Updates sentry sdk configuration #6175
Conversation
- Upgrades sentry gems to versions 5.22.3 in order to use 'sentry/test_helper' - Adds require and include statements for sentry test helper in `rails_helper` - Adds filtering to sentry events before they are sent to Sentry using `config.excluded_exceptions` and `config.before_send` - Adds `sentry_spec` to ensure filtering works as expected and normal sentry event sending logic is not affected.
empty-codes
changed the title
- Filters `Non-Error promise rejection` errors which are Known errors caused by Microsoft Outlook SafeSearch or bot crawlers that use the CefSharp library. - Filters `chrome-extension://` errors which like the name implies are caused by user extensions.
|
Hello @empty-codes the failure in your build has been fixed here waiting for it to be merged |
| # Appends to IGNORE_DEFAULT array of exception classes that should never be sent to Sentry | ||
| config.excluded_exceptions += ['WikiApi::PageFetchError'] # error code 429 | ||
|
|
||
| filter_patterns = [ |
There was a problem hiding this comment.
Hmm...
I don't think I like using pattern matching to filter errors here. I'd rather take it very slowly and only filter out specific error classes that we have a clear understanding of, in terms of why they are happening.
There was a problem hiding this comment.
I did that because the TypeErrors don't follow the pattern of Ruby / Rails exceptions but I do understand, I'll remove that if case
| stacktrace = exception&.stacktrace | ||
| if stacktrace | ||
| stacktrace.frames.each do |frame| | ||
| if frame.function&.include?('@webkit-masked-url') |
There was a problem hiding this comment.
What of this check? Should I leave it in? the errors with chrome-extension'are well caused by chrome extensions manipulating the code in ways not meant to, while the webkit-masked url is a webkit bug caused by safari extensions.
These errors are actually part of the ones automatically filtered out by Sentry if we toggle the filter 'Filter out errors known to be caused by browser extensions' in the UI (see their source code here) but since better safe than sorry, I filtered out just these
There was a problem hiding this comment.
Also , I added a check for web crawler errors (that caused the ReferenceError: stores is not defined error) this way:
# Ignore web crawler errors
browser_name = event.tags&.dig(:browser_name)
return nil if %w[Baiduspider Googlebot].include?(browser_name)
It's also part of the web crawler errors Sentry filters out if we toggle the option, (see their source code here). Baiduspider and Googlebot are the ones I've noticed so I also filtered just these.
Is this okay?
|
I think we should set this Sentry config aside entirely for now. I want to be really confident that we don't accidentally start losing meaningful errors, and the 429 errors can definitely be potentially meaningful ones. |
Okay got it! I'll go back to fixing the fixable errors, cause they're still a lot😅 |
What this PR does
Reduce noise in Sentry by filtering out benign / unactionable errors.
rails_helperconfig.excluded_exceptionsandconfig.before_sendsentry_specto ensure filtering works as expected and normal sentry event sending logic is not affected.Screenshots
Before changes to
sentry.rb:Only the
does not filter out normal eventspasses:After changes to
sentry.rb: