Push the span context to gracefully handle multiple routers #15
Conversation
This file contains hidden or 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
In the previous version, I did not anticipate the possibility that one router might delegate to another for code-organization reasons, and so we would enter the telemetry handler code twice while trying to service a request and lose the parent context we attempted to save on the first time around. We witnessed this as traces in our aggregator/visualizer that were missing root spans. This change brings the code closer to what opentelemetry_telemetry does without adding that dependency; namely, it maintains the parent context as a list/stack and pushes onto it when a new span is created, popping it when a span completes. Also, if a context already exists, we don't attempt to extract propagation from the request headers. This change will also make the library more compatible with webserver instrumentation like opentelemetry_cowboy, which does the header extraction and context creation.
jamcito
approved these changes
Oct 19, 2021
|
@seancribbs @tsloughter trying to bring this back to life.. Any thoughts on what you'd like to see in this PR in order for it to be ready to be merged ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
In the previous version, I did not anticipate the possibility that one router
might delegate to another for code-organization reasons, and so we would enter
the telemetry handler code twice while trying to service a request and lose the
parent context we attempted to save on the first time around. We witnessed this
as traces in our aggregator/visualizer that were missing root spans.
This change brings the code closer to what
opentelemetry_telemetrydoes withoutadding that dependency; namely, it maintains the parent context as a list/stack
and pushes onto it when a new span is created, popping it when a span completes.
Also, if a context already exists, we don't attempt to extract propagation from
the request headers. This change will also make the library more compatible with
webserver instrumentation like
opentelemetry_cowboy, which does the headerextraction and context creation.