-
Notifications
You must be signed in to change notification settings - Fork 576
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 ot-tracer-* propagator #562
Add ot-tracer-* propagator #562
Conversation
This commit adds support for the OpenTracing header format
Codecov Report
@@ Coverage Diff @@
## main #562 +/- ##
=======================================
+ Coverage 77.6% 77.9% +0.3%
=======================================
Files 54 55 +1
Lines 2536 2590 +54
=======================================
+ Hits 1969 2020 +51
- Misses 437 439 +2
- Partials 130 131 +1
|
carrier.Set(spanIDHeader, sc.SpanID.String()) | ||
} | ||
|
||
if sc.IsSampled() { |
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.
Wondering whether there's any value on injecting the sampled part if the trace/span ids are invalid.
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.
Yeah, probably this whole if/else can move inside the ids-validated branch above.
carrier.Set(spanIDHeader, sc.SpanID.String()) | ||
} | ||
|
||
if sc.IsSampled() { |
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.
Yeah, probably this whole if/else can move inside the ids-validated branch above.
if err != nil || !sc.IsValid() { | ||
return ctx | ||
} | ||
// TODO: implement extracting baggage |
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.
Please mention open-telemetry/opentelemetry-go#1493
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.
done!
It's not "semi-official OpenTracing". OpenTracing does not define propagation formats. Is there a spec for this format? Who is asking to support it and why (no ticket linked to the PR)? |
Updated the name to "OT" propagator based on the conversation in: open-telemetry/opentelemetry-specification#1391 |
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
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.
Please add a CHANGELOG.md
entry for this addition.
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Thanks for the review @Aneurysm9! Added an entry to the changelog |
This adds support for the semi-official OpenTracing (aka ot-tracer) header format. Supports propagation of trace context.
NOTE: The support for baggage is incomplete, as this requires iterating through all the keys in a carrier and prefix matching all keys starting with
ot-baggage-{key}
.Other implementations: