Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .agents/skills/apm-integrations/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ After `updateAgentJarIntegrationsGoldenFile` runs, commit the updated `metadata/

**If `checkDecoratorAnalyticsConfigurations` fails:** a name returned by the decorator's `instrumentationNames()` is missing `DD_TRACE_<NAME>_ANALYTICS_ENABLED` / `DD_TRACE_<NAME>_ANALYTICS_SAMPLE_RATE` entries in `metadata/supported-configurations.json` — add them per [Supported Configurations](references/supported-configurations.md).

**If tests fail:** verify span lifecycle order (start → activate → error → close → finish), helper registration,
and `contextStore()` map entries match actual usage.
**If tests fail:** verify span lifecycle order (start → activate → error → close → finish), helper
registration, and `contextStore()` map entries match actual usage. If the output contains
`Scope/continuation timeline`, read and follow
`.agents/skills/fix-continuation-leakage/SKILL.md`; fix the broken ownership edge rather than
adding `strictTraceWrites(false)` or disabling the diagnostic.

## Step 11 – Checklist before finishing

Expand Down
59 changes: 59 additions & 0 deletions .agents/skills/fix-continuation-leakage/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
name: fix-continuation-leakage
description: >
Diagnose and fix scope or continuation lifecycle failures in dd-trace-java instrumentation
tests. Use when a test reports a continuation leak, double resolution, activation after resolve,
or an unclosed scope, or when strictTraceWrites(false) appears to hide one. Reads the automatic
diagnostic timeline, finds the broken lifecycle edge, fixes it, and explains it with a compact
Mermaid diagram.
user-invocable: true
context: fork
allowed-tools:
- Bash
- Read
- Edit
- Glob
- Grep
- AskUserQuestion
---

# Fix continuation leakage

Instrumentation tests run the diagnostic automatically. A failure includes the capture, resume,
resolution, scope, thread, timing, and callsite data needed to find the missing lifecycle edge.

## Work the failure

1. Run the smallest failing test with full output:

```bash
./gradlew :dd-java-agent:instrumentation:<framework>-<minVersion>:test --tests '<FQCN-or-pattern>' --info 2>&1 | tee /tmp/scopediag-run.txt
```

2. Find `Scope/continuation timeline` in the output. If Gradle hides it, inspect the test XML's
`<system-out>` under the module's `build/test-results` directory.
3. Follow the failing record from its first event:
- `LEAKED` / `NEVER_CLOSED`: find the success, error, cancellation, and rejection exits that
skipped `release()` or `close()`.
- `DOUBLE_FINISH`: find two owners of the same cleanup.
- `ACTIVATE_AFTER_RESOLVE`: find work scheduled after ownership ended.
- `LATE_FINISH` / `CLOSE_WRONG_THREAD`: advisory evidence; verify whether ordering is valid.
- `[deferred-cleanup]`: a root iteration scope transferred cleanup to the bounded iteration
cleaner. It may remain open at the test boundary and is not a leak. Do not generalize this to
other `ITERATION` scopes; an unregistered iteration scope must still close normally.
4. Fix ownership where it breaks. Prefer one owner and `try/finally` cleanup across every exit.
5. Rerun the failing test, then its module.

Do not make the test green with `strictTraceWrites(false)` or
`@TrackScopeContinuations(enabled=false, reason="...")`. Those hide evidence. The opt-out requires
a reason and is only for a proven diagnostic incompatibility. If the failure is genuinely
intermittent, treat that as a flaky-test finding, keep diagnostics enabled, and link the `@Flaky`
annotation to a tracked issue.

## Explain it to a human

Lead with one sentence: what was captured, which cleanup edge was missing, and where. Cite the
timeline callsites. Then include a small Mermaid `flowchart LR`; use green for healthy edges, red
for the broken edge, and label thread handoffs. Use a Gantt only when timing itself caused the bug.

End with the code fix and the exact tests that passed.
23 changes: 23 additions & 0 deletions .claude/skills/fix-continuation-leakage/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
name: fix-continuation-leakage
description: >
Diagnose and fix scope or continuation lifecycle failures in dd-trace-java instrumentation
tests. Use when a test reports a continuation leak, double resolution, activation after resolve,
or an unclosed scope, or when strictTraceWrites(false) appears to hide one. Reads the automatic
diagnostic timeline, finds the broken lifecycle edge, fixes it, and explains it with a compact
Mermaid diagram.
user-invocable: true
context: fork
allowed-tools:
- Bash
- Read
- Edit
- Glob
- Grep
- AskUserQuestion
---

# Fix continuation leakage

Read `.agents/skills/fix-continuation-leakage/SKILL.md` in full and follow it. That file is the
shared playbook for repository agents.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import datadog.metrics.impl.DDSketchHistograms
import datadog.metrics.impl.MonitoringImpl
import datadog.trace.agent.test.asserts.ListWriterAssert
import datadog.trace.agent.test.asserts.TagsAssert
import datadog.trace.agent.test.scopediag.ScopeDiagnostics
import datadog.trace.agent.test.scopediag.TrackScopeContinuations
import datadog.trace.agent.test.datastreams.MockFeaturesDiscovery
import datadog.trace.agent.test.datastreams.RecordingDatastreamsPayloadWriter
import datadog.trace.agent.tooling.AgentInstaller
Expand Down Expand Up @@ -467,6 +469,9 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
}

TEST_WRITER.start()
if (scopeDiagnosticsEnabled()) {
ScopeDiagnostics.startRecording()
}
TEST_DATA_STREAMS_WRITER.clear()
TEST_DATA_STREAMS_MONITORING.clear()

Expand Down Expand Up @@ -500,27 +505,73 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
}
TEST_TRACER.flush()

def util = new MockUtil()
util.detachMock(STATS_D_CLIENT)
def scopeDiagnosticsFailure = reportScopeDiagnostics()

try {
def util = new MockUtil()
util.detachMock(STATS_D_CLIENT)

ActiveSubsystems.APPSEC_ACTIVE = originalAppSecRuntimeValue

if (Config.get().isDebuggerCodeOriginEnabled()) {
injectSysConfig(CODE_ORIGIN_FOR_SPANS_ENABLED, "false", true)
rebuildConfig()
}

ActiveSubsystems.APPSEC_ACTIVE = originalAppSecRuntimeValue
try {
if (enabledFinishTimingChecks()) {
doCheckRepeatedFinish()
}
} finally {
spanFinishLocations.clear()
originalToTrackingSpan.clear()
}

if (Config.get().isDebuggerCodeOriginEnabled()) {
injectSysConfig(CODE_ORIGIN_FOR_SPANS_ENABLED, "false", true)
rebuildConfig()
// check for instrumentation issues while running each test
assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors()
} catch (Throwable cleanupFailure) {
if (scopeDiagnosticsFailure != null) {
cleanupFailure.addSuppressed(scopeDiagnosticsFailure)
}
throw cleanupFailure
}
if (scopeDiagnosticsFailure != null) {
throw scopeDiagnosticsFailure
}
}

private TrackScopeContinuations scopeDiagConfig() {
def method = specificationContext?.currentFeature?.featureMethod?.reflection
def ann = method?.getAnnotation(TrackScopeContinuations)
if (ann == null) {
ann = this.class.getAnnotation(TrackScopeContinuations)
}
return ann
}

private boolean scopeDiagnosticsEnabled() {
return ScopeDiagnostics.isEnabled(scopeDiagConfig())
}

private Throwable reportScopeDiagnostics() {
def config = scopeDiagConfig()
if (!ScopeDiagnostics.isEnabled(config)) {
return null
}
try {
if (enabledFinishTimingChecks()) {
doCheckRepeatedFinish()
ScopeDiagnostics.awaitQuiescence()
ScopeDiagnostics.stop()
def report = ScopeDiagnostics.report()
if (report.hasFindings()) {
println(report.renderTimeline())
}
ScopeDiagnostics.assertNoLeaks(report)
return null
} catch (Throwable failure) {
return failure
} finally {
spanFinishLocations.clear()
originalToTrackingSpan.clear()
ScopeDiagnostics.reset()
}

// check for instrumentation issues while running each test
assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors()
}

private void doCheckRepeatedFinish() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import datadog.instrument.classinject.ClassInjector;
import datadog.trace.agent.test.assertions.TraceAssertions;
import datadog.trace.agent.test.assertions.TraceMatcher;
import datadog.trace.agent.test.scopediag.ScopeDiagnosticsExtension;
import datadog.trace.agent.tooling.AgentInstaller;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.agent.tooling.TracerInstaller;
Expand Down Expand Up @@ -58,7 +59,8 @@
@ExtendWith({
TestClassShadowingExtension.class,
AllowContextTestingExtension.class,
LegacyContextTestingExtension.class
LegacyContextTestingExtension.class,
ScopeDiagnosticsExtension.class
})
public abstract class AbstractInstrumentationTest {
static final Instrumentation INSTRUMENTATION = ByteBuddyAgent.getInstrumentation();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package datadog.trace.agent.test.scopediag;

import net.bytebuddy.asm.Advice;

/** Test-only advice for {@code ContinuableScope}. */
public final class ContinuableScopeAdvice {
private ContinuableScopeAdvice() {}

public static final class OnProperClose {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void exit(@Advice.This Object scope) {
ScopeContinuationProbe.onScopeClose(scope);
}
}

public static final class Close {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void enter(@Advice.This Object scope) {
ScopeContinuationProbe.onScopeClosing(scope);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package datadog.trace.agent.test.scopediag;

import net.bytebuddy.asm.Advice;

/** Test-only advice for scopes owned by the iteration cleaner. */
public final class ContinuableScopeManagerAdvice {
private ContinuableScopeManagerAdvice() {}

public static final class ScheduleRootIterationCleanup {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void exit(@Advice.Argument(1) Object scope) {
ScopeContinuationProbe.onDeferredScopeCleanup(scope);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package datadog.trace.agent.test.scopediag;

import net.bytebuddy.asm.Advice;

/** Test-only advice for {@code ScopeContinuation}. */
public final class ContinuationAdvice {
private ContinuationAdvice() {}

public static final class Register {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void exit(@Advice.This Object self) {
ScopeContinuationProbe.onCapture(self);
}
}

/** Timestamps entry because {@code resume()} may resolve the continuation before returning. */
public static final class Activate {
@Advice.OnMethodEnter
public static long enter() {
return System.nanoTime();
}

@Advice.OnMethodExit(suppress = Throwable.class)
public static void exit(
@Advice.This Object self, @Advice.Enter long ddActivateNanos, @Advice.Return Object scope) {
ScopeContinuationProbe.onActivate(self, scope, ddActivateNanos);
}
}

/** Timestamps entry because resolution may write the trace before the method returns. */
public static final class Cancel {
@Advice.OnMethodEnter
public static int enter(
@Advice.FieldValue("count") int count,
@Advice.Local("ddResolveNanos") long ddResolveNanos) {
ddResolveNanos = System.nanoTime();
return count;
}

@Advice.OnMethodExit(suppress = Throwable.class)
public static void exit(
@Advice.This Object self,
@Advice.Origin("#m") String method,
@Advice.Enter int countBefore,
@Advice.Local("ddResolveNanos") long ddResolveNanos,
@Advice.FieldValue("count") int countAfter) {
ScopeContinuationProbe.onResolve(self, method, countBefore, countAfter, ddResolveNanos);
}
}
}
Loading