add sentry #2786
add sentry #2786
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces Sentry integration across various components of the Danswer application, enhancing error tracking and monitoring capabilities.
- Added Sentry initialization in
backend/danswer/main.pyandbackend/model_server/main.pyfor the main API and model servers - Implemented Sentry integration in
backend/danswer/background/celery/celery_app.pyfor Celery tasks - Introduced
SENTRY_DSNenvironment variable inbackend/shared_configs/configs.pyanddeployment/docker_compose/docker-compose.dev.yml - Reorganized configuration imports, moving several variables to
shared_configs/configs.pyfor centralized management - Enhanced error handling and logging in various components, particularly in the tenant tracking middleware
13 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
| if SENTRY_DSN: | ||
| sentry_sdk.init( | ||
| dsn=SENTRY_DSN, | ||
| integrations=[CeleryIntegration()], | ||
| traces_sample_rate=1.0, | ||
| ) | ||
| logger.info("Sentry initialized") | ||
| else: | ||
| logger.debug("Sentry DSN not provided, skipping Sentry initialization") |
There was a problem hiding this comment.
style: Consider moving Sentry initialization to a separate function for better modularity and testability
| logger.info("Sentry initialized") | ||
| else: |
There was a problem hiding this comment.
style: Use logger.info for consistency with other log levels in the file
There was a problem hiding this comment.
Most users won't set Sentry so better to keep this quiet
backend/danswer/main.py
Outdated
| integrations=[StarletteIntegration(), FastApiIntegration()], | ||
| traces_sample_rate=1.0, |
There was a problem hiding this comment.
style: traces_sample_rate of 1.0 may be too high for production. Consider making this configurable
backend/model_server/main.py
Outdated
| if SENTRY_DSN: | ||
| sentry_sdk.init( | ||
| dsn=SENTRY_DSN, | ||
| integrations=[StarletteIntegration(), FastApiIntegration()], | ||
| traces_sample_rate=1.0, | ||
| ) | ||
| logger.info("Sentry initialized") | ||
| else: | ||
| logger.debug("Sentry DSN not provided, skipping Sentry initialization") |
There was a problem hiding this comment.
style: Consider moving Sentry initialization to a separate function for better modularity
backend/shared_configs/configs.py
Outdated
| # Set up Sentry integration | ||
| SENTRY_DSN = os.environ.get("SENTRY_DSN") |
There was a problem hiding this comment.
style: Consider adding a comment explaining the purpose of SENTRY_DSN and how it should be used
pablonyx
force-pushed
the
sentry_on_main
branch
from
daad216 to
a73c089
Compare
pablonyx
force-pushed
the
sentry_on_main
branch
from
a73c089 to
46c439e
Compare
pablonyx
force-pushed
the
sentry_on_main
branch
from
46c439e to
3dce229
Compare
pablonyx
force-pushed
the
sentry_on_main
branch
from
b83e5b3 to
1e582f1
Compare
pablonyx
force-pushed
the
sentry_on_main
branch
from
00b62c7 to
712d7e6
Compare
Description
Adds Sentry to background, model server, API server, and frontend.
Keeping mostly defaults so that we can evolve our approach as we determine what is/isn't useful