Add Sentry.with_child_span for easier span recording#1783
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1783 +/- ##
==========================================
- Coverage 98.40% 98.40% -0.01%
==========================================
Files 145 145
Lines 8480 8536 +56
==========================================
+ Hits 8345 8400 +55
- Misses 135 136 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
ping @sl0thentr0py |
|
|
||
| begin | ||
| current_span.with_child_span(**attributes) do |child_span| | ||
| get_current_scope.set_span(child_span) |
There was a problem hiding this comment.
Just an observation, all the current usages of set_span set it to the transaction and we don't really use scope#get_span either so this will be the first time the span on the scope gets used right?
➜ sentry-ruby git:(master) ✗ ag --ignore 'spec' set_span
sentry-rails/lib/sentry/rails/action_cable.rb
18: scope.set_span(transaction) if transaction
sentry-rails/lib/sentry/rails/active_job.rb
31: scope.set_span(transaction) if transaction
sentry-resque/lib/sentry/resque.rb
28: scope.set_span(transaction) if transaction
sentry-delayed_job/lib/sentry/delayed_job/plugin.rb
22: scope.set_span(transaction) if transaction
sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb
21: scope.set_span(transaction) if transaction
sentry-ruby/lib/sentry/scope.rb
132: def set_span(span)
sentry-ruby/lib/sentry/rack/capture_exceptions.rb
23: scope.set_span(transaction) if transaction
There was a problem hiding this comment.
Yes, because most the usages in the SDK are to create the top-level transaction. But as described in #1784, sentry-rails's subscriber should use spans instead. And users will generally use get_span and set_span for spans more than transactions.
0807404 to
4af72c0
Compare
|
|
||
| second_span = transaction.spans.last | ||
| expect(second_span[:op]).to eq("second level") | ||
| expect(second_span[:parent_span_id]).to eq(first_span[:span_id]) |
There was a problem hiding this comment.
@sl0thentr0py I've added the integration test here.
This PR adds a new
Sentry.with_child_spanAPI. It'll allow users to record a child span with a block without the need to getting the current span from scope, checking its presence...etc. A simple block wrapping will do the trick:Note that if there's no span in the current scope, the block will still be executed. But the yield
child_spanwill benilinstead.With this API, it's also easier to fix the child span nesting issue reported in #1723.