Split out :instrumentation:netty:netty-4.1 library #6820
Conversation
...entelemetry/instrumentation/netty/v4_1/internal/client/HttpClientResponseTracingHandler.java
Show resolved
Hide resolved
...c/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyClientSingletons.java
Show resolved
Hide resolved
...c/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyServerSingletons.java
Show resolved
Hide resolved
| } else if (handler | ||
| .getClass() | ||
| .getName() | ||
| .startsWith("io.opentelemetry.instrumentation.netty.")) { | ||
| pipeline.removeLast(); |
There was a problem hiding this comment.
it's not clear to me why this is needed?
There was a problem hiding this comment.
This was a bit of a thorn in my side. So this instrumentation ensures that when a handler is removed, the associated otel tracing handler is also removed. But in splitting out the netty-4.1 javaagent stuff into a library, the package that the handlers lives in changes from io.opentelemetry.javaagent.instrumentation.netty.* to io.opentelemetry.instrumentation.netty.*.
There was a problem hiding this comment.
inside the Java agent, io.opentelemetry.instrumentation.netty.* should be shaded to io.opentelemetry.javaagent.shaded.instrumentation.netty.*
oh! but this string will get correspondingly shaded. ok, makes sense to me now, thx
There was a problem hiding this comment.
I didn't know that shading detects and changes string usage.. That makes sense now that I think about it because I tried to use a matches with a regex instead of startsWith and it wouldn't work.
...y/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/NettyClientTelemetryBuilder.java
Outdated
Show resolved
Hide resolved
Sure I can definitely do that. And happy to do it in a follow up or in this PR. Do you have a preference? |
Nah, just do whatever's more comfortable to you. |
|
I'm going to merge to make sure this makes it into the release, I'll open an issue to track adding tests |
Resolves #6734. Builds on #6811.