Avoid false activation-after-resolution findings for overlapping resumes

Author: amarziali

dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ContinuationRecord.java

@@ -140,7 +140,8 @@ public synchronized EnumSet<Failure> failures(Long rootWrittenNanos) {
     if (duplicateTerminalAttempt || !extraTerminals.isEmpty()) {
       failures.add(Failure.DOUBLE_FINISH);
     }
-    if ((terminal != null && !failedActivations.isEmpty()) || resumedAfterTerminal()) {
+    // Cleanup entry timestamps do not order resolution against concurrent successful resumes.
+    if (terminal != null && !failedActivations.isEmpty()) {
       failures.add(Failure.ACTIVATE_AFTER_RESOLVE);
     }
     if (rootWrittenNanos != null
@@ -150,18 +151,6 @@ public synchronized EnumSet<Failure> failures(Long rootWrittenNanos) {
     return failures;
   }
 
-  private boolean resumedAfterTerminal() {
-    if (terminal == null) {
-      return false;
-    }
-    for (ScopeEvent r : resumes) {
-      if (r.nanos > terminal.nanos) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   /** {@code true} when capture and any resume/terminal happened on different threads. */
   public synchronized boolean threadHandoff() {
     if (capture == null) {

dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java

@@ -122,16 +122,19 @@ void multipleResolutionsAreFlaggedDouble() {
   }
 
   @Test
-  void activationAfterResolveIsFailure() {
+  void successfulResumeAfterCleanupEntryIsNotActivationAfterResolve() {
     ContinuationRecord r = record(0, DDTraceId.from(14));
-    r.setTerminalOrExtra(event(ScopeEvent.Type.RESOLVE_RELEASE, "pool-1", 2000));
+    r.addResume(event(ScopeEvent.Type.ACTIVATE, "pool-1", 1500));
     r.addResume(event(ScopeEvent.Type.ACTIVATE, "pool-2", 3000));
+    r.setTerminalOrExtra(event(ScopeEvent.Type.RESOLVE_FINISH, "pool-1", 2000));
 
     ScopeDiagnosticsReport report = report(list(r), map());
 
-    assertEquals(1, report.activateAfterResolveCount());
+    assertEquals(2, report.records().get(0).resumes().size());
+    assertEquals(ContinuationStatus.FINISHED, report.records().get(0).status());
+    assertEquals(0, report.activateAfterResolveCount());
     assertEquals(0, report.doubleCount());
-    assertTrue(report.hasProblems());
+    assertFalse(report.hasProblems());
   }
 
   @Test

dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeResolutionTest.java

@@ -58,6 +58,24 @@ void nestedReleaseBelongsToTheScopeClose() {
     assertEquals(ContinuationStatus.FINISHED, ScopeDiagnostics.report().records().get(0).status());
   }
 
+  @Test
+  void delayedSuccessfulResumeCallbackAfterCleanupIsNotRejected() {
+    Object window = ScopeDiagnostics.recordingWindow();
+    ScopeContinuationProbe.ResolveAttempt close = enter("cancelFromContinuedScopeClose", 1);
+    ScopeContinuationProbe.onResolveExit(close, CANCELLED);
+    long cleanupEntryNanos = ScopeDiagnostics.report().records().get(0).terminal().nanos;
+
+    // The resume succeeded during cleanup, but its exit callback arrives after cleanup's callback.
+    ScopeDiagnostics.recordActivate(
+        window, continuation, DDTraceId.from(1), 2, "op", (byte) 0, cleanupEntryNanos + 1);
+
+    ScopeDiagnosticsReport report = ScopeDiagnostics.report();
+    assertEquals(1, report.records().get(0).resumes().size());
+    assertEquals(ContinuationStatus.FINISHED, report.records().get(0).status());
+    assertEquals(0, report.activateAfterResolveCount());
+    assertResolvedOnce();
+  }
+
   @Test
   void overlappingLegitimateScopeClosesResolveOnce() {
     Object window = ScopeDiagnostics.recordingWindow();