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

Apache http client 4.3 low level instrumentation #10253

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

laurit
Copy link
Contributor

@laurit laurit commented Jan 17, 2024

Not needing to keep track of redirects greatly simplifies the instrumentation.

@laurit laurit requested a review from a team January 17, 2024 10:12
@jeanbisutti
Copy link
Member

Not needing to keep track of redirects greatly simplifies the instrumentation.

Coud you please explain why it's not needed? Perhaps add a comment in the Java code about this?

@laurit
Copy link
Contributor Author

laurit commented Jan 18, 2024

@jeanbisutti Have a look at https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-client Spec says that Instrumentations SHOULD create an HTTP span for each attempt to send an HTTP request over the wire and If for some reason it is not possible to emit a span for each send attempt (because e.g. the instrumented library does not expose hooks that would allow this), instrumentations MAY create an HTTP span for the top-most operation of the HTTP client. We call the former a low level instrumentation and the latter high level instrumentation. Historically spec required high level instrumentation so that is what we usually implement. In this case besides being recommended by the spec low level instrumentation is also simpler.

Coud you please explain why it's not needed? Perhaps add a comment in the Java code about this?

With low level instrumentation we can start/end the span each time interceptor is called. With high level instrumentation we had to start the span when interceptor was first called and end it in the last call. Figuring out which of the interceptor calls was last required figuring out whether the client would be following the redirect or giving up.

@laurit laurit merged commit 32b3326 into open-telemetry:main Feb 14, 2024
47 checks passed
@laurit laurit deleted the apache-httpclient-4.3-low-level branch February 14, 2024 07:26
steverao pushed a commit to steverao/opentelemetry-java-instrumentation that referenced this pull request Feb 16, 2024
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.

3 participants