AcademySoftwareFoundation / OpenCue

A render management system you can deploy for visual effects and animation productions.
https://www.opencue.io
Apache License 2.0
832 stars 202 forks source link

[rqd] Add sentry support #1433

Closed DiegoTavares closed 3 months ago

DiegoTavares commented 4 months ago

Add sentry as an optional integration to collect events from the rqd module.

If SENTRY_DSN_PATH is configured on rqd.conf, a sentry collector will be initialized.

lithorus commented 4 months ago

Isn't there a better way to have the sentry-sdk added than in the main requirements.txt? I sorta have the same problem with the loki integration. I don't think it should be in requirements.txt. Perhaps an idea to split up requirements.txt a bit?

ramonfigueiredo commented 4 months ago

Isn't there a better way to have the sentry-sdk added than in the main requirements.txt? I sorta have the same problem with the loki integration. I don't think it should be in requirements.txt. Perhaps an idea to split up requirements.txt a bit?

Good point, @lithorus ! I thought about that yesterday as well.

My suggestion to you and Diego is to split the requirements.txt file into more files for optional dependencies. This can help keep the main dependencies clean and manage optional integrations like Sentry and Loki more effectively. This approach allows users to install only the dependencies they need. The same approach is currently applied to the CueGUI dependencies, using the extra requirements_gui.txt.

@DiegoTavares and @lithorus

I recommend creating separate files for optional dependencies like requirements-sentry.txt and requirements-loki.txt.

@DiegoTavares

# requirements-sentry.txt
sentry-sdk==2.11.0

2) Update the documentation (opencue.io) and README.md file to inform users how to install these optional dependencies if they need them.

Example:

## Optional dependencies

### Sentry Integration
To enable Sentry support, install the additional requirements:
```bash
pip install -r requirements-sentry.txt
DiegoTavares commented 4 months ago

Isn't there a better way to have the sentry-sdk added than in the main requirements.txt? I sorta have the same problem with the loki integration. I don't think it should be in requirements.txt. Perhaps an idea to split up requirements.txt a bit?

I confess I spend some time pondering the options and I still haven't found one that I'm completely satisfied with.

If a new requirements file is added for each new optional dependency as suggested by @ramonfigueiredo we get the benefit of giving users the option to install what they need, but it makes our life harder down the line. Here are my cons:

lithorus commented 3 months ago

All good points, however I would still suggest that some of it was split up a bit more. Some of the modules are only needed for testing, some are only needed for rqd similar to how some only used for the GUI apps.

Someting like this perhaps? : requirements.txt (Core libraries used by outline and opencue modules) requirements_rqd.txt (Additional used by rqd) requirements_gui.txt (Additional used by cuegui and cuesubmit)

If we also use this as sort of a dependency tree (rqd can depend on things in opencue, but not the other way), we can centralize some of the functions, like the loki/sentry functions or a centralized way of handling config files.

It might even make testing more straightforward, since we have a clear sequence of when modules are tested/installed.

Edit: Either way it makes for a great topic on next meeting :)

DiegoTavares commented 3 months ago

All good points, however I would still suggest that some of it was split up a bit more. Some of the modules are only needed for testing, some are only needed for rqd similar to how some only used for the GUI apps.

Someting like this perhaps? : requirements.txt (Core libraries used by outline and opencue modules) requirements_rqd.txt (Additional used by rqd) requirements_gui.txt (Additional used by cuegui and cuesubmit)

If we also use this as sort of a dependency tree (rqd can depend on things in opencue, but not the other way), we can centralize some of the functions, like the loki/sentry functions or a centralized way of handling config files.

It might even make testing more straightforward, since we have a clear sequence of when modules are tested/installed.

Edit: Either way it makes for a great topic on next meeting :)

Sounds reasonable to me. Let's discuss this further on Wednesday on the TSC meeting.