diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java index 137ca240f19..9055ceaf3aa 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java @@ -3,6 +3,7 @@ import datadog.trace.api.civisibility.config.TestIdentifier; import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -31,12 +32,13 @@ TestSuiteImpl testSuiteStart( boolean isModified(TestSourceData testSourceData); /** - * Checks if a given test can be skipped with Intelligent Test Runner or not. + * Returns the reason for skipping a test, IF it can be skipped. * * @param test Test to be checked - * @return {@code true} if the test can be skipped, {@code false} otherwise + * @return skip reason, or {@code null} if the test cannot be skipped */ - boolean isSkippable(TestIdentifier test); + @Nullable + SkipReason skipReason(TestIdentifier test); @Nonnull TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 115b140b5a8..d657c71429a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -8,7 +8,6 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.civisibility.CIConstants; import datadog.trace.api.civisibility.DDTest; -import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.InstrumentationTestBridge; import datadog.trace.api.civisibility.config.TestIdentifier; import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; @@ -23,6 +22,7 @@ import datadog.trace.api.civisibility.telemetry.tag.IsRetry; import datadog.trace.api.civisibility.telemetry.tag.IsRum; import datadog.trace.api.civisibility.telemetry.tag.RetryReason; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentScope; @@ -208,9 +208,10 @@ public void setSkipReason(String skipReason) { if (skipReason != null) { span.setTag(Tags.TEST_SKIP_REASON, skipReason); - if (skipReason.equals(InstrumentationBridge.ITR_SKIP_REASON)) { + if (skipReason.equals(SkipReason.ITR.getDescription())) { span.setTag(Tags.TEST_SKIPPED_BY_ITR, true); metricCollector.add(CiVisibilityCountMetric.ITR_SKIPPED, 1, EventType.TEST); + executionResults.incrementTestsSkippedByItr(); } } } @@ -262,10 +263,6 @@ public void end(@Nullable Long endTime) { span.finish(); } - if (InstrumentationBridge.ITR_SKIP_REASON.equals(span.getTag(Tags.TEST_SKIP_REASON))) { - executionResults.incrementTestsSkippedByItr(); - } - metricCollector.add( CiVisibilityCountMetric.EVENT_FINISHED, 1, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java index d086c27244d..9506402afb7 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java @@ -7,6 +7,7 @@ import datadog.trace.api.civisibility.coverage.CoverageStore; import datadog.trace.api.civisibility.retry.TestRetryPolicy; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; @@ -102,9 +103,10 @@ public boolean isModified(TestSourceData testSourceData) { return executionStrategy.isModified(testSourceData); } + @Nullable @Override - public boolean isSkippable(TestIdentifier test) { - return executionStrategy.isSkippable(test); + public SkipReason skipReason(TestIdentifier test) { + return executionStrategy.skipReason(test); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java index 7d5df44490b..83302b8fc31 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java @@ -8,6 +8,7 @@ import datadog.trace.api.civisibility.coverage.CoverageStore; import datadog.trace.api.civisibility.retry.TestRetryPolicy; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; @@ -87,9 +88,10 @@ public boolean isModified(TestSourceData testSourceData) { return executionStrategy.isModified(testSourceData); } + @Nullable @Override - public boolean isSkippable(TestIdentifier test) { - return executionStrategy.isSkippable(test); + public SkipReason skipReason(TestIdentifier test) { + return executionStrategy.skipReason(test); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java index f5187516dad..16286ebe4e1 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java @@ -6,6 +6,7 @@ import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import datadog.trace.civisibility.retry.NeverRetry; @@ -91,8 +92,8 @@ public void onTestIgnore( } @Override - public boolean isSkippable(TestIdentifier test) { - return false; + public SkipReason skipReason(TestIdentifier test) { + return null; } @NotNull diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java index 787bc882ae4..4405312d82f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java @@ -12,6 +12,7 @@ import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.tag.EventType; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -187,7 +188,7 @@ public void onTestStart( test.setTag(Tags.TEST_ITR_UNSKIPPABLE, true); metricCollector.add(CiVisibilityCountMetric.ITR_UNSKIPPABLE, 1, EventType.TEST); - if (testModule.isSkippable(thisTest)) { + if (testModule.skipReason(thisTest) == SkipReason.ITR) { test.setTag(Tags.TEST_ITR_FORCED_RUN, true); metricCollector.add(CiVisibilityCountMetric.ITR_FORCED_RUN, 1, EventType.TEST); } @@ -276,9 +277,10 @@ public boolean isFlaky(TestIdentifier test) { return testModule.isFlaky(test); } + @Nullable @Override - public boolean isSkippable(TestIdentifier test) { - return testModule.isSkippable(test); + public SkipReason skipReason(TestIdentifier test) { + return testModule.skipReason(test); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java index 39e781b1469..52ef2a6a713 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java @@ -5,6 +5,7 @@ import datadog.trace.api.civisibility.config.TestMetadata; import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings; import datadog.trace.civisibility.config.ExecutionSettings; import datadog.trace.civisibility.retry.NeverRetry; @@ -17,6 +18,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,18 +60,23 @@ public boolean isFlaky(TestIdentifier test) { return flakyTests != null && flakyTests.contains(test.withoutParameters()); } - public boolean isSkippable(TestIdentifier test) { + @Nullable + public SkipReason skipReason(TestIdentifier test) { if (test == null) { - return false; + return null; } if (!executionSettings.isTestSkippingEnabled()) { - return false; + return null; } Map skippableTests = executionSettings.getSkippableTests(); TestMetadata testMetadata = skippableTests.get(test); - return testMetadata != null - && !(config.isCiVisibilityCoverageLinesEnabled() - && testMetadata.isMissingLineCodeCoverage()); + if (testMetadata == null) { + return null; + } + if (config.isCiVisibilityCoverageLinesEnabled() && testMetadata.isMissingLineCodeCoverage()) { + return null; + } + return SkipReason.ITR; } @Nonnull diff --git a/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy b/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy index 7a6511fe21b..e4d32c9867a 100644 --- a/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy +++ b/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy @@ -7,6 +7,7 @@ import com.jayway.jsonpath.JsonPath import com.jayway.jsonpath.Option import com.jayway.jsonpath.ReadContext import com.jayway.jsonpath.WriteContext +import freemarker.core.InvalidReferenceException import freemarker.template.Template import freemarker.template.TemplateExceptionHandler import org.skyscreamer.jsonassert.JSONAssert @@ -130,6 +131,16 @@ abstract class CiVisibilityTestUtils { StringWriter coveragesOut = new StringWriter() coveragesTemplate.process(replacements, coveragesOut) return coveragesOut.toString() + } catch (InvalidReferenceException e) { + def missingExpression = e.getBlamedExpressionString() + if (missingExpression != null) { + replacements.put(missingExpression, "") // to simplify debugging failures + return getFreemarkerTemplate(templatePath, replacements, replacementsSource) + + } else { + throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e) + } + } catch (Exception e) { throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e) } diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java index 60d63bba1c8..55ab788ee60 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java @@ -36,7 +36,7 @@ public String[] helperClassNames() { return new String[] { packageName + ".CucumberUtils", packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".CucumberTracingListener", diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberSkipInstrumentation.java similarity index 74% rename from dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java rename to dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberSkipInstrumentation.java index c6dce0fa24e..cdb999f531a 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberSkipInstrumentation.java @@ -11,6 +11,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.cucumber.core.gherkin.Pickle; import java.util.List; @@ -24,10 +25,10 @@ import org.junit.runner.notification.RunNotifier; @AutoService(InstrumenterModule.class) -public class JUnit4CucumberItrInstrumentation extends InstrumenterModule.CiVisibility +public class JUnit4CucumberSkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public JUnit4CucumberItrInstrumentation() { + public JUnit4CucumberSkipInstrumentation() { super("ci-visibility", "junit-4", "junit-4-cucumber"); } @@ -51,7 +52,7 @@ public String[] helperClassNames() { return new String[] { packageName + ".CucumberUtils", packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".CucumberTracingListener", @@ -67,10 +68,11 @@ public Reference[] additionalMuzzleReferences() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("run").and(takesArgument(0, named("org.junit.runner.notification.RunNotifier"))), - JUnit4CucumberItrInstrumentation.class.getName() + "$CucumberItrAdvice"); + JUnit4CucumberSkipInstrumentation.class.getName() + "$CucumberSkipAdvice"); } - public static class CucumberItrAdvice { + public static class CucumberSkipAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL") @Advice.OnMethodEnter(skipOn = Boolean.class) public static Boolean run( @@ -78,23 +80,24 @@ public static Boolean run( @Advice.FieldValue("description") Description description, @Advice.Argument(0) RunNotifier notifier) { - List tags = pickle.getTags(); - for (String tag : tags) { - if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { - return null; - } - } - TestIdentifier test = CucumberUtils.toTestIdentifier(description); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { - notifier.fireTestAssumptionFailed( - new Failure( - description, - new AssumptionViolatedException(InstrumentationBridge.ITR_SKIP_REASON))); - return Boolean.FALSE; - } else { + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test); + if (skipReason == null) { return null; } + + if (skipReason == SkipReason.ITR) { + List tags = pickle.getTags(); + for (String tag : tags) { + if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { + return null; + } + } + } + + notifier.fireTestAssumptionFailed( + new Failure(description, new AssumptionViolatedException(skipReason.getDescription()))); + return Boolean.FALSE; } } } diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/Cucumber4RetryInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/Cucumber4RetryInstrumentation.java index 06de5a7d397..5c85c9117ae 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/Cucumber4RetryInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/Cucumber4RetryInstrumentation.java @@ -49,7 +49,7 @@ public String instrumentedType() { @Override public String[] helperClassNames() { return new String[] { - parentPackageName + ".SkippedByItr", + parentPackageName + ".SkippedByDatadog", parentPackageName + ".CucumberUtils", parentPackageName + ".JUnit4Utils", parentPackageName + ".TracingListener", @@ -79,6 +79,7 @@ public void methodAdvice(MethodTransformer transformer) { } public static class RetryAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL") @Advice.OnMethodEnter(skipOn = Boolean.class) public static Boolean retryIfNeeded( diff --git a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitInstrumentation.java index 1414a51bf60..4d392d324f7 100644 --- a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitInstrumentation.java @@ -33,7 +33,7 @@ public String instrumentedType() { public String[] helperClassNames() { return new String[] { packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".MUnitUtils", packageName + ".TracingListener", diff --git a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/MUnitRetryInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/MUnitRetryInstrumentation.java index 640d6983b82..36b733fbf6a 100644 --- a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/MUnitRetryInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/MUnitRetryInstrumentation.java @@ -50,7 +50,7 @@ public String instrumentedType() { public String[] helperClassNames() { return new String[] { parentPackageName + ".MUnitUtils", - parentPackageName + ".SkippedByItr", + parentPackageName + ".SkippedByDatadog", parentPackageName + ".JUnit4Utils", parentPackageName + ".TracingListener", parentPackageName + ".TestEventsHandlerHolder", @@ -72,6 +72,7 @@ public void methodAdvice(MethodTransformer transformer) { } public static class RetryAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @Advice.OnMethodEnter(skipOn = Future.class) public static Future retryIfNeeded( @Advice.Origin Method runTest, @@ -118,6 +119,7 @@ public static Future retryIfNeeded( return result; } + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings( value = "UC_USELESS_OBJECT", justification = "result is the return value of the original method") diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java index 8b1b4f1d294..816ed835cc4 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java @@ -65,7 +65,7 @@ public ElementMatcher hierarchyMatcher() { public String[] helperClassNames() { return new String[] { packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".JUnit4TracingListener", diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SkipInstrumentation.java similarity index 73% rename from dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java rename to dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SkipInstrumentation.java index 0755f08fbf2..87cd9394c48 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SkipInstrumentation.java @@ -12,6 +12,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.lang.reflect.Method; import java.util.List; @@ -26,10 +27,10 @@ import org.junit.runners.ParentRunner; @AutoService(InstrumenterModule.class) -public class JUnit4ItrInstrumentation extends InstrumenterModule.CiVisibility +public class JUnit4SkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public JUnit4ItrInstrumentation() { + public JUnit4SkipInstrumentation() { super("ci-visibility", "junit-4"); } @@ -54,7 +55,7 @@ public ElementMatcher hierarchyMatcher() { public String[] helperClassNames() { return new String[] { packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".JUnit4TracingListener", @@ -67,10 +68,11 @@ public void methodAdvice(MethodTransformer transformer) { named("runChild") .and(takesArguments(2)) .and(takesArgument(1, named("org.junit.runner.notification.RunNotifier"))), - JUnit4ItrInstrumentation.class.getName() + "$JUnit4ItrInstrumentationAdvice"); + JUnit4SkipInstrumentation.class.getName() + "$JUnit4SkipInstrumentationAdvice"); } - public static class JUnit4ItrInstrumentationAdvice { + public static class JUnit4SkipInstrumentationAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL") @Advice.OnMethodEnter(skipOn = Boolean.class) public static Boolean runChild( @@ -79,33 +81,35 @@ public static Boolean runChild( @Advice.Argument(1) RunNotifier notifier) { Description description = JUnit4Utils.getDescription(runner, child); if (description == null || !description.isTest()) { - // ITR only skips individual tests return null; } Ignore ignoreAnnotation = description.getAnnotation(Ignore.class); if (ignoreAnnotation != null) { - // class is ignored, ITR not applicable + // class is ignored return null; } - Class testClass = description.getTestClass(); - Method testMethod = JUnit4Utils.getTestMethod(description); - List categories = JUnit4Utils.getCategories(testClass, testMethod); - for (String category : categories) { - if (category.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { - return null; - } - } - TestIdentifier test = JUnit4Utils.toTestIdentifier(description); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { - Description skippedDescription = JUnit4Utils.getSkippedDescription(description); - notifier.fireTestIgnored(skippedDescription); - return Boolean.FALSE; - } else { + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test); + if (skipReason == null) { return null; } + + if (skipReason == SkipReason.ITR) { + Class testClass = description.getTestClass(); + Method testMethod = JUnit4Utils.getTestMethod(description); + List categories = JUnit4Utils.getCategories(testClass, testMethod); + for (String category : categories) { + if (category.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { + return null; + } + } + } + + Description skippedDescription = JUnit4Utils.getSkippedDescription(description, skipReason); + notifier.fireTestIgnored(skippedDescription); + return Boolean.FALSE; } // JUnit 4.10 and above diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SuiteEventsInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SuiteEventsInstrumentation.java index cd9269780da..37c4f3d5fe5 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SuiteEventsInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4SuiteEventsInstrumentation.java @@ -40,7 +40,7 @@ public ElementMatcher hierarchyMatcher() { public String[] helperClassNames() { return new String[] { packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".JUnit4TracingListener", diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java index ef221174480..97c1f42f14d 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Utils.java @@ -6,6 +6,7 @@ import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.events.TestDescriptor; import datadog.trace.api.civisibility.events.TestSuiteDescriptor; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.util.MethodHandles; import java.lang.annotation.Annotation; import java.lang.invoke.MethodHandle; @@ -276,14 +277,14 @@ public static Description getDescription(ParentRunner runner, Object child) { return METHOD_HANDLES.invoke(PARENT_RUNNER_DESCRIBE_CHILD, runner, child); } - public static Description getSkippedDescription(Description description) { + public static Description getSkippedDescription(Description description, SkipReason skipReason) { Collection annotations = description.getAnnotations(); Annotation[] updatedAnnotations = new Annotation[annotations.size() + 1]; int idx = 0; for (Annotation annotation : annotations) { updatedAnnotations[idx++] = annotation; } - updatedAnnotations[idx] = new SkippedByItr(); + updatedAnnotations[idx] = new SkippedByDatadog(skipReason.getDescription()); String displayName = description.getDisplayName(); Class testClass = description.getTestClass(); diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnitLegacySuiteEventsInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnitLegacySuiteEventsInstrumentation.java index 2e80a36d130..71ad815128c 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnitLegacySuiteEventsInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnitLegacySuiteEventsInstrumentation.java @@ -38,7 +38,7 @@ public String[] knownMatchingTypes() { public String[] helperClassNames() { return new String[] { packageName + ".TestEventsHandlerHolder", - packageName + ".SkippedByItr", + packageName + ".SkippedByDatadog", packageName + ".JUnit4Utils", packageName + ".TracingListener", packageName + ".JUnit4TracingListener", diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByItr.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByDatadog.java similarity index 55% rename from dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByItr.java rename to dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByDatadog.java index c0cdcfecf77..57eadfff031 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByItr.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/SkippedByDatadog.java @@ -1,13 +1,19 @@ package datadog.trace.instrumentation.junit4; -import datadog.trace.api.civisibility.InstrumentationBridge; import java.lang.annotation.Annotation; import org.junit.Ignore; -public final class SkippedByItr implements Ignore { +public final class SkippedByDatadog implements Ignore { + + private final String description; + + public SkippedByDatadog(String description) { + this.description = description; + } + @Override public String value() { - return InstrumentationBridge.ITR_SKIP_REASON; + return description; } @Override diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/retry/JUnit4RetryInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/retry/JUnit4RetryInstrumentation.java index 02e662fd40a..def177dbab4 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/retry/JUnit4RetryInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/retry/JUnit4RetryInstrumentation.java @@ -57,7 +57,7 @@ public ElementMatcher hierarchyMatcher() { @Override public String[] helperClassNames() { return new String[] { - parentPackageName + ".SkippedByItr", + parentPackageName + ".SkippedByDatadog", parentPackageName + ".JUnit4Utils", parentPackageName + ".TracingListener", parentPackageName + ".TestEventsHandlerHolder", @@ -82,6 +82,7 @@ public void methodAdvice(MethodTransformer transformer) { } public static class RetryAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL") @Advice.OnMethodEnter(skipOn = Boolean.class) public static Boolean retryIfNeeded( diff --git a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberSkipInstrumentation.java similarity index 81% rename from dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java rename to dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberSkipInstrumentation.java index c39fa1c179e..f2c68d8e1d3 100644 --- a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberSkipInstrumentation.java @@ -11,6 +11,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collection; import java.util.Set; @@ -23,10 +24,10 @@ import org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService; @AutoService(InstrumenterModule.class) -public class JUnit5CucumberItrInstrumentation extends InstrumenterModule.CiVisibility +public class JUnit5CucumberSkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public JUnit5CucumberItrInstrumentation() { + public JUnit5CucumberSkipInstrumentation() { super("ci-visibility", "junit-5", "junit-5-cucumber"); } @@ -66,7 +67,7 @@ public String[] helperClassNames() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("shouldBeSkipped").and(takesArguments(1)), - JUnit5CucumberItrInstrumentation.class.getName() + "$JUnit5ItrAdvice"); + JUnit5CucumberSkipInstrumentation.class.getName() + "$JUnit5SkipAdvice"); } /** @@ -74,7 +75,7 @@ public void methodAdvice(MethodTransformer transformer) { * org.junit.platform.launcher} package in here: in some Gradle projects this package is not * available in CL where this instrumentation is injected */ - public static class JUnit5ItrAdvice { + public static class JUnit5SkipAdvice { @SuppressFBWarnings( value = "UC_USELESS_OBJECT", @@ -93,17 +94,26 @@ public static void shouldBeSkipped( return; } - Collection tags = testDescriptor.getTags(); - for (TestTag tag : tags) { - if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) { - return; - } + TestIdentifier test = CucumberUtils.toTestIdentifier(testDescriptor); + if (test == null) { + return; } - TestIdentifier test = CucumberUtils.toTestIdentifier(testDescriptor); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { - skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test); + if (skipReason == null) { + return; } + + if (skipReason == SkipReason.ITR) { + Collection tags = testDescriptor.getTags(); + for (TestTag tag : tags) { + if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) { + return; + } + } + } + + skipResult = Node.SkipResult.skip(skipReason.getDescription()); } // JUnit 5.3.0 and above diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockSkipInstrumentation.java similarity index 72% rename from dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java rename to dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockSkipInstrumentation.java index e3f962b2bac..298aec07f05 100644 --- a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockSkipInstrumentation.java @@ -9,8 +9,8 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.Config; -import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Set; @@ -24,10 +24,10 @@ import org.spockframework.runtime.SpockNode; @AutoService(InstrumenterModule.class) -public class JUnit5SpockItrInstrumentation extends InstrumenterModule.CiVisibility +public class JUnit5SpockSkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public JUnit5SpockItrInstrumentation() { + public JUnit5SpockSkipInstrumentation() { super("ci-visibility", "junit-5", "junit-5-spock"); } @@ -65,7 +65,7 @@ public String[] helperClassNames() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("shouldBeSkipped").and(takesArguments(1)), - JUnit5SpockItrInstrumentation.class.getName() + "$JUnit5ItrAdvice"); + JUnit5SpockSkipInstrumentation.class.getName() + "$JUnit5SkipAdvice"); } /** @@ -73,7 +73,7 @@ public void methodAdvice(MethodTransformer transformer) { * org.junit.platform.launcher} package in here: in some Gradle projects this package is not * available in CL where this instrumentation is injected */ - public static class JUnit5ItrAdvice { + public static class JUnit5SkipAdvice { @Advice.OnMethodEnter public static void beforeSkipCheck() { @@ -103,34 +103,55 @@ public static void shouldBeSkipped( return; } - if (SpockUtils.isUnskippable(spockNode)) { - return; - } - if (spockNode instanceof SpecNode) { // suite SpecNode specNode = (SpecNode) spockNode; Set features = specNode.getChildren(); + + SkipReason suiteSkipReason = null; for (TestDescriptor feature : features) { - if (feature instanceof SpockNode && SpockUtils.isUnskippable((SpockNode) feature)) { + if (feature instanceof SpockNode && SpockUtils.isItrUnskippable((SpockNode) feature)) { return; } TestIdentifier featureIdentifier = SpockUtils.toTestIdentifier(feature); - if (featureIdentifier == null - || !TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(featureIdentifier)) { + if (featureIdentifier == null) { + return; + } + SkipReason skipReason = + TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(featureIdentifier); + if (skipReason == null) { return; } + if (suiteSkipReason != null && suiteSkipReason != skipReason) { + // we cannot have a mix of skip reasons in the suite for technical reasons + return; + } + suiteSkipReason = skipReason; } - skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); + if (suiteSkipReason == null) { + return; + } + if (suiteSkipReason == SkipReason.ITR && SpockUtils.isItrUnskippable(specNode)) { + return; + } + skipResult = Node.SkipResult.skip(suiteSkipReason.getDescription()); } else { // individual test case TestIdentifier test = SpockUtils.toTestIdentifier(spockNode); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { - skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); + if (test == null) { + return; + } + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test); + if (skipReason == null) { + return; + } + if (skipReason == SkipReason.ITR && SpockUtils.isItrUnskippable(spockNode)) { + return; } + skipResult = Node.SkipResult.skip(skipReason.getDescription()); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java index e70b7560946..2c2899f1cc0 100644 --- a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java +++ b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java @@ -66,7 +66,7 @@ public static Collection getTags(SpockNode spockNode) { } } - public static boolean isUnskippable(SpockNode spockNode) { + public static boolean isItrUnskippable(SpockNode spockNode) { Collection tags = getTags(spockNode); for (TestTag tag : tags) { if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) { diff --git a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SkipInstrumentation.java similarity index 81% rename from dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java rename to dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SkipInstrumentation.java index 731b48fcb9d..ba0c9fdbcba 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SkipInstrumentation.java @@ -12,6 +12,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collection; import java.util.Set; @@ -24,10 +25,10 @@ import org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService; @AutoService(InstrumenterModule.class) -public class JUnit5ItrInstrumentation extends InstrumenterModule.CiVisibility +public class JUnit5SkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public JUnit5ItrInstrumentation() { + public JUnit5SkipInstrumentation() { super("ci-visibility", "junit-5"); } @@ -62,7 +63,7 @@ public String[] helperClassNames() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("shouldBeSkipped").and(takesArguments(1)), - JUnit5ItrInstrumentation.class.getName() + "$JUnit5ItrAdvice"); + JUnit5SkipInstrumentation.class.getName() + "$JUnit5SkipAdvice"); } /** @@ -70,7 +71,7 @@ public void methodAdvice(MethodTransformer transformer) { * org.junit.platform.launcher} package in here: in some Gradle projects this package is not * available in CL where this instrumentation is injected */ - public static class JUnit5ItrAdvice { + public static class JUnit5SkipAdvice { @SuppressFBWarnings( value = "UC_USELESS_OBJECT", @@ -89,17 +90,25 @@ public static void shouldBeSkipped( return; } - Collection tags = testDescriptor.getTags(); - for (TestTag tag : tags) { - if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) { - return; - } + TestIdentifier test = JUnitPlatformUtils.toTestIdentifier(testDescriptor); + if (test == null) { + return; + } + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test); + if (skipReason == null) { + return; } - TestIdentifier test = JUnitPlatformUtils.toTestIdentifier(testDescriptor); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { - skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); + if (skipReason == SkipReason.ITR) { + Collection tags = testDescriptor.getTags(); + for (TestTag tag : tags) { + if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) { + return; + } + } } + + skipResult = Node.SkipResult.skip(skipReason.getDescription()); } // JUnit 5.3.0 and above diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java index 030d736d0ff..2bcbd5fbb2e 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java @@ -19,6 +19,7 @@ import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.events.TestDescriptor; import datadog.trace.api.civisibility.events.TestSuiteDescriptor; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.instrumentation.api.AgentScope; @@ -112,10 +113,13 @@ public boolean beforeScenario(ScenarioRuntime sr) { String parameters = KarateUtils.getParameters(scenario); Collection categories = scenario.getTagsEffective().getTagKeys(); - if (Config.get().isCiVisibilityTestSkippingEnabled() - && !categories.contains(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { + if (Config.get().isCiVisibilityTestSkippingEnabled()) { TestIdentifier skippableTest = KarateUtils.toTestIdentifier(scenario); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(skippableTest)) { + SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(skippableTest); + + if (skipReason != null + && !(skipReason == SkipReason.ITR + && categories.contains(InstrumentationBridge.ITR_UNSKIPPABLE_TAG))) { TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestIgnore( suiteDescriptor, testDescriptor, @@ -125,7 +129,7 @@ public boolean beforeScenario(ScenarioRuntime sr) { parameters, categories, TestSourceData.UNKNOWN, - InstrumentationBridge.ITR_SKIP_REASON); + skipReason.getDescription()); return false; } } diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java index ce28947a8b8..8351688c1f7 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java @@ -7,6 +7,7 @@ import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.events.TestSuiteDescriptor; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.instrumentation.scalatest.retry.SuppressedTestFailedException; import java.util.Collection; @@ -138,7 +139,7 @@ private static void onTestStart(TestStarting event) { String testParameters = null; Collection categories; TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - if (context.unskippable(testIdentifier)) { + if (context.itrUnskippable(testIdentifier)) { categories = Collections.singletonList(InstrumentationBridge.ITR_UNSKIPPABLE_TAG); } else { categories = Collections.emptyList(); @@ -203,13 +204,8 @@ private static void onTestIgnore(TestIgnored event) { Collection categories = Collections.emptyList(); Class testClass = ScalatestUtils.getClass(event.suiteClassName()); - String reason; TestIdentifier skippableTest = new TestIdentifier(testSuiteName, testName, null); - if (context.skipped(skippableTest)) { - reason = InstrumentationBridge.ITR_SKIP_REASON; - } else { - reason = null; - } + SkipReason reason = context.getSkipReason(skippableTest); eventHandler.onTestIgnore( new TestSuiteDescriptor(testSuiteName, testClass), @@ -220,7 +216,7 @@ private static void onTestIgnore(TestIgnored event) { testParameters, categories, new TestSourceData(testClass, null, null), - reason); + reason != null ? reason.getDescription() : null); } private static void onTestCancel(TestCanceled event) { diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java index 4b0da0601b9..e6165ce9ec8 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java @@ -7,6 +7,7 @@ import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.events.TestSuiteDescriptor; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import java.util.ArrayList; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -34,8 +35,9 @@ public static void destroy(int runStamp) { private final int runStamp; private final TestEventsHandler eventHandler = InstrumentationBridge.createTestEventsHandler("scalatest", null, null); - private final java.util.Set skippedTests = ConcurrentHashMap.newKeySet(); - private final java.util.Set unskippableTests = ConcurrentHashMap.newKeySet(); + private final java.util.Map skipReasonByTest = + new ConcurrentHashMap<>(); + private final java.util.Set itrUnskippableTests = ConcurrentHashMap.newKeySet(); private final java.util.Map retryPolicies = new ConcurrentHashMap<>(); @@ -51,12 +53,12 @@ public TestEventsHandler getEventHandler() return eventHandler; } - public boolean skipped(TestIdentifier test) { - return skippedTests.remove(test); + public SkipReason getSkipReason(TestIdentifier test) { + return skipReasonByTest.remove(test); } - public boolean unskippable(TestIdentifier test) { - return unskippableTests.remove(test); + public boolean itrUnskippable(TestIdentifier test) { + return itrUnskippableTests.remove(test); } public scala.collection.immutable.List> skip( @@ -83,32 +85,41 @@ private Tuple2 skip( String testName = testNameAndSkipStatus._1(); TestIdentifier test = new TestIdentifier(suiteId, testName, null); - if (isUnskippable(test, tags)) { - unskippableTests.add(test); - return testNameAndSkipStatus; - } else if (eventHandler.isSkippable(test)) { - skippedTests.add(test); - return new Tuple2<>(testName, true); + boolean itrUnskippable = isItrUnskippable(test, tags); + if (itrUnskippable) { + itrUnskippableTests.add(test); + } - } else { + SkipReason skipReason = eventHandler.skipReason(test); + if (skipReason == null || skipReason == SkipReason.ITR && itrUnskippable) { return testNameAndSkipStatus; } + + skipReasonByTest.put(test, skipReason); + return new Tuple2<>(testName, true); } public boolean skip(TestIdentifier test, Map> tags) { - if (isUnskippable(test, tags)) { - unskippableTests.add(test); + boolean itrUnskippable = isItrUnskippable(test, tags); + if (itrUnskippable) { + itrUnskippableTests.add(test); + } + + SkipReason skipReason = eventHandler.skipReason(test); + if (skipReason == null) { return false; - } else if (eventHandler.isSkippable(test)) { - skippedTests.add(test); - return true; - } else { + } + + if (skipReason == SkipReason.ITR && itrUnskippable) { return false; } + + skipReasonByTest.put(test, skipReason); + return true; } - public boolean isUnskippable(TestIdentifier test, Map> tags) { + private boolean isItrUnskippable(TestIdentifier test, Map> tags) { Option> testTagsOption = tags.get(test.getName()); if (testTagsOption.isEmpty()) { return false; diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestItrInstrumentation.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestSkipInstrumentation.java similarity index 93% rename from dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestItrInstrumentation.java rename to dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestSkipInstrumentation.java index 1952dffe4ea..5c5ee59dd63 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestItrInstrumentation.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/ScalatestSkipInstrumentation.java @@ -21,10 +21,10 @@ import scala.Tuple2; @AutoService(InstrumenterModule.class) -public class ScalatestItrInstrumentation extends InstrumenterModule.CiVisibility +public class ScalatestSkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { - public ScalatestItrInstrumentation() { + public ScalatestSkipInstrumentation() { super("ci-visibility", "scalatest"); } @@ -59,7 +59,7 @@ public void methodAdvice(MethodTransformer transformer) { isConstructor() .and(takesArgument(2, named("org.scalatest.Filter"))) .and(takesArgument(5, named("org.scalatest.Tracker"))), - ScalatestItrInstrumentation.class.getName() + "$ArgsContructorAdvice"); + ScalatestSkipInstrumentation.class.getName() + "$ArgsContructorAdvice"); // org.scalatest.Filter transformer.applyAdvice( named("apply") @@ -67,14 +67,14 @@ public void methodAdvice(MethodTransformer transformer) { .and(takesArgument(0, String.class)) .and(takesArgument(1, named("scala.collection.immutable.Map"))) .and(takesArgument(2, String.class)), - ScalatestItrInstrumentation.class.getName() + "$SingleTestFilterAdvice"); + ScalatestSkipInstrumentation.class.getName() + "$SingleTestFilterAdvice"); transformer.applyAdvice( named("apply") .and(takesArguments(3)) .and(takesArgument(0, named("scala.collection.immutable.Set"))) .and(takesArgument(1, named("scala.collection.immutable.Map"))) .and(takesArgument(2, String.class)), - ScalatestItrInstrumentation.class.getName() + "$MultipleTestsFilterAdvice"); + ScalatestSkipInstrumentation.class.getName() + "$MultipleTestsFilterAdvice"); } public static class ArgsContructorAdvice { diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java similarity index 74% rename from dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java rename to dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java index 1bba703c618..c029e399f54 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java @@ -10,6 +10,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import java.lang.reflect.Method; import java.util.List; import java.util.Set; @@ -18,9 +19,9 @@ import org.testng.annotations.DataProvider; @AutoService(InstrumenterModule.class) -public class TestNGItrInstrumentation extends InstrumenterModule.CiVisibility +public class TestNGSkipInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { - public TestNGItrInstrumentation() { + public TestNGSkipInstrumentation() { super("testng", "testng-itr"); } @@ -45,7 +46,7 @@ public void methodAdvice(MethodTransformer transformer) { .and(takesArgument(0, Method.class)) .and(takesArgument(1, Object.class)) .and(takesArgument(2, Object[].class)), - TestNGItrInstrumentation.class.getName() + "$InvokeMethodAdvice"); + TestNGSkipInstrumentation.class.getName() + "$InvokeMethodAdvice"); } @Override @@ -63,15 +64,21 @@ public static void invokeMethod( @Advice.Argument(0) final Method method, @Advice.Argument(1) final Object instance, @Advice.Argument(2) final Object[] parameters) { - List groups = TestNGUtils.getGroups(method); - if (groups.contains(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { + TestIdentifier testIdentifier = TestNGUtils.toTestIdentifier(method, instance, parameters); + SkipReason skipReason = + TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(testIdentifier); + if (skipReason == null) { return; } - TestIdentifier skippableTest = TestNGUtils.toTestIdentifier(method, instance, parameters); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(skippableTest)) { - throw new SkipException(InstrumentationBridge.ITR_SKIP_REASON); + if (skipReason == SkipReason.ITR) { + List groups = TestNGUtils.getGroups(method); + if (groups.contains(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { + return; + } } + + throw new SkipException(skipReason.getDescription()); } // TestNG 6.4 and above diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/InstrumentationBridge.java b/internal-api/src/main/java/datadog/trace/api/civisibility/InstrumentationBridge.java index 3dea5eccef6..18e8cc860cf 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/InstrumentationBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/InstrumentationBridge.java @@ -8,7 +8,6 @@ public abstract class InstrumentationBridge { - public static final String ITR_SKIP_REASON = "Skipped by Datadog Intelligent Test Runner"; public static final String ITR_UNSKIPPABLE_TAG = "datadog_itr_unskippable"; private static volatile TestEventsHandler.Factory TEST_EVENTS_HANDLER_FACTORY; diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java index 5a30079dadb..e8ec1cf143f 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java @@ -5,6 +5,7 @@ import datadog.trace.api.civisibility.config.TestIdentifier; import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.bootstrap.ContextStore; import java.io.Closeable; @@ -86,12 +87,19 @@ void onTestIgnore( @Nonnull TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData source); + /** + * Returns the reason for skipping a test IF it can be skipped. + * + * @param test Test to be checked + * @return skip reason, or {@code null} if the test cannot be skipped + */ + @Nullable + SkipReason skipReason(TestIdentifier test); + boolean isNew(TestIdentifier test); boolean isFlaky(TestIdentifier test); - boolean isSkippable(TestIdentifier test); - @Override void close(); diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/SkipReason.java b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/SkipReason.java new file mode 100644 index 00000000000..258370d0d97 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/SkipReason.java @@ -0,0 +1,24 @@ +package datadog.trace.api.civisibility.telemetry.tag; + +import datadog.trace.api.civisibility.telemetry.TagValue; + +public enum SkipReason implements TagValue { + ITR("Skipped by Datadog Intelligent Test Runner"); + + private final String s; + private final String description; + + SkipReason(String description) { + this.description = description; + this.s = "retry_reason:" + name().toLowerCase(); + } + + public String getDescription() { + return description; + } + + @Override + public String asString() { + return s; + } +}