-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Sentry error monitoring #253
Conversation
fce923d
to
77f1366
Compare
Rebased to clean up merge conflicts |
Sentry by default will try to use SENTRY_DSN for the dsn option if no argument is passed in, but this makes it clearer
the value is not secret and can be driven by a Terraform local var
default is explicitly unset
b322204
to
0d68e4d
Compare
I should update the docs with this, putting back into draft -- sorry! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm able to get Sentry errors locally to appear. But having some trouble with the local env file settings not being ready properly.
From @thekaveman on 2023-10-26:
Also see #355 (comment) |
@angela-tran @machikoyasuda @allejo I think Sentry is back working again, so we can proceed with testing this again! Also, my notes were wrong about how to configure locally for testing. I've updated that -- you need to use a custom |
I second Machiko's findings. I have to set Also, changing (maybe "fixing") the path to the settings file did not change anything. I'm not sure where the
|
Yeah we should probably have a comment in there about this. It's because this environment variable has to be relative to the code that loads the settings file, and not to where the environment file is. The code that loads the settings is in a sibling to the location of the settings file ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
Thank you for the clarifying comment
Closes #252
Closes #312
How to test / review
Create a new Python settings file in your local
config/
directory, e.g.config/sentry.py
, with the following contents:In your local
.env
file, make sure to point to your custom settings file:Get this branch and rebuild and reopen in the devcontainer.
F5
to run the appGo to
http://localhost:PORT/error
to trigger a Sentry errorLog in to Sentry to confirm your details were recorded in the error.
TODOs
Figure out / decide on a way to single-source the version number and read it when settingdetermined that we don't need this. Benefits usesrelease
for SentryVERSION
as a fallback, but in practice, we're always getting the git hash.Before merging
SENTRY_DSN
as a setting for current deployments ofeligibility-server
(it will be within the file that is specified byELIGIBILITY_SERVER_SETTINGS
)AGENCY_NAME
as a setting for current deployments ofeligibility-server
(it will be within the file that is specified byELIGIBILITY_SERVER_SETTINGS
). This should match the correspondingagency.long_name
from Benefits