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

DR-2982: Send Sentry errors in GlobalExceptionHandler #1476

Closed
wants to merge 3 commits into from

Conversation

snf2ye
Copy link
Contributor

@snf2ye snf2ye commented Jun 23, 2023

Background

Original Sentry integration is not working (Original PR: #1364). I was able to successfully register an error in sentry locally, but since merging the change, we have not seen any errors logged in our other environments.

So, what's going on?
My understanding of sentry is that we should be able to automagically capture errors that we're otherwise routing to stackdriver to also log in sentry via the logback definition. It seems that Sentry picks up some errors in my local environment, but not on my personal dev environment. Something about the deployed instance is not working. So, I'm not sure what's going wrong. I've spent a good amount of time investigating this and I think there is a better solution, so I'm instead pivoting to a different approach.

The Solution

Even if we got the logback configuration to work, it wouldn't really serve the desired purpose for Sentry. It would capture too many errors, creating a lot of noise and would overuse our allocated resources in Sentry (there is a limit to how many errors we can log in Sentry).
Another solution is to manually capture specific types of errors directly to sentry. We can do this the GlobalExceptionHandler.

Which exceptions should we capture?
We want Sentry to log unexpected exceptions that we should fix/handle differently. With this changes, this currently includes:

  • Internal Server Errors
  • Service unavailable errors
  • Job response exceptions
  • Unexpected SAM ApiExceptions
  • Catchall

Reviewers: Please let me know if you think I should include more/less of these exceptions.

More notes can be found in this doc: https://docs.google.com/document/d/1E6UTXHbj4Qs9hwujIMHO0vGCLfdrGIsylJZCdLHvMJQ/edit

@snf2ye snf2ye force-pushed the sh-dr-2982-sentry-global-exception branch from 0547914 to 0e7b060 Compare July 10, 2023 12:04
@snf2ye
Copy link
Contributor Author

snf2ye commented Jul 13, 2023

Closing this in favor of #1479

@snf2ye snf2ye closed this Jul 13, 2023
@snf2ye snf2ye deleted the sh-dr-2982-sentry-global-exception branch July 13, 2023 11:32
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.

1 participant