Skip to content

Support cleaning up exception backtrace with customized backtrace_cleaner#1011

Merged
st0012 merged 2 commits into
masterfrom
fix-#957
Sep 4, 2020
Merged

Support cleaning up exception backtrace with customized backtrace_cleaner#1011
st0012 merged 2 commits into
masterfrom
fix-#957

Conversation

@st0012

@st0012 st0012 commented Sep 3, 2020

Copy link
Copy Markdown
Contributor

This PR adds 2 new features:

New config option: backtrace_cleanup_callback

It takes a lambda/proc object (default is nil) and will be called with exception's backtrace. Here's an example usage:

Raven.configure do |config|
  config.backtrace_cleanup_callback = lambda do |backtrace|
    Rails.backtrace_cleaner.clean(backtrace)
  end
end

Automatically clean Rails app's backtrace

With the Rails integration, it'll automatically use a customized Raven::Rails::BacktraceCleaner to clean up exception's backtrace. This can be disabled by:

Raven.configure do |config|
  config.backtrace_cleanup_callback = nil
end

Raven::Rails::BacktraceCleaner

This cleaner is pretty much like Rails 6's backtrace cleaner but without silencers. The main reason to add this cleaner is to remove template methods from the trace, e.g.

app/views/welcome/view_error.html.erb in _app_views_welcome_view_error_html_erb__2807287320172182514_65600 at line 1

will become

app/views/welcome/view_error.html.erb at line 1

This can help Sentry group issues more accurately. See #957 for more information about this.

Closes #957

@st0012 st0012 added this to the 3.1.0 milestone Sep 3, 2020
@st0012 st0012 self-assigned this Sep 3, 2020
@codecov-commenter

codecov-commenter commented Sep 3, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1011 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1011      +/-   ##
==========================================
+ Coverage   97.89%   97.94%   +0.04%     
==========================================
  Files          49       50       +1     
  Lines        2238     2283      +45     
==========================================
+ Hits         2191     2236      +45     
  Misses         47       47              
Impacted Files Coverage Δ
lib/raven/integrations/rails.rb 86.04% <100.00%> (+1.43%) ⬆️
lib/raven/integrations/rails/backtrace_cleaner.rb 100.00% <100.00%> (ø)
spec/raven/configuration_spec.rb 98.67% <100.00%> (+0.03%) ⬆️
spec/raven/integrations/rails_spec.rb 100.00% <100.00%> (ø)
spec/support/test_rails_app/app.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f5dd4d...14c5903. Read the comment docs.

@davidstosik davidstosik left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👏 Thanks a lot, this is looking great, and provides a nice modularity!

a proc/lambda that takes an array of stack traces
it'll be used to cleanup backtrace of the exception

for example:

```ruby
Raven.configuration.backtrace_cleanup_callback = lambda do |backtrace|
  Rails.backtrace_cleaner.clean(backtrace)
end
```
@st0012 st0012 changed the title Support silencing exception backtrace with Rails' backtrace_cleaner Support cleaning up exception backtrace with customized backtrace_cleaner Sep 4, 2020
@st0012
st0012 merged commit cd628e0 into master Sep 4, 2020
@st0012
st0012 deleted the fix-#957 branch September 4, 2020 06:02
ChrisBAshton added a commit to alphagov/govuk_app_config that referenced this pull request Oct 19, 2020
In order to improve Sentry's grouping of errors, we want to make
use of the `backtrace_cleanup_callback` introduced in 3.1.0:
getsentry/sentry-ruby#1011

`backtrace_cleanup_callback` automatically uses a
[customised version of Rails::BacktraceCleaner](https://github.com/getsentry/sentry-ruby/pull/1011/files#diff-c01d5cbc846720e47a4185cb501a55225247ec698aa169b0a2773ca9b65ae35dR4-R29)
if we don't specify our own. It means that for stack traces like:

`app/views/welcome/view_error.html.erb in _app_views_welcome_view_error_html_erb__2807287320172182514_65600 at line 1`

...these lines get normalised to:

`app/views/welcome/view_error.html.erb at line 1`

So if the same `ActionView::Template::Error` error happens twice,
it will now be grouped instead of erroneously treated as separate
errors.

This behaviour differs from the [native `Rails::BacktraceCleaner`](https://github.com/rails/rails/blob/b66235d432d0505857ff667e0a1ddecfb5640a56/railties/lib/rails/backtrace_cleaner.rb),
which has 'silencers' that remove any "framework trace" from your
stacktrace, leaving only the "application trace" behind. This
would actually be beneficial for grouping, as Sentry is very
sensitive to stack traces that differ only slightly, which can
happen when a dependency is updated or a Ruby version is upgraded.

We could specify sentry-raven to use the Rails backtrace cleaner like so:

```ruby
config.backtrace_cleanup_callback = lambda do |backtrace|
  Rails.backtrace_cleaner.clean(backtrace)
end
```

...however, this is not _only_ used for the comparison, but would alter
the stacktrace itself, meaning we lose key diagnostic information.
This isn't a price worth paying for the sake of 'neatness' in our
Sentry groupings.

Trello: https://trello.com/c/NZNjFHWO/2162-5-improve-sentrys-grouping-of-exceptions
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.

Ungroupable error stacktraces: can Raven-Ruby leverage Rails.backtrace_cleaner?

3 participants