Add agent instrumentation for Ratpack 1.7

[johnrengelman]

Implements agent based instrumentation for ratpack 1.7 including http client instrumentation.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: johnrengelman / name: John Engelman (75455b6, 2ddcf3a, 093aeaa, d8d4bf5)

[johnrengelman]

These tests run locally and the test error seems to be occurring in any PR that is triggering a run of the test partitions (see https://github.com/open-telemetry/opentelemetry-java-instrumentation/actions/runs/11629696246/job/32387158964?pr=12553) and not loading from cache. Not sure what to do with this since it doesn't appear to be related to this change.

[laurit]

These tests run locally and the test error seems to be occurring in any PR that is triggering a run of the test partitions (see https://github.com/open-telemetry/opentelemetry-java-instrumentation/actions/runs/11629696246/job/32387158964?pr=12553) and not loading from cache. Not sure what to do with this since it doesn't appear to be related to this change.

I tried running tests from your PR and the http client tests fail for me too. I think the issue is that you are missing instrumentation that sets up https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInitializer.java Also I think you should remove https://github.com/ratpack/opentelemetry-java-instrumentation/blob/9e0c64a0fb8776111b8aed709e7f74c32ada8055/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts#L30-L43 and include https://github.com/ratpack/opentelemetry-java-instrumentation/blob/9e0c64a0fb8776111b8aed709e7f74c32ada8055/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts#L17 using testInstrumentation. Though you will need to make sure that the 1.4 instrumentation is disabled for 1.7.

[johnrengelman]

@laurit Thanks, can you send which test fails for you and which gradle command you are running? ./gradlew :instrumentation:ratpack:ratpack-1.7:javaagent:test passes for me without issues.

Though you will need to make sure that the 1.4 instrumentation is disabled for 1.7.

Can you explain why you think I should do that? I would expect in a ratpack 1.7 application, that the 1.4 instrumentation is there and it's critical for passing along the OTEL context information through ratpack's async model.

[laurit]

@laurit Thanks, can you send which test fails for you and which gradle command you are running? ./gradlew :instrumentation:ratpack:ratpack-1.7:javaagent:test passes for me without issues.

It fails for me. Try running io.opentelemetry.javaagent.instrumentation.ratpack.v1_7.RatpackHttpClientTest from intellij.

Can you explain why you think I should do that? I would expect in a ratpack 1.7 application, that the 1.4 instrumentation is there and it's critical for passing along the OTEL context information through ratpack's async model.

Our tests are set up so that we test with the earliest version, which is also the version in the module name, and with the latest version -PtestLatestDeps=true flag. The latest dep test replaces version for library and testLibrary dependencies with the latest version. So the tests in the 1.4 module are actually also run against 1.9.0 (we ignore milestone and rc versions). The 1.7 instrumentation you built isn't really needed to make tests pass, if you'd disable it you'd get more tests passing than with the current incomplete instrumentation. You need to ensure that 1.4 instrumentation does not trigger for versions 1.7 and up so that it wouldn't interfere with your instrumentation. I had assumed that you wanted to have agent instrumentation based on 1.7 because it is simpler and possibly has more correct context propagation.

[johnrengelman]

This adds instrumentation that only works starting in ratpack 1.6 because of methods that were introduced to allow HttpClient interception. All the server side instrumentation from 1.4 works fine in later versions.

All this change really does is add ratpack level instrumentation for http client calls instead of netty level (which causes supression).
So it's the right thing to do here:

1. Make the ratpack-1.4 instrumentation turn off for ratpack 1.7+
2. Add the ratpack 1.4 instrumentation to also be added for ratpack 1.7 (https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/ratpack/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java#L32-L38)

It fails for me. Try running io.opentelemetry.javaagent.instrumentation.ratpack.v1_7.RatpackHttpClientTest from intellij.

Yeah, this all passes for me. I'm running with OpenJDK Termurin 17.

[trask]

Yeah, this all passes for me.

I checked and ./gradlew :instrumentation:ratpack:ratpack-1.7:javaagent:test fails locally for me on your PR branch as well. Maybe try --no-build-cache and/or :clean first?

(btw a couple of the latest CI failures outside of the ratpack module are due to #12579 which will get cleared up if you rebase)

[johnrengelman]

Figured this out - I had disabled the shadow relocation so I could debug the instrumentation. Reenabling it causes the failure. I'll find some time to work out the issues.

[johnrengelman]

So I suspect the issue is that in the test class, I'm not working with the shaded versions of some of these classes - https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/12572/files#diff-0a231ce184aa699e90d480be72001173b246f78a2104827ae473df46f9a6595dR37
Inside the javaagent code base ContextHolder is relocated to io.opentelemetry.javaagent.shaded.instrumentation.ratpack.v1_7.internal.ContextHolder, so the classes don't match up.

Any suggestions on the idiomatic way to do this? In an actual Ratpack app, the ExecInitializer gets bound into the server's registry when it's built, but in these tests, we're using the Ratpack ExecHarness which doesn't use that instrumentation.

[johnrengelman]

Got this all working now and the tests pass. I had to change a bit of the instrumentation to make it work consistently with with ExecHarness, but that's actually a bit nicer. Rebased this to the current main branch as well.