Repository navigation
test(debugger): add snapshot correlation tests with Go, .NET and Node.js weblog endpoints - #7425
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 14 commits intoOct 23, 2026
Conversation
5 tasks
Contributor
|
|
grantseltzer
marked this pull request as ready for review
August 3, 2026 13:58
grantseltzer
requested review from
dromanol,
florentinl,
manuel-alvarez-alvarez and
ncybul
and removed request for
a team
August 3, 2026 13:58
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The new correlation chain returns the expected value, and the loop handler safely returns Loop 0 for both zero and negative counts. The shared line-probe mapping targets the newly added loop body and sibling lines consistently across Go weblog variants.
📊 Validated against 3 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 8431afa · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
nccatoni
reviewed
Aug 5, 2026
nccatoni
left a comment
nccatoni
left a comment
Collaborator
There was a problem hiding this comment.
LGTM (from @DataDog/system-tests-core) but you should get a review from someone familiar with the feature
tylfin
reviewed
Aug 5, 2026
tylfin
reviewed
Aug 5, 2026
tylfin
reviewed
Aug 5, 2026
tylfin
reviewed
Aug 5, 2026
tylfin
reviewed
Aug 5, 2026
watson
changed the title
Every probe had the same one snapshot per second limit, and each request hits the probes at the same offsets, so independent per-probe samplers stay in lockstep and emit whole chains too. The test passed without coordinated sampling, e.g. for Node.js before coordinated sampling was implemented. Let the leaf, which is the first probe hit in each request and therefore decides for the trace, sample every request, and keep the other probes at one snapshot per second. A request takes about 800ms, so without coordination those probes are skipped in about every other request and the chains split. Replaying the scenario against Node.js weblog routes: 15 of 30 chains were partial without coordinated sampling, and all 30 were complete with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the `/debugger/correlation` and `/debugger/correlation/loop/:count` endpoints to the Node.js Express, TypeScript Express and Fastify weblogs, mirroring the Go endpoints: the probed functions return 400ms apart, and the loop sleeps for a second per iteration. The debugger scenarios run the weblogs with IAST enabled, and Dynamic Instrumentation places line probes two lines too early in files that the IAST rewriter rewrites. So the endpoints avoid everything the rewriter instruments (`+`, `+=`, template literals and some string methods) and respond with constant strings. Node.js doesn't support method probes, so the correlation tests point each probe at the line its method returns on for Node.js instead, using new Node.js entries in the line map. The new routes move the capture timeout line from 157 to 159, and the budgets line from 163 to 165. Enable `Test_Debugger_Coordinated_Sampling` and `test_per_span_budget` from v6.21.0 and v5.132.0, the releases that ship coordinated snapshot sampling (DataDog/dd-trace-js#10677). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The new debugger fixtures change the content of the express4, express4-typescript, express5 and fastify base images. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…test Node.js doesn't support method probes. Instead of branching on the library in the test, add `Test_Debugger_Coordinated_Sampling_Line_Probe`, which points each probe at the line its method returns on, using the line map. Enable it for Node.js, and mark the method-probe variant as irrelevant there, like the evaluation timeout guardrail test variants. The other libraries keep the method-probe variant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
watson
changed the title
watson
changed the title
P403n1x87
approved these changes
Oct 23, 2026
Remove Test_Debugger_Snapshot_Correlation_Fields. No tracer sends the generation token it asserts on, and nothing defines that field yet. Its thread_id check also fails Java, which sends the key flat, and PHP, which has no logger object. The runtime ID part is dropped for now. Rename test_per_span_budget to test_per_trace_budget, since tracers cap each probe at one snapshot per trace, not per span. Count every collected snapshot, so tracers that send the flat debugger.snapshot key are counted. Use a ref for the Node.js 6.21.0 / 5.132.0 versions. Declare the Java entries incomplete_test_app. dd-trace-java 1.67.0 has coordinated sampling and the per-trace cap, but the Java weblog has no /debugger/correlation endpoints. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The RFC says the first snapshot probe hit in a trace decides for the whole trace, and that once it emits, every probe in the trace may emit until its cap of one snapshot per trace. The test relies on that: the leaf samples every request, and the other probes emit past their own one-per-second rate. Say so in the class docstring. The leaf is only the first probe hit for line probes on the return lines. Method probes may count the outer method first, so reword that comment. Environment: Datadog workspace Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts with #7995. Keep its Python CaptureTimeout line mapping next to the moved Node.js line, and its version-scoped DEBUG-5743 entry next to the new test_per_trace_budget entry in the Python manifest. Environment: Datadog workspace Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment named Ruby as sending both the nested dd object and the flat dd.trace_id key, and Go as sending the flat key. In CI, Ruby sends only the flat key and Go sends neither. Describe the two formats without language lists, which go stale. Environment: Datadog workspace Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tylfin
approved these changes
Oct 23, 2026
nccatoni
approved these changes
Oct 23, 2026
Member
Author
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
grantseltzer/correlation-tests-go
branch
October 23, 2026 15:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The RFC: Improving Correlation for Live Debugger Snapshots asks tracers to make one snapshot sampling decision per trace, so a trace emits its whole probe chain or none of it, and to cap each probe at one snapshot per trace. This adds the shared tests for that behavior (feature 564) and the weblog endpoints they need. It enables them for .NET (DataDog/dd-trace-dotnet#9266) and Node.js (DataDog/dd-trace-js#10677, DEBUG-5831).
Note
Taken over from @grantseltzer, with his permission, by @watson. The branch was rebased onto
mainand the remaining issues fixed. The Node.js enablement, previously stacked on top in #7985, is now part of this PR.Changes
Tests:
Test_Debugger_Coordinated_Samplingsends 30 requests to/debugger/correlation. It asserts that every trace emits either all of the chain's probes or none of them, with at least one full chain.Test_Debugger_Coordinated_Sampling_Line_Proberuns the same test with line probes on the line each method returns on, for tracers without method probes. It's activated through the manifests, like the evaluation timeout guardrail variants.Test_Debugger_Probe_Budgets::test_per_trace_budgetprobes a loop body and a sibling line via/debugger/correlation/loop/:count. It asserts exactly one snapshot per probe per request, which is one trace.Weblogs, all documented in
end-to-end_weblog.md:chi,echo,gin,net-httpandnet-http-orchestrion, with the Go line mappings.+,+=, template literals and some string methods). The debugger scenarios run with IAST enabled, and Dynamic Instrumentation places line probes two lines too early in a file the rewriter rewrites.CaptureTimeoutline from 157 to 159 andBudgetsfrom 163 to 165.Manifests:
test_per_trace_budgetfrom>=6.21.0 || ^5.132.0(a newref_6_21_0), and marks the method-probe variant as irrelevant.incomplete_test_app. dd-trace-java 1.67.0 implements coordinated sampling and the per-trace cap, but the Java weblog has no correlation endpoints yet.missing_feature, orirrelevantfor the line-probe variant.Fixes made while taking this over:
main.using System.Runtime.CompilerServices;added to the .NETDebuggerController.csmoved every line of the file down by one, which broke all existing .NET line-probe tests. TheMethodImplattributes are now fully qualified instead.dd.trace_idkey that .NET sends./debugger/correlation/loop/<count>, which a review comment asked for.Test_Debugger_Coordinated_Samplingdiscriminating. With the same rate on every probe, independent samplers stayed in lockstep and also emitted only whole chains, so the test passed without coordinated sampling.Testing
The real Node.js
express/debugger/index.jsroutes ran under dd-trace-js with a fake agent and IAST enabled, as in CI, replaying both scenarios with the same probes:/debugger/correlation×30/debugger/correlation/loop/3With IAST enabled and disabled, a snapshot probe on each mapped Node.js line (20, 159, 165, 178, 184, 188, 194 and 197) pauses on exactly that line.
In CI, all .NET and Node.js end-to-end jobs pass, on both
devandprod. The Node.js jobs run the line-probe variant andtest_per_trace_budget.Checks:
./format.sh --check: mypy, import policy, ruff, yamlfmt, yamllint, the manifest parser and shellcheck pass.debugger/index.js: both pass.tsc --strictagainst a minimal Express type stub.Not addressed here:
fiber-v2-orchestrionandnet-http-span-pooldon't have the correlation endpoints yet. Go's manifest keeps these tests disabled.runtime_idin the snapshot envelope and an execution-context generation token. An earlier version of this PR tested both. Runtime ID is on hold, and no tracer sends a generation token yet, with no agreed field for it, so the test was removed.Workflow
system-tests-reviewers: ask to one of youre co-worker familiar with the tested feature.system-tests-core(slack)🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
🤖 Generated with Claude Code