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

Extract HTTP request & response content length from headers #6415

Merged
merged 13 commits into from
Aug 5, 2022

Conversation

mateuszrzeszutek
Copy link
Member

... and deprecate the requestContentLength() and responseContentLength() methods in the getter interface. I figured that almost all instrumentations (except Armeria) get these from the headers anyway, so we might as well keep the API simple and remove them altogether.

@mateuszrzeszutek mateuszrzeszutek requested a review from a team as a code owner August 3, 2022 11:30
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@trask trask merged commit b2c90c7 into open-telemetry:main Aug 5, 2022
@anuragagarwal561994
Copy link
Contributor

@mateuszrzeszutek there is a slight catch here for apache http clients.

The instrumentation flows are as follows:

  1. User submits the request to client
  2. Instrumentation is applied on the request given by the user
  3. Client duplicates the request from the user
  4. Client applies request interceptor to this duplicated request where it adds the content length and other headers
  5. This new request is internal and hence never recorded

Thus in case of all apache http clients response content length is coming fine because the one that comes is the one delivered to the user back but the request content length headers and attributes are missing because the one submitted by the user is not the actual one that goes in the request.

@mateuszrzeszutek mateuszrzeszutek deleted the content-length branch October 3, 2022 11:13
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Oct 23, 2022
…emetry#6415)

* Extract HTTP request & response content length from headers

* remove unused method

* fix camel tests

* fix google http client tests

* fix HttpUrlConnection tests

* fix k8s and jaxrs tests

* fix aws tests

* actually fix aws tests 🤞

* fix elasticsearch tests

* fix ratpack tests

* fix spring webflux tests

* fix vertx tests

* fix reactor netty tests
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Oct 31, 2022
…emetry#6415)

* Extract HTTP request & response content length from headers

* remove unused method

* fix camel tests

* fix google http client tests

* fix HttpUrlConnection tests

* fix k8s and jaxrs tests

* fix aws tests

* actually fix aws tests 🤞

* fix elasticsearch tests

* fix ratpack tests

* fix spring webflux tests

* fix vertx tests

* fix reactor netty tests
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Dec 4, 2022
…emetry#6415)

* Extract HTTP request & response content length from headers

* remove unused method

* fix camel tests

* fix google http client tests

* fix HttpUrlConnection tests

* fix k8s and jaxrs tests

* fix aws tests

* actually fix aws tests 🤞

* fix elasticsearch tests

* fix ratpack tests

* fix spring webflux tests

* fix vertx tests

* fix reactor netty tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants