Library instrumentation for Java Servlet 3.0 Filters. #15188
Conversation
Includes handling for attaching all the async stuff, but can't support everything the java agent does. I could use help figuring out the right way to shadow all the necessary dependencies. I'm a bit rusty with our shadow usage. Depending on how aggressive we want to be, the dependency could be reversed so that `:servlet-3.0:javaagent` depends on this new module, but that's more aggressive. Perhaps shadow could prune unused classes (with `minimize()`) instead. Once we get the modules figured out, I can work on adding some unit tests. If we like this approach, it could probably be copied and modified pretty easily to support servlet 5, but I'm not familiar with that instrumentation.
| library("javax.servlet:javax.servlet-api:3.0.1") | ||
|
|
||
| // FIXME: These dependencies need to be shadowed into the library. | ||
| library(project(":instrumentation:servlet:servlet-3.0:javaagent")) |
There was a problem hiding this comment.
library instrumentations don't typically depend on javaagent instrumentation, could you move the helper into a common module or swap the dependency and have the javaagent depend on the library?
There was a problem hiding this comment.
I understand that isn't usually how it's done. I was trying to figure out if there was a simpler option besides moving everything around. There are a lot of related classes that would need to move, not just the helper class.
You shouldn't shadow anything. If you need to share code you can extract common code between the javaagent and library instrumentation into separate module or make javaagent module depend on the library module if that make sense. In your place I would focus on building a functioning instrumentation along with tests and explore options for sharing code later. It is quite possible that the agent code does things that don't make sense for the library instrumentation which could limit the amount of code that can be shared between them. |
| @@ -0,0 +1,43 @@ | |||
| package io.opentelemetry.javaagent.instrumentation.servlet.v3_0; | |||
|
|
|||
| import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Singletons.helper; | |||
There was a problem hiding this comment.
@laurit I'm not sure how to handle the dependency on classes like io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator from Servlet3Accessor referenced by Servlet3Singletons that seem to expect to be on the bootstrap classpath and are part of the javaagent-extension-api.
More and more it seems like there will be relatively little code that can be shared between the javaagent and library instrumentation.
There was a problem hiding this comment.
That one shouldn't be too hard to handle. You can easily remove that interface from Servlet3Accessor and have the usage in advice create a HttpServerResponseMutator that calls the appendHeader method in Servlet3Accessor.
3 participants
Includes handling for attaching all the async stuff, but can't support everything the java agent does.
I could use help figuring out the right way to shadow all the necessary dependencies. I'm a bit rusty with our shadow usage. Depending on how aggressive we want to be, the dependency could be reversed so that
:servlet-3.0:javaagentdepends on this new module, but that's more aggressive. Perhaps shadow could prune unused classes (withminimize()) instead.Once we get the modules figured out, I can work on adding some unit tests.
If we like this approach, it could probably be copied and modified pretty easily to support servlet 5, but I'm not familiar with that instrumentation.