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

Allow request body to be collected as a span attribute #8778

Open
bhogayatakb opened this issue Jun 21, 2023 · 10 comments
Open

Allow request body to be collected as a span attribute #8778

bhogayatakb opened this issue Jun 21, 2023 · 10 comments
Assignees
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request

Comments

@bhogayatakb
Copy link

Proposing a new feature, where user can get the request parameters, i.e. headers, body etc. in resource attributes.
This can be controlled via a feature toggle as @zeitlinger suggested to me on slack

I would like to give a PR for this myself.
Need inputs from the team on this.
Also, I would like to know if this can be done via any central class instead of implementing this in each instrumentation ?!

@bhogayatakb bhogayatakb added the enhancement New feature or request label Jun 21, 2023
@laurit
Copy link
Contributor

laurit commented Jun 21, 2023

Instead of resource attribute you probably meant span attribute. Resource is immutable and can not be altered after sdk initialization. There already is an option to capture request & response headers, see https://opentelemetry.io/docs/instrumentation/java/automatic/agent-config/#capturing-http-request-and-response-headers also there is an experimental option to capture servlet request parameters https://opentelemetry.io/docs/instrumentation/java/automatic/agent-config/#capturing-servlet-request-parameters Check whether these would be sufficient for you, if not please do elaborate what exactly you need. Capturing request body is going to be tricky, you'd need to read the request and then somehow restore it so that the application can also read it. It would probably be easier to handle this inside the application and call setAttribute on the server span once you have access to the request body.

@bhogayatakb
Copy link
Author

bhogayatakb commented Jun 21, 2023

@laurit Yes, Span attribute makes more sense compared to resource attributes. Agreed !

Understood that using setAttribute from application side is a solution that can be performed without modifying anything in this project.

I need to capture request body in the span attributes for all the request handlers, in an application, irrespective of their framework,
I want user to just use otel JAR as -javaagent and provide a property i.e. -Dotel.traces.request.data-capture=true
So that user do not have to do setAttribute in all the request handlers.

3 Quick questions:

  1. Is there a simpler way for user to perform setAttribute, instead of doing it in each and every request handler ?
  2. Do you think this change might be helpful to this particular project ?
  3. Is there a central class (framework-agnostic) that has access to incoming requests, where I can do the needful code ?

@zeitlinger
Copy link
Member

I've heard this feature request before, so I think it might be useful to have.

As for getting started, you can look at the usages for

.
You will probably create one along side it (boolean).
You will see that the usages branch out to different server technologies, such as tomcat - because each server framework has a different request object.

@bhogayatakb bhogayatakb changed the title Allow request body to be collected as a resource attribute Allow request body to be collected as a span attribute Jun 21, 2023
@laurit
Copy link
Contributor

laurit commented Jun 21, 2023

Understood that using setAttribute from application side is a solution that can be performed without modifying anything in this project.

I need to capture request body in the span attributes for all the request handlers, in an application, irrespective of their framework, I want user to just use otel JAR as -javaagent and provide a property i.e. -Dotel.traces.request.data-capture=true So that user do not have to do setAttribute in all the request handlers.

I don't think you realize how much work implementing this for all frameworks could be and what kind of problems you might run into. I'd still suggest you start with the framework you are using and attempt to prototype this inside the application to get a feel how this might work. Once you have it figured out inside your application you can try to build a generic instrumentation for the framework you use.

  1. Is there a simpler way for user to perform setAttribute, instead of doing it in each and every request handler ?

Depends on the application and frameworks at hand. If the framework provides a way to intercept all the requests and has request body available at that point then it could be easy, if it does not you may need to instrument internals of the framework or resort to setting the attribute in multiple places. For example for java servlets you could use a servlet filter that wraps the http request. You could read the request input stream and in your request wrapper implement getInputStream/getReader to return what you already read.

  1. Do you think this change might be helpful to this particular project ?

As this feature has been requested before we'd gladly accept it.

  1. Is there a central class (framework-agnostic) that has access to incoming requests, where I can do the needful code ?

All http server instrumentations use https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java this extractor calls into various getter that extract information from the underlying request. The code that adds http headers to span attributes gets called in

for (String name : capturedRequestHeaders) {
List<String> values = getter.getHttpRequestHeader(request, name);
if (!values.isEmpty()) {
internalSet(attributes, requestAttributeKey(name), values);
}
}

Though this is not where the complexity of adding request body as span attribute lies. The real problem is that the request body is often exposed as a stream that you can read only once. If you read it before the application does you need to somehow restore the stream to its original state or replace it with another stream. Additionally you generally won't know what character encoding the data will be in or whether you'll get a multipart request. The request you get might also be very large so can't just read it all and keep in the memory. For every framework and server you intend to support you'll probably have to come up with a separate instrumentation.

@bhogayatakb
Copy link
Author

ohk, I see
Thanks @laurit , your inputs gave a lot more clarity on this.
At first, I'll be focused on implementing this in the Spring framework. Keeping in mind, the challenges you just mentioned above.

@trask trask added the contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome label Aug 10, 2023
@trask
Copy link
Member

trask commented Sep 18, 2023

linking related open-telemetry/oteps#234

@trask
Copy link
Member

trask commented Apr 11, 2024

linking related open-telemetry/semantic-conventions#857

@Cirilla-zmh
Copy link

Thanks @trask , I am currently in the same boat with this requirement.

I concur with @laurit's point that capturing the HTTP request and response bodies for all HTTP Servers is a challenging task, requiring an in-depth exploration of each type of HTTP server framework. However, for applications integrated with specific web development frameworks (such as Spring MVC), we can leverage some provided hook methods or advice internal methods to capture the bodies. (For instance, ResponseBodyAdvice).

I have completed the preliminary validation and would be delighted to contribute the implementation of this feature. If feasible, please consider assigning this issue to me.

@steverao
Copy link
Contributor

I have completed the preliminary validation and would be delighted to contribute the implementation of this feature. If feasible, please consider assigning this issue to me.

Feel free to create PR to solve it!

@oliver-zhang
Copy link
Contributor

oliver-zhang commented Jul 10, 2024

Thanks @trask , I am currently in the same boat with this requirement.

I concur with @laurit's point that capturing the HTTP request and response bodies for all HTTP Servers is a challenging task, requiring an in-depth exploration of each type of HTTP server framework. However, for applications integrated with specific web development frameworks (such as Spring MVC), we can leverage some provided hook methods or advice internal methods to capture the bodies. (For instance, ResponseBodyAdvice).

I have completed the preliminary validation and would be delighted to contribute the implementation of this feature. If feasible, please consider assigning this issue to me.

@Cirilla-zmh Could you share how you did it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants