-
Notifications
You must be signed in to change notification settings - Fork 791
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
Add javaagent<->application context bridge for HttpRouteHolder #5838
Conversation
f35dd49
to
1abbad3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves #4319
🎉
...nv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java
Outdated
Show resolved
Hide resolved
...ry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java
Show resolved
Hide resolved
...ry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java
Show resolved
Hide resolved
06d6fd8
to
02733bc
Compare
// instrumentation-api specific instrumentation | ||
new HttpRouteStateInstrumentation()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution to this issue turned out to be a bit more complex than expected -- I don't particularly like this, but I had to insert additional instrumentation-api instrumentation into the opentelemetry-api module, cause it uses the AgentContextWrapper
. It doesn't add any muzzle dependencies though, so it should be completely neutral for applications that just use opentelemetry-api without instrumentation-api.
I haven't thought this through, so maybe missing something, but wondering if we can instead instrument |
(a bit like #4786) |
b3f1c93
to
28ef87c
Compare
I thought that this would be a bit more difficult, since we'd have to bridge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx, I have a couple more thoughts about bridging, will explore in a follow-up 👍
…telemetry#5838) * Add javaagent<->application context bridge for HttpRouteHolder * remove comments * fix broken http.route bridge * spotless * Move to a separate module
Resolves #4319 -- there are no more context keys worth bridging in both instrumentation-api and instrumentation-api-semconv, this was the last useful one.