open-telemetry / opentelemetry-java-instrumentation

OpenTelemetry auto-instrumentation and instrumentation libraries for Java
https://opentelemetry.io
Apache License 2.0
1.92k stars 839 forks source link

Cross test javaagent instrumentation against other instrumentations targeted at the same library? #5261

Open trask opened 2 years ago

trask commented 2 years ago

E.g. netty-3.8 instrumentation is tested with netty-4.0 and netty-4.1 instrumentations applied.

Should we apply this pattern to all instrumentations with multiple versions, or decide that the muzzle range checks are all we need and abandon this practice?

iNikem commented 2 years ago

I already expressed my opinion, that we should test with the whole agent, not partial agents prepared specifically for a given module :)

mateuszrzeszutek commented 2 years ago

I already expressed my opinion, that we should test with the whole agent, not partial agents prepared specifically for a given module :)

That indeed sounds like a good idea; especially in light of our recent Armeria vs. Netty discussion 😄 And if we ever need to test a particular instrumentation in isolation, we can create a test suite/set that disables unwanted instrumentations.

trask commented 2 years ago

we should test with the whole agent, not partial agents prepared specifically for a given module

this sounds good to me, any thoughts how to implement?

iNikem commented 2 years ago

we should test with the whole agent, not partial agents prepared specifically for a given module

this sounds good to me, any thoughts how to implement?

io.opentelemetry.instrumentation.javaagent-testing.gradle.kts uses a minimal agent-for-testing and adds module-specific extension. It may depend on the full javaagent instead.

iNikem commented 2 years ago

@anuraaga was against this idea in the past because allegedly building the full agent will take too much time.

trask commented 2 years ago

sounds worth giving it a try and seeing what the perf impact is