Skip to content

Commit a1adf8e

Browse files
dougqhclaude
andcommitted
Address review feedback on okhttp AsyncCall instrumentation
- Scope AsyncCallInstrumentation's muzzle to a named [3.0,4.0) directive (okhttp-async-3) so the 3.x-only async-call capture no longer claims the module-wide [3.0,) range; real version gate remains knownMatchingTypes. - Remove two verbose comments amarziali flagged (the rationale lives in the vthread21Test javadoc, which documents and verifies it). - Remove the maxJavaVersion=24 cap on both okhttp vthread suites. The cap was mis-inherited from java-concurrent-21.0, whose cap exists because its own test uses StructuredTaskScope.ShutdownOnFailure (a preview API removed in JDK 25) - not an instrumentation limit. Verified okhttp 3.x/4.x/5.x vthread suites pass on JDK 25. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fbcd0a1 commit a1adf8e

3 files changed

Lines changed: 21 additions & 36 deletions

File tree

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
11
muzzle {
2+
// Core tracing (OkHttp3Instrumentation -> TracingInterceptor), IAST and AppSec reference OkHttp
3+
// types and apply to 3.x/4.x/5.x.
24
pass {
5+
name = 'okhttp-3'
36
group = "com.squareup.okhttp3"
47
module = "okhttp"
58
versions = "[3.0,)"
69
assertInverse = true
710
}
11+
// AsyncCallInstrumentation is 3.x-only: the matched type okhttp3.RealCall$AsyncCall relocated to
12+
// okhttp3.internal.connection.RealCall$AsyncCall in 4.x (handled by the okhttp-4.0 module). Its
13+
// advice references only java.lang.Runnable, never an OkHttp type, so muzzle has nothing
14+
// version-specific to enforce here — this directive documents the [3.0,4.0) boundary; the real
15+
// gate is knownMatchingTypes. No assertInverse for the same reason: with no OkHttp reference
16+
// muzzle would pass outside the range (nothing to be missing), contradicting the assertion.
17+
pass {
18+
name = 'okhttp-async-3'
19+
group = "com.squareup.okhttp3"
20+
module = "okhttp"
21+
versions = "[3.0,4.0)"
22+
}
823
}
924

1025
apply from: "$rootDir/gradle/java.gradle"
@@ -25,11 +40,6 @@ tasks.named("compileVthread21TestJava", JavaCompile) {
2540
tasks.named("vthread21Test", Test) {
2641
testJvmConstraints {
2742
minJavaVersion = JavaVersion.VERSION_21
28-
// Cap at 24 to match java-concurrent-21.0, which provides the virtual-thread scope propagation
29-
// (TaskRunner/VirtualThread instrumentation) this suite relies on and is only supported through
30-
// JDK 24. Without this, the suite runs on the JDK 25 / "tip" (26) CI shards and times out
31-
// because that propagation chain isn't active there.
32-
maxJavaVersion = JavaVersion.VERSION_24
3343
}
3444
}
3545

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

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,20 @@
1212
import java.util.Map;
1313
import net.bytebuddy.asm.Advice;
1414

15-
/**
16-
* Captures the active scope at {@code AsyncCall.<init>} (i.e. the moment {@code
17-
* Call.enqueue(callback)} was invoked on the user's thread) and stores it in the {@code
18-
* ContextStore<Runnable, State>} shared with the rest of the concurrent instrumentation. {@code
19-
* RunnableInstrumentation} then re-activates that scope on {@code AsyncCall.run()} entry, which
20-
* overrides whatever scope {@code TaskRunner.run()} (or {@code beforeExecute}) put in place from
21-
* the dispatcher's worker thread.
22-
*
23-
* <p>Without this, {@code TaskRunnerInstrumentation} captures whatever scope happens to be active
24-
* on the worker thread when {@code Dispatcher.promoteAndExecute()} dequeues and submits the call —
25-
* and when promotion runs from inside {@code Dispatcher.finished()} (i.e. recursively from a
26-
* <em>different</em> AsyncCall's run()), that scope belongs to the finishing call, not to the
27-
* caller who actually enqueued this AsyncCall. Result: under concurrent OkHttp load, {@code
28-
* okhttp.request} spans cross-contaminate between traces.
29-
*
30-
* <p>{@code AsyncCall} is an inner class of {@code RealCall} and transitively implements {@link
31-
* Runnable}. This module targets OkHttp 3.x, where it lives at {@code okhttp3.RealCall$AsyncCall}.
32-
* OkHttp 4.x+ relocated it to {@code okhttp3.internal.connection.RealCall$AsyncCall}, which is
33-
* handled by the separate {@code okhttp-4.0} module.
34-
*/
3515
@AutoService(InstrumenterModule.class)
3616
public final class AsyncCallInstrumentation extends InstrumenterModule.ContextTracking
3717
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
3818

3919
public AsyncCallInstrumentation() {
40-
// Re-use the existing "okhttp" / "okhttp-3" instrumentation names so we don't introduce a
41-
// separately-toggleable feature flag (DD_TRACE_OKHTTP_ASYNC_CALL_ENABLED). The capture here
42-
// is conceptually part of the OkHttp instrumentation — if you disable OkHttp tracing, you
43-
// also disable this capture, which is the right behavior.
44-
//
45-
// This is a ContextTracking module (like RunnableInstrumentation, which consumes the state we
46-
// write) rather than a Tracing module: its sole job is to propagate the captured scope through
47-
// the shared ContextStore<Runnable, State>, not to create spans of its own.
4820
super("okhttp", "okhttp-3");
4921
}
5022

23+
@Override
24+
public String muzzleDirective() {
25+
// 3.x only: okhttp3.RealCall$AsyncCall relocated in 4.x (handled by the okhttp-4.0 module).
26+
return "okhttp-async-3";
27+
}
28+
5129
@Override
5230
public String[] knownMatchingTypes() {
5331
return new String[] {

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ addTestSuiteForDir('vthread21Test5', 'vthread21Test')
3333
tasks.named(suite, Test) {
3434
testJvmConstraints {
3535
minJavaVersion = JavaVersion.VERSION_21
36-
// Cap at 24 to match java-concurrent-21.0, which provides the virtual-thread scope propagation
37-
// these suites rely on and is only supported through JDK 24. See okhttp-3.0 for details.
38-
maxJavaVersion = JavaVersion.VERSION_24
3936
}
4037
}
4138
tasks.named("check") {

0 commit comments

Comments
 (0)