fix(okhttp): split AsyncCall scope capture by major version, verify 4.x/5.x

Address review feedback on the virtual-thread Dispatcher trace-contamination fix:

- Split the AsyncCall.<init> scope capture into per-major instrumentations so
  each is tested against its own OkHttp version. okhttp-3.0 keeps the v3 type
  (okhttp3.RealCall$AsyncCall); a new okhttp-4.0 module matches the relocated
  v4+ type (okhttp3.internal.connection.RealCall$AsyncCall). The okhttp-4.0
  suite runs the regression against OkHttp 4.12.0 and 5.4.0 (5.x is Kotlin
  Multiplatform; classes ship in okhttp-jvm, resolved via Gradle variant
  metadata). Muzzle covers 4.0.0 -> 5.4.0.
- Both instrumentations now extend InstrumenterModule.ContextTracking (matching
  RunnableInstrumentation, the consumer of the state they write) rather than
  Tracing -- their job is scope propagation, not span creation.
- Drop the redundant cachedThreadPoolDispatcher test (covered by OkHttp3AsyncTest)
  and document that only the concurrent test exercises the contamination path.
- Cap the vthread suites at maxJavaVersion=24 to match java-concurrent-21.0,
  which provides the virtual-thread propagation they depend on and is only
  supported through JDK 24. This fixes the TimeoutExceptions on the JDK 25 / 26
  ("tip") CI shards, where that propagation chain isn't active.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: dougqh

dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle

@@ -25,6 +25,11 @@ tasks.named("compileVthread21TestJava", JavaCompile) {
 tasks.named("vthread21Test", Test) {
   testJvmConstraints {
     minJavaVersion = JavaVersion.VERSION_21
+    // Cap at 24 to match java-concurrent-21.0, which provides the virtual-thread scope propagation
+    // (TaskRunner/VirtualThread instrumentation) this suite relies on and is only supported through
+    // JDK 24. Without this, the suite runs on the JDK 25 / "tip" (26) CI shards and times out
+    // because that propagation chain isn't active there.
+    maxJavaVersion = JavaVersion.VERSION_24
   }
 }
 

dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AsyncCallInstrumentation.java

@@ -28,24 +28,30 @@
  * okhttp.request} spans cross-contaminate between traces.
  *
  * <p>{@code AsyncCall} is an inner class of {@code RealCall} and transitively implements {@link
- * Runnable}.
+ * Runnable}. This module targets OkHttp 3.x, where it lives at {@code okhttp3.RealCall$AsyncCall}.
+ * OkHttp 4.x+ relocated it to {@code okhttp3.internal.connection.RealCall$AsyncCall}, which is
+ * handled by the separate {@code okhttp-4.0} module.
  */
 @AutoService(InstrumenterModule.class)
-public final class AsyncCallInstrumentation extends InstrumenterModule.Tracing
+public final class AsyncCallInstrumentation extends InstrumenterModule.ContextTracking
     implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
 
   public AsyncCallInstrumentation() {
     // Re-use the existing "okhttp" / "okhttp-3" instrumentation names so we don't introduce a
     // separately-toggleable feature flag (DD_TRACE_OKHTTP_ASYNC_CALL_ENABLED). The capture here
     // is conceptually part of the OkHttp instrumentation — if you disable OkHttp tracing, you
     // also disable this capture, which is the right behavior.
+    //
+    // This is a ContextTracking module (like RunnableInstrumentation, which consumes the state we
+    // write) rather than a Tracing module: its sole job is to propagate the captured scope through
+    // the shared ContextStore<Runnable, State>, not to create spans of its own.
     super("okhttp", "okhttp-3");
   }
 
   @Override
   public String[] knownMatchingTypes() {
     return new String[] {
-      "okhttp3.RealCall$AsyncCall",
+      "okhttp3.RealCall$AsyncCall", // OkHttp 3.x
     };
   }
 

dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/vthread21Test/java/OkHttpVirtualThreadDispatcherTest.java

@@ -34,16 +34,19 @@
  * {@code Executors.newCachedThreadPool(...)} to {@code
  * Executors.newThreadPerTaskExecutor(Thread.ofVirtual().name(prefix, start).factory())}.
  *
- * <p>The test runs both shapes against the same {@link HttpServer} mock. Inside a manually
- * activated "parent" span it does {@code client.newCall(request).enqueue(callback)} and waits on a
- * latch for the callback. The agent's OkHttp instrumentation injects {@code TracingInterceptor},
- * which creates the {@code okhttp.request} client span using whatever scope is active on the
- * dispatcher worker. The assertions verify the client span lands under the parent &mdash; i.e., the
- * dispatcher's worker thread saw the propagated scope.
+ * <p>Each test runs against the same {@link HttpServer} mock. Inside a manually activated "parent"
+ * span it does {@code client.newCall(request).enqueue(callback)} and waits on a latch for the
+ * callback. The agent's OkHttp instrumentation injects {@code TracingInterceptor}, which creates
+ * the {@code okhttp.request} client span using whatever scope is active on the dispatcher worker.
+ * The assertions verify the client span lands under the parent &mdash; i.e., the dispatcher's
+ * worker thread saw the propagated scope.
  *
- * <p>If propagation fails for the virtual-thread shape (the failure profiling-backend is reporting)
- * the {@code okhttp.request} span will either become a root span in its own trace or be parented
- * under nothing, and {@code assertEquals(parentSpan.getSpanId(), okhttpSpan.getParentId())} fails.
+ * <p>{@code concurrentVirtualThreadPerTaskDispatcher_keepsEachTraceSeparate} is the regression test
+ * for the fix: it forces dispatcher-queue contention so calls are promoted from {@code
+ * Dispatcher.finished()} on a sibling's worker thread, and fails (cross-trace contamination)
+ * without the {@code AsyncCall.<init>} scope capture. {@code virtualThreadPerTaskDispatcher_…} is a
+ * single-shot baseline: it confirms basic propagation through the virtual-thread dispatcher but
+ * does not by itself exercise the contamination path (single-shot requests never queue).
  */
 class OkHttpVirtualThreadDispatcherTest extends AbstractInstrumentationTest {
 
@@ -114,19 +117,6 @@ public void onFailure(Call call, IOException e) {
     }
   }
 
-  @Test
-  void cachedThreadPoolDispatcher_parentsOkHttpSpanUnderParent() throws Exception {
-    ExecutorService dispatcherExecutor = Executors.newCachedThreadPool();
-    OkHttpClient client = buildClient(dispatcherExecutor);
-    try {
-      runUnderParent(client, new CountDownLatch(1));
-    } finally {
-      dispatcherExecutor.shutdown();
-    }
-
-    assertOkHttpSpanParentedUnderParent();
-  }
-
   @Test
   void virtualThreadPerTaskDispatcher_parentsOkHttpSpanUnderParent() throws Exception {
     // Exact shape from profiling-backend PR#8520.

dd-java-agent/instrumentation/okhttp/okhttp-4.0/build.gradle

@@ -0,0 +1,67 @@
+muzzle {
+  // No assertInverse: the advice references only java.lang.Runnable plus agent-bootstrap helpers,
+  // never an OkHttp type, so muzzle has nothing version-specific to fail on for pre-4.x. The real
+  // version gate is knownMatchingTypes — okhttp3.internal.connection.RealCall$AsyncCall only exists
+  // in 4.x+, so the instrumentation simply doesn't match on 3.x (which okhttp-3.0 handles instead).
+  pass {
+    group = "com.squareup.okhttp3"
+    module = "okhttp"
+    versions = "[4.0,)"
+  }
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+// Use slf4j-simple for tests; logback's synchronized appenders can pin virtual-thread carriers
+// when many vthreads log concurrently, which deadlocks the virtual-thread suites.
+apply from: "$rootDir/gradle/slf4j-simple.gradle"
+
+// The only coverage here is the virtual-thread regression suite, which needs the JDK 21+ API. We
+// run the same test source against both OkHttp 4.x and 5.x:
+//   - 4.x lives at com.squareup.okhttp3:okhttp
+//   - 5.x became a Kotlin-Multiplatform library; com.squareup.okhttp3:okhttp is a metadata shell and
+//     the JVM classes ship in com.squareup.okhttp3:okhttp-jvm (Gradle resolves this automatically
+//     via variant metadata). The matched type okhttp3.internal.connection.RealCall$AsyncCall is at
+//     the same FQN in both, so a single instrumentation covers both majors.
+// Both suites share src/vthread21Test/java and are kept apart only to bind different OkHttp versions.
+addTestSuiteForDir('vthread21Test4', 'vthread21Test')
+addTestSuiteForDir('vthread21Test5', 'vthread21Test')
+
+['vthread21Test4', 'vthread21Test5'].each { suite ->
+  tasks.named("compile${suite.capitalize()}Java", JavaCompile) {
+    configureCompiler(it, 21)
+  }
+  tasks.named(suite, Test) {
+    testJvmConstraints {
+      minJavaVersion = JavaVersion.VERSION_21
+      // Cap at 24 to match java-concurrent-21.0, which provides the virtual-thread scope propagation
+      // these suites rely on and is only supported through JDK 24. See okhttp-3.0 for details.
+      maxJavaVersion = JavaVersion.VERSION_24
+    }
+  }
+  tasks.named("check") {
+    dependsOn suite
+  }
+}
+
+dependencies {
+  compileOnly(group: 'com.squareup.okhttp3', name: 'okhttp', version: '4.0.0')
+
+  // OkHttp 4.x (Kotlin) pulls kotlin-stdlib and okio 2.x transitively.
+  vthread21Test4Implementation(group: 'com.squareup.okhttp3', name: 'okhttp', version: '4.12.0')
+  // OkHttp 5.x resolves to okhttp-jvm via Gradle variant metadata; pulls kotlin-stdlib 2.x + okio 3.x.
+  vthread21Test5Implementation(group: 'com.squareup.okhttp3', name: 'okhttp', version: '5.4.0')
+
+  // The core OkHttp tracing (OkHttp3Instrumentation -> TracingInterceptor, which creates the
+  // okhttp.request span) lives in the okhttp-3.0 module and applies to 4.x/5.x too. Pull it in so
+  // the tests exercise the full chain: core tracing + this module's AsyncCall scope capture.
+  vthread21Test4RuntimeOnly project(':dd-java-agent:instrumentation:okhttp:okhttp-3.0')
+  vthread21Test5RuntimeOnly project(':dd-java-agent:instrumentation:okhttp:okhttp-3.0')
+
+  // Pull in the JDK-21+ concurrent / lang instrumentations so the tests install the same
+  // TaskRunnerInstrumentation + VirtualThreadInstrumentation chain that profiling-backend
+  // exercises in production.
+  vthread21Test4RuntimeOnly project(':dd-java-agent:instrumentation:java:java-concurrent:java-concurrent-21.0')
+  vthread21Test4RuntimeOnly project(':dd-java-agent:instrumentation:java:java-lang:java-lang-21.0')
+  vthread21Test5RuntimeOnly project(':dd-java-agent:instrumentation:java:java-concurrent:java-concurrent-21.0')
+  vthread21Test5RuntimeOnly project(':dd-java-agent:instrumentation:java:java-lang:java-lang-21.0')
+}

dd-java-agent/instrumentation/okhttp/okhttp-4.0/src/main/java/datadog/trace/instrumentation/okhttp4/AsyncCallInstrumentation.java

@@ -0,0 +1,65 @@
+package datadog.trace.instrumentation.okhttp4;
+
+import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.capture;
+import static java.util.Collections.singletonMap;
+import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.bootstrap.InstrumentationContext;
+import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
+import java.util.Map;
+import net.bytebuddy.asm.Advice;
+
+/**
+ * OkHttp 4.x+ variant of the {@code AsyncCall.<init>} scope capture. Identical in behavior to the
+ * {@code okhttp-3.0} module's instrumentation — see that class for the full explanation of the
+ * dispatcher-recursion failure mode — but matches the relocated 4.x type.
+ *
+ * <p>{@code AsyncCall} is an inner class of {@code RealCall} and transitively implements {@link
+ * Runnable}. OkHttp 4.x moved {@code RealCall} from {@code okhttp3} into {@code
+ * okhttp3.internal.connection}, so the nested type is {@code
+ * okhttp3.internal.connection.RealCall$AsyncCall}.
+ */
+@AutoService(InstrumenterModule.class)
+public final class AsyncCallInstrumentation extends InstrumenterModule.ContextTracking
+    implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
+
+  public AsyncCallInstrumentation() {
+    // Re-use the existing "okhttp" instrumentation name so this capture is enabled/disabled with
+    // OkHttp tracing as a whole, rather than introducing a separately-toggleable feature flag.
+    //
+    // This is a ContextTracking module (like RunnableInstrumentation, which consumes the state we
+    // write) rather than a Tracing module: its sole job is to propagate the captured scope through
+    // the shared ContextStore<Runnable, State>, not to create spans of its own.
+    super("okhttp", "okhttp-4");
+  }
+
+  @Override
+  public String[] knownMatchingTypes() {
+    return new String[] {
+      "okhttp3.internal.connection.RealCall$AsyncCall", // OkHttp 4.x+
+    };
+  }
+
+  @Override
+  public Map<String, String> contextStore() {
+    // Same Runnable -> State store that RunnableInstrumentation reads from.
+    return singletonMap("java.lang.Runnable", State.class.getName());
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(isConstructor(), getClass().getName() + "$Construct");
+  }
+
+  public static final class Construct {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void captureScope(@Advice.This Runnable asyncCall) {
+      // AdviceUtils.capture is a no-op when async propagation is disabled or there's no active
+      // span — same behavior as the rest of the concurrent instrumentation.
+      capture(InstrumentationContext.get(Runnable.class, State.class), asyncCall);
+    }
+  }
+}