From f9f7b35d30c63326e8ccf406bd6bbd797fe67eae Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 5 Oct 2026 10:07:34 +0200 Subject: [PATCH 1/4] Allow instrumentation tests to select diagnostic checks --- .../test/InstrumentationSpecification.groovy | 2 +- .../test/scopediag/ContinuationRecord.java | 12 +-- .../test/scopediag/ScopeDiagnostics.java | 46 ++++++--- ...ailure.java => ScopeDiagnosticsCheck.java} | 7 +- .../scopediag/ScopeDiagnosticsExtension.java | 2 +- .../scopediag/ScopeDiagnosticsReport.java | 98 +++++++++++++------ .../agent/test/scopediag/ScopeRecord.java | 8 +- .../scopediag/TrackScopeContinuations.java | 20 +++- .../ScopeDiagnosticsAdmissionTest.java | 4 +- .../ScopeDiagnosticsConfigurationTest.java | 71 ++++++++++++++ .../ScopeDiagnosticsIntegrationTest.java | 18 ++-- .../scopediag/ScopeDiagnosticsReportTest.java | 28 ++++-- .../agent/test/scopediag/ScopeRecordTest.java | 16 +-- .../test/scopediag/ScopeResolutionTest.java | 2 +- docs/how_to_test.md | 22 ++++- 15 files changed, 269 insertions(+), 87 deletions(-) rename dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/{Failure.java => ScopeDiagnosticsCheck.java} (73%) diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index fb94da94ae7..60357ed625b 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -565,7 +565,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A if (report.hasFindings()) { println(report.renderTimeline()) } - ScopeDiagnostics.assertNoLeaks(report) + ScopeDiagnostics.assertNoViolations(report, config) return null } catch (Throwable failure) { return failure diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ContinuationRecord.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ContinuationRecord.java index 3758977f646..545f3f3716a 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ContinuationRecord.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ContinuationRecord.java @@ -132,21 +132,21 @@ public synchronized ContinuationStatus status() { } /** Derives failures, including events after {@code rootWrittenNanos} when provided. */ - public synchronized EnumSet failures(Long rootWrittenNanos) { - EnumSet failures = EnumSet.noneOf(Failure.class); + public synchronized EnumSet failures(Long rootWrittenNanos) { + EnumSet failures = EnumSet.noneOf(ScopeDiagnosticsCheck.class); if (terminal == null) { - failures.add(Failure.LEAKED); + failures.add(ScopeDiagnosticsCheck.LEAKED); } if (duplicateTerminalAttempt || !extraTerminals.isEmpty()) { - failures.add(Failure.DOUBLE_FINISH); + failures.add(ScopeDiagnosticsCheck.DOUBLE_FINISH); } // Cleanup entry timestamps do not order resolution against concurrent successful resumes. if (terminal != null && !failedActivations.isEmpty()) { - failures.add(Failure.ACTIVATE_AFTER_RESOLVE); + failures.add(ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE); } if (rootWrittenNanos != null && (laterThan(terminal, rootWrittenNanos) || laterThan(resumes, rootWrittenNanos))) { - failures.add(Failure.LATE_FINISH); + failures.add(ScopeDiagnosticsCheck.LATE_FINISH); } return failures; } diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java index c26b8739065..b704c0390cd 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java @@ -4,6 +4,7 @@ import datadog.trace.api.DDTraceId; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.IdentityHashMap; import java.util.Map; @@ -102,30 +103,51 @@ public static void awaitQuiescence() { /** * Fails with an {@link AssertionError} (carrying the problem summary) if the report flags a - * genuine bug (see {@link ScopeDiagnosticsReport#hasProblems()}). Report-only signals such as + * genuine bug (see {@link ScopeDiagnosticsReport#hasViolations()}). Report-only signals such as * late-after-root and close-on-wrong-thread do not fail. */ - public static void assertNoLeaks() { - assertNoLeaks(report()); + public static void assertNoViolations() { + assertNoViolations(report()); } /** Fails using the supplied snapshot, so rendering and assertion examine the same events. */ - public static void assertNoLeaks(ScopeDiagnosticsReport report) { - if (report.hasProblems()) { - throw new AssertionError("Scope continuation problems detected:\n" + report.renderSummary()); + public static void assertNoViolations(ScopeDiagnosticsReport report) { + assertNoViolations(report, null); + } + + /** Fails when the supplied configuration selects an enforced violation in the snapshot. */ + public static void assertNoViolations( + ScopeDiagnosticsReport report, TrackScopeContinuations config) { + if (report.hasViolations(enabledChecks(config))) { + throw new AssertionError( + "Scope/continuation lifecycle violations detected:\n" + report.renderSummary()); } } - /** Resolves the default-on policy and rejects opt-outs without a reason. */ + /** Returns whether the configuration selects any checks to evaluate. */ public static boolean isEnabled(TrackScopeContinuations config) { - if (config == null || config.enabled()) { - return true; + return !enabledChecks(config).isEmpty(); + } + + static EnumSet enabledChecks(TrackScopeContinuations config) { + EnumSet allChecks = EnumSet.allOf(ScopeDiagnosticsCheck.class); + if (config == null) { + return allChecks; } - if (config.reason().trim().isEmpty()) { + + EnumSet configured = EnumSet.noneOf(ScopeDiagnosticsCheck.class); + Collections.addAll(configured, config.checks()); + EnumSet enabled = configured; + if (!config.enabled()) { + enabled = allChecks.clone(); + enabled.removeAll(configured); + } + + if (!enabled.equals(allChecks) && config.reason().trim().isEmpty()) { throw new IllegalArgumentException( - "@TrackScopeContinuations(enabled = false) requires a reason"); + "@TrackScopeContinuations requires a reason when checks are disabled"); } - return false; + return enabled; } private void clear() { diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/Failure.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsCheck.java similarity index 73% rename from dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/Failure.java rename to dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsCheck.java index 64a596bf473..448e0429dd8 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/Failure.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsCheck.java @@ -1,7 +1,10 @@ package datadog.trace.agent.test.scopediag; -/** A derived scope or continuation lifecycle finding. */ -public enum Failure { +/** + * A scope or continuation lifecycle condition evaluated by the test diagnostic. Enforcement or + * advisory treatment is defined independently by the diagnostic policy. + */ +public enum ScopeDiagnosticsCheck { /** Continuation captured but never resolved within the window. */ LEAKED, /** Continuation resolved/resumed after the root span of its trace was already written. */ diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java index 7ba889284d0..295304ca364 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java @@ -34,7 +34,7 @@ public void afterEach(ExtensionContext context) { if (report.hasFindings()) { log.info("[{}] {}", context.getDisplayName(), report.renderTimeline()); } - ScopeDiagnostics.assertNoLeaks(report); + ScopeDiagnostics.assertNoViolations(report, config); } finally { ScopeDiagnostics.reset(); } diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java index d25d9020853..f9965b963ff 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java @@ -6,14 +6,22 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; /** Immutable scope and continuation lifecycle snapshot with derived failures. */ public final class ScopeDiagnosticsReport { + private static final EnumSet ENFORCED_CHECKS = + EnumSet.of( + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.NEVER_CLOSED); + private final List continuations; private final List scopes; private final long t0; - private final Map> continuationFailures; - private final Map> scopeFailures; + private final Map> continuationFailures; + private final Map> scopeFailures; private final Map scopeBySeq; ScopeDiagnosticsReport( @@ -50,11 +58,11 @@ private static long computeT0(List continuations, List> classifyContinuations( + private static Map> classifyContinuations( List records, Map rootWrittenNanos) { - Map> result = new LinkedHashMap<>(); + Map> result = new LinkedHashMap<>(); for (ContinuationRecord r : records) { - EnumSet failures = r.failures(rootWrittenNanos.get(r.traceId)); + EnumSet failures = r.failures(rootWrittenNanos.get(r.traceId)); if (!failures.isEmpty()) { result.put(r, failures); } @@ -62,10 +70,11 @@ private static Map> classifyContinuations( return result; } - private static Map> classifyScopes(List scopes) { - Map> result = new LinkedHashMap<>(); + private static Map> classifyScopes( + List scopes) { + Map> result = new LinkedHashMap<>(); for (ScopeRecord s : scopes) { - EnumSet failures = s.failures(); + EnumSet failures = s.failures(); if (!failures.isEmpty()) { result.put(s, failures); } @@ -81,36 +90,36 @@ public List scopeRecords() { return new ArrayList<>(scopes); } - public Map> findings() { + public Map> findings() { return new LinkedHashMap<>(continuationFailures); } - public Map> scopeFindings() { + public Map> scopeFindings() { return new LinkedHashMap<>(scopeFailures); } public int leakCount() { - return countWith(continuationFailures, Failure.LEAKED); + return countWith(continuationFailures, ScopeDiagnosticsCheck.LEAKED); } public int lateCount() { - return countWith(continuationFailures, Failure.LATE_FINISH); + return countWith(continuationFailures, ScopeDiagnosticsCheck.LATE_FINISH); } public int doubleCount() { - return countWith(continuationFailures, Failure.DOUBLE_FINISH); + return countWith(continuationFailures, ScopeDiagnosticsCheck.DOUBLE_FINISH); } public int activateAfterResolveCount() { - return countWith(continuationFailures, Failure.ACTIVATE_AFTER_RESOLVE); + return countWith(continuationFailures, ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE); } public int neverClosedScopeCount() { - return countWith(scopeFailures, Failure.NEVER_CLOSED); + return countWith(scopeFailures, ScopeDiagnosticsCheck.NEVER_CLOSED); } public int closeWrongThreadCount() { - return countWith(scopeFailures, Failure.CLOSE_WRONG_THREAD); + return countWith(scopeFailures, ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD); } public int deferredCleanupScopeCount() { @@ -127,22 +136,49 @@ boolean hasIncompleteLifecycles() { return leakCount() > 0 || neverClosedScopeCount() > 0; } - private static int countWith(Map> findings, Failure failure) { + private static int countWith( + Map> findings, ScopeDiagnosticsCheck check) { int n = 0; - for (EnumSet f : findings.values()) { - if (f.contains(failure)) { + for (EnumSet f : findings.values()) { + if (f.contains(check)) { n++; } } return n; } - /** Returns whether the report contains a failure that should fail the test. */ - public boolean hasProblems() { - return leakCount() > 0 - || doubleCount() > 0 - || activateAfterResolveCount() > 0 - || neverClosedScopeCount() > 0; + /** Returns whether the report contains a violation that should fail the test. */ + public boolean hasViolations() { + return hasViolations(EnumSet.allOf(ScopeDiagnosticsCheck.class)); + } + + /** Returns whether an enabled check contains a finding enforced by the current policy. */ + boolean hasViolations(Set enabledChecks) { + for (ScopeDiagnosticsCheck check : enabledChecks) { + if (ENFORCED_CHECKS.contains(check) && count(check) > 0) { + return true; + } + } + return false; + } + + private int count(ScopeDiagnosticsCheck check) { + switch (check) { + case LEAKED: + return leakCount(); + case LATE_FINISH: + return lateCount(); + case DOUBLE_FINISH: + return doubleCount(); + case ACTIVATE_AFTER_RESOLVE: + return activateAfterResolveCount(); + case CLOSE_WRONG_THREAD: + return closeWrongThreadCount(); + case NEVER_CLOSED: + return neverClosedScopeCount(); + default: + throw new IllegalStateException("Unknown scope diagnostic check: " + check); + } } /** True when the report contains either a failing problem or an advisory signal. */ @@ -181,7 +217,8 @@ public String renderSummary() { sb.append(" (none)\n"); return sb.toString(); } - for (Map.Entry> e : continuationFailures.entrySet()) { + for (Map.Entry> e : + continuationFailures.entrySet()) { ContinuationRecord r = e.getKey(); ScopeEvent capture = r.capture(); sb.append(" ") @@ -196,7 +233,7 @@ public String renderSummary() { .append(capture == null || capture.callsite() == null ? "" : capture.callsite()) .append('\n'); } - for (Map.Entry> e : scopeFailures.entrySet()) { + for (Map.Entry> e : scopeFailures.entrySet()) { ScopeRecord s = e.getKey(); ScopeEvent open = s.open(); sb.append(" ") @@ -226,8 +263,8 @@ public String renderTimeline() { } for (ContinuationRecord r : continuations) { - EnumSet failures = - continuationFailures.getOrDefault(r, EnumSet.noneOf(Failure.class)); + EnumSet failures = + continuationFailures.getOrDefault(r, EnumSet.noneOf(ScopeDiagnosticsCheck.class)); sb.append("\n#") .append(r.seq) .append(' ') @@ -308,7 +345,8 @@ private String timing(ContinuationRecord r) { } private void appendScopeLine(StringBuilder sb, String indent, ScopeRecord scope) { - EnumSet failures = scopeFailures.getOrDefault(scope, EnumSet.noneOf(Failure.class)); + EnumSet failures = + scopeFailures.getOrDefault(scope, EnumSet.noneOf(ScopeDiagnosticsCheck.class)); sb.append(indent).append("scope#").append(scope.seq).append(' ').append(scope.sourceName()); if (scope.spanName != null) { sb.append(" \"").append(scope.spanName).append('"'); diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeRecord.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeRecord.java index ec7f72b4117..c11f22e72a2 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeRecord.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeRecord.java @@ -107,13 +107,13 @@ public synchronized Long activeDurationNanos() { return close.nanos - open.nanos; } - public synchronized EnumSet failures() { - EnumSet failures = EnumSet.noneOf(Failure.class); + public synchronized EnumSet failures() { + EnumSet failures = EnumSet.noneOf(ScopeDiagnosticsCheck.class); if (open != null && close == null && !deferredCleanup) { - failures.add(Failure.NEVER_CLOSED); + failures.add(ScopeDiagnosticsCheck.NEVER_CLOSED); } if (!wrongThreadCloses.isEmpty()) { - failures.add(Failure.CLOSE_WRONG_THREAD); + failures.add(ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD); } return failures; } diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java index dcef5bb15a3..a3939b427ff 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java @@ -11,9 +11,25 @@ @Target({ElementType.TYPE, ElementType.METHOD}) @Inherited public @interface TrackScopeContinuations { - /** Set to {@code false} only for a proven incompatibility with the diagnostic itself. */ + /** + * Selects whether {@link #checks()} is the complete set of active checks ({@code true}) or the + * checks to remove from the default full set ({@code false}). + */ boolean enabled() default true; - /** Explains why the diagnostic is disabled. Required when {@link #enabled()} is false. */ + /** + * Checks selected by {@link #enabled()}. The default lists every available check; a configuration + * test guards that invariant as new checks are added. + */ + ScopeDiagnosticsCheck[] checks() default { + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.LATE_FINISH, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD, + ScopeDiagnosticsCheck.NEVER_CLOSED + }; + + /** Explains any reduction from the default full check set. */ String reason() default ""; } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsAdmissionTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsAdmissionTest.java index 9b571a36842..71b1269a9a5 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsAdmissionTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsAdmissionTest.java @@ -32,7 +32,7 @@ void failedActivationBeforeResolutionAdviceIsRetained() { ScopeDiagnosticsReport report = ScopeDiagnostics.report(); assertEquals(0, report.leakCount()); assertEquals(1, report.activateAfterResolveCount()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); } @Test @@ -80,6 +80,6 @@ void delayedEventsCannotEnterTheNextRecordingWindow() { assertEquals(1, report.records().size()); assertEquals(0, report.leakCount()); assertEquals(0, report.lateCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java index 2b0ada14e7a..df797ca49e2 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java @@ -1,9 +1,12 @@ package datadog.trace.agent.test.scopediag; +import static java.util.Arrays.asList; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.EnumSet; import org.junit.jupiter.api.Test; class ScopeDiagnosticsConfigurationTest { @@ -14,11 +17,71 @@ private static class UndocumentedOptOut {} @TrackScopeContinuations(enabled = false, reason = "incompatible synthetic tracer") private static class DocumentedOptOut {} + @TrackScopeContinuations( + checks = {ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH}, + reason = "only these checks apply") + private static class CheckWhitelist {} + + @TrackScopeContinuations( + enabled = false, + checks = {ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH}, + reason = "these checks do not apply") + private static class CheckBlacklist {} + + @TrackScopeContinuations( + enabled = false, + checks = {}) + private static class EmptyBlacklist {} + + @TrackScopeContinuations(checks = ScopeDiagnosticsCheck.LEAKED) + private static class UndocumentedWhitelist {} + + @TrackScopeContinuations + private static class Defaults {} + @Test void diagnosticsAreEnabledByDefault() { assertTrue(ScopeDiagnostics.isEnabled(null)); } + @Test + void defaultChecksIncludeEveryEnumValue() { + TrackScopeContinuations config = Defaults.class.getAnnotation(TrackScopeContinuations.class); + + assertEquals( + EnumSet.allOf(ScopeDiagnosticsCheck.class), EnumSet.copyOf(asList(config.checks()))); + } + + @Test + void enabledChecksAreAWhitelist() { + TrackScopeContinuations config = + CheckWhitelist.class.getAnnotation(TrackScopeContinuations.class); + + assertEquals( + EnumSet.of(ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH), + ScopeDiagnostics.enabledChecks(config)); + } + + @Test + void disabledChecksAreABlacklist() { + TrackScopeContinuations config = + CheckBlacklist.class.getAnnotation(TrackScopeContinuations.class); + EnumSet expected = EnumSet.allOf(ScopeDiagnosticsCheck.class); + expected.remove(ScopeDiagnosticsCheck.LEAKED); + expected.remove(ScopeDiagnosticsCheck.LATE_FINISH); + + assertEquals(expected, ScopeDiagnostics.enabledChecks(config)); + } + + @Test + void emptyBlacklistKeepsEveryCheckEnabled() { + TrackScopeContinuations config = + EmptyBlacklist.class.getAnnotation(TrackScopeContinuations.class); + + assertEquals( + EnumSet.allOf(ScopeDiagnosticsCheck.class), ScopeDiagnostics.enabledChecks(config)); + } + @Test void documentedOptOutDisablesDiagnostics() { assertFalse( @@ -33,4 +96,12 @@ void undocumentedOptOutIsRejected() { assertThrows(IllegalArgumentException.class, () -> ScopeDiagnostics.isEnabled(config)); } + + @Test + void undocumentedWhitelistIsRejected() { + TrackScopeContinuations config = + UndocumentedWhitelist.class.getAnnotation(TrackScopeContinuations.class); + + assertThrows(IllegalArgumentException.class, () -> ScopeDiagnostics.isEnabled(config)); + } } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java index 55b9adbb408..0f4b826d413 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java @@ -49,7 +49,7 @@ void capturesRealLeakAndResolvedContinuation() { assertEquals(2, report.records().size(), "both captures recorded"); assertEquals(1, report.leakCount(), "exactly the un-resolved continuation leaks"); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); assertFalse(leaked.toString().isEmpty()); span.finish(); @@ -84,7 +84,7 @@ void sameSpanReactivationIsNotFlaggedActivateAfterResolve() { report.activateAfterResolveCount(), "a same-span re-activation resolved during activate() is not activate-after-resolve"); assertEquals(0, report.leakCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -114,14 +114,14 @@ void repeatedContinuedScopeCloseIsFlagged() { ContextContinuation continuation = tracer.capture(span); ContextScope scope = continuation.resume(); scope.close(); - assertFalse(ScopeDiagnostics.report().hasProblems()); + assertFalse(ScopeDiagnostics.report().hasViolations()); scope.close(); span.finish(); ScopeDiagnosticsReport report = ScopeDiagnostics.report(); assertEquals(1, report.doubleCount()); assertEquals(0, report.leakCount()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); assertTrue(report.renderTimeline().contains("DOUBLE_FINISH")); } @@ -147,7 +147,7 @@ void heldContinuationScopeCloseAndReleaseResolveOnce() { assertFalse(report.renderTimeline().contains("cancel")); assertEquals(0, report.doubleCount()); assertEquals(0, report.leakCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -163,7 +163,7 @@ void releaseWithoutResumeIsReportedAsReleased() { assertEquals(1, report.records().size()); assertEquals(ContinuationStatus.RELEASED, report.records().get(0).status()); assertEquals(ScopeEvent.Type.RESOLVE_RELEASE, report.records().get(0).terminal().type); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -228,7 +228,7 @@ void rootIterationScopeDelegatesCleanup() { ScopeDiagnosticsReport report = ScopeDiagnostics.report(); assertEquals(1, report.deferredCleanupScopeCount()); assertEquals(0, report.neverClosedScopeCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); tracer.closePrevious(true); } @@ -263,7 +263,7 @@ void waitsForAsynchronousScopeCleanup() throws Exception { ScopeDiagnosticsReport report = ScopeDiagnostics.report(); assertFalse(report.hasIncompleteLifecycles()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); span.finish(); } @@ -281,7 +281,7 @@ void neverClosedScopeIsFlagged() { assertEquals(1, report.neverClosedScopeCount(), "the open scope never closed"); assertEquals(1, report.leakCount(), "and the continuation it backs also leaks"); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); scope.close(); span.finish(); diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java index 232a7d18b2e..1f42a8cabd4 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java @@ -7,6 +7,7 @@ import datadog.trace.api.DDTraceId; import java.util.ArrayList; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -61,7 +62,7 @@ void resolvedContinuationHasNoFailures() { assertEquals(0, report.doubleCount()); assertEquals(ContinuationStatus.FINISHED, r.status()); assertTrue(r.threadHandoff()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -72,7 +73,7 @@ void neverResolvedIsFlaggedAsLeak() { assertEquals(1, report.leakCount()); assertEquals(ContinuationStatus.LEAKED, r.status()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); assertTrue(report.renderSummary().contains("LEAKED")); assertTrue(report.renderSummary().contains("Worker.java:42")); } @@ -105,7 +106,20 @@ void lateFinishDoesNotFail() { ScopeDiagnosticsReport report = report(list(r), rootWritten); assertEquals(1, report.lateCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); + assertFalse(report.hasViolations(EnumSet.of(ScopeDiagnosticsCheck.LATE_FINISH))); + } + + @Test + void enabledChecksSelectEnforcementWithoutChangingFindings() { + ContinuationRecord r = record(0, DDTraceId.from(121)); + ScopeDiagnosticsReport report = report(list(r), map()); + + assertTrue(report.hasViolations(EnumSet.of(ScopeDiagnosticsCheck.LEAKED))); + assertFalse(report.hasViolations(EnumSet.of(ScopeDiagnosticsCheck.DOUBLE_FINISH))); + assertEquals(1, report.leakCount()); + assertTrue(report.hasFindings()); + assertTrue(report.renderSummary().contains("LEAKED")); } @Test @@ -118,7 +132,7 @@ void multipleResolutionsAreFlaggedDouble() { ScopeDiagnosticsReport report = report(list(r), map()); assertEquals(1, report.doubleCount()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); } @Test @@ -134,7 +148,7 @@ void successfulResumeAfterCleanupEntryIsNotActivationAfterResolve() { assertEquals(ContinuationStatus.FINISHED, report.records().get(0).status()); assertEquals(0, report.activateAfterResolveCount()); assertEquals(0, report.doubleCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -146,7 +160,7 @@ void failedActivationIsActivateAfterResolve() { ScopeDiagnosticsReport report = report(list(r), map()); assertEquals(1, report.activateAfterResolveCount()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); } @Test @@ -157,7 +171,7 @@ void timelineRendersResolvedContinuationEvenWithoutProblems() { ScopeDiagnosticsReport report = report(list(r), map()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); assertTrue(report.renderSummary().contains("(none)")); String timeline = report.renderTimeline(); diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java index 5e0b09cb41a..6474476136e 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java @@ -49,7 +49,7 @@ void openAndClosedHasNoFailures() { assertEquals(0, s.failures().size()); assertFalse(s.threadHandoff()); assertEquals(Long.valueOf(2000), s.activeDurationNanos()); - assertFalse(report(s).hasProblems()); + assertFalse(report(s).hasViolations()); } @Test @@ -74,7 +74,7 @@ void timelineIncludesEveryWrongThreadCloseCallsite() { s.setClose(event(ScopeEvent.Type.SCOPE_CLOSE, "main", 4000)); ScopeDiagnosticsReport report = report(s); - assertFalse(report.hasProblems(), "wrong-thread cleanup remains advisory"); + assertFalse(report.hasViolations(), "wrong-thread cleanup remains advisory"); String timeline = report.renderTimeline(); assertTrue(timeline.contains("wrong-thread close")); assertTrue(timeline.contains("@ worker-one at com.app.First.close(First.java:12)")); @@ -86,11 +86,11 @@ void openWithoutCloseIsNeverClosed() { ScopeRecord s = scope(0, null, "main", 1000); assertFalse(s.closed()); - assertTrue(s.failures().contains(Failure.NEVER_CLOSED)); + assertTrue(s.failures().contains(ScopeDiagnosticsCheck.NEVER_CLOSED)); ScopeDiagnosticsReport report = report(s); assertEquals(1, report.neverClosedScopeCount()); - assertTrue(report.hasProblems()); + assertTrue(report.hasViolations()); } @Test @@ -99,10 +99,10 @@ void deferredCleanupIsNotALeak() { s.markDeferredCleanup(); ScopeDiagnosticsReport report = report(s); - assertFalse(s.failures().contains(Failure.NEVER_CLOSED)); + assertFalse(s.failures().contains(ScopeDiagnosticsCheck.NEVER_CLOSED)); assertEquals(1, report.deferredCleanupScopeCount()); assertEquals(0, report.neverClosedScopeCount()); - assertFalse(report.hasProblems()); + assertFalse(report.hasViolations()); } @Test @@ -119,10 +119,10 @@ void wrongThreadCloseIsReportedButDoesNotFail() { s.setClose(event(ScopeEvent.Type.SCOPE_CLOSE, "main", 2000)); s.addWrongThreadClose(event(ScopeEvent.Type.SCOPE_CLOSE_WRONG_THREAD, "pool-2", 1500)); - assertTrue(s.failures().contains(Failure.CLOSE_WRONG_THREAD)); + assertTrue(s.failures().contains(ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD)); ScopeDiagnosticsReport report = report(s); assertEquals(1, report.closeWrongThreadCount()); - assertFalse(report.hasProblems()); // wrong-thread is report-only + assertFalse(report.hasViolations()); // wrong-thread is report-only } } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeResolutionTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeResolutionTest.java index 103d503b769..ef47c91d5f1 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeResolutionTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeResolutionTest.java @@ -184,6 +184,6 @@ private void assertDuplicateWithoutLeak() { private void assertResolvedOnce() { assertEquals(0, ScopeDiagnostics.report().leakCount()); - assertFalse(ScopeDiagnostics.report().hasProblems()); + assertFalse(ScopeDiagnostics.report().hasViolations()); } } diff --git a/docs/how_to_test.md b/docs/how_to_test.md index ad01aaa7a6c..7a146ab04ae 100644 --- a/docs/how_to_test.md +++ b/docs/how_to_test.md @@ -137,8 +137,26 @@ If a fix cannot be included immediately, quarantine the Spock test with `@Flaky` `@Flaky` is not yet supported for JUnit tests. Keep continuation tracking enabled so the diagnostic evidence is preserved. -Disable tracking with `@TrackScopeContinuations(enabled = false, reason = "...")` only for a proven incompatibility with the diagnostic itself, never for an unresolved leak. -Keep the opt-out narrow and document the incompatibility and its removal condition. +`@TrackScopeContinuations` checks every detected lifecycle condition by default. Its `checks` value +and `enabled` flag select the active checks: + +```java +// Run every check except the known-incompatible late-finish check. +@TrackScopeContinuations( + enabled = false, + checks = ScopeDiagnosticsCheck.LATE_FINISH, + reason = "the fixture deliberately writes the root before draining callbacks; remove with ABC-123") +``` + +With `enabled = true`, `checks` is a whitelist. With `enabled = false`, `checks` is removed from the +default full set. Omitting `checks` therefore preserves the existing full enable/disable behavior. +Any reduction from the full set requires a reason documenting the incompatibility and its removal +condition. + +When at least one check is active, all conditions remain recorded and reported. The diagnostic +policy independently decides which conditions fail a test and which are advisory, so advisory +conditions are also valid selectors. Disable tracking or individual checks only for a proven +incompatibility with the diagnostic itself, never to hide an unresolved lifecycle bug. Strict trace writes remain enabled. ## Running Tests From ab5716dce2f24fc7ebcbd1318a6d7c65a568d4ed Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 5 Oct 2026 13:40:55 +0200 Subject: [PATCH 2/4] Update docs/how_to_test.md Co-authored-by: Brice Dutheil --- docs/how_to_test.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how_to_test.md b/docs/how_to_test.md index 7a146ab04ae..3e9800ba6a3 100644 --- a/docs/how_to_test.md +++ b/docs/how_to_test.md @@ -148,7 +148,7 @@ and `enabled` flag select the active checks: reason = "the fixture deliberately writes the root before draining callbacks; remove with ABC-123") ``` -With `enabled = true`, `checks` is a whitelist. With `enabled = false`, `checks` is removed from the +With `enabled = true`, `checks` is an allow-list. With `enabled = false`, `checks` is removed from the default full set. Omitting `checks` therefore preserves the existing full enable/disable behavior. Any reduction from the full set requires a reason documenting the incompatibility and its removal condition. From d513cb21f5fb5908d9c358ae299cf323312a1b73 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 6 Oct 2026 09:15:59 +0200 Subject: [PATCH 3/4] suggestions --- .../test/InstrumentationSpecification.groovy | 2 +- .../test/scopediag/ScopeDiagnostics.java | 53 ++++++--- .../scopediag/ScopeDiagnosticsExtension.java | 2 +- .../scopediag/ScopeDiagnosticsReport.java | 55 +++++++-- .../scopediag/TrackScopeContinuations.java | 20 +--- .../ScopeDiagnosticsConfigurationTest.java | 108 ++++++++++++------ .../ScopeDiagnosticsIntegrationTest.java | 59 ++++++++++ .../scopediag/ScopeDiagnosticsReportTest.java | 39 +++++++ .../agent/test/scopediag/ScopeRecordTest.java | 22 ++++ docs/how_to_test.md | 36 +++--- 10 files changed, 308 insertions(+), 88 deletions(-) diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index 60357ed625b..2ca53880461 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -470,7 +470,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A TEST_WRITER.start() if (scopeDiagnosticsEnabled()) { - ScopeDiagnostics.startRecording() + ScopeDiagnostics.startRecording(scopeDiagConfig()) } TEST_DATA_STREAMS_WRITER.clear() TEST_DATA_STREAMS_MONITORING.clear() diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java index b704c0390cd..c674b8731c3 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnostics.java @@ -9,9 +9,13 @@ import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Records test-time scope and continuation lifecycles and reports leaks. */ public final class ScopeDiagnostics { + private static final Logger log = LoggerFactory.getLogger(ScopeDiagnostics.class); + private static final int DEFAULT_MAX_FRAMES = 6; private static final long DEFAULT_QUIESCENCE_TIMEOUT_MILLIS = 250; @@ -38,6 +42,21 @@ public final class ScopeDiagnostics { private ScopeDiagnostics() {} + /** Starts configured recording, warning when no remaining check can fail the test. */ + public static void startRecording(TrackScopeContinuations config) { + EnumSet checks = enabledChecks(config); + if (config != null && !config.enabled()) { + return; + } + if (ScopeDiagnosticsReport.enforcedChecks(checks).isEmpty()) { + log.warn( + "@TrackScopeContinuations excludes all enforced checks; recording remains enabled, " + + "but no diagnostic finding can fail the test. Reason: {}", + config.reason()); + } + startRecording(); + } + /** Clears any prior data and starts recording with the default stack depth. */ public static void startRecording() { startRecording(DEFAULT_MAX_FRAMES); @@ -118,15 +137,17 @@ public static void assertNoViolations(ScopeDiagnosticsReport report) { /** Fails when the supplied configuration selects an enforced violation in the snapshot. */ public static void assertNoViolations( ScopeDiagnosticsReport report, TrackScopeContinuations config) { - if (report.hasViolations(enabledChecks(config))) { + EnumSet checks = enabledChecks(config); + if (report.hasViolations(checks)) { throw new AssertionError( - "Scope/continuation lifecycle violations detected:\n" + report.renderSummary()); + "Scope/continuation lifecycle violations detected:\n" + report.renderSummary(checks)); } } - /** Returns whether the configuration selects any checks to evaluate. */ + /** Validates the configuration and returns whether recording is enabled. */ public static boolean isEnabled(TrackScopeContinuations config) { - return !enabledChecks(config).isEmpty(); + enabledChecks(config); + return config == null || config.enabled(); } static EnumSet enabledChecks(TrackScopeContinuations config) { @@ -135,17 +156,23 @@ static EnumSet enabledChecks(TrackScopeContinuations conf return allChecks; } - EnumSet configured = EnumSet.noneOf(ScopeDiagnosticsCheck.class); - Collections.addAll(configured, config.checks()); - EnumSet enabled = configured; - if (!config.enabled()) { - enabled = allChecks.clone(); - enabled.removeAll(configured); + if (!config.enabled() && config.disabledChecks().length > 0) { + throw new IllegalArgumentException( + "@TrackScopeContinuations(enabled = false) requires empty disabledChecks"); } - - if (!enabled.equals(allChecks) && config.reason().trim().isEmpty()) { + if ((!config.enabled() || config.disabledChecks().length > 0) + && config.reason().trim().isEmpty()) { throw new IllegalArgumentException( - "@TrackScopeContinuations requires a reason when checks are disabled"); + "@TrackScopeContinuations requires a reason when recording or checks are disabled"); + } + + EnumSet enabled = allChecks; + if (!config.enabled()) { + enabled.clear(); + } else { + for (ScopeDiagnosticsCheck check : config.disabledChecks()) { + enabled.remove(check); + } } return enabled; } diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java index 295304ca364..3c8f8a1137b 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java @@ -17,7 +17,7 @@ public final class ScopeDiagnosticsExtension implements BeforeEachCallback, Afte public void beforeEach(ExtensionContext context) { TrackScopeContinuations config = resolve(context); if (ScopeDiagnostics.isEnabled(config)) { - ScopeDiagnostics.startRecording(); + ScopeDiagnostics.startRecording(config); } } diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java index f9965b963ff..9152617042b 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReport.java @@ -152,10 +152,16 @@ public boolean hasViolations() { return hasViolations(EnumSet.allOf(ScopeDiagnosticsCheck.class)); } + static EnumSet enforcedChecks(Set checks) { + EnumSet enforced = ENFORCED_CHECKS.clone(); + enforced.retainAll(checks); + return enforced; + } + /** Returns whether an enabled check contains a finding enforced by the current policy. */ boolean hasViolations(Set enabledChecks) { - for (ScopeDiagnosticsCheck check : enabledChecks) { - if (ENFORCED_CHECKS.contains(check) && count(check) > 0) { + for (ScopeDiagnosticsCheck check : enforcedChecks(enabledChecks)) { + if (count(check) > 0) { return true; } } @@ -211,18 +217,38 @@ private void appendHeader(StringBuilder sb, String title) { /** Renders flagged continuations and scopes with their call sites. */ public String renderSummary() { - StringBuilder sb = new StringBuilder(); - appendHeader(sb, "Scope/continuation problems"); - if (continuationFailures.isEmpty() && scopeFailures.isEmpty()) { - sb.append(" (none)\n"); - return sb.toString(); - } + return renderSummary(EnumSet.allOf(ScopeDiagnosticsCheck.class)); + } + + /** Renders enforced violations first, followed by advisory and excluded findings. */ + String renderSummary(Set checks) { + StringBuilder sb = new StringBuilder("Scope/continuation findings\n"); + EnumSet enforced = enforcedChecks(checks); + EnumSet advisory = EnumSet.allOf(ScopeDiagnosticsCheck.class); + advisory.retainAll(checks); + advisory.removeAll(ENFORCED_CHECKS); + EnumSet excluded = EnumSet.allOf(ScopeDiagnosticsCheck.class); + excluded.removeAll(checks); + appendFindings(sb, "Enforced violations", enforced); + appendFindings(sb, "Advisory findings (not enforced)", advisory); + appendFindings(sb, "Excluded findings (not enforced)", excluded); + return sb.toString(); + } + + private void appendFindings(StringBuilder sb, String title, Set checks) { + sb.append(title).append(":\n"); + int start = sb.length(); for (Map.Entry> e : continuationFailures.entrySet()) { + EnumSet findings = e.getValue().clone(); + findings.retainAll(checks); + if (findings.isEmpty()) { + continue; + } ContinuationRecord r = e.getKey(); ScopeEvent capture = r.capture(); sb.append(" ") - .append(e.getValue()) + .append(findings) .append(" #") .append(r.seq) .append(" trace=") @@ -234,10 +260,15 @@ public String renderSummary() { .append('\n'); } for (Map.Entry> e : scopeFailures.entrySet()) { + EnumSet findings = e.getValue().clone(); + findings.retainAll(checks); + if (findings.isEmpty()) { + continue; + } ScopeRecord s = e.getKey(); ScopeEvent open = s.open(); sb.append(" ") - .append(e.getValue()) + .append(findings) .append(" scope#") .append(s.seq) .append(" trace=") @@ -248,7 +279,9 @@ public String renderSummary() { .append(open == null || open.callsite() == null ? "" : open.callsite()) .append('\n'); } - return sb.toString(); + if (sb.length() == start) { + sb.append(" (none)\n"); + } } private static final int TIMELINE_FRAMES = 3; diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java index a3939b427ff..6e834932012 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/TrackScopeContinuations.java @@ -12,24 +12,14 @@ @Inherited public @interface TrackScopeContinuations { /** - * Selects whether {@link #checks()} is the complete set of active checks ({@code true}) or the - * checks to remove from the default full set ({@code false}). + * Enables recording and diagnostics. Disabling requires a reason and an empty {@link + * #disabledChecks()} list. */ boolean enabled() default true; - /** - * Checks selected by {@link #enabled()}. The default lists every available check; a configuration - * test guards that invariant as new checks are added. - */ - ScopeDiagnosticsCheck[] checks() default { - ScopeDiagnosticsCheck.LEAKED, - ScopeDiagnosticsCheck.LATE_FINISH, - ScopeDiagnosticsCheck.DOUBLE_FINISH, - ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, - ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD, - ScopeDiagnosticsCheck.NEVER_CLOSED - }; + /** Checks excluded from enforcement while recording remains enabled. Requires a reason. */ + ScopeDiagnosticsCheck[] disabledChecks() default {}; - /** Explains any reduction from the default full check set. */ + /** Required when recording is disabled or {@link #disabledChecks()} is nonempty. */ String reason() default ""; } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java index df797ca49e2..f1c60eb5a38 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsConfigurationTest.java @@ -1,6 +1,5 @@ package datadog.trace.agent.test.scopediag; -import static java.util.Arrays.asList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -18,26 +17,49 @@ private static class UndocumentedOptOut {} private static class DocumentedOptOut {} @TrackScopeContinuations( - checks = {ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH}, - reason = "only these checks apply") - private static class CheckWhitelist {} - - @TrackScopeContinuations( - enabled = false, - checks = {ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH}, + disabledChecks = {ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH}, reason = "these checks do not apply") - private static class CheckBlacklist {} + private static class ExcludedChecks {} + + @TrackScopeContinuations(disabledChecks = {}) + private static class EmptyExclusions {} + + @TrackScopeContinuations + private static class Defaults {} @TrackScopeContinuations( enabled = false, - checks = {}) - private static class EmptyBlacklist {} + disabledChecks = ScopeDiagnosticsCheck.LEAKED, + reason = "incompatible fixture") + private static class DisabledWithExclusions {} - @TrackScopeContinuations(checks = ScopeDiagnosticsCheck.LEAKED) - private static class UndocumentedWhitelist {} + @TrackScopeContinuations(disabledChecks = ScopeDiagnosticsCheck.LEAKED, reason = " ") + private static class UndocumentedExclusion {} - @TrackScopeContinuations - private static class Defaults {} + @TrackScopeContinuations(disabledChecks = ScopeDiagnosticsCheck.LATE_FINISH) + private static class UndocumentedAdvisoryExclusion {} + + @TrackScopeContinuations( + disabledChecks = { + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.NEVER_CLOSED + }, + reason = "diagnostic investigation only") + private static class AdvisoryOnly {} + + @TrackScopeContinuations( + disabledChecks = { + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.LATE_FINISH, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD, + ScopeDiagnosticsCheck.NEVER_CLOSED + }, + reason = "diagnostic investigation only") + private static class AllExcluded {} @Test void diagnosticsAreEnabledByDefault() { @@ -45,27 +67,17 @@ void diagnosticsAreEnabledByDefault() { } @Test - void defaultChecksIncludeEveryEnumValue() { + void defaultSelectionIncludesEveryEnumValue() { TrackScopeContinuations config = Defaults.class.getAnnotation(TrackScopeContinuations.class); assertEquals( - EnumSet.allOf(ScopeDiagnosticsCheck.class), EnumSet.copyOf(asList(config.checks()))); - } - - @Test - void enabledChecksAreAWhitelist() { - TrackScopeContinuations config = - CheckWhitelist.class.getAnnotation(TrackScopeContinuations.class); - - assertEquals( - EnumSet.of(ScopeDiagnosticsCheck.LEAKED, ScopeDiagnosticsCheck.LATE_FINISH), - ScopeDiagnostics.enabledChecks(config)); + EnumSet.allOf(ScopeDiagnosticsCheck.class), ScopeDiagnostics.enabledChecks(config)); } @Test - void disabledChecksAreABlacklist() { + void excludedChecksAreRemovedFromDefaults() { TrackScopeContinuations config = - CheckBlacklist.class.getAnnotation(TrackScopeContinuations.class); + ExcludedChecks.class.getAnnotation(TrackScopeContinuations.class); EnumSet expected = EnumSet.allOf(ScopeDiagnosticsCheck.class); expected.remove(ScopeDiagnosticsCheck.LEAKED); expected.remove(ScopeDiagnosticsCheck.LATE_FINISH); @@ -74,9 +86,9 @@ void disabledChecksAreABlacklist() { } @Test - void emptyBlacklistKeepsEveryCheckEnabled() { + void emptyExclusionsKeepEveryCheckEnabled() { TrackScopeContinuations config = - EmptyBlacklist.class.getAnnotation(TrackScopeContinuations.class); + EmptyExclusions.class.getAnnotation(TrackScopeContinuations.class); assertEquals( EnumSet.allOf(ScopeDiagnosticsCheck.class), ScopeDiagnostics.enabledChecks(config)); @@ -98,10 +110,40 @@ void undocumentedOptOutIsRejected() { } @Test - void undocumentedWhitelistIsRejected() { + void fullOptOutCannotSupplyDisabledChecks() { + assertInvalid(DisabledWithExclusions.class); + } + + @Test + void undocumentedExclusionIsRejected() { + assertInvalid(UndocumentedExclusion.class); + } + + @Test + void excludingAnAdvisoryCheckStillRequiresAReason() { + assertInvalid(UndocumentedAdvisoryExclusion.class); + } + + @Test + void excludingAllEnforcedChecksKeepsRecordingEnabled() { TrackScopeContinuations config = - UndocumentedWhitelist.class.getAnnotation(TrackScopeContinuations.class); + AdvisoryOnly.class.getAnnotation(TrackScopeContinuations.class); + + assertTrue(ScopeDiagnostics.isEnabled(config)); + assertTrue( + ScopeDiagnosticsReport.enforcedChecks(ScopeDiagnostics.enabledChecks(config)).isEmpty()); + } + + @Test + void excludingEveryCheckKeepsRecordingEnabled() { + TrackScopeContinuations config = AllExcluded.class.getAnnotation(TrackScopeContinuations.class); + + assertTrue(ScopeDiagnostics.isEnabled(config)); + assertTrue(ScopeDiagnostics.enabledChecks(config).isEmpty()); + } + private static void assertInvalid(Class fixture) { + TrackScopeContinuations config = fixture.getAnnotation(TrackScopeContinuations.class); assertThrows(IllegalArgumentException.class, () -> ScopeDiagnostics.isEnabled(config)); } } diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java index 0f4b826d413..83f4d2e2bb3 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsIntegrationTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.context.Context; @@ -23,6 +24,31 @@ /** Exercises diagnostics against continuations created by a real {@link CoreTracer}. */ class ScopeDiagnosticsIntegrationTest { + @TrackScopeContinuations( + disabledChecks = { + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.NEVER_CLOSED + }, + reason = "test recording without enforcement") + private static class AdvisoryOnly {} + + @TrackScopeContinuations( + disabledChecks = { + ScopeDiagnosticsCheck.LEAKED, + ScopeDiagnosticsCheck.LATE_FINISH, + ScopeDiagnosticsCheck.DOUBLE_FINISH, + ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + ScopeDiagnosticsCheck.CLOSE_WRONG_THREAD, + ScopeDiagnosticsCheck.NEVER_CLOSED + }, + reason = "test recording without enforcement") + private static class AllExcluded {} + + @TrackScopeContinuations(enabled = false, reason = "test full recording opt-out") + private static class Disabled {} + private CoreTracer tracer; @AfterEach @@ -34,6 +60,39 @@ void tearDown() { } } + @Test + void excludingAllEnforcedChecksRecordsLeaksWithoutFailing() { + assertRecordsWithoutEnforcement(AdvisoryOnly.class); + } + + @Test + void excludingAllChecksRecordsLeaksWithoutFailing() { + assertRecordsWithoutEnforcement(AllExcluded.class); + } + + @Test + void fullOptOutDoesNotStartRecording() { + ScopeDiagnostics.startRecording(Disabled.class.getAnnotation(TrackScopeContinuations.class)); + assertNull(ScopeDiagnostics.recordingWindow()); + } + + private void assertRecordsWithoutEnforcement(Class fixture) { + TrackScopeContinuations config = fixture.getAnnotation(TrackScopeContinuations.class); + tracer = CoreTracer.builder().writer(new ListWriter()).strictTraceWrites(false).build(); + ScopeDiagnostics.startRecording(config); + assertNotNull(ScopeDiagnostics.recordingWindow()); + AgentSpan span = tracer.startSpan("test", "op"); + ContextContinuation continuation = tracer.capture(span); + + ScopeDiagnosticsReport report = ScopeDiagnostics.report(); + assertEquals(1, report.leakCount()); + assertTrue(report.renderTimeline().contains("LEAKED")); + ScopeDiagnostics.assertNoViolations(report, config); + + continuation.release(); + span.finish(); + } + @Test void capturesRealLeakAndResolvedContinuation() { tracer = CoreTracer.builder().writer(new ListWriter()).strictTraceWrites(false).build(); diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java index 1f42a8cabd4..092c34c4717 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsReportTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.trace.api.DDTraceId; @@ -28,6 +29,11 @@ private static ContinuationRecord record(long seq, DDTraceId trace) { seq, trace, 7L, "op", (byte) 0, false, event(ScopeEvent.Type.CAPTURE, "main", 1000)); } + @TrackScopeContinuations( + disabledChecks = ScopeDiagnosticsCheck.LEAKED, + reason = "synthetic report tests exclusion") + private static class ExcludeLeaks {} + @Test void firstResumeTimingUsesEarliestTimestampRegardlessOfRecordingOrder() { ContinuationRecord r = record(0, DDTraceId.from(10)); @@ -122,6 +128,39 @@ void enabledChecksSelectEnforcementWithoutChangingFindings() { assertTrue(report.renderSummary().contains("LEAKED")); } + @Test + void assertionSeparatesEnforcedAdvisoryAndExcludedFindings() { + DDTraceId trace = DDTraceId.from(122); + ContinuationRecord leaked = record(0, trace); + ContinuationRecord doubled = record(1, trace); + doubled.setTerminalOrExtra(event(ScopeEvent.Type.RESOLVE_FINISH, "main", 3000)); + doubled.setTerminalOrExtra(event(ScopeEvent.Type.RESOLVE_FINISH, "main", 4000)); + Map rootWritten = map(); + rootWritten.put(trace, 2000L); + List records = list(leaked); + records.add(doubled); + ScopeDiagnosticsReport report = report(records, rootWritten); + TrackScopeContinuations config = + ExcludeLeaks.class.getAnnotation(TrackScopeContinuations.class); + + AssertionError failure = + assertThrows( + AssertionError.class, () -> ScopeDiagnostics.assertNoViolations(report, config)); + String message = failure.getMessage(); + assertTrue(message.contains("[DOUBLE_FINISH] #1")); + assertTrue(message.contains("[LATE_FINISH] #1")); + assertTrue(message.contains("[LEAKED] #0")); + int advisory = message.indexOf("Advisory findings (not enforced)"); + int excluded = message.indexOf("Excluded findings (not enforced)"); + assertTrue(message.indexOf("[DOUBLE_FINISH] #1") < advisory); + assertTrue(message.indexOf("[LATE_FINISH] #1") > advisory); + assertTrue(message.indexOf("[LATE_FINISH] #1") < excluded); + assertTrue(message.indexOf("[LEAKED] #0") > excluded); + assertEquals(1, report.leakCount()); + assertEquals(1, report.doubleCount()); + assertTrue(report.renderTimeline().contains("LEAKED")); + } + @Test void multipleResolutionsAreFlaggedDouble() { ContinuationRecord r = record(0, DDTraceId.from(13)); diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java index 6474476136e..1f0c546bb9b 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeRecordTest.java @@ -6,6 +6,7 @@ import datadog.trace.api.DDTraceId; import java.util.ArrayList; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import org.junit.jupiter.api.Test; @@ -40,6 +41,27 @@ private static ScopeDiagnosticsReport report(ScopeRecord... scopes) { return new ScopeDiagnosticsReport(new ArrayList<>(), list, new HashMap<>()); } + @Test + void summarySeparatesScopeFindingsOnTheSameRecord() { + ScopeRecord scope = scope(0, null, "main", 1000); + scope.addWrongThreadClose(event(ScopeEvent.Type.SCOPE_CLOSE_WRONG_THREAD, "worker", 2000)); + EnumSet checks = EnumSet.allOf(ScopeDiagnosticsCheck.class); + checks.remove(ScopeDiagnosticsCheck.NEVER_CLOSED); + + String summary = report(scope).renderSummary(checks); + assertTrue(summary.contains("[CLOSE_WRONG_THREAD] scope#0")); + assertTrue(summary.contains("[NEVER_CLOSED] scope#0")); + assertTrue( + summary.indexOf("[CLOSE_WRONG_THREAD] scope#0") + > summary.indexOf("Advisory findings (not enforced)")); + assertTrue( + summary.indexOf("[CLOSE_WRONG_THREAD] scope#0") + < summary.indexOf("Excluded findings (not enforced)")); + assertTrue( + summary.indexOf("[NEVER_CLOSED] scope#0") + > summary.indexOf("Excluded findings (not enforced)")); + } + @Test void openAndClosedHasNoFailures() { ScopeRecord s = scope(0, null, "main", 1000); diff --git a/docs/how_to_test.md b/docs/how_to_test.md index 3e9800ba6a3..61d5d32291a 100644 --- a/docs/how_to_test.md +++ b/docs/how_to_test.md @@ -137,26 +137,34 @@ If a fix cannot be included immediately, quarantine the Spock test with `@Flaky` `@Flaky` is not yet supported for JUnit tests. Keep continuation tracking enabled so the diagnostic evidence is preserved. -`@TrackScopeContinuations` checks every detected lifecycle condition by default. Its `checks` value -and `enabled` flag select the active checks: +`@TrackScopeContinuations` records every detected lifecycle condition by default. Use `disabledChecks` +to exclude checks from enforcement: ```java -// Run every check except the known-incompatible late-finish check. +// Exclude an enforced check only for a proven diagnostic incompatibility. @TrackScopeContinuations( - enabled = false, - checks = ScopeDiagnosticsCheck.LATE_FINISH, - reason = "the fixture deliberately writes the root before draining callbacks; remove with ABC-123") + disabledChecks = ScopeDiagnosticsCheck.ACTIVATE_AFTER_RESOLVE, + reason = "the synthetic fixture models failed activation without a real continuation; remove with ABC-123") ``` -With `enabled = true`, `checks` is an allow-list. With `enabled = false`, `checks` is removed from the -default full set. Omitting `checks` therefore preserves the existing full enable/disable behavior. -Any reduction from the full set requires a reason documenting the incompatibility and its removal -condition. +An empty `disabledChecks` list keeps all checks active, including checks added in the future. A +nonempty list excludes only the listed checks and includes future checks automatically. A method +annotation replaces the class configuration; exclusion lists are not merged. -When at least one check is active, all conditions remain recorded and reported. The diagnostic -policy independently decides which conditions fail a test and which are advisory, so advisory -conditions are also valid selectors. Disable tracking or individual checks only for a proven -incompatibility with the diagnostic itself, never to hide an unresolved lifecycle bug. +`enabled = false` stops recording entirely and requires an empty `disabledChecks` list. A reason is +required whenever `enabled = false` or `disabledChecks` is nonempty. Document the incompatibility +and its removal condition. + +With recording enabled, all conditions remain recorded and reported even when every check is +excluded. Excluding checks changes enforcement only; it does not disable recording, classification, +or the bounded wait for asynchronous cleanup. + +The diagnostic policy determines which findings fail a test and which are advisory. Excluding a +check affects enforcement without removing its diagnostic evidence. Reports distinguish findings +that caused a failure from those retained for context. + +Disable recording or individual checks only for a proven incompatibility with the diagnostic itself, +never to hide an unresolved lifecycle bug. Strict trace writes remain enabled. ## Running Tests From f1f11464fa271be4776dfd2662543816c9fe93f4 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 7 Oct 2026 23:16:21 +0200 Subject: [PATCH 4/4] fix compilation --- .../scopediag/ScopeDiagnosticsExtension.java | 15 ++-- .../ScopeDiagnosticsSpockFixtureTest.groovy | 2 +- .../ScopeDiagnosticsExtensionTest.java | 74 +++++++++++++++++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java index 103a4c55ad2..d8d178287d0 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtension.java @@ -44,7 +44,9 @@ public void interceptBeforeAllMethod( if (suiteEnabled && suiteSetupPending) { suiteSetupPending = false; try { - report(extensionContext.getRequiredTestClass().getSimpleName() + " suite setup"); + report( + extensionContext.getRequiredTestClass().getSimpleName() + " suite setup", + resolveClass(extensionContext)); } catch (Throwable diagnosticFailure) { setupFailure.addSuppressed(diagnosticFailure); } finally { @@ -68,7 +70,8 @@ public void beforeEach(ExtensionContext context) { if (suiteEnabled) { if (suiteSetupPending) { suiteSetupPending = false; - report(context.getRequiredTestClass().getSimpleName() + " suite setup"); + report( + context.getRequiredTestClass().getSimpleName() + " suite setup", resolveClass(context)); } else { ScopeDiagnostics.reset(); } @@ -84,7 +87,7 @@ public void afterEach(ExtensionContext context) { try { TrackScopeContinuations config = resolve(context); if (ScopeDiagnostics.isEnabled(config)) { - report(context.getDisplayName()); + report(context.getDisplayName(), config); } } finally { if (suiteEnabled) { @@ -104,7 +107,9 @@ public void interceptAfterAllMethod( return; } try { - report(extensionContext.getRequiredTestClass().getSimpleName() + " suite cleanup"); + report( + extensionContext.getRequiredTestClass().getSimpleName() + " suite cleanup", + resolveClass(extensionContext)); } catch (Throwable diagnosticFailure) { try { invocation.proceed(); @@ -139,7 +144,7 @@ private static boolean isHarnessLifecycleMethod( == AbstractInstrumentationTest.class; } - private static void report(String displayName) { + private static void report(String displayName, TrackScopeContinuations config) { try { ScopeDiagnostics.awaitQuiescence(); ScopeDiagnostics.stop(); diff --git a/dd-java-agent/instrumentation-testing/src/test/groovy/datadog/trace/agent/test/scopediag/ScopeDiagnosticsSpockFixtureTest.groovy b/dd-java-agent/instrumentation-testing/src/test/groovy/datadog/trace/agent/test/scopediag/ScopeDiagnosticsSpockFixtureTest.groovy index 083c581bd4e..aae086844ed 100644 --- a/dd-java-agent/instrumentation-testing/src/test/groovy/datadog/trace/agent/test/scopediag/ScopeDiagnosticsSpockFixtureTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/test/groovy/datadog/trace/agent/test/scopediag/ScopeDiagnosticsSpockFixtureTest.groovy @@ -43,7 +43,7 @@ class ScopeDiagnosticsSpockFixtureTest extends InstrumentationSpecification { then: def failure = thrown(AssertionError) - failure.message.contains("Scope continuation problems detected") + failure.message.contains("Scope/continuation lifecycle violations detected") ScopeDiagnostics.report().records().isEmpty() cleanup: diff --git a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtensionTest.java b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtensionTest.java index a0c6020e7c8..73aed481430 100644 --- a/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtensionTest.java +++ b/dd-java-agent/instrumentation-testing/src/test/java/datadog/trace/agent/test/scopediag/ScopeDiagnosticsExtensionTest.java @@ -23,6 +23,8 @@ import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.InvocationInterceptor.Invocation; import org.junit.jupiter.api.extension.ReflectiveInvocationContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class ScopeDiagnosticsExtensionTest { @@ -31,6 +33,14 @@ private static class FailingFixture { static void failSuiteSetup() {} } + @TrackScopeContinuations(disabledChecks = ScopeDiagnosticsCheck.LEAKED, reason = "fixture leak") + private static class SelectiveFixture { + @TrackScopeContinuations + void enforceLeaks() {} + + void inheritExcludedLeaks() {} + } + @AfterEach void tearDown() { ScopeDiagnostics.reset(); @@ -80,4 +90,68 @@ void reportsPendingSuiteSetupBeforeStartingCleanupWindow() throws Throwable { continuation.release(); } + + @Test + @SuppressWarnings("unchecked") + void appliesClassConfigurationToSuiteFixtures() throws Throwable { + ScopeDiagnosticsExtension extension = new ScopeDiagnosticsExtension(); + ExtensionContext context = selectiveContext("enforceLeaks"); + extension.beforeAll(context); + + Invocation harnessSetup = mock(Invocation.class); + ReflectiveInvocationContext harnessContext = mock(ReflectiveInvocationContext.class); + when(harnessContext.getExecutable()) + .thenReturn(AbstractInstrumentationTest.class.getDeclaredMethod("initAll")); + extension.interceptBeforeAllMethod(harnessSetup, harnessContext, context); + recordLeak(); + + // Suite setup uses the class exclusion even when the test method enforces leaks. + extension.beforeEach(context); + extension.afterEach(context); + recordLeak(); + + when(harnessContext.getExecutable()) + .thenReturn(AbstractInstrumentationTest.class.getDeclaredMethod("tearDownAll")); + Invocation harnessCleanup = mock(Invocation.class); + extension.interceptAfterAllMethod(harnessCleanup, harnessContext, context); + } + + @ParameterizedTest + @ValueSource(strings = {"enforceLeaks", "inheritExcludedLeaks"}) + void appliesMethodConfigurationToTestFindings(String methodName) throws Exception { + ScopeDiagnosticsExtension extension = new ScopeDiagnosticsExtension(); + ExtensionContext context = selectiveContext(methodName); + extension.beforeAll(context); + extension.beforeEach(context); + recordLeak(); + + if (methodName.equals("enforceLeaks")) { + assertThrows(AssertionError.class, () -> extension.afterEach(context)); + } else { + extension.afterEach(context); + } + } + + private static ExtensionContext selectiveContext(String methodName) throws Exception { + ExtensionContext context = mock(ExtensionContext.class); + when(context.getTestClass()).thenReturn(Optional.of(SelectiveFixture.class)); + doReturn(SelectiveFixture.class).when(context).getRequiredTestClass(); + when(context.getElement()) + .thenReturn(Optional.of(SelectiveFixture.class.getDeclaredMethod(methodName))); + return context; + } + + private static void recordLeak() { + ContextContinuation continuation = Context.root().capture(); + ScopeDiagnostics.recordCapture( + ScopeDiagnostics.recordingWindow(), + continuation, + DDTraceId.from(1), + 2, + "fixture", + (byte) 0); + // The diagnostic is recorded explicitly; release the real continuation to avoid a resource + // leak. + continuation.release(); + } }