From 5e64fba2fc9e974721e1dd3f60ec3b8d2063c13c Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Fri, 8 May 2026 15:15:09 +0200 Subject: [PATCH 1/2] fix: proper handling of bazel test runner suite events --- .../junit4/JUnit4TracingListener.java | 75 --------------- .../instrumentation/junit4/JUnit4Utils.java | 7 +- ...azelRunNotifierWrapperInstrumentation.java | 92 +++++++++++++++++++ 3 files changed, 96 insertions(+), 78 deletions(-) create mode 100644 dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java index fd6954d007b..afa43e45493 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java @@ -8,11 +8,8 @@ import datadog.trace.bootstrap.ContextStore; import java.lang.reflect.Method; import java.util.List; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import org.junit.Ignore; import org.junit.runner.Description; -import org.junit.runner.Result; import org.junit.runner.notification.Failure; public class JUnit4TracingListener extends TracingListener { @@ -22,26 +19,6 @@ public class JUnit4TracingListener extends TracingListener { private final ContextStore executionTrackers; - /** - * Suites for which {@code onTestSuiteStart} has been fired (from either the normal - * ParentRunner-based flow or via lazy-registration in {@link #testStarted}). Used to keep - * lifecycle events idempotent and to know which auto-started suite still needs closing. - */ - private final Set startedSuites = ConcurrentHashMap.newKeySet(); - - /** - * Last suite lazy-started from {@link #testStarted} because no {@link #testSuiteStarted} event - * was observed for it first. This has been seen under {@code - * com.google.testing.junit.runner.BazelTestRunner}, where the suite-start advice in {@code - * JUnit4SuiteEventsInstrumentation} does not fire for reasons still to be pinpointed (likely a - * classloader or runner-wrapping quirk specific to the Bazel test launcher). Closed when the next - * test belongs to a different suite, or when the whole test run finishes. - * - *

TODO: investigate the exact cause under {@code BazelTestRunner} and add a dedicated - * instrumentation that emits proper suite-lifecycle events instead of relying on this fallback. - */ - private volatile TestSuiteDescriptor autoStartedSuite; - public JUnit4TracingListener(ContextStore executionTrackers) { this.executionTrackers = executionTrackers; } @@ -55,9 +32,6 @@ public void testSuiteStarted(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); - if (!startedSuites.add(suiteDescriptor)) { - return; // already started (idempotent vs. lazy-registration or duplicate events) - } Class testClass = description.getTestClass(); String testSuiteName = JUnit4Utils.getSuiteName(testClass, description); List categories = JUnit4Utils.getCategories(testClass, null); @@ -84,9 +58,6 @@ public void testSuiteFinished(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); - if (!startedSuites.remove(suiteDescriptor)) { - return; // never started - } TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.JUNIT4) .onTestSuiteFinish(suiteDescriptor, null); @@ -102,8 +73,6 @@ public void testStarted(final Description description) { TestDescriptor testDescriptor = JUnit4Utils.toTestDescriptor(description); TestSourceData testSourceData = JUnit4Utils.toTestSourceData(description); - lazyStartSuiteIfNeeded(suiteDescriptor, description, testSourceData); - String testName = JUnit4Utils.getTestName(description, testSourceData.getTestMethod()); String testParameters = JUnit4Utils.getParameters(description); List categories = @@ -124,50 +93,6 @@ public void testStarted(final Description description) { executionTrackers.get(description)); } - @Override - public void testRunFinished(Result result) { - closeAutoStartedSuite(); - } - - private void lazyStartSuiteIfNeeded( - TestSuiteDescriptor newSuite, Description description, TestSourceData testSourceData) { - if (startedSuites.contains(newSuite)) { - return; - } - closeAutoStartedSuite(); - - Class testClass = testSourceData.getTestClass(); - String testSuiteName = JUnit4Utils.getSuiteName(testClass, description); - List categories = JUnit4Utils.getCategories(testClass, null); - TestEventsHandlerHolder.HANDLERS - .get(TestFrameworkInstrumentation.JUNIT4) - .onTestSuiteStart( - newSuite, - testSuiteName, - FRAMEWORK_NAME, - FRAMEWORK_VERSION, - testClass, - categories, - false, - TestFrameworkInstrumentation.JUNIT4, - null); - startedSuites.add(newSuite); - autoStartedSuite = newSuite; - } - - private void closeAutoStartedSuite() { - TestSuiteDescriptor suite = autoStartedSuite; - if (suite == null) { - return; - } - autoStartedSuite = null; - if (startedSuites.remove(suite)) { - TestEventsHandlerHolder.HANDLERS - .get(TestFrameworkInstrumentation.JUNIT4) - .onTestSuiteFinish(suite, null); - } - } - @Override public void testFinished(final Description description) { if (JUnit4Utils.isJUnitPlatformRunnerTest(description)) { diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java index 697c38d56e2..63dc45df761 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java @@ -117,10 +117,11 @@ public static List runListenersFromRunNotifier(final RunNotifier ru } /** - * Walks through {@link RunNotifier} wrappers (e.g. Bazel's {@code RunNotifierWrapper}) so the - * effective {@code listeners} field is read, not the wrapper's own (forwarded) one. + * Walks through {@link RunNotifier} wrappers (e.g. Bazel's {@code RunNotifierWrapper}) and + * returns the inner notifier whose {@code listeners} field actually receives {@code addListener} + * calls. Returns the input untouched when it is not a known wrapper. */ - private static RunNotifier unwrapRunNotifier(RunNotifier notifier) { + public static RunNotifier unwrapRunNotifier(RunNotifier notifier) { RunNotifier current = notifier; for (int i = 0; i < 8 && current != null; i++) { if (!isBazelRunNotifierWrapper(current.getClass())) { diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java new file mode 100644 index 00000000000..b9bcae00eb7 --- /dev/null +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java @@ -0,0 +1,92 @@ +package datadog.trace.instrumentation.junit4; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import net.bytebuddy.asm.Advice; +import org.junit.runner.Description; +import org.junit.runner.notification.RunNotifier; + +/** + * Restores suite lifecycle events when JUnit 4.13+ tests run under Bazel's {@code + * com.google.testing.junit.runner.BazelTestRunner}. + * + *

Bazel's {@code com.google.testing.junit.junit4.runner.RunNotifierWrapper} (last touched in + * 2015) explicitly delegates {@code addListener}, {@code fireTestStarted}, etc. to the inner + * notifier, but does not override {@link RunNotifier#fireTestSuiteStarted(Description)} and {@link + * RunNotifier#fireTestSuiteFinished(Description)} — those were added in JUnit 4.13. {@link + * org.junit.runners.ParentRunner#run} therefore fires the suite-lifecycle events on the wrapper's + * own (always empty) listener list, and our tracing listener — installed on the inner notifier via + * the wrapper's delegating {@code addListener} — never sees them. + * + *

This advice intercepts {@link RunNotifier#fireTestSuiteStarted(Description)} and {@link + * RunNotifier#fireTestSuiteFinished(Description)} on the wrapper instance and re-fires the event on + * the inner notifier, where the listener actually lives. Calls on a non-wrapper notifier are a + * no-op. + */ +@AutoService(InstrumenterModule.class) +public class BazelRunNotifierWrapperInstrumentation extends InstrumenterModule.CiVisibility + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public BazelRunNotifierWrapperInstrumentation() { + super("ci-visibility", "junit-4"); + } + + @Override + public String instrumentedType() { + return "org.junit.runner.notification.RunNotifier"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JUnit4Utils", + packageName + ".TracingListener", + packageName + ".SkippedByDatadog", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("fireTestSuiteStarted").and(takesArgument(0, named("org.junit.runner.Description"))), + BazelRunNotifierWrapperInstrumentation.class.getName() + "$FireSuiteStartedAdvice"); + transformer.applyAdvice( + named("fireTestSuiteFinished").and(takesArgument(0, named("org.junit.runner.Description"))), + BazelRunNotifierWrapperInstrumentation.class.getName() + "$FireSuiteFinishedAdvice"); + } + + public static class FireSuiteStartedAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void fireOnInnerNotifier( + @Advice.This final RunNotifier self, @Advice.Argument(0) final Description description) { + RunNotifier inner = JUnit4Utils.unwrapRunNotifier(self); + if (inner != null && inner != self) { + inner.fireTestSuiteStarted(description); + } + } + + // JUnit 4.13 muzzle marker: fireTestSuiteStarted exists from 4.13. + public static void muzzleCheck(final RunNotifier notifier) { + notifier.fireTestSuiteStarted(null); + } + } + + public static class FireSuiteFinishedAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void fireOnInnerNotifier( + @Advice.This final RunNotifier self, @Advice.Argument(0) final Description description) { + RunNotifier inner = JUnit4Utils.unwrapRunNotifier(self); + if (inner != null && inner != self) { + inner.fireTestSuiteFinished(description); + } + } + + public static void muzzleCheck(final RunNotifier notifier) { + notifier.fireTestSuiteFinished(null); + } + } +} From ece2ddc78dc471f71566e21b5a1a06b29c5e9604 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 11 May 2026 15:32:45 +0200 Subject: [PATCH 2/2] fix: only fire suite event on tracing listener --- .../instrumentation/junit4/JUnit4Utils.java | 14 ++--- ...azelRunNotifierWrapperInstrumentation.java | 54 +++++++++++++------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java index 63dc45df761..ca6f383c2cf 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java @@ -41,15 +41,6 @@ public abstract class JUnit4Utils { private static final String SYNCHRONIZED_LISTENER = "org.junit.runner.notification.SynchronizedRunListener"; - - /** - * Bazel's test launcher wraps the {@link RunNotifier} that our instrumentation advice receives. - * {@link RunNotifier#addListener} on the wrapper forwards to its inner delegate, but {@link - * org.junit.runner.notification.RunNotifier#listeners} on the wrapper is a separate (empty) - * field. Without unwrapping, our idempotency check fails to see a listener installed via a prior - * advice call on the inner notifier, and we end up adding a second tracing listener that the - * wrapper also forwards to the delegate. - */ private static final String BAZEL_RUN_NOTIFIER_WRAPPER = "com.google.testing.junit.junit4.runner.RunNotifierWrapper"; @@ -124,7 +115,7 @@ public static List runListenersFromRunNotifier(final RunNotifier ru public static RunNotifier unwrapRunNotifier(RunNotifier notifier) { RunNotifier current = notifier; for (int i = 0; i < 8 && current != null; i++) { - if (!isBazelRunNotifierWrapper(current.getClass())) { + if (!isBazelRunNotifierWrapper(current)) { return current; } RunNotifier delegate = METHOD_HANDLES.invoke(BAZEL_RUN_NOTIFIER_WRAPPER_DELEGATE, current); @@ -136,7 +127,8 @@ public static RunNotifier unwrapRunNotifier(RunNotifier notifier) { return current; } - private static boolean isBazelRunNotifierWrapper(Class cls) { + private static boolean isBazelRunNotifierWrapper(RunNotifier notifier) { + Class cls = notifier.getClass(); while (cls != null && cls != Object.class) { if (BAZEL_RUN_NOTIFIER_WRAPPER.equals(cls.getName())) { return true; diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java index b9bcae00eb7..782e6e1ebf8 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.13/src/main/java/datadog/trace/instrumentation/junit4/BazelRunNotifierWrapperInstrumentation.java @@ -6,26 +6,28 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import java.util.List; import net.bytebuddy.asm.Advice; import org.junit.runner.Description; +import org.junit.runner.notification.RunListener; import org.junit.runner.notification.RunNotifier; /** * Restores suite lifecycle events when JUnit 4.13+ tests run under Bazel's {@code * com.google.testing.junit.runner.BazelTestRunner}. * - *

Bazel's {@code com.google.testing.junit.junit4.runner.RunNotifierWrapper} (last touched in - * 2015) explicitly delegates {@code addListener}, {@code fireTestStarted}, etc. to the inner - * notifier, but does not override {@link RunNotifier#fireTestSuiteStarted(Description)} and {@link - * RunNotifier#fireTestSuiteFinished(Description)} — those were added in JUnit 4.13. {@link - * org.junit.runners.ParentRunner#run} therefore fires the suite-lifecycle events on the wrapper's - * own (always empty) listener list, and our tracing listener — installed on the inner notifier via - * the wrapper's delegating {@code addListener} — never sees them. + *

Bazel's {@code com.google.testing.junit.junit4.runner.RunNotifierWrapper} explicitly delegates + * {@code addListener}, {@code fireTestStarted}, etc. to the inner notifier, but does not override + * {@link RunNotifier#fireTestSuiteStarted(Description)} and {@link + * RunNotifier#fireTestSuiteFinished(Description)}. The runner therefore fires the suite-lifecycle + * events on the wrapper's own (always empty) listener list, and our tracing listener — installed on + * the inner notifier via the wrapper's delegating {@code addListener} — never sees them. * *

This advice intercepts {@link RunNotifier#fireTestSuiteStarted(Description)} and {@link - * RunNotifier#fireTestSuiteFinished(Description)} on the wrapper instance and re-fires the event on - * the inner notifier, where the listener actually lives. Calls on a non-wrapper notifier are a - * no-op. + * RunNotifier#fireTestSuiteFinished(Description)} on the wrapper instance, looks up our tracing + * listener inside the inner notifier's listener list, and invokes it directly. Other listeners + * installed on the inner notifier are intentionally skipped so Bazel's default dispatch behavior is + * unchanged. Calls on a non-wrapper notifier are a no-op. */ @AutoService(InstrumenterModule.class) public class BazelRunNotifierWrapperInstrumentation extends InstrumenterModule.CiVisibility @@ -61,11 +63,21 @@ public void methodAdvice(MethodTransformer transformer) { public static class FireSuiteStartedAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void fireOnInnerNotifier( + public static void fireOnTracingListener( @Advice.This final RunNotifier self, @Advice.Argument(0) final Description description) { RunNotifier inner = JUnit4Utils.unwrapRunNotifier(self); - if (inner != null && inner != self) { - inner.fireTestSuiteStarted(description); + if (inner == null || inner == self) { + return; + } + List listeners = JUnit4Utils.runListenersFromRunNotifier(inner); + if (listeners == null) { + return; + } + for (RunListener listener : listeners) { + TracingListener tracingListener = JUnit4Utils.toTracingListener(listener); + if (tracingListener != null) { + tracingListener.testSuiteStarted(description); + } } } @@ -77,11 +89,21 @@ public static void muzzleCheck(final RunNotifier notifier) { public static class FireSuiteFinishedAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void fireOnInnerNotifier( + public static void fireOnTracingListener( @Advice.This final RunNotifier self, @Advice.Argument(0) final Description description) { RunNotifier inner = JUnit4Utils.unwrapRunNotifier(self); - if (inner != null && inner != self) { - inner.fireTestSuiteFinished(description); + if (inner == null || inner == self) { + return; + } + List listeners = JUnit4Utils.runListenersFromRunNotifier(inner); + if (listeners == null) { + return; + } + for (RunListener listener : listeners) { + TracingListener tracingListener = JUnit4Utils.toTracingListener(listener); + if (tracingListener != null) { + tracingListener.testSuiteFinished(description); + } } }