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

Add query source support to sentry-rails #2313

Merged
merged 4 commits into from
May 31, 2024
Merged

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented May 26, 2024

Closes #2242

This PR adds query source information to the tracing spans captured by sentry-rails's ActiveSupportSubscriber, which should be picked up by Sentry and displayed like:

Screenshot 2024-05-26 at 16 43 23

The implementation is inspired by Rails' ActiveRecord::LogSubscriber but doesn't depend on it.

There are several requirements for this feature to work:

  • Ruby 3.2+ because we need Thread.each_caller_location to minimise the performance impact
  • Rails 7.1+ because we need ActiveSupport::BacktraceCleaner#clean_frame
  • config.rails.enable_db_query_source is set to true (default)
  • sentry-rails's ActiveRecordSubscriber is included in the config.rails.tracing_subscribers list (default)

And to reduce the feature's overhead, only queries that exceed config.rails.db_query_source_threshold_ms will have source recorded, which is set to 100ms by default.

Copy link

codecov bot commented May 26, 2024

Codecov Report

Attention: Patch coverage is 95.14563% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 98.66%. Comparing base (41ab3ba) to head (a47f0c2).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2313   +/-   ##
=======================================
  Coverage   98.66%   98.66%           
=======================================
  Files         205      205           
  Lines       13339    13422   +83     
=======================================
+ Hits        13161    13243   +82     
- Misses        178      179    +1     
Components Coverage Δ
sentry-ruby 99.03% <100.00%> (+0.01%) ⬆️
sentry-rails 97.42% <94.94%> (+0.05%) ⬆️
sentry-sidekiq 96.98% <ø> (ø)
sentry-resque 96.76% <ø> (-0.33%) ⬇️
sentry-delayed_job 98.91% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-rails/lib/sentry/rails/configuration.rb 97.43% <100.00%> (+0.29%) ⬆️
...try/rails/tracing/active_record_subscriber_spec.rb 100.00% <100.00%> (ø)
sentry-ruby/lib/sentry/span.rb 100.00% <100.00%> (ø)
...b/sentry/rails/tracing/active_record_subscriber.rb 90.38% <88.63%> (+4.67%) ⬆️

... and 2 files with indirect coverage changes

@sl0thentr0py sl0thentr0py self-assigned this May 27, 2024
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!
We also have an option called db_query_source_threshold_ms that defaults to 100ms and the source is recorded on the span only if it takes more than that threshold.
Could you also add that?

https://github.com/getsentry/sentry-python/blob/b922f40ad9cff1f97286030f71a22e0000e9b4e2/sentry_sdk/tracing_utils.py#L185-L186

st0012 added 4 commits May 27, 2024 23:20
This change mimics Rails' ActiveRecord::LogSubscriber to extract the source
of the query and add it to the span data. The added source information can
then be displayed along with the query SQL in the Sentry UI.

The feature only works with Ruby 3.2+ and Rails 7.1+.
…shold_ms

This avoids the overhead of recording the source in fast queries, which
in general don't need to be traced.

The threshold is configurable via `config.rails.db_query_source_threshold_ms`
and the default is 100ms.
@st0012 st0012 force-pushed the implement-#2242 branch from 834bcd9 to a47f0c2 Compare May 27, 2024 22:21
@st0012 st0012 requested a review from sl0thentr0py May 27, 2024 22:21
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

ty <3

@sl0thentr0py sl0thentr0py merged commit 7b00b25 into master May 31, 2024
123 of 124 checks passed
@sl0thentr0py sl0thentr0py deleted the implement-#2242 branch May 31, 2024 13:40
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.

Add "Query Sources" Ruby support
2 participants