ASGI: Capture custom request response headers #1004
Conversation
…ders as span attributes. Issue: open-telemetry#919
| @@ -342,6 +384,13 @@ async def __call__(self, scope, receive, send): | |||
| for key, value in attributes.items(): | |||
| current_span.set_attribute(key, value) | |||
|
|
|||
| if span.kind == trace.SpanKind.SERVER: | |||
There was a problem hiding this comment.
I guess if current_span.kind == trace.SpanKind.SERVER: should be used here.
| @@ -395,6 +444,17 @@ async def otel_send(message): | |||
| set_status_code(server_span, 200) | |||
| set_status_code(send_span, 200) | |||
| send_span.set_attribute("type", message["type"]) | |||
| if ( | |||
| server_span.kind == trace.SpanKind.SERVER | |||
There was a problem hiding this comment.
We should check if server_span is a recording span before checking its kind. NonRecordingSpan do not contain kind attribute. (#998 was reported for wsgi based frameworks for the same)
...tation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py
Outdated
Show resolved
Hide resolved
| return attributes | ||
|
|
||
|
|
||
| def collect_custom_response_headers_attributes(message): |
There was a problem hiding this comment.
This can be ok, but please make sure any new public (non-underscore prefixed) function or symbol is really necessary and meant to be used directly by the user. If not, please prefix them with underscores.
There was a problem hiding this comment.
I understand. This functions will be used by other frameworks like django or FastAPI to fetch the custom headers and add them to spans as attributes. (just like collect_request_attributes/add_response_attributes)
…ure the span is recording before adding the attributes to span. - changed span to current_span to make sure attributes are being added to proper span.
|
@ocelotl @codeboten @lzchen Please do review and provide your approval on this PR. |
sanketmehta28
changed the title
Description
This PR will add a feature to add custom request and response headers of http as well as websocket application to span as attributes.
Fixes # #919
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
I have added a new sample http_app_with_custom_headers and new websocket_app_with_custom_headers which will gives us sample apps to test and I have created a test class which will check expected as well as not expected span attributes in both type of applications
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.