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 8ad03a7685f..0fdd0b2bce6 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 @@ -17,6 +17,7 @@ import datadog.trace.api.civisibility.telemetry.tag.BrowserDriver; import datadog.trace.api.civisibility.telemetry.tag.EventType; import datadog.trace.api.civisibility.telemetry.tag.IsNew; +import datadog.trace.api.civisibility.telemetry.tag.IsRetry; import datadog.trace.api.civisibility.telemetry.tag.IsRum; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.api.gateway.RequestContextSlot; @@ -245,6 +246,7 @@ public void end(@Nullable Long endTime) { instrumentation, EventType.TEST, span.getTag(Tags.TEST_IS_NEW) != null ? IsNew.TRUE : null, + span.getTag(Tags.TEST_IS_RETRY) != null ? IsRetry.TRUE : null, span.getTag(Tags.TEST_IS_RUM_ACTIVE) != null ? IsRum.TRUE : null, CIConstants.SELENIUM_BROWSER_DRIVER.equals(span.getTag(Tags.TEST_BROWSER_DRIVER)) ? BrowserDriver.SELENIUM 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 30fd785c428..f0530afc3fb 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 @@ -66,6 +66,7 @@ public class ProxyTestModule implements TestFrameworkModule { private final Collection knownTests; private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings; private final AtomicInteger earlyFlakeDetectionsUsed = new AtomicInteger(0); + private final AtomicInteger autoRetriesUsed = new AtomicInteger(0); private final Collection testFrameworks = ConcurrentHashMap.newKeySet(); public ProxyTestModule( @@ -142,8 +143,9 @@ public TestRetryPolicy retryPolicy(TestIdentifier test) { return new RetryNTimes(earlyFlakeDetectionSettings); } if (flakyTestRetriesEnabled - && (flakyTests == null || flakyTests.contains(test.withoutParameters()))) { - return new RetryIfFailed(config.getCiVisibilityFlakyRetryCount()); + && (flakyTests == null || flakyTests.contains(test.withoutParameters())) + && autoRetriesUsed.get() < config.getCiVisibilityTotalFlakyRetryCount()) { + return new RetryIfFailed(config.getCiVisibilityFlakyRetryCount(), autoRetriesUsed); } } return NeverRetry.INSTANCE; 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 e93aa38bce8..f86a2bd118b 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 @@ -51,6 +51,7 @@ public class HeadlessTestModule extends AbstractTestModule implements TestFramew private final Collection knownTests; private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings; private final AtomicInteger earlyFlakeDetectionsUsed = new AtomicInteger(0); + private final AtomicInteger autoRetriesUsed = new AtomicInteger(0); private final boolean codeCoverageEnabled; private final boolean testSkippingEnabled; @@ -131,8 +132,9 @@ public TestRetryPolicy retryPolicy(TestIdentifier test) { return new RetryNTimes(earlyFlakeDetectionSettings); } if (flakyTestRetriesEnabled - && (flakyTests == null || flakyTests.contains(test.withoutParameters()))) { - return new RetryIfFailed(config.getCiVisibilityFlakyRetryCount()); + && (flakyTests == null || flakyTests.contains(test.withoutParameters())) + && autoRetriesUsed.get() < config.getCiVisibilityTotalFlakyRetryCount()) { + return new RetryIfFailed(config.getCiVisibilityFlakyRetryCount(), autoRetriesUsed); } } return NeverRetry.INSTANCE; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryIfFailed.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryIfFailed.java index 2595d7f6c5e..45437990a14 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryIfFailed.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryIfFailed.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.retry; import datadog.trace.api.civisibility.retry.TestRetryPolicy; +import java.util.concurrent.atomic.AtomicInteger; /** Retries a test case if it failed, up to a maximum number of times. */ public class RetryIfFailed implements TestRetryPolicy { @@ -8,8 +9,12 @@ public class RetryIfFailed implements TestRetryPolicy { private final int maxExecutions; private int executions; - public RetryIfFailed(int maxExecutions) { + /** Total execution counter that is shared by all retry policies */ + private final AtomicInteger totalExecutions; + + public RetryIfFailed(int maxExecutions, AtomicInteger totalExecutions) { this.maxExecutions = maxExecutions; + this.totalExecutions = totalExecutions; this.executions = 0; } @@ -25,7 +30,12 @@ public boolean suppressFailures() { @Override public boolean retry(boolean successful, long duration) { - return !successful && ++executions < maxExecutions; + if (!successful && ++executions < maxExecutions) { + totalExecutions.incrementAndGet(); + return true; + } else { + return false; + } } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/TestImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy similarity index 97% rename from dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/TestImplTest.groovy rename to dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy index ee1793d39cc..d305832980d 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/TestImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy @@ -1,4 +1,4 @@ -package datadog.trace.civisibility +package datadog.trace.civisibility.domain import datadog.trace.agent.test.asserts.ListWriterAssert import datadog.trace.agent.tooling.TracerInstaller @@ -11,9 +11,9 @@ import datadog.trace.api.civisibility.coverage.CoverageStore import datadog.trace.api.civisibility.coverage.NoOpCoverageStore import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.civisibility.InstrumentationType import datadog.trace.civisibility.codeowners.NoCodeowners import datadog.trace.civisibility.decorator.TestDecoratorImpl -import datadog.trace.civisibility.domain.TestImpl import datadog.trace.civisibility.source.MethodLinesResolver import datadog.trace.civisibility.source.NoOpSourcePathResolver import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy new file mode 100644 index 00000000000..f578c09ff7a --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy @@ -0,0 +1,66 @@ +package datadog.trace.civisibility.domain.buildsystem + +import datadog.trace.api.Config +import datadog.trace.api.civisibility.config.EarlyFlakeDetectionSettings +import datadog.trace.api.civisibility.config.ModuleExecutionSettings +import datadog.trace.api.civisibility.config.TestIdentifier +import datadog.trace.api.civisibility.coverage.CoverageDataSupplier +import datadog.trace.api.civisibility.coverage.CoverageStore +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector +import datadog.trace.civisibility.codeowners.Codeowners +import datadog.trace.civisibility.decorator.TestDecorator +import datadog.trace.civisibility.ipc.SignalClient +import datadog.trace.civisibility.source.MethodLinesResolver +import datadog.trace.civisibility.source.SourcePathResolver +import datadog.trace.test.util.DDSpecification + +class ProxyTestModuleTest extends DDSpecification { + + def "test total retries limit is applied across test cases"() { + def moduleExecutionSettings = Stub(ModuleExecutionSettings) + moduleExecutionSettings.getEarlyFlakeDetectionSettings() >> EarlyFlakeDetectionSettings.DEFAULT + moduleExecutionSettings.isFlakyTestRetriesEnabled() >> true + moduleExecutionSettings.getFlakyTests(_) >> null + + def config = Stub(Config) + config.getCiVisibilityFlakyRetryCount() >> 2 // this counts all executions of a test case (first attempt is counted too) + config.getCiVisibilityTotalFlakyRetryCount() >> 2 // this counts retries across all tests (first attempt is not a retry, so it is not counted) + + given: + def proxyTestModule = new ProxyTestModule( + 1L, + 1L, + "test-module", + moduleExecutionSettings, + config, + Stub(CiVisibilityMetricCollector), + Stub(TestDecorator), + Stub(SourcePathResolver), + Stub(Codeowners), + Stub(MethodLinesResolver), + Stub(CoverageStore.Factory), + Stub(CoverageDataSupplier), + GroovyMock(SignalClient.Factory) + ) + + when: + def retryPolicy1 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-1", null, null)) + + then: + retryPolicy1.retry(false, 1L) // 2nd test execution, 1st retry globally + !retryPolicy1.retry(false, 1L) // asking for 3rd test execution - local limit reached + + when: + def retryPolicy2 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-2", null, null)) + + then: + retryPolicy2.retry(false, 1L) // 2nd test execution, 2nd retry globally (since previous test was retried too) + !retryPolicy2.retry(false, 1L) // asking for 3rd test execution - local limit reached + + when: + def retryPolicy3 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-3", null, null)) + + then: + !retryPolicy3.retry(false, 1L) // asking for 3rd retry globally - global limit reached + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy new file mode 100644 index 00000000000..63eb3921858 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy @@ -0,0 +1,67 @@ +package datadog.trace.civisibility.domain.headless + +import datadog.trace.api.Config +import datadog.trace.api.civisibility.config.EarlyFlakeDetectionSettings +import datadog.trace.api.civisibility.config.ModuleExecutionSettings +import datadog.trace.api.civisibility.config.TestIdentifier +import datadog.trace.api.civisibility.coverage.CoverageStore +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.civisibility.codeowners.Codeowners +import datadog.trace.civisibility.decorator.TestDecorator +import datadog.trace.civisibility.source.MethodLinesResolver +import datadog.trace.civisibility.source.SourcePathResolver +import datadog.trace.test.util.DDSpecification + +class HeadlessTestModuleTest extends DDSpecification { + + def "test total retries limit is applied across test cases"() { + def moduleExecutionSettings = Stub(ModuleExecutionSettings) + moduleExecutionSettings.getEarlyFlakeDetectionSettings() >> EarlyFlakeDetectionSettings.DEFAULT + moduleExecutionSettings.isFlakyTestRetriesEnabled() >> true + moduleExecutionSettings.getFlakyTests(_) >> null + + def config = Stub(Config) + config.getCiVisibilityFlakyRetryCount() >> 2 // this counts all executions of a test case (first attempt is counted too) + config.getCiVisibilityTotalFlakyRetryCount() >> 2 // this counts retries across all tests (first attempt is not a retry, so it is not counted) + + given: + def headlessTestModule = new HeadlessTestModule( + Stub(AgentSpan.Context), + 1L, + "test-module", + null, + config, + Stub(CiVisibilityMetricCollector), + Stub(TestDecorator), + Stub(SourcePathResolver), + Stub(Codeowners), + Stub(MethodLinesResolver), + Stub(CoverageStore.Factory), + moduleExecutionSettings, + (span) -> {} + ) + + when: + def retryPolicy1 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-1", null, null)) + + then: + retryPolicy1.retry(false, 1L) // 2nd test execution, 1st retry globally + !retryPolicy1.retry(false, 1L) // asking for 3rd test execution - local limit reached + + when: + def retryPolicy2 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-2", null, null)) + + then: + retryPolicy2.retry(false, 1L) // 2nd test execution, 2nd retry globally (since previous test was retried too) + !retryPolicy2.retry(false, 1L) // asking for 3rd test execution - local limit reached + + when: + def retryPolicy3 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-3", null, null)) + + then: + !retryPolicy3.retry(false, 1L) // asking for 3rd retry globally - global limit reached + } + + +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java index 7d02647b943..c3af998b75d 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java @@ -60,6 +60,8 @@ public final class CiVisibilityConfig { public static final String CIVISIBILITY_EARLY_FLAKE_DETECTION_LOWER_LIMIT = "civisibility.early.flake.detection.lower.limit"; public static final String CIVISIBILITY_FLAKY_RETRY_COUNT = "civisibility.flaky.retry.count"; + public static final String CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT = + "civisibility.total.flaky.retry.count"; public static final String CIVISIBILITY_MODULE_NAME = "civisibility.module.name"; public static final String CIVISIBILITY_TELEMETRY_ENABLED = "civisibility.telemetry.enabled"; public static final String CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS = diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 9ed2b362b2e..6d90320dbc0 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -212,6 +212,7 @@ import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_SOURCE_DATA_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_TELEMETRY_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_TEST_SKIPPING_ENABLED; +import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_TRACE_SANITATION_ENABLED; import static datadog.trace.api.config.CrashTrackingConfig.CRASH_TRACKING_AGENTLESS; import static datadog.trace.api.config.CrashTrackingConfig.CRASH_TRACKING_AGENTLESS_DEFAULT; @@ -831,6 +832,7 @@ static class HostNameHolder { private final boolean ciVisibilityFlakyRetryEnabled; private final boolean ciVisibilityFlakyRetryOnlyKnownFlakes; private final int ciVisibilityFlakyRetryCount; + private final int ciVisibilityTotalFlakyRetryCount; private final boolean ciVisibilityEarlyFlakeDetectionEnabled; private final int ciVisibilityEarlyFlakeDetectionLowerLimit; private final String ciVisibilityModuleName; @@ -1893,6 +1895,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) ciVisibilityEarlyFlakeDetectionLowerLimit = configProvider.getInteger(CIVISIBILITY_EARLY_FLAKE_DETECTION_LOWER_LIMIT, 30); ciVisibilityFlakyRetryCount = configProvider.getInteger(CIVISIBILITY_FLAKY_RETRY_COUNT, 5); + ciVisibilityTotalFlakyRetryCount = + configProvider.getInteger(CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT, 1000); ciVisibilityModuleName = configProvider.getString(CIVISIBILITY_MODULE_NAME); ciVisibilityTelemetryEnabled = configProvider.getBoolean(CIVISIBILITY_TELEMETRY_ENABLED, true); ciVisibilityRumFlushWaitMillis = @@ -3226,6 +3230,10 @@ public int getCiVisibilityFlakyRetryCount() { return ciVisibilityFlakyRetryCount; } + public int getCiVisibilityTotalFlakyRetryCount() { + return ciVisibilityTotalFlakyRetryCount; + } + public String getCiVisibilityModuleName() { return ciVisibilityModuleName; } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityCountMetric.java b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityCountMetric.java index ed29be6d4f4..bdbfbc163fe 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityCountMetric.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityCountMetric.java @@ -15,6 +15,7 @@ import datadog.trace.api.civisibility.telemetry.tag.IsBenchmark; import datadog.trace.api.civisibility.telemetry.tag.IsHeadless; import datadog.trace.api.civisibility.telemetry.tag.IsNew; +import datadog.trace.api.civisibility.telemetry.tag.IsRetry; import datadog.trace.api.civisibility.telemetry.tag.IsRum; import datadog.trace.api.civisibility.telemetry.tag.IsUnsupportedCI; import datadog.trace.api.civisibility.telemetry.tag.ItrEnabled; @@ -53,6 +54,7 @@ public enum CiVisibilityCountMetric { IsBenchmark.class, EarlyFlakeDetectionAbortReason.class, IsNew.class, + IsRetry.class, IsRum.class, BrowserDriver.class), /** The number of successfully collected code coverages that are empty */ diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/IsRetry.java b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/IsRetry.java new file mode 100644 index 00000000000..f8139b0027d --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/IsRetry.java @@ -0,0 +1,13 @@ +package datadog.trace.api.civisibility.telemetry.tag; + +import datadog.trace.api.civisibility.telemetry.TagValue; + +/** Whether a test case is a retry or not. */ +public enum IsRetry implements TagValue { + TRUE; + + @Override + public String asString() { + return "is_retry:true"; + } +}