Enhancement: lossless filtering of backtraces for fingerprinting #1076
Assignees
Comments
|
hi @ChrisBAshton, thanks for clearing explaining the issue! I'm sorry that with that being said, I will introduce something like |
|
Hi @st0012, thanks for the quick response! |
|
Hi @st0012, I was wondering if you had a rough timeline for this now that v4 has been released? Thanks in advance. |
|
@ChrisBAshton hey sorry for the delay. I've started working on this again. I'm still thinking if I should treat |
|
@ChrisBAshton can you screenshot the backtrace information you find missing because of |
|
Hi @st0012, sorry for the slow response. Here are the stack traces taken from clicking "RAW" next to the Stack Trace section in the Sentry UI. I've tried three approaches:
With no backtrace cleaningWith backtrace cleaningi.e. by supplying this config: config.backtrace_cleanup_callback = lambda do |backtrace|
Rails.backtrace_cleaner.clean(backtrace)
endWe can see that the formatting is different, but no frames are lost: With backtrace cleaning and custom silencersconfig.backtrace_cleanup_callback = lambda do |backtrace|
Rails.backtrace_cleaner.add_silencer { |line| /action_dispatch/.match?(line) }
Rails.backtrace_cleaner.clean(backtrace)
endWe can see that, again, no frames are lost - Further investigationI must admit, I was surprised that 2) retained every frame trace (albeit looking different), and even more surprised when 3) retained all the frame traces too. So I tried an experiment: config.backtrace_cleanup_callback = lambda do |backtrace|
Rails.backtrace_cleaner.add_silencer { |line| true }
Rails.backtrace_cleaner.clean(backtrace)
endThe I added this to my routes.rb: get "/trigger-error" => "trigger_error#now"
get "/trigger-another-error" => "trigger_error#then"...and this in the TriggerErrorController: def now
raise StandardError.new("Custom exception from Chris #1")
end
def then
raise StandardError.new("Custom exception from Chris #2")
endHowever, when I triggered the two error types, they did not appear in Sentry as one issue as I expected. They appeared as two separate issues, and when I scrolled to the bottom, I could see that they were each grouped "By Exception Stack-trace", and that the stack traces appeared in full. This makes me think that the Sentry backtrace cleaner isn't actually doing what we think it is. For the record, I also tried out the Rails backtrace cleaner in a REPL, and it does seem to work how I've documented above: SummaryThe |
|
@ChrisBAshton thanks for the detailed investigation 🙏 I found the result weird too so I did an investigation myself and here's what I found:
|
st0012
added a commit
that referenced
this issue
Jul 22, 2021
`sentry-rails` should check the presence `backtrace_cleanup_callback` before assigning the default one. The problem was spotted in #1076 (comment).
st0012
added a commit
that referenced
this issue
Jul 23, 2021
* Remove perform_basic_setup test helper to avoid misuse * Don't override backtrace_cleanup_callback set by the user `sentry-rails` should check the presence `backtrace_cleanup_callback` before assigning the default one. The problem was spotted in #1076 (comment). * Update changelog
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
Thanks, bot, for reminding me about this! I believe next steps are for me to try out my experiment again, but with the latest version of |
|
@ChrisBAshton I believe this should no longer be a problem, so closing. Please reopen if necessary! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hello 👋 We're facing a situation similar to #957, whereby we have lots of errors that differ slightly in their stacktrace, meaning they are grouped differently when they are ostensibly the same error. Stacktrace differences typically occur in the 'framework traces', caused by slight variations in dependency versions over time. What we really care about for the purposes of fingerprinting are the 'application traces'.
The
backtrace_cleanup_callbackadded in #1011 was a useful addition meaning that the application traces of our stacktraces no longer differ on dynamically generated template methods (such asapp/views/welcome/view_error.html.erb in _app_views_welcome_view_error_html_erb__2807287320172182514_65600 at line 1). I understand that we could provide our own callback to this method and use the Rails::BacktraceCleaner to filter out framework traces and keep only our application traces, like so:The problem with this approach is that it is lossy. We only want to filter the stacktrace for the purposes of fingerprinting, but we still want to be able to dig into the whole stack trace when we come to debug the error.
Proposal
I propose that the implementation of
backtrace_cleanup_callbackbe changed slightly (as it's still new and presumably has limited adoption so far), so that it doesn't alter the original stack trace. It would have exactly the same API (i.e. alambda) but would just store a filtered version in memory for the purposes of fingerprinting.If changing the current property is not an option, a new config option could be provided (perhaps
filter_backtrace_for_fingerprinting).Or, if I've completely misunderstood how this works and it isn't lossy, could the documentation make this explicit?
Other approaches considered
I've toyed with the idea of manipulating the fingerprint itself using
before_send, though there is no documentation on this and I'm not sure if it's supported:What stopped me from investigating this further was that it felt like using a sledgehammer to crack a nut. The default Sentry grouping algorithm is great, most of the time, and I'd rather not reimplement it - I just want to tweak it to be a little more forgiving of environmental differences in stack trace.
I did also consider server-side fingerprinting to filter out framework traces, but we have a lot of applications to manage and a strong version control mantra, whereby manually editing this kind of thing in the Sentry UI really isn't ideal.
Thanks in advance for your time!
The text was updated successfully, but these errors were encountered: