-
-
Notifications
You must be signed in to change notification settings - Fork 165
feat(core): implement Tracing without Performance #811
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #811 +/- ##
==========================================
- Coverage 73.23% 72.99% -0.25%
==========================================
Files 62 62
Lines 7238 7279 +41
==========================================
+ Hits 5301 5313 +12
- Misses 1937 1966 +29 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, I have some (mostly minor) suggestions though
Co-authored-by: Daniel Szoke <[email protected]>
Co-authored-by: Daniel Szoke <[email protected]>
Co-authored-by: Daniel Szoke <[email protected]>
Co-authored-by: Daniel Szoke <[email protected]>
Co-authored-by: Daniel Szoke <[email protected]>
/// Use [`crate::Scope::iter_trace_propagation_headers`] to obtain the active | ||
/// span's/transaction's distributed tracing headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that this could also return the ids from PropagationContext if there is no ongoing span, not sure if this comment correctly reflects that.
But on the other hand we should also hide PropagationContext as that's an "implementation detail".
@@ -1133,6 +1140,15 @@ pub struct TraceHeadersIter { | |||
sentry_trace: Option<String>, | |||
} | |||
|
|||
impl TraceHeadersIter { | |||
#[cfg_attr(not(feature = "client"), allow(dead_code))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If client
is not active then we will use the noop scope that doesn't call this
Implements Tracing without Performance (spec).
This is done by having an additional
PropagationContext
attribute on the scope that holds a "fake"trace_id
andspan_id
.When applying the scope to an event, if there is no ongoing span set on the scope, we create the
trace
context from thePropagationContext
.Additionally, adds an API
scope.iter_trace_propagation_headers
that should be preferred over something like:because it will use the
PropagationContext
as a fallback.Closes #796
Also makes a minor adjustment to trace and span ids where we now use the
Display
impl forDebug
, otherwise we were getting u8 arrays when using e.g.dbg!
with the derived one which is not very useful.