-
Notifications
You must be signed in to change notification settings - Fork 277
feat: Implement the Trace Response propagator #1397
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
feat: Implement the Trace Response propagator #1397
Conversation
|
|
...mental/lib/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator.rb
Outdated
Show resolved
Hide resolved
plantfansam
left a comment
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.
Looking good! I had a few questions/comments about the tests. I'd suggest adding a few test cases that push the edges: e.g. can we add assertions for what happens if span context is not valid?
...mental/lib/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
...mental/lib/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator.rb
Outdated
Show resolved
Hide resolved
Adds a TextMapPropagator in the experimental SDK to inject `traceresponse` headers as defined in the [Trace Context][1] draft specification. [1]: https://w3c.github.io/trace-context/#traceresponse-header
84a7f25 to
bd6402f
Compare
...mental/lib/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator.rb
Outdated
Show resolved
Hide resolved
.../test/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator_test.rb
Outdated
Show resolved
Hide resolved
plantfansam
left a comment
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!
...mental/lib/opentelemetry/sdk/trace/propagation/trace_context/response_text_map_propagator.rb
Outdated
Show resolved
Hide resolved
…_context/response_text_map_propagator.rb Co-authored-by: Francis Bogsanyi <[email protected]>
Adds an optional config for the rack instrumentation to use the [new traceresponse propagator][1]. [1]: open-telemetry/opentelemetry-ruby#1397
Adds an optional config for the rack instrumentation to use the [new traceresponse propagator][1]. [1]: open-telemetry/opentelemetry-ruby#1397
Adds a TextMapPropagator in the experimental SDK to inject
traceresponseheaders as defined in the Trace Context draft specification.Here is a gist demonstrating the propagator, based off of the existing http server example from this repo.