Skip to content

Detect scope and continuation leaks in instrumentation tests - #12458

Open
amarziali wants to merge 11 commits into
masterfrom
andrea.marziali/diag2
Open

amarziali wants to merge 11 commits into
masterfrom
andrea.marziali/diag2

Conversation

@amarziali

@amarziali amarziali commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Adds default-on scope and continuation lifecycle diagnostics to instrumentation tests.

The diagnostic records continuation and scope events during each test, waits briefly for asynchronous cleanup, and fails with an actionable timeline when it detects problems such as:

  • A continuation that was never released
  • A scope that was never closed
  • A continuation resolved more than once
  • Activation after resolution
  • Cleanup performed from an unexpected thread
  • Cleanup occurring after the test boundary

The timeline includes elapsed time, thread transitions, ownership state, and relevant call sites so the missing lifecycle edge can be identified without reconstructing it from a trace timeout.

  flowchart LR
      A[Capture continuation] --> B[Resume context]
      B --> C[Run asynchronous work]
      C --> D[Close scope]
      D --> E[Release continuation]
      E --> F[Write complete trace]

      C -. missing cleanup .-> G[Test ends]
      G --> H[Diagnostic failure]
      H --> I[Timeline with thread and callsite]

      style E fill:#d5f5e3,stroke:#239b56
      style H fill:#fadbd8,stroke:#c0392b
Loading

How is it enabled?

The diagnostic runs automatically for both instrumentation-test harnesses:

  • Spock tests extending InstrumentationSpecification
  • JUnit tests extending AbstractInstrumentationTest

A narrow opt-out is available only for tests that are incompatible with the diagnostic itself:

@TrackScopeContinuations(
enabled = false,
reason = "Explain the incompatibility and removal condition")

The reason is mandatory. The opt-out is not intended for unresolved continuation leaks.

Additional behavior

  • Allows a bounded quiescence period for legitimate asynchronous cleanup.
  • Recognizes iteration scopes registered for deferred cleanup without ignoring unrelated iteration-scope leaks.
  • Keeps recording isolated to the test harness.
  • Preserves an original test failure while attaching diagnostic failures as additional evidence.
  • Includes focused tests for lifecycle classification, reporting, configuration, stack filtering, and concurrent snapshots.
  • Adds a troubleshooting skill that guides an LLM from the emitted timeline to the broken ownership edge and a compact human-readable diagram.

Motivation

Strict trace writing can reveal that asynchronous work is still pending, but it does not explain where ownership was lost.

This diagnostic connects capture, activation, scope closure, and continuation resolution into one timeline, making continuation failures reproducible and actionable for both humans and automated integration work.

Fixed issues leveraging that:

Additional Notes

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@amarziali amarziali added the tag: do not merge Do not merge changes label Sep 11, 2026
@datadog-datadog-us1-prod

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 13.98 s 13.98 s [-0.7%; +0.8%] (no difference)
startup:insecure-bank:tracing:Agent 12.90 s 13.08 s [-2.1%; -0.6%] (maybe better)
startup:petclinic:appsec:Agent 17.61 s 17.57 s [-0.6%; +1.2%] (no difference)
startup:petclinic:iast:Agent 17.45 s 16.99 s [-1.7%; +7.1%] (no difference)
startup:petclinic:profiling:Agent 17.23 s 17.48 s [-2.5%; -0.3%] (maybe better)
startup:petclinic:sca:Agent 17.63 s 17.46 s [+0.1%; +1.8%] (maybe worse)
startup:petclinic:tracing:Agent 16.59 s 16.81 s [-2.3%; -0.3%] (maybe better)

Commit: 7dcb0903 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@amarziali
amarziali added this pull request to stack #12465 September 11, 2026 13:34
@amarziali
amarziali removed this pull request from stack #12465 September 11, 2026 13:39
@amarziali
amarziali force-pushed the andrea.marziali/diag2 branch from aa21edf to 506a231 Compare September 14, 2026 12:33
@amarziali
amarziali force-pushed the andrea.marziali/diag2 branch from 4bfc4b6 to 113957f Compare September 15, 2026 15:20
@pr-commenter

pr-commenter Bot commented Sep 15, 2026

Copy link
Copy Markdown

Kafka / consumer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master andrea.marziali/diag2
git_commit_date 1789549637 1789636371
git_commit_sha 6c9c7ee b1f76b3
See matching parameters
Baseline Candidate
ci_job_date 1789637607 1789637607
ci_job_id 2052599415 2052599415
ci_pipeline_id 138107322 138107322
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-f6rz4kye 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-f6rz4kye 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaConsumerBenchmark.benchConsume same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaConsumerBenchmark.benchConsume same
scenario:only-tracing-dsm-enabled-benchmarks/KafkaConsumerBenchmark.benchConsume same

@amarziali
amarziali force-pushed the andrea.marziali/diag2 branch from e7e6409 to b7fa1e8 Compare September 16, 2026 09:27
@pr-commenter

pr-commenter Bot commented Sep 16, 2026

Copy link
Copy Markdown

Kafka / producer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master andrea.marziali/diag2
git_commit_date 1789729041 1789730514
git_commit_sha 0854571 c034c2c
See matching parameters
Baseline Candidate
ci_job_date 1789733675 1789733675
ci_job_id 2057388097 2057388097
ci_pipeline_id 138445198 138445198
cpu_model Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-2mls8hku 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-2mls8hku 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-enabled-benchmarks/KafkaProduceBenchmark.benchProduce same

@amarziali
amarziali added this pull request to stack #12526 September 16, 2026 14:11
@amarziali amarziali changed the title Test scope diagnostics Detect scope and continuation leaks in instrumentation tests Sep 16, 2026
@amarziali
amarziali force-pushed the andrea.marziali/diag2 branch from 13947ba to c034c2c Compare September 18, 2026 11:55
@amarziali
amarziali marked this pull request as ready for review September 18, 2026 13:25
@amarziali
amarziali requested review from a team as code owners September 18, 2026 13:25
@amarziali
amarziali requested review from jordan-wong and removed request for a team September 18, 2026 13:25
@amarziali
amarziali requested review from dougqh and removed request for a team September 18, 2026 13:25
@dd-octo-sts

dd-octo-sts Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

  • Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-18T14:44:11.819692Z 5401fc8 Manual request
🔒 Security Review Completed 2026-09-18T14:36:58.398732Z 5401fc8 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@amarziali amarziali added type: feature Enhancements and improvements comp: testing Testing tag: no release notes Changes to exclude from release notes and removed tag: do not merge Do not merge changes labels Sep 18, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c034c2c2b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .agents/skills/fix-continuation-leakage/SKILL.md Outdated
Comment thread .agents/skills/fix-continuation-leakage/SKILL.md Outdated

@datadog-datadog-us1-prod datadog-datadog-us1-prod Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: FAIL

The new guard ignores a second close of a continued scope, so the diagnostic does not report the repeated resolution.

Open Bits AI session

🤖 Datadog Autotest · Commit c034c2c · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@amarziali

Copy link
Copy Markdown
Contributor Author

@DataDog review

@datadog-datadog-us1-prod datadog-datadog-us1-prod Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: PASS

More details

Focused checks cover diagnostic classification, both test harnesses, and an affected integration. They show no reportable failure mode.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Datadog Autotest · Commit 5401fc8 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@amarziali

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 5401fc8543

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5401fc8543

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -15,12 +15,6 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace

class HystrixObservableTest extends HystrixTestRunner {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep strict writes disabled until the Hystrix race is fixed

When the asynchronous scheduler variants run in CI, this class now inherits useStrictTraceWrites() == true, which uses the discarding pending-trace buffer; the deleted override explicitly documented that this suite still times out in that mode. Because this change modifies neither the Hystrix continuation lifecycle nor the test's synchronization—and removes the same safeguard from HystrixObservableChainTest—those variants can again reach their trace assertion while async references remain outstanding and time out. Fix that lifecycle or synchronization before removing these overrides.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a great catch. I managed reproducing the CI issue by stress testing and got a failure due to a race in the rxjava dependency (not hystrix itself) used for testing. Now not to lower the coverage we found a way to remove the race in the testing. So I'll solve this comment as soon as #12570 will be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: testing Testing tag: no release notes Changes to exclude from release notes type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant