From 80643ba01b7e250d4294da579d4d0e76dc6a75f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Antonio=20Fern=C3=A1ndez=20de=20Alba?= Date: Thu, 29 Oct 2026 17:00:30 +0100 Subject: [PATCH 1/3] Fix ScalaTest suites run through JUnit 4 --- .../junit4/JUnit4Instrumentation.java | 24 +- .../junit4/JUnit4TracingListener.java | 52 ++-- .../src/test/groovy/JUnit4Test.groovy | 20 ++ .../org/example/TestSucceedCustomRunner.java | 45 ++++ .../test-custom-runner/coverages.ftl | 1 + .../resources/test-custom-runner/events.ftl | 241 ++++++++++++++++++ 6 files changed, 360 insertions(+), 23 deletions(-) create mode 100644 dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java create mode 100644 dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/coverages.ftl create mode 100644 dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/events.ftl diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java index 8fdf749a5aa..f0b16304693 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java @@ -99,34 +99,48 @@ public static void onRunnerCreation(@Advice.This final Runner runner) { public static class JUnit4Advice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void addTracingListener( + public static JUnit4TracingListener addTracingListener( @Advice.This final Runner runner, @Advice.Argument(0) final RunNotifier runNotifier) { if (!JUnit4Utils.runnerToFramework(runner).equals(TestFrameworkInstrumentation.JUNIT4)) { // checking class names in hierarchyMatcher alone is not enough: // for example, Karate calls #run method of its super class, // that was transformed - return; + return null; } // No public accessor to get already installed listeners. // The installed RunListeners list are obtained using reflection. final List runListeners = JUnit4Utils.runListenersFromRunNotifier(runNotifier); if (runListeners == null) { - return; + return null; } for (final RunListener listener : runListeners) { RunListener tracingListener = JUnit4Utils.toTracingListener(listener); if (tracingListener != null) { // prevents installing TracingListener multiple times - return; + return null; } } - final TracingListener tracingListener = + final JUnit4TracingListener tracingListener = new JUnit4TracingListener( InstrumentationContext.get(Description.class, TestExecutionTracker.class)); runNotifier.addListener(tracingListener); + return tracingListener; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void finishTestSuites( + @Advice.Enter final JUnit4TracingListener tracingListener, + @Advice.Argument(0) final RunNotifier runNotifier) { + if (tracingListener != null) { + try { + tracingListener.finishTestSuites(); + } finally { + runNotifier.removeListener(tracingListener); + } + } } // JUnit 4.10 and above 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 afa43e45493..2e7be419c99 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,6 +8,8 @@ import datadog.trace.bootstrap.ContextStore; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.junit.Ignore; import org.junit.runner.Description; import org.junit.runner.notification.Failure; @@ -18,6 +20,7 @@ public class JUnit4TracingListener extends TracingListener { private static final String FRAMEWORK_VERSION = JUnit4Utils.getVersion(); private final ContextStore executionTrackers; + private final Map startedSuites = new ConcurrentHashMap<>(); public JUnit4TracingListener(ContextStore executionTrackers) { this.executionTrackers = executionTrackers; @@ -32,6 +35,19 @@ public void testSuiteStarted(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); + startTestSuite(suiteDescriptor, description); + } + + private void startTestSuite(TestSuiteDescriptor suiteDescriptor, Description description) { + startedSuites.computeIfAbsent( + suiteDescriptor, + key -> { + onTestSuiteStart(key, description); + return description; + }); + } + + private void onTestSuiteStart(TestSuiteDescriptor suiteDescriptor, Description description) { Class testClass = description.getTestClass(); String testSuiteName = JUnit4Utils.getSuiteName(testClass, description); List categories = JUnit4Utils.getCategories(testClass, null); @@ -58,11 +74,25 @@ public void testSuiteFinished(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); + finishTestSuite(suiteDescriptor); + } + + private void finishTestSuite(TestSuiteDescriptor suiteDescriptor) { + if (startedSuites.remove(suiteDescriptor) == null) { + return; + } TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.JUNIT4) .onTestSuiteFinish(suiteDescriptor, null); } + public void finishTestSuites() { + // Some runners, including ScalaTest's JUnitRunner, never emit suite lifecycle events. + for (TestSuiteDescriptor suiteDescriptor : startedSuites.keySet()) { + finishTestSuite(suiteDescriptor); + } + } + @Override public void testStarted(final Description description) { if (JUnit4Utils.isJUnitPlatformRunnerTest(description)) { @@ -70,6 +100,7 @@ public void testStarted(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); + startTestSuite(suiteDescriptor, description); TestDescriptor testDescriptor = JUnit4Utils.toTestDescriptor(description); TestSourceData testSourceData = JUnit4Utils.toTestSourceData(description); @@ -179,21 +210,7 @@ public void testIgnored(final Description description) { TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); Class testClass = description.getTestClass(); - String testSuiteName = JUnit4Utils.getSuiteName(testClass, description); - List categories = JUnit4Utils.getCategories(testClass, null); - - TestEventsHandlerHolder.HANDLERS - .get(TestFrameworkInstrumentation.JUNIT4) - .onTestSuiteStart( - suiteDescriptor, - testSuiteName, - FRAMEWORK_NAME, - FRAMEWORK_VERSION, - testClass, - categories, - false, - TestFrameworkInstrumentation.JUNIT4, - null); + startTestSuite(suiteDescriptor, description); TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.JUNIT4) .onTestSuiteSkip(suiteDescriptor, reason); @@ -203,9 +220,7 @@ public void testIgnored(final Description description) { testIgnored(description, testMethod, reason); } - TestEventsHandlerHolder.HANDLERS - .get(TestFrameworkInstrumentation.JUNIT4) - .onTestSuiteFinish(suiteDescriptor, null); + finishTestSuite(suiteDescriptor); } } @@ -215,6 +230,7 @@ private void testIgnored(Description description, Method testMethod, String reas } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); + startTestSuite(suiteDescriptor, description); TestDescriptor testDescriptor = JUnit4Utils.toTestDescriptor(description); Class testClass = description.getTestClass(); diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy index f38332ccdac..107f3ae4476 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy @@ -15,6 +15,26 @@ class JUnit4Test extends CiVisibilityInstrumentationTest { def runner = new JUnitCore() + def "test runner without suite lifecycle events"() { + when: + TestEventsHandlerHolder.start(TestFrameworkInstrumentation.JUNIT4, JUnit4Utils.capabilities(false)) + def result + try { + result = runner.run(TestSucceedCustomRunner) + } finally { + TestEventsHandlerHolder.stop(TestFrameworkInstrumentation.JUNIT4) + } + + assert result.runCount == 2 + assert result.ignoreCount == 1 + assert result.failures*.description*.methodName == ["existing failure"] + assert result.failures*.exception*.class == [AssertionError] + assertSpansData("test-custom-runner") + + then: + noExceptionThrown() + } + def "test #testcaseName"() { runTests(tests, success) diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java new file mode 100644 index 00000000000..a03a0f12f2a --- /dev/null +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java @@ -0,0 +1,45 @@ +package org.example; + +import org.junit.runner.Description; +import org.junit.runner.RunWith; +import org.junit.runner.Runner; +import org.junit.runner.notification.Failure; +import org.junit.runner.notification.RunNotifier; + +@RunWith(TestSucceedCustomRunner.CustomRunner.class) +public class TestSucceedCustomRunner { + + public static class CustomRunner extends Runner { + private final Class testClass; + + public CustomRunner(Class testClass) { + this.testClass = testClass; + } + + @Override + public Description getDescription() { + Description suite = Description.createSuiteDescription(testClass); + suite.addChild(Description.createTestDescription(testClass, "adds")); + suite.addChild(Description.createTestDescription(testClass, "existing failure")); + suite.addChild(Description.createTestDescription(testClass, "ignored")); + return suite; + } + + @Override + public void run(RunNotifier notifier) { + // Like ScalaTest's JUnitRunner, emit test callbacks without suite lifecycle callbacks. + notifier.fireTestIgnored(testDescription("ignored")); + Description passing = testDescription("adds"); + notifier.fireTestStarted(passing); + notifier.fireTestFinished(passing); + Description failing = testDescription("existing failure"); + notifier.fireTestStarted(failing); + notifier.fireTestFailure(new Failure(failing, new AssertionError("existing failure"))); + notifier.fireTestFinished(failing); + } + + private Description testDescription(String name) { + return Description.createSuiteDescription(name + "(" + testClass.getName() + ")"); + } + } +} diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/coverages.ftl b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/coverages.ftl new file mode 100644 index 00000000000..8878e547a79 --- /dev/null +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/coverages.ftl @@ -0,0 +1 @@ +[ ] \ No newline at end of file diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/events.ftl b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/events.ftl new file mode 100644 index 00000000000..07a5ec05941 --- /dev/null +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/resources/test-custom-runner/events.ftl @@ -0,0 +1,241 @@ +[ { + "content" : { + "duration" : ${content_duration}, + "error" : 0, + "meta" : { + "_dd.p.tid" : ${content_meta__dd_p_tid}, + "_dd.profiling.ctx" : "test", + "_dd.tracer_host" : ${content_meta__dd_tracer_host}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "language" : "jvm", + "library_version" : ${content_meta_library_version}, + "runtime-id" : ${content_meta_runtime_id}, + "span.kind" : "test_session_end", + "test.command" : "junit-4.10", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.status" : "fail", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count}, + "_dd.profiling.enabled" : 0, + "_dd.trace_span_attribute_schema" : 0, + "process_id" : ${content_metrics_process_id} + }, + "name" : "junit4.test_session", + "resource" : "junit-4.10", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "start" : ${content_start}, + "test_session_id" : ${content_test_session_id} + }, + "type" : "test_session_end", + "version" : 1 +}, { + "content" : { + "duration" : ${content_duration_2}, + "error" : 0, + "meta" : { + "_dd.p.tid" : ${content_meta__dd_p_tid_2}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "library_version" : ${content_meta_library_version}, + "span.kind" : "test_module_end", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.module" : "junit-4.10", + "test.status" : "fail", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_2} + }, + "name" : "junit4.test_module", + "resource" : "junit-4.10", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "start" : ${content_start_2}, + "test_module_id" : ${content_test_module_id}, + "test_session_id" : ${content_test_session_id} + }, + "type" : "test_module_end", + "version" : 1 +}, { + "content" : { + "duration" : ${content_duration_3}, + "error" : 0, + "meta" : { + "_dd.p.tid" : ${content_meta__dd_p_tid_3}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "library_version" : ${content_meta_library_version}, + "span.kind" : "test_suite_end", + "test.codeowners" : "[\"owner1\",\"owner2\"]", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.module" : "junit-4.10", + "test.source.file" : "dummy_source_path", + "test.status" : "fail", + "test.suite" : "org.example.TestSucceedCustomRunner", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_3}, + "test.source.end" : 19, + "test.source.start" : 11 + }, + "name" : "junit4.test_suite", + "resource" : "org.example.TestSucceedCustomRunner", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "start" : ${content_start_3}, + "test_module_id" : ${content_test_module_id}, + "test_session_id" : ${content_test_session_id}, + "test_suite_id" : ${content_test_suite_id} + }, + "type" : "test_suite_end", + "version" : 1 +}, { + "content" : { + "duration" : ${content_duration_4}, + "error" : 0, + "meta" : { + "_dd.profiling.ctx" : "test", + "_dd.tracer_host" : ${content_meta__dd_tracer_host}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "language" : "jvm", + "library_version" : ${content_meta_library_version}, + "runtime-id" : ${content_meta_runtime_id}, + "span.kind" : "test", + "test.codeowners" : "[\"owner1\",\"owner2\"]", + "test.final_status" : "pass", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.module" : "junit-4.10", + "test.name" : "adds", + "test.source.file" : "dummy_source_path", + "test.status" : "pass", + "test.suite" : "org.example.TestSucceedCustomRunner", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_4}, + "_dd.profiling.enabled" : 0, + "_dd.trace_span_attribute_schema" : 0, + "process_id" : ${content_metrics_process_id} + }, + "name" : "junit4.test", + "parent_id" : ${content_parent_id}, + "resource" : "org.example.TestSucceedCustomRunner.adds", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "span_id" : ${content_span_id}, + "start" : ${content_start_4}, + "test_module_id" : ${content_test_module_id}, + "test_session_id" : ${content_test_session_id}, + "test_suite_id" : ${content_test_suite_id}, + "trace_id" : ${content_trace_id} + }, + "type" : "test", + "version" : 2 +}, { + "content" : { + "duration" : ${content_duration_5}, + "error" : 1, + "meta" : { + "_dd.profiling.ctx" : "test", + "_dd.tracer_host" : ${content_meta__dd_tracer_host}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "error.message" : ${content_meta_error_message}, + "error.stack" : ${content_meta_error_stack}, + "error.type" : "java.lang.AssertionError", + "language" : "jvm", + "library_version" : ${content_meta_library_version}, + "runtime-id" : ${content_meta_runtime_id}, + "span.kind" : "test", + "test.codeowners" : "[\"owner1\",\"owner2\"]", + "test.final_status" : "fail", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.module" : "junit-4.10", + "test.name" : "existing failure", + "test.source.file" : "dummy_source_path", + "test.status" : "fail", + "test.suite" : "org.example.TestSucceedCustomRunner", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_5}, + "_dd.profiling.enabled" : 0, + "_dd.trace_span_attribute_schema" : 0, + "process_id" : ${content_metrics_process_id} + }, + "name" : "junit4.test", + "parent_id" : ${content_parent_id}, + "resource" : "org.example.TestSucceedCustomRunner.existing failure", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "span_id" : ${content_span_id_2}, + "start" : ${content_start_5}, + "test_module_id" : ${content_test_module_id}, + "test_session_id" : ${content_test_session_id}, + "test_suite_id" : ${content_test_suite_id}, + "trace_id" : ${content_trace_id_2} + }, + "type" : "test", + "version" : 2 +}, { + "content" : { + "duration" : ${content_duration_6}, + "error" : 0, + "meta" : { + "_dd.profiling.ctx" : "test", + "_dd.tracer_host" : ${content_meta__dd_tracer_host}, + "component" : "junit4", + "dummy_ci_tag" : "dummy_ci_tag_value", + "env" : "none", + "language" : "jvm", + "library_version" : ${content_meta_library_version}, + "runtime-id" : ${content_meta_runtime_id}, + "span.kind" : "test", + "test.codeowners" : "[\"owner1\",\"owner2\"]", + "test.final_status" : "skip", + "test.framework" : "junit4", + "test.framework_version" : ${content_meta_test_framework_version}, + "test.module" : "junit-4.10", + "test.name" : "ignored", + "test.source.file" : "dummy_source_path", + "test.status" : "skip", + "test.suite" : "org.example.TestSucceedCustomRunner", + "test.type" : "test", + "test_session.name" : "session-name" + }, + "metrics" : { + "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_6}, + "_dd.profiling.enabled" : 0, + "_dd.trace_span_attribute_schema" : 0, + "process_id" : ${content_metrics_process_id} + }, + "name" : "junit4.test", + "parent_id" : ${content_parent_id}, + "resource" : "org.example.TestSucceedCustomRunner.ignored", + "service" : "worker.org.gradle.process.internal.worker.gradleworkermain", + "span_id" : ${content_span_id_3}, + "start" : ${content_start_6}, + "test_module_id" : ${content_test_module_id}, + "test_session_id" : ${content_test_session_id}, + "test_suite_id" : ${content_test_suite_id}, + "trace_id" : ${content_trace_id_3} + }, + "type" : "test", + "version" : 2 +} ] \ No newline at end of file From 73cdd95118ea3216692fba7dba59876bdcf506a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Antonio=20Fern=C3=A1ndez=20de=20Alba?= Date: Fri, 30 Oct 2026 09:39:17 +0100 Subject: [PATCH 2/3] Fix JUnit 4 suite cleanup for nested and parallel runners --- .../junit4/JUnit4Instrumentation.java | 21 ++++++++-- .../junit4/JUnit4TracingListener.java | 24 ++++++++++-- .../src/test/groovy/JUnit4Test.groovy | 39 +++++++++++++++++++ .../org/example/TestSucceedCustomRunner.java | 2 + 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java index f0b16304693..3b8f09b60e7 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java @@ -100,7 +100,10 @@ public static void onRunnerCreation(@Advice.This final Runner runner) { public static class JUnit4Advice { @Advice.OnMethodEnter(suppress = Throwable.class) public static JUnit4TracingListener addTracingListener( - @Advice.This final Runner runner, @Advice.Argument(0) final RunNotifier runNotifier) { + @Advice.This final Runner runner, + @Advice.Argument(0) final RunNotifier runNotifier, + @Advice.Local("suiteCount") int suiteCount, + @Advice.Local("listenerAdded") boolean listenerAdded) { if (!JUnit4Utils.runnerToFramework(runner).equals(TestFrameworkInstrumentation.JUNIT4)) { // checking class names in hierarchyMatcher alone is not enough: // for example, Karate calls #run method of its super class, @@ -117,6 +120,11 @@ public static JUnit4TracingListener addTracingListener( for (final RunListener listener : runListeners) { RunListener tracingListener = JUnit4Utils.toTracingListener(listener); + if (tracingListener instanceof JUnit4TracingListener) { + JUnit4TracingListener junit4Listener = (JUnit4TracingListener) tracingListener; + suiteCount = junit4Listener.getStartedSuiteCount(); + return junit4Listener; + } if (tracingListener != null) { // prevents installing TracingListener multiple times return null; @@ -127,18 +135,23 @@ public static JUnit4TracingListener addTracingListener( new JUnit4TracingListener( InstrumentationContext.get(Description.class, TestExecutionTracker.class)); runNotifier.addListener(tracingListener); + listenerAdded = true; return tracingListener; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void finishTestSuites( @Advice.Enter final JUnit4TracingListener tracingListener, - @Advice.Argument(0) final RunNotifier runNotifier) { + @Advice.Argument(0) final RunNotifier runNotifier, + @Advice.Local("suiteCount") final int suiteCount, + @Advice.Local("listenerAdded") final boolean listenerAdded) { if (tracingListener != null) { try { - tracingListener.finishTestSuites(); + tracingListener.finishTestSuites(suiteCount); } finally { - runNotifier.removeListener(tracingListener); + if (listenerAdded) { + runNotifier.removeListener(tracingListener); + } } } } 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 2e7be419c99..519d195a47f 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 @@ -7,6 +7,7 @@ import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -21,6 +22,8 @@ public class JUnit4TracingListener extends TracingListener { private final ContextStore executionTrackers; private final Map startedSuites = new ConcurrentHashMap<>(); + private final ThreadLocal> suitesByThread = + ThreadLocal.withInitial(ArrayList::new); public JUnit4TracingListener(ContextStore executionTrackers) { this.executionTrackers = executionTrackers; @@ -43,6 +46,7 @@ private void startTestSuite(TestSuiteDescriptor suiteDescriptor, Description des suiteDescriptor, key -> { onTestSuiteStart(key, description); + suitesByThread.get().add(key); return description; }); } @@ -86,10 +90,22 @@ private void finishTestSuite(TestSuiteDescriptor suiteDescriptor) { .onTestSuiteFinish(suiteDescriptor, null); } - public void finishTestSuites() { - // Some runners, including ScalaTest's JUnitRunner, never emit suite lifecycle events. - for (TestSuiteDescriptor suiteDescriptor : startedSuites.keySet()) { - finishTestSuite(suiteDescriptor); + public int getStartedSuiteCount() { + return suitesByThread.get().size(); + } + + public void finishTestSuites(int previousSuiteCount) { + // Nested runners share the listener, but must finish their own suites on the originating + // thread. + List suites = suitesByThread.get(); + try { + for (int i = suites.size() - 1; i >= previousSuiteCount; i--) { + finishTestSuite(suites.remove(i)); + } + } finally { + if (suites.isEmpty()) { + suitesByThread.remove(); + } } } diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy index 107f3ae4476..f874e0c1681 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy @@ -2,12 +2,15 @@ import datadog.trace.api.DisableTestTrace import datadog.trace.api.civisibility.config.TestFQN import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation +import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.civisibility.CiVisibilityInstrumentationTest import datadog.trace.civisibility.diff.LineDiff import datadog.trace.instrumentation.junit4.JUnit4Utils import datadog.trace.instrumentation.junit4.TestEventsHandlerHolder import org.example.* import org.junit.jupiter.api.Assumptions +import org.junit.experimental.ParallelComputer +import org.junit.runner.Computer import org.junit.runner.JUnitCore @DisableTestTrace(reason = "avoid self-tracing") @@ -35,6 +38,42 @@ class JUnit4Test extends CiVisibilityInstrumentationTest { noExceptionThrown() } + def "test multiple runners without suite lifecycle events #mode"() { + when: + def initialSpan = AgentTracer.activeSpan() + TestEventsHandlerHolder.start(TestFrameworkInstrumentation.JUNIT4, JUnit4Utils.capabilities(false)) + def result + try { + result = runner.run(computer, tests as Class[]) + } finally { + TestEventsHandlerHolder.stop(TestFrameworkInstrumentation.JUNIT4) + } + + assert result.runCount == 4 + assert result.ignoreCount == 2 + assert result.failures*.description*.methodName == ["existing failure", "existing failure"] + assert result.failures*.exception*.class == [AssertionError, AssertionError] + + def events = getEventsAsJson(TEST_WRITER.toList()) + def suites = events.findAll { it.type == "test_suite_end" } + assert suites*.content*.meta*.get("test.suite").sort() == [TestSucceedCustomRunner.name, TestSucceedCustomRunner.AnotherSuite.name].sort() + suites.each { suite -> + def suiteTests = events.findAll { it.type == "test" && it.content.test_suite_id == suite.content.test_suite_id } + assert suiteTests.size() == 3 + assert suiteTests.collectEntries { [(it.content.meta["test.name"]): it.content.meta["test.status"]] } == ["adds": "pass", "existing failure": "fail", "ignored": "skip"] + } + assert AgentTracer.activeSpan() == initialSpan + + then: + noExceptionThrown() + + where: + mode | computer | tests + "parallel" | new ParallelComputer(true, false) | [TestSucceedCustomRunner, TestSucceedCustomRunner.AnotherSuite] + "sequential" | new Computer() | [TestSucceedCustomRunner, TestSucceedCustomRunner.AnotherSuite] + "sequential reversed" | new Computer() | [TestSucceedCustomRunner.AnotherSuite, TestSucceedCustomRunner] + } + def "test #testcaseName"() { runTests(tests, success) diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java index a03a0f12f2a..2f72b361100 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java @@ -9,6 +9,8 @@ @RunWith(TestSucceedCustomRunner.CustomRunner.class) public class TestSucceedCustomRunner { + public static class AnotherSuite extends TestSucceedCustomRunner {} + public static class CustomRunner extends Runner { private final Class testClass; From e9f1539daa26e6edb1d20d1ef2f3eb93f2ab206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Antonio=20Fern=C3=A1ndez=20de=20Alba?= Date: Fri, 30 Oct 2026 10:17:45 +0100 Subject: [PATCH 3/3] Fix JUnit 4 suites with callbacks on executor threads --- .../junit4/JUnit4Instrumentation.java | 21 ++------- .../junit4/JUnit4TracingListener.java | 47 +++++++------------ .../src/test/groovy/JUnit4Test.groovy | 43 +++++++++++++++++ .../org/example/TestSucceedCustomRunner.java | 4 ++ .../example/TestSucceedExecutorRunner.java | 23 +++++++++ 5 files changed, 92 insertions(+), 46 deletions(-) create mode 100644 dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedExecutorRunner.java diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java index 3b8f09b60e7..f0b16304693 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java @@ -100,10 +100,7 @@ public static void onRunnerCreation(@Advice.This final Runner runner) { public static class JUnit4Advice { @Advice.OnMethodEnter(suppress = Throwable.class) public static JUnit4TracingListener addTracingListener( - @Advice.This final Runner runner, - @Advice.Argument(0) final RunNotifier runNotifier, - @Advice.Local("suiteCount") int suiteCount, - @Advice.Local("listenerAdded") boolean listenerAdded) { + @Advice.This final Runner runner, @Advice.Argument(0) final RunNotifier runNotifier) { if (!JUnit4Utils.runnerToFramework(runner).equals(TestFrameworkInstrumentation.JUNIT4)) { // checking class names in hierarchyMatcher alone is not enough: // for example, Karate calls #run method of its super class, @@ -120,11 +117,6 @@ public static JUnit4TracingListener addTracingListener( for (final RunListener listener : runListeners) { RunListener tracingListener = JUnit4Utils.toTracingListener(listener); - if (tracingListener instanceof JUnit4TracingListener) { - JUnit4TracingListener junit4Listener = (JUnit4TracingListener) tracingListener; - suiteCount = junit4Listener.getStartedSuiteCount(); - return junit4Listener; - } if (tracingListener != null) { // prevents installing TracingListener multiple times return null; @@ -135,23 +127,18 @@ public static JUnit4TracingListener addTracingListener( new JUnit4TracingListener( InstrumentationContext.get(Description.class, TestExecutionTracker.class)); runNotifier.addListener(tracingListener); - listenerAdded = true; return tracingListener; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void finishTestSuites( @Advice.Enter final JUnit4TracingListener tracingListener, - @Advice.Argument(0) final RunNotifier runNotifier, - @Advice.Local("suiteCount") final int suiteCount, - @Advice.Local("listenerAdded") final boolean listenerAdded) { + @Advice.Argument(0) final RunNotifier runNotifier) { if (tracingListener != null) { try { - tracingListener.finishTestSuites(suiteCount); + tracingListener.finishTestSuites(); } finally { - if (listenerAdded) { - runNotifier.removeListener(tracingListener); - } + runNotifier.removeListener(tracingListener); } } } 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 519d195a47f..72443566543 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 @@ -7,7 +7,6 @@ import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import java.lang.reflect.Method; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -21,9 +20,7 @@ public class JUnit4TracingListener extends TracingListener { private static final String FRAMEWORK_VERSION = JUnit4Utils.getVersion(); private final ContextStore executionTrackers; - private final Map startedSuites = new ConcurrentHashMap<>(); - private final ThreadLocal> suitesByThread = - ThreadLocal.withInitial(ArrayList::new); + private final Map startedSuites = new ConcurrentHashMap<>(); public JUnit4TracingListener(ContextStore executionTrackers) { this.executionTrackers = executionTrackers; @@ -38,20 +35,21 @@ public void testSuiteStarted(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); - startTestSuite(suiteDescriptor, description); + startTestSuite(suiteDescriptor, description, false); } - private void startTestSuite(TestSuiteDescriptor suiteDescriptor, Description description) { + private void startTestSuite( + TestSuiteDescriptor suiteDescriptor, Description description, boolean parallelized) { startedSuites.computeIfAbsent( suiteDescriptor, key -> { - onTestSuiteStart(key, description); - suitesByThread.get().add(key); - return description; + onTestSuiteStart(key, description, parallelized); + return parallelized; }); } - private void onTestSuiteStart(TestSuiteDescriptor suiteDescriptor, Description description) { + private void onTestSuiteStart( + TestSuiteDescriptor suiteDescriptor, Description description, boolean parallelized) { Class testClass = description.getTestClass(); String testSuiteName = JUnit4Utils.getSuiteName(testClass, description); List categories = JUnit4Utils.getCategories(testClass, null); @@ -64,7 +62,7 @@ private void onTestSuiteStart(TestSuiteDescriptor suiteDescriptor, Description d FRAMEWORK_VERSION, testClass, categories, - false, + parallelized, TestFrameworkInstrumentation.JUNIT4, null); } @@ -90,21 +88,12 @@ private void finishTestSuite(TestSuiteDescriptor suiteDescriptor) { .onTestSuiteFinish(suiteDescriptor, null); } - public int getStartedSuiteCount() { - return suitesByThread.get().size(); - } - - public void finishTestSuites(int previousSuiteCount) { - // Nested runners share the listener, but must finish their own suites on the originating - // thread. - List suites = suitesByThread.get(); - try { - for (int i = suites.size() - 1; i >= previousSuiteCount; i--) { - finishTestSuite(suites.remove(i)); - } - } finally { - if (suites.isEmpty()) { - suitesByThread.remove(); + public void finishTestSuites() { + // Fallback suites have no thread-bound scope: their callbacks may run on an executor. + // Suites with native lifecycle events are finished by their own callbacks. + for (Map.Entry suite : startedSuites.entrySet()) { + if (suite.getValue()) { + finishTestSuite(suite.getKey()); } } } @@ -116,7 +105,7 @@ public void testStarted(final Description description) { } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); - startTestSuite(suiteDescriptor, description); + startTestSuite(suiteDescriptor, description, true); TestDescriptor testDescriptor = JUnit4Utils.toTestDescriptor(description); TestSourceData testSourceData = JUnit4Utils.toTestSourceData(description); @@ -226,7 +215,7 @@ public void testIgnored(final Description description) { TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); Class testClass = description.getTestClass(); - startTestSuite(suiteDescriptor, description); + startTestSuite(suiteDescriptor, description, false); TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.JUNIT4) .onTestSuiteSkip(suiteDescriptor, reason); @@ -246,7 +235,7 @@ private void testIgnored(Description description, Method testMethod, String reas } TestSuiteDescriptor suiteDescriptor = JUnit4Utils.toSuiteDescriptor(description); - startTestSuite(suiteDescriptor, description); + startTestSuite(suiteDescriptor, description, true); TestDescriptor testDescriptor = JUnit4Utils.toTestDescriptor(description); Class testClass = description.getTestClass(); diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy index f874e0c1681..9c402aad390 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy @@ -13,6 +13,9 @@ import org.junit.experimental.ParallelComputer import org.junit.runner.Computer import org.junit.runner.JUnitCore +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executors + @DisableTestTrace(reason = "avoid self-tracing") class JUnit4Test extends CiVisibilityInstrumentationTest { @@ -74,6 +77,46 @@ class JUnit4Test extends CiVisibilityInstrumentationTest { "sequential reversed" | new Computer() | [TestSucceedCustomRunner.AnotherSuite, TestSucceedCustomRunner] } + def "test executor callbacks without suite lifecycle events"() { + setup: + def executor = Executors.newSingleThreadExecutor() + TestSucceedExecutorRunner.executor = executor + + when: + def initialSpan = AgentTracer.activeSpan() + def initialWorkerSpan = CompletableFuture.supplyAsync({ AgentTracer.activeSpan() }, executor).join() + TestEventsHandlerHolder.start(TestFrameworkInstrumentation.JUNIT4, JUnit4Utils.capabilities(false)) + def result + def workerSpan + try { + result = runner.run(TestSucceedExecutorRunner) + workerSpan = CompletableFuture.supplyAsync({ AgentTracer.activeSpan() }, executor).join() + } finally { + TestEventsHandlerHolder.stop(TestFrameworkInstrumentation.JUNIT4) + } + + assert workerSpan == initialWorkerSpan + assert AgentTracer.activeSpan() == initialSpan + assert result.runCount == 2 + assert result.ignoreCount == 1 + assert result.failures*.description*.methodName == ["existing failure"] + assert result.failures*.exception*.class == [AssertionError] + + def events = getEventsAsJson(TEST_WRITER.toList()) + def suites = events.findAll { it.type == "test_suite_end" } + assert suites*.content*.meta*.get("test.suite") == [TestSucceedExecutorRunner.name] + def suiteTests = events.findAll { it.type == "test" && it.content.test_suite_id == suites[0].content.test_suite_id } + assert suiteTests.size() == 3 + assert suiteTests.collectEntries { [(it.content.meta["test.name"]): it.content.meta["test.status"]] } == ["adds": "pass", "existing failure": "fail", "ignored": "skip"] + + then: + noExceptionThrown() + + cleanup: + executor.shutdownNow() + TestSucceedExecutorRunner.executor = null + } + def "test #testcaseName"() { runTests(tests, success) diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java index 2f72b361100..7a797b4a5fd 100644 --- a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedCustomRunner.java @@ -29,6 +29,10 @@ public Description getDescription() { @Override public void run(RunNotifier notifier) { + emitTestEvents(notifier); + } + + protected void emitTestEvents(RunNotifier notifier) { // Like ScalaTest's JUnitRunner, emit test callbacks without suite lifecycle callbacks. notifier.fireTestIgnored(testDescription("ignored")); Description passing = testDescription("adds"); diff --git a/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedExecutorRunner.java b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedExecutorRunner.java new file mode 100644 index 00000000000..85879e08e7a --- /dev/null +++ b/dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/java/org/example/TestSucceedExecutorRunner.java @@ -0,0 +1,23 @@ +package org.example; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import org.junit.runner.RunWith; +import org.junit.runner.notification.RunNotifier; + +@RunWith(TestSucceedExecutorRunner.ExecutorRunner.class) +public class TestSucceedExecutorRunner { + + public static ExecutorService executor; + + public static class ExecutorRunner extends TestSucceedCustomRunner.CustomRunner { + public ExecutorRunner(Class testClass) { + super(testClass); + } + + @Override + public void run(RunNotifier notifier) { + CompletableFuture.runAsync(() -> emitTestEvents(notifier), executor).join(); + } + } +}