Skip to content
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

Capture HTTP request/response headers as span attributes #906

Closed
ashu658 opened this issue Feb 8, 2022 · 10 comments
Closed

Capture HTTP request/response headers as span attributes #906

ashu658 opened this issue Feb 8, 2022 · 10 comments
Assignees

Comments

@ashu658
Copy link
Member

ashu658 commented Feb 8, 2022

Is your feature request related to a problem?
Python agent is not able to capture custom headers as of now. The specifications for capturing request/response headers is defined here http-request-response-header

Describe the solution you'd like
This feature aims to capture custom request/response headers with the help of environment variables.I think these environment variables should be same for all the frameworks (something like OTEL_PYTHON_CAPTURE_REQUEST_HEADERS, OTEL_PYTHON_CAPTURE_RESPONSE_HEADERS) and the request/response headers will only be added in case of a server span.

Currently, some frameworks like falcon and django captures custom request headers with the help of environment variables (i.e. OTEL_PYTHON_FALCON_TRACED_REQUEST_ATTRS etc). I suggest we should change the environment variable from being framework specific to be general for all the frameworks.

We can open multiple sub-issues to add support and test cases for each framework.

Additional context
With conditional server span creation #445 added we can ensure single server span creation and no two frameworks capturing the same headers.

@owais
Copy link
Contributor

owais commented Feb 9, 2022

I think these environment variables should be same for all the frameworks (something like OTEL_PYTHON_CAPTURE_REQUEST_HEADERS, OTEL_PYTHON_CAPTURE_RESPONSE_HEADERS) and the request/response headers will only be added in case of a server span.

It would be ideal if we could have same env vars for all Otel SDKs. Does the spec recommend anything?

@ashu658
Copy link
Member Author

ashu658 commented Feb 10, 2022

It would be ideal if we could have same env vars for all Otel SDKs.

Sure we can have the same env vars for all SDKs. We might want to keep the prefix 'OTEL' to distinguish that these env vars are used by otel sdks.

Does the spec recommend anything?

No, the spec doesn’t recommend anything. I see you have already asked for the specifications of env vars here but I don't think there has been further discussion around that.
Should I create a new issue for the specification of env vars? Not sure how to proceed here.

@owais
Copy link
Contributor

owais commented Feb 10, 2022

I think we can go ahead with OTEL_PYTHON_ env vars for now but you could also submit a PR in parallel the SDK spec proposing env vars for this functionality.

@ashu658
Copy link
Member Author

ashu658 commented Feb 11, 2022

sure :)

@ashu658
Copy link
Member Author

ashu658 commented Feb 18, 2022

Found that Java SIG is using language independent environment variables. Otel-Java-Env-Vars
Another thing to note is they have also captured client headers.

@lzchen
Copy link
Contributor

lzchen commented Mar 29, 2022

As part of this work, we should add an example of this usage for each instrumentation under configuration section of docstrings.

@ashu658
Copy link
Member Author

ashu658 commented Mar 31, 2022

I think we can go ahead with OTEL_PYTHON_ env vars for now but you could also submit a PR in parallel the SDK spec proposing env vars for this functionality.

Raised a PR proposing env vars open-telemetry/opentelemetry-specification#2461. Please have a look.

@ashu658
Copy link
Member Author

ashu658 commented Mar 31, 2022

As part of this work, we should add an example of this usage for each instrumentation under configuration section of docstrings.

Created #1023 for adding docstrings for instrumentations where changes are already merged.

@srikanthccv
Copy link
Member

@ashu658 Can be this be closed now?

@ashu658
Copy link
Member Author

ashu658 commented Apr 22, 2022

Yes @srikanthccv it can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants