Skip to content

Commit bcddb29

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
authored
Generalize test retry logic (#8289)
1 parent f06d59c commit bcddb29

File tree

21 files changed

+76
-64
lines changed

21 files changed

+76
-64
lines changed

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
import datadog.trace.api.civisibility.domain.TestContext;
1616
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
1717
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
18+
import datadog.trace.api.civisibility.telemetry.TagValue;
1819
import datadog.trace.api.civisibility.telemetry.tag.BrowserDriver;
1920
import datadog.trace.api.civisibility.telemetry.tag.EventType;
2021
import datadog.trace.api.civisibility.telemetry.tag.IsModified;
2122
import datadog.trace.api.civisibility.telemetry.tag.IsNew;
2223
import datadog.trace.api.civisibility.telemetry.tag.IsRetry;
2324
import datadog.trace.api.civisibility.telemetry.tag.IsRum;
24-
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
2525
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
2626
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
2727
import datadog.trace.api.gateway.RequestContextSlot;
@@ -263,6 +263,7 @@ public void end(@Nullable Long endTime) {
263263
span.finish();
264264
}
265265

266+
Object retryReason = span.getTag(Tags.TEST_RETRY_REASON);
266267
metricCollector.add(
267268
CiVisibilityCountMetric.EVENT_FINISHED,
268269
1,
@@ -271,25 +272,13 @@ public void end(@Nullable Long endTime) {
271272
span.getTag(Tags.TEST_IS_NEW) != null ? IsNew.TRUE : null,
272273
span.getTag(Tags.TEST_IS_MODIFIED) != null ? IsModified.TRUE : null,
273274
span.getTag(Tags.TEST_IS_RETRY) != null ? IsRetry.TRUE : null,
274-
getRetryReason(),
275+
retryReason instanceof TagValue ? (TagValue) retryReason : null,
275276
span.getTag(Tags.TEST_IS_RUM_ACTIVE) != null ? IsRum.TRUE : null,
276277
CIConstants.SELENIUM_BROWSER_DRIVER.equals(span.getTag(Tags.TEST_BROWSER_DRIVER))
277278
? BrowserDriver.SELENIUM
278279
: null);
279280
}
280281

281-
private RetryReason getRetryReason() {
282-
String retryReason = (String) span.getTag(Tags.TEST_RETRY_REASON);
283-
if (retryReason != null) {
284-
try {
285-
return RetryReason.valueOf(retryReason.toUpperCase());
286-
} catch (IllegalArgumentException e) {
287-
log.debug("Non-standard retry-reason: {}", retryReason);
288-
}
289-
}
290-
return null;
291-
}
292-
293282
/**
294283
* Tests often perform operations that involve APM instrumentations: sending an HTTP request,
295284
* executing a database query, etc. APM instrumentations create spans that correspond to those

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import datadog.trace.api.civisibility.config.TestSourceData;
77
import datadog.trace.api.civisibility.events.TestEventsHandler;
88
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
9+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
910
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1011
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1112
import datadog.trace.bootstrap.ContextStore;
@@ -57,7 +58,7 @@ public void onTestStart(
5758
@Nullable String testParameters,
5859
@Nullable Collection<String> categories,
5960
@Nonnull TestSourceData testSourceData,
60-
String retryReason,
61+
RetryReason retryReason,
6162
@Nullable Long startTime) {
6263
// do nothing
6364
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
1313
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
1414
import datadog.trace.api.civisibility.telemetry.tag.EventType;
15+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
1516
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1617
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1718
import datadog.trace.bootstrap.ContextStore;
@@ -138,7 +139,7 @@ public void onTestStart(
138139
final @Nullable String testParameters,
139140
final @Nullable Collection<String> categories,
140141
final @Nonnull TestSourceData testSourceData,
141-
final @Nullable String retryReason,
142+
final @Nullable RetryReason retryReason,
142143
final @Nullable Long startTime) {
143144
if (skipTrace(testSourceData.getTestClass())) {
144145
return;

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/NeverRetry.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.civisibility.retry;
22

33
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
4+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
45
import org.jetbrains.annotations.Nullable;
56

67
public class NeverRetry implements TestRetryPolicy {
@@ -20,18 +21,13 @@ public boolean suppressFailures() {
2021
}
2122

2223
@Override
23-
public boolean retry(boolean successful, long duration) {
24-
return false;
25-
}
26-
27-
@Override
28-
public boolean currentExecutionIsRetry() {
24+
public boolean retry(boolean successful, long durationMillis) {
2925
return false;
3026
}
3127

3228
@Nullable
3329
@Override
34-
public String currentExecutionRetryReason() {
30+
public RetryReason currentExecutionRetryReason() {
3531
return null;
3632
}
3733
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryIfFailed.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
package datadog.trace.civisibility.retry;
22

33
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
4+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
45
import java.util.concurrent.atomic.AtomicInteger;
56
import org.jetbrains.annotations.Nullable;
67

78
/** Retries a test case if it failed, up to a maximum number of times. */
89
public class RetryIfFailed implements TestRetryPolicy {
910

10-
private static final String AUTO_TEST_RETRIES = "atr";
11-
1211
private final int maxExecutions;
1312
private int executions;
1413

@@ -28,11 +27,13 @@ public boolean retriesLeft() {
2827

2928
@Override
3029
public boolean suppressFailures() {
31-
return true;
30+
// if this isn't the last attempt,
31+
// possible failures should be suppressed
32+
return retriesLeft();
3233
}
3334

3435
@Override
35-
public boolean retry(boolean successful, long duration) {
36+
public boolean retry(boolean successful, long durationMillis) {
3637
if (!successful && ++executions < maxExecutions) {
3738
totalExecutions.incrementAndGet();
3839
return true;
@@ -41,14 +42,13 @@ public boolean retry(boolean successful, long duration) {
4142
}
4243
}
4344

45+
@Nullable
4446
@Override
45-
public boolean currentExecutionIsRetry() {
46-
return executions > 0;
47+
public RetryReason currentExecutionRetryReason() {
48+
return currentExecutionIsRetry() ? RetryReason.atr : null;
4749
}
4850

49-
@Nullable
50-
@Override
51-
public String currentExecutionRetryReason() {
52-
return currentExecutionIsRetry() ? AUTO_TEST_RETRIES : null;
51+
private boolean currentExecutionIsRetry() {
52+
return executions > 0;
5353
}
5454
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/retry/RetryNTimes.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
package datadog.trace.civisibility.retry;
22

33
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
4+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
45
import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings;
56
import org.jetbrains.annotations.Nullable;
67

78
/** Retries a test case N times (N depends on test duration) regardless of success or failure. */
89
public class RetryNTimes implements TestRetryPolicy {
910

10-
private static final String EARLY_FLAKINESS_DETECTION = "efd";
11-
1211
private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
1312
private int executions;
1413
private int maxExecutions;
@@ -30,21 +29,20 @@ public boolean suppressFailures() {
3029
}
3130

3231
@Override
33-
public boolean retry(boolean successful, long duration) {
32+
public boolean retry(boolean successful, long durationMillis) {
3433
// adjust maximum retries based on the now known test duration
35-
int maxExecutionsForGivenDuration = earlyFlakeDetectionSettings.getExecutions(duration);
34+
int maxExecutionsForGivenDuration = earlyFlakeDetectionSettings.getExecutions(durationMillis);
3635
maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration);
3736
return ++executions < maxExecutions;
3837
}
3938

39+
@Nullable
4040
@Override
41-
public boolean currentExecutionIsRetry() {
42-
return executions > 0;
41+
public RetryReason currentExecutionRetryReason() {
42+
return currentExecutionIsRetry() ? RetryReason.efd : null;
4343
}
4444

45-
@Nullable
46-
@Override
47-
public String currentExecutionRetryReason() {
48-
return currentExecutionIsRetry() ? EARLY_FLAKINESS_DETECTION : null;
45+
private boolean currentExecutionIsRetry() {
46+
return executions > 0;
4947
}
5048
}

‎dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/retry/RetryAwareNotifier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public RetryAwareNotifier(TestRetryPolicy retryPolicy, RunNotifier notifier) {
2727
public void fireTestFailure(Failure failure) {
2828
this.failed = true;
2929

30-
if (!retryPolicy.retriesLeft() || !retryPolicy.suppressFailures()) {
30+
if (!retryPolicy.suppressFailures()) {
3131
super.fireTestFailure(failure);
3232
return;
3333
}

‎dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnitPlatformUtils.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import datadog.trace.api.civisibility.config.TestIdentifier;
66
import datadog.trace.api.civisibility.config.TestSourceData;
7+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
78
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
89
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
910
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -185,8 +186,9 @@ public static boolean isParameterizedTest(TestDescriptor testDescriptor) {
185186
return "test-template".equals(lastSegment.getType());
186187
}
187188

188-
public static String retryReason(TestDescriptor testDescriptor) {
189-
return getIDSegmentValue(testDescriptor, RETRY_DESCRIPTOR_REASON_SUFFIX);
189+
public static RetryReason retryReason(TestDescriptor testDescriptor) {
190+
String retryReasonSegment = getIDSegmentValue(testDescriptor, RETRY_DESCRIPTOR_REASON_SUFFIX);
191+
return retryReasonSegment != null ? RetryReason.valueOf(retryReasonSegment) : null;
190192
}
191193

192194
public static boolean isRetry(TestDescriptor testDescriptor) {

‎dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/retry/JUnit5RetryInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public static Boolean execute(@Advice.This HierarchicalTestExecutorService.TestT
158158
int retryAttemptIdx = 0;
159159
boolean retry;
160160
while (true) {
161-
factory.setSuppressFailures(retryPolicy.retriesLeft() && retryPolicy.suppressFailures());
161+
factory.setSuppressFailures(retryPolicy.suppressFailures());
162162

163163
long startTimestamp = System.currentTimeMillis();
164164
CallDepthThreadLocalMap.incrementCallDepth(HierarchicalTestExecutorService.TestTask.class);

‎dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/retry/TestDescriptorHandle.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,12 @@ public TestDescriptorHandle(TestDescriptor testDescriptor) {
4343
}
4444

4545
public TestDescriptor withIdSuffix(
46-
String segmentName, String segmentValue, String otherSegmentName, String otherSegmentValue) {
46+
String segmentName, Object segmentValue, String otherSegmentName, Object otherSegmentValue) {
4747
UniqueId uniqueId = testDescriptor.getUniqueId();
4848
UniqueId updatedId =
49-
uniqueId.append(segmentName, segmentValue).append(otherSegmentName, otherSegmentValue);
49+
uniqueId
50+
.append(segmentName, String.valueOf(segmentValue))
51+
.append(otherSegmentName, String.valueOf(otherSegmentValue));
5052
TestDescriptor descriptorClone = UnsafeUtils.tryShallowClone(testDescriptor);
5153
METHOD_HANDLES.invoke(UNIQUE_ID_SETTER, descriptorClone, updatedId);
5254
return descriptorClone;

‎dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateRetryInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public static void onAddingStepResult(
124124
retryContext.setFailed(true);
125125

126126
TestRetryPolicy retryPolicy = retryContext.getRetryPolicy();
127-
if (retryPolicy.suppressFailures() && retryPolicy.retriesLeft()) {
127+
if (retryPolicy.suppressFailures()) {
128128
stepResult = new StepResult(stepResult.getStep(), KarateUtils.abortedResult());
129129
stepResult.setFailedReason(result.getError());
130130
stepResult.setErrorIgnored(true);

‎dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import datadog.trace.api.civisibility.config.TestSourceData;
2020
import datadog.trace.api.civisibility.events.TestDescriptor;
2121
import datadog.trace.api.civisibility.events.TestSuiteDescriptor;
22+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
2223
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
2324
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
2425
import datadog.trace.bootstrap.ContextStore;
@@ -143,7 +144,7 @@ public boolean beforeScenario(ScenarioRuntime sr) {
143144
parameters,
144145
categories,
145146
TestSourceData.UNKNOWN,
146-
(String) sr.magicVariables.get(KarateUtils.RETRY_MAGIC_VARIABLE),
147+
(RetryReason) sr.magicVariables.get(KarateUtils.RETRY_MAGIC_VARIABLE),
147148
null);
148149
return true;
149150
}

‎dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/retry/TestExecutionWrapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public Outcome apply(SuperEngine<?>.TestLeaf testLeaf) {
2929

3030
if (outcome.isFailed()) {
3131
executionFailed = true;
32-
if (retryPolicy.retriesLeft() && retryPolicy.suppressFailures()) {
32+
if (retryPolicy.suppressFailures()) {
3333
Throwable t = outcome.toOption().get();
3434
return Canceled.apply(
3535
new SuppressedTestFailedException("Test failed and will be retried", t, 0));

‎dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TracingListener.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import datadog.trace.api.civisibility.config.TestSourceData;
44
import datadog.trace.api.civisibility.events.TestSuiteDescriptor;
5+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
56
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
67
import datadog.trace.instrumentation.testng.retry.RetryAnalyzer;
78
import java.util.List;
@@ -94,7 +95,7 @@ public void onTestStart(final ITestResult result) {
9495
null);
9596
}
9697

97-
private String retryReason(final ITestResult result) {
98+
private RetryReason retryReason(final ITestResult result) {
9899
IRetryAnalyzer retryAnalyzer = TestNGUtils.getRetryAnalyzer(result);
99100
if (retryAnalyzer instanceof RetryAnalyzer) {
100101
RetryAnalyzer datadogAnalyzer = (RetryAnalyzer) retryAnalyzer;

‎dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/retry/RetryAnalyzer.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.trace.api.civisibility.config.TestIdentifier;
44
import datadog.trace.api.civisibility.config.TestSourceData;
55
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
6+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
67
import datadog.trace.instrumentation.testng.TestEventsHandlerHolder;
78
import datadog.trace.instrumentation.testng.TestNGUtils;
89
import org.testng.IRetryAnalyzer;
@@ -31,7 +32,7 @@ public boolean retry(ITestResult result) {
3132
return retryPolicy.retry(result.isSuccess(), result.getEndMillis() - result.getStartMillis());
3233
}
3334

34-
public String currentExecutionRetryReason() {
35+
public RetryReason currentExecutionRetryReason() {
3536
return retryPolicy != null ? retryPolicy.currentExecutionRetryReason() : null;
3637
}
3738
}

‎dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGClassListenerInstrumentation.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public String[] helperClassNames() {
6060
}
6161

6262
public static class InvokeBeforeClassAdvice {
63+
@SuppressWarnings("bytebuddy-exception-suppression")
6364
@Advice.OnMethodEnter
6465
public static void invokeBeforeClass(
6566
@Advice.FieldValue("m_testContext") final ITestContext testContext,
@@ -81,6 +82,7 @@ public static String muzzleCheck(final CustomAttribute customAttribute) {
8182
}
8283

8384
public static class InvokeAfterClassAdvice {
85+
@SuppressWarnings("bytebuddy-exception-suppression")
8486
@Advice.OnMethodExit
8587
public static void invokeAfterClass(
8688
@Advice.FieldValue("m_testContext") final ITestContext testContext,

‎dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGRetryInstrumentation.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public String[] helperClassNames() {
5555
}
5656

5757
public static class RetryAdvice {
58+
@SuppressWarnings("bytebuddy-exception-suppression")
5859
@SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
5960
@Advice.OnMethodExit
6061
public static void shouldRetryTestMethod(

‎dd-java-agent/instrumentation/weaver/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import datadog.trace.api.civisibility.events.TestDescriptor;
66
import datadog.trace.api.civisibility.events.TestEventsHandler;
77
import datadog.trace.api.civisibility.events.TestSuiteDescriptor;
8+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
89
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
910
import datadog.trace.api.time.SystemTimeSource;
1011
import datadog.trace.util.AgentThreadFactory;
@@ -91,7 +92,7 @@ public static void onTestFinished(TestFinished event, TaskDef taskDef) {
9192
new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier);
9293
String testMethodName = null;
9394
Method testMethod = null;
94-
String retryReason = null;
95+
RetryReason retryReason = null;
9596

9697
// Only test finish is reported, so fake test start timestamp
9798
long endMicros = SystemTimeSource.INSTANCE.getCurrentTimeMicros();

‎internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import datadog.trace.api.civisibility.config.TestIdentifier;
66
import datadog.trace.api.civisibility.config.TestSourceData;
77
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
8+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
89
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
910
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1011
import datadog.trace.bootstrap.ContextStore;
@@ -64,7 +65,7 @@ void onTestStart(
6465
@Nullable String testParameters,
6566
@Nullable Collection<String> categories,
6667
@Nonnull TestSourceData testSourceData,
67-
@Nullable String retryReason,
68+
@Nullable RetryReason retryReason,
6869
@Nullable Long startTime);
6970

7071
void onTestSkip(TestKey descriptor, @Nullable String reason);

0 commit comments

Comments
 (0)