-
Notifications
You must be signed in to change notification settings - Fork 373
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
[OpenTracing] Context propagation implementation #495
[OpenTracing] Context propagation implementation #495
Conversation
I'm looking into adding some integration tests for the tracer, to test round-trip |
end | ||
|
||
def header_to_baggage(key) | ||
key[BAGGAGE_PREFIX_FORMATTED.length, key.length].downcase |
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.
There's a possible issue with round-trip propagation here. I'm anticipating there's a case where you could have baggage items with names containing either -
s or uppercase characters. These should serialize to the carrier without issue, however, they'd probably fail to be extracted back to their original form, because how Rack forces headers to uppercase and underscores.
e.g. My-Baggage-Item
--> #inject
--> HTTP_OT_BAGGAGE_MY_BAGGAGE_ITEM
--> #extract
--> my_baggage_item
.
I'm not sure this is possible to correct for, because we lose resolution on whether the key should have these dashes or uppercase characters when it goes through this conversion. Just something to watch out for.
3e1d548
to
0c4cc15
Compare
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 👍
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.
👍
* Added: Propagators to OpenTracer. * Added: Specs for OpenTracer::Propagators. * Fixed: OpenTracer::Tracer#start_span not using provided SpanContext. * Added: OpenTracer context propagation integration specs.
* Added: Propagators to OpenTracer. * Added: Specs for OpenTracer::Propagators. * Fixed: OpenTracer::Tracer#start_span not using provided SpanContext. * Added: OpenTracer context propagation integration specs.
* Added: Propagators to OpenTracer. * Added: Specs for OpenTracer::Propagators. * Fixed: OpenTracer::Tracer#start_span not using provided SpanContext. * Added: OpenTracer context propagation integration specs.
This pull request implements context propagation using
Propagator
classes, which serialize/deserialize trace context from carriers.It provides full implementation for
Rack
andTextMap
formats.Binary
format is still a no-op.