Context propagation in opentracing bridge #525
Merged
+448
−119
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krnowak
requested review from
iredelmeier,
jmacd,
lizthegrey,
MrAlias,
paivagustavo,
rghetia and
tedsuo
as code owners
jmacd
approved these changes
Mar 6, 2020
We will put this map into the context with correlation context functions, and that is easier if we have correlation.Map, not map[string]string.
The otel propagators are now kinda sorta usable for opentracing bridge. Some more work is needed to make it fully work, though - correlation context set with the otel API is not propagated to OT spans as baggage items yet.
This introduces two kinds of hooks into the correlation context code. The set hook gets called every time we set a Map in the context. The hook receives a context with the Map and returns a new context. The get hook gets called every time we get a Map from the context. The hook receives the context and the map, and returns a new Map. These hooks will be used for correlation context and baggage items propagation between the Otel and OT APIs.
So I do not need to remember about initializing a newly added member in several places now.
This uses the set hook functionality to propagate correlation context changes from Otel to OT spans by inserting keys and values into the baggage items. The get hook functionality is used to propagate baggage items from active OT span into the otel correlation context.
This prepares the context by installing the hooks, so the correlation context and baggage items can be propagated between the APIs.
We will need this for testing the correlation context and baggage items propagation between the APIs.
The test adds a baggage item to active OT span and some correlation key value to current Otel span. Then makes sure that the OT span contains both the baggage item and some translated version of the correlation key value its Otel sibling got, and that the Otel span contains both the correlation key value and the baggage item its OT sibling got.
krnowak
force-pushed
the
ctx-prop-in-ot-bridge
branch
from
0580976 to
9a42b69
Compare
MikeGoldsmith
pushed a commit
to MikeGoldsmith/opentelemetry-go
that referenced
this pull request
Mar 13, 2020
* Propagate context changes in mix tests We will need this for testing the correlation context and baggage items propagation between the APIs. * Add baggage interoperation tests The test adds a baggage item to active OT span and some correlation key value to current Otel span. Then makes sure that the OT span contains both the baggage item and some translated version of the correlation key value its Otel sibling got, and that the Otel span contains both the correlation key value and the baggage item its OT sibling got. * Add hooks functionality to baggage propagation This introduces two kinds of hooks into the correlation context code. The set hook gets called every time we set a Map in the context. The hook receives a context with the Map and returns a new context. The get hook gets called every time we get a Map from the context. The hook receives the context and the map, and returns a new Map. These hooks will be used for correlation context and baggage items propagation between the Otel and OT APIs. * Warn on foreign opentracing span * fixup for using otel propagators * Add utility function for setting up bridge and context This prepares the context by installing the hooks, so the correlation context and baggage items can be propagated between the APIs. * Add bridge span constructor So I do not need to remember about initializing a newly added member in several places now. * Propagate baggage across otel and OT APIs This uses the set hook functionality to propagate correlation context changes from Otel to OT spans by inserting keys and values into the baggage items. The get hook functionality is used to propagate baggage items from active OT span into the otel correlation context. * Use correlation Map for baggage items We will put this map into the context with correlation context functions, and that is easier if we have correlation.Map, not map[string]string. * Use otel propagators in bridge The otel propagators are now kinda sorta usable for opentracing bridge. Some more work is needed to make it fully work, though - correlation context set with the otel API is not propagated to OT spans as baggage items yet. Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the context propagation in opentracing bridge in terms of OpenTelemetry context propagation.
I had to add some hacky hooky code to correlation package to make the otel span to know about baggage items set with the OT API, and to make the OT span to know about correlation key values set with Otel API. Hopefully it's not too scary, but it's always making me wonder if things won't go boom with threads. I think I'll need to go through the code with this in mind once again, but in the mean time I'd like to get a review of the current state.