Added support for traceresponse headers #436
Merged
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
owais
requested review from
a team,
aabmass and
lzchen
and removed request for
a team
owais
force-pushed
the
traceresponse-header-propagation
branch
2 times, most recently
from
72589a6 to
10ea879
Compare
|
I created individual PRs first but they were very repetitive and quite small. I thought it'd be better to have them bundled as a single PR. This way they can be reviewed at once by the same reviewers. It should be more efficient I think. |
This was referenced Apr 13, 2021
owais
force-pushed
the
traceresponse-header-propagation
branch
8 times, most recently
from
4028808 to
f453f21
Compare
lzchen
reviewed
Apr 19, 2021
| @@ -273,5 +279,9 @@ def process_response( | |||
| ) | |||
| ) | |||
|
|
|||
| propagator = get_global_response_propagator() | |||
| if propagator: | |||
| propagator.inject(resp, setter=self.back_propagation_setter) | |||
There was a problem hiding this comment.
You are able to access a class attribute like this via self?
There was a problem hiding this comment.
Yes but it can lead to bugs later. Will change.
lzchen
reviewed
Apr 19, 2021
| @@ -264,3 +264,11 @@ def _end_span_after_iterating(iterable, span, tracer, token): | |||
| close() | |||
| span.end() | |||
| context.detach(token) | |||
|
|
|||
|
|
|||
| class BackPropagationSetter: | |||
owais
force-pushed
the
traceresponse-header-propagation
branch
4 times, most recently
from
4bf4ec0 to
9f33735
Compare
|
After sub-classing, docs job is failing with: The core repo GIT SHA was updated to tip of core main which contains the change. The file for which docs are being generated is obviously able to import the said class as it passes all tests and lint checks. I tried invalidating tox cache but it isn't helping either. Looks like docs job is not installing the latest core main or is not using the locally checked out copy for some reason. |
owais
force-pushed
the
traceresponse-header-propagation
branch
from
9f33735 to
c0afe1d
Compare
…umentations. This allows users to configure their web apps to inject trace context as headers in HTTP responses. This is useful when client side apps need to connect their spans with the server side spans e.g, in RUM products. Today the most practical way to do this is to use the Server-Timing header but in near future we might use the traceresponse header as described here: https://w3c.github.io/trace-context/#trace-context-http-response-headers-format Added trace response propagation support for: Django Falcon Flask Pyramid Tornado
owais
force-pushed
the
traceresponse-header-propagation
branch
from
c0afe1d to
9a710fb
Compare
|
@lzchen I'm at a loss. I'm reverting inheriting from ABC setter for now and added a TODO to unblock release for now. Will need more time to figure out why docs doesn't like it. I remember Srikanth had a similar issue last week so probably something is wrong with docs tox job that we need to look into anyway. |
|
@owais |
lzchen
approved these changes
Apr 19, 2021
phillipuniverse
added a commit
to phillipuniverse/opentelemetry-python-contrib
that referenced
this pull request
Nov 28, 2021
This asgi version is modeled after the original wsgi version in open-telemetry#436.
6 tasks
phillipuniverse
added a commit
to phillipuniverse/opentelemetry-python-contrib
that referenced
this pull request
Nov 28, 2021
This asgi version is modeled after the original wsgi version in open-telemetry#436.
phillipuniverse
added a commit
to phillipuniverse/opentelemetry-python-contrib
that referenced
this pull request
Dec 21, 2021
This asgi version is modeled after the original wsgi version in open-telemetry#436.
phillipuniverse
added a commit
to phillipuniverse/opentelemetry-python-contrib
that referenced
this pull request
Dec 23, 2021
This asgi version is modeled after the original wsgi version in open-telemetry#436 and corresponds to the SERVER span. Also cleans up some of the existing ASGI functionality to reduce complexity and make future contributions more straightforward.
owais
pushed a commit
that referenced
this pull request
Dec 25, 2021
This asgi version is modeled after the original wsgi version in #436 and corresponds to the SERVER span. Also cleans up some of the existing ASGI functionality to reduce complexity and make future contributions more straightforward.
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.
Description
Added opt-in support to return traceresponse headers for server instrumentations.
This allows users to configure their web apps to inject trace context
as headers in HTTP responses. This is useful when client side apps need
to connect their spans with the server side spans e.g, in RUM products.
Today the most practical way to do this is to use the
Server-Timingheader but in near future we might use the
traceresponseheader asdescribed here: https://w3c.github.io/trace-context/#trace-context-http-response-headers-format
Added trace response propagation support for:
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.