Skip to content
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

Merged
merged 16 commits into from
Nov 8, 2023
Merged

Feat: Sentry error monitoring #253

merged 16 commits into from
Nov 8, 2023

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Apr 13, 2023

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:

# get the DSN from https://sentry.calitp.org/settings/sentry/projects/eligibility-server/keys/
SENTRY_DSN = "https://..."  

# make up a name for testing
AGENCY_NAME = "Your Transit Agency"

In your local .env file, make sure to point to your custom settings file:

ELIGIBILITY_SERVER_SETTINGS=../config/sentry.py

Get this branch and rebuild and reopen in the devcontainer.

F5 to run the app

Go to http://localhost:PORT/error to trigger a Sentry error

Log 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 setting release for Sentry determined that we don't need this. Benefits uses VERSION as a fallback, but in practice, we're always getting the git hash.

Before merging

@angela-tran angela-tran self-assigned this Apr 13, 2023
@angela-tran angela-tran changed the title Feat: Sentry Feat: Sentry error monitoring Apr 13, 2023
@angela-tran angela-tran marked this pull request as ready for review April 17, 2023 22:00
@angela-tran angela-tran requested a review from a team as a code owner April 17, 2023 22:00
eligibility_server/sentry.py Outdated Show resolved Hide resolved
terraform/app_service.tf Show resolved Hide resolved
@thekaveman
Copy link
Member

Rebased to clean up merge conflicts

@thekaveman
Copy link
Member

Confirmed the custom AGENCY_NAME shows up in Sentry as a tag agency_name:

image

@thekaveman thekaveman marked this pull request as ready for review October 11, 2023 22:33
@thekaveman
Copy link
Member

I should update the docs with this, putting back into draft -- sorry!

@thekaveman thekaveman marked this pull request as draft October 12, 2023 17:52
@thekaveman thekaveman marked this pull request as ready for review October 12, 2023 17:54
@thekaveman thekaveman mentioned this pull request Oct 26, 2023
Copy link
Member

@machikoyasuda machikoyasuda left a 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.

@angela-tran angela-tran marked this pull request as draft October 31, 2023 17:38
@angela-tran
Copy link
Member Author

From @thekaveman on 2023-10-26:

between Sentry having issues today and not being able to verify the agency name, we need to hold this and keep working on it.

Also see #355 (comment)

@thekaveman thekaveman marked this pull request as ready for review October 31, 2023 23:02
@thekaveman
Copy link
Member

@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 settings.py file rather than putting all this in your .env.

@machikoyasuda
Copy link
Member

I still keep getting this: SENTRY_DSN not set, so won't send events despite following the new instructions.
image

@allejo
Copy link

allejo commented Nov 7, 2023

I second Machiko's findings. I have to set SENTRY_DSN in my local .env file and source that file before I get this reporting correctly to Sentry. Following the instructions above got me the same results as #253 (comment); mind you, doing an install of python-dotfiles did not help.

Also, changing (maybe "fixing") the path to the settings file did not change anything. I'm not sure where the ../ from the original PR comment is coming from.

ELIGIBILITY_SERVER_SETTINGS=./config/sentry.py

@thekaveman
Copy link
Member

Also, changing (maybe "fixing") the path to the settings file did not change anything. I'm not sure where the ../ from the original PR comment is coming from.

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 (eligibility_server/ vs. config/), so we have to back out to get to config/.

machikoyasuda
machikoyasuda previously approved these changes Nov 8, 2023
Copy link
Member

@machikoyasuda machikoyasuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me now locally
image

Always re-build your Devcontainer without cache after changing the .env file.

Copy link
Member

@machikoyasuda machikoyasuda left a 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

@thekaveman thekaveman merged commit 0a2dccd into dev Nov 8, 2023
6 of 7 checks passed
@thekaveman thekaveman deleted the feat/sentry branch November 8, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce an agency environment variable Configure Sentry for error monitoring
4 participants