Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,30 @@ muzzle {
}

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 this module's vthread21Test suite.
apply from: "$rootDir/gradle/slf4j-simple.gradle"

addTestSuiteForDir('latestDepTest', 'test')

// Separate suite for tests that need the JDK 21+ virtual-thread API. Kept apart from the
// default test suite so that the existing OkHttp 3.0 tests keep their Java 1.8 baseline.
addTestSuite('vthread21Test')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test can simply put under test - no need to have a different test suite


tasks.named("compileVthread21TestJava", JavaCompile) {
configureCompiler(it, 21)
}

tasks.named("vthread21Test", Test) {
testJvmConstraints {
minJavaVersion = JavaVersion.VERSION_21
}
Comment on lines +25 to +28
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add JDK 21 constraints for the virtual-thread suite

This suite is forced to run with a JDK 21 launcher, but it does not declare a task-specific testJvmConstraints { minJavaVersion = JavaVersion.VERSION_21 }. The repo's test-JVM plugin gates tasks from the requested -PtestJvm, not from a later launcher override, so lower-JDK shards such as -PtestJvm=8/11 still consider vthread21Test allowed and then run or resolve this JDK 21-only suite from check. Other per-suite JDK 21 tests in this repo add the constraint on the Test task; doing the same here keeps lower-JDK checks from unexpectedly executing this virtual-thread coverage.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DataDog fix this

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only run on private repositories.

}

tasks.named("check") {
dependsOn "vthread21Test"
}
Comment on lines +31 to +33
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this is needed? Normally you should use the testJvmConstraints to asses min jvm version for testing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that doesn't quite work here because it is a mix of okhttp and virtual threads that we need to test. The fix is with okhttp, but using virtual threads requires Java 21.


dependencies {
compileOnly(group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0')

Expand Down Expand Up @@ -38,4 +59,14 @@ dependencies {

testRuntimeOnly(project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter'))
testRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-net:java-net-1.8'))

// vthread21Test inherits testImplementation via addTestSuite's extendsFrom wiring.
vthread21TestImplementation(group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.11.0')
vthread21TestImplementation(group: 'com.squareup.okio', name: 'okio', version: '1.14.0')

// Pull in the JDK-21+ concurrent / lang instrumentations so the test installs the same
// TaskRunnerInstrumentation + VirtualThreadInstrumentation chain that profiling-backend
// exercises in production.
vthread21TestRuntimeOnly project(':dd-java-agent:instrumentation:java:java-concurrent:java-concurrent-21.0')
vthread21TestRuntimeOnly project(':dd-java-agent:instrumentation:java:java-lang:java-lang-21.0')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package datadog.trace.instrumentation.okhttp3;

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;

/**
* Captures the active scope at {@code AsyncCall.<init>} (i.e. the moment {@code
* Call.enqueue(callback)} was invoked on the user's thread) and stores it in the {@code
* ContextStore<Runnable, State>} shared with the rest of the concurrent instrumentation. {@code
* RunnableInstrumentation} then re-activates that scope on {@code AsyncCall.run()} entry, which
* overrides whatever scope {@code TaskRunner.run()} (or {@code beforeExecute}) put in place from
* the dispatcher's worker thread.
*
* <p>Without this, {@code TaskRunnerInstrumentation} captures whatever scope happens to be active
* on the worker thread when {@code Dispatcher.promoteAndExecute()} dequeues and submits the call —
* and when promotion runs from inside {@code Dispatcher.finished()} (i.e. recursively from a
* <em>different</em> AsyncCall's run()), that scope belongs to the finishing call, not to the
* caller who actually enqueued this AsyncCall. Result: under concurrent OkHttp load, {@code
* okhttp.request} spans cross-contaminate between traces.
*
* <p>{@code AsyncCall} is an inner class of {@code RealCall} and transitively implements {@link
* Runnable}.
*/
@AutoService(InstrumenterModule.class)
public final class AsyncCallInstrumentation extends InstrumenterModule.Tracing
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.
super("okhttp", "okhttp-3");
}

@Override
public String[] knownMatchingTypes() {
return new String[] {
"okhttp3.RealCall$AsyncCall",
};
}

@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);
}
}
}
Loading