-
Notifications
You must be signed in to change notification settings - Fork 417
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
[API] Propagators: do not overwrite the active span with a default invalid span #2511
[API] Propagators: do not overwrite the active span with a default invalid span #2511
Conversation
ae019ff
to
f9e6e95
Compare
Bonjour @ecourreges-orange Thanks for the PR, CI build in progress. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2511 +/- ##
==========================================
+ Coverage 87.08% 87.12% +0.05%
==========================================
Files 200 200
Lines 6103 6109 +6
==========================================
+ Hits 5314 5322 +8
+ Misses 789 787 -2
|
f1dde84
to
9ef1ed6
Compare
… active span with a default invalid span, which is especially useful when used with CompositePropagator (open-telemetry#2504) [TEST] Change wrong string "current-span" in unit tests to proper trace:kSpan which is "active_span"
9ef1ed6
to
5b7fa3c
Compare
You're welcome, happy to be contributing! |
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.
LGTM, thanks for everything:
- the initial report
- the analysis
- the fix
- the unit tests
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.
LGTM. Thanks for the fix.
[API] Fix b3, w3c and jaeger propagators: they will not overwrite the active span with a default invalid span
This is especially useful when used with CompositePropagator (#2504)
[TEST] Change wrong string "current-span" in unit tests to proper trace:kSpan which is "active_span"
Fixes #2504
Changes
This changes how Extract returns when invalid or missing headers are provided: it now returns the original context with potentially no active_span (or not overwritten if one was present).
But this should not have side effects on client code which calls GetSpan(context), which answers a default invalid span when it does not find an active_span
CHANGELOG.md
updated for non-trivial changes