search

instana / ruby-sensor

💎 Ruby Distributed Tracing & Metrics Sensor for Instana
https://www.instana.com/
MIT License
26 stars 25 forks source link

Add backtrace_path_filter config option #266

Closed jeremycw closed 2 years ago

jeremycw commented 2 years ago

What

Adds a config option called backtrace_path_filter that allows the lines in backtraces to be compared against the filter to see if they should be included in the backtrace

Why

Backtraces are limited to a number of lines (default 30) and are often coming from so deep in library/framework code that you may not even see the originating user code in the backtrace. However, library code is often located in a different path than user code. This can be leveraged to filter the call stack to relevant code.

[
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...',
  # potentially many more lines...
  '/var/www/app/my/model.rb',
  '/var/www/app/my/controller.rb',
  '/usr/local/bundle/rails/...',
  '/usr/local/bundle/rails/...'
]

# With `backtrace_path_filter` set to `/var/www/app/` becomes:

[
  '/var/www/app/my/model.rb',
  '/var/www/app/my/controller.rb'
]
hmadison commented 2 years ago

Hey @jeremycw,

Since this seems to be a Rails related pull request, would it be possible to make the configuration option take a lambda instead of a string so users could pass closures to ActiveSupport::BacktraceCleaner? I would also ask that we add tests for this behavior. For core functionality we would like to keep our 100% branch coverage.

Thanks, Hunter

jeremycw commented 2 years ago

@hmadison Good suggestion. I confirmed that it also works doing ->(trace) { Rails.backtrace_cleaner.clean(trace) }

daande commented 2 years ago

@hmadison right now we are running off of a fork of this gem as this functionality we find helpful so hopefully we can get it merged in

hmadison commented 2 years ago

Hey @daande,

If you can reach out to your TAM and have them find me internally I can get the CLA stuff handled through them and get this merged Monday.

Thanks, Hunter

daande commented 2 years ago

@hmadison we created a support request in instana (https://support.instana.com/hc/en-us/requests/25636) hopefully that is sufficient

daande commented 2 years ago

@hmadison any update on this? See attached ticket above.

hmadison commented 2 years ago

I've merged the change and will work on getting a new release pushed tomorrow morning.

daande commented 2 years ago

@hmadison can you cut a release with this change please?

hmadison commented 2 years ago

@daande https://rubygems.org/gems/instana/versions/1.209.1