Conversation
Codecov Report
@@ Coverage Diff @@
## master #1446 +/- ##
==========================================
+ Coverage 98.22% 98.85% +0.62%
==========================================
Files 213 118 -95
Lines 10034 6261 -3773
==========================================
- Hits 9856 6189 -3667
+ Misses 178 72 -106
Continue to review full report at Codecov.
|
rhcarvalho
left a comment
There was a problem hiding this comment.
I think we just need a method name change to match the SDK guidelines and we're good to merge.
Thanks @st0012 !
sentry-ruby/lib/sentry/hub.rb
Outdated
| def generate_sentry_trace(span = nil) | ||
| return unless current_client.configuration.propagate_traces | ||
|
|
||
| span ||= current_scope.get_span |
There was a problem hiding this comment.
@rhcarvalho let's move the discussion here.
Another reason I choose Hub instead of Client is because in Hub we can also access the current scope.
Consider that this method needs to access different both Client and Scope, I still think Hub is currently the best place to have this method.
There was a problem hiding this comment.
#1446 (comment)
I'd say we don't need the Hub because it is only used to get the current scope when the span is nil, but we never call it with a nil span.
I have the impression this line is unnecessary, isn't it? The only non-test place where this method is called is set_sentry_trace_header and we never call with a nil span.
So my thought was that this didn't necessarily need to access the current scope.
There was a problem hiding this comment.
I thought being able to read the span from current scope is a hard requirement when I checked sentry-python's implementation.
But if it's not, I'm happy to remove it and move this method to Client.
b996870 to
080de9e
Compare
|
@rhcarvalho thanks for the review! |
This implements and closes #1445
Summary
The SDK will insert the
sentry-traceto outgoing requests made withNet::HTTP. Its value would look liked827317d25d5420aa3aa97a0257db998-57757614642bdff5-1.If the receiver service also uses Sentry and the SDK supports performance monitoring, its tracing event will be connected with the sender application's.
Example:
This feature is activated by default. But users can use the new
config.propagate_tracesconfig option to disable it.