Repository navigation
Add coordinated sampling for snapshot probes - #12452
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f56387e2e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
Debugger benchmarksParameters
See matching parameters
SummaryFound 2 performance improvements and 0 performance regressions! Performance is the same for 8 metrics, 5 unstable metrics.
See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (343.62 µs) : 313, 374
. : milestone, 344,
basic (324.825 µs) : 319, 331
. : milestone, 325,
loop (8.088 ms) : 8025, 8151
. : milestone, 8088,
section candidate
noprobe (338.421 µs) : 304, 372
. : milestone, 338,
basic (315.199 µs) : 309, 322
. : milestone, 315,
loop (8.085 ms) : 8021, 8149
. : milestone, 8085,
|
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
There was a problem hiding this comment.
An ordinary snapshot probe and an exception probe now share one root-span sampling state. The first probe can stop the other probe or let it emit without its own sampling decision.
🤖 Datadog Autotest · Commit f56387e · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Adds coordinated sampling for full debugger snapshots using the active Datadog context, using Context API. State of Coordinated Sampling is stored into a Context attached to the root local span. The first probe’s sampling decision controls related probes, avoiding fragmented snapshot sets. Ensures each probe emits at most once per coordinated context. Log-only probes remain independent. Encapsulates ProbeDefinition.probeId, updating callers to use getProbeId().
f56387e to
94fbfeb
Compare
|
/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
|
## Summary of changes - Coordinate Live Debugger sampling once per local trace for full-snapshot and capture-expression probes. - Share the first decision across all participating probes in the trace; each probe emits at most one snapshot per trace. - Preserve independent sampling for template-only logs, metric and span-decoration probes, Exception Replay, and hits without an active trace. - Correlate snapshots with the span context captured when probe processing begins, not when the snapshot is finalized. ## Reason for change - Independent per-probe sampling produces incomplete parent/child/sibling snapshot chains within the same request, with no signal that anything is missing. - Align .NET with [dd-trace-java#12452](DataDog/dd-trace-java#12452), the "Improving Correlation for Live Debugger Snapshots" RFC (Tier 1: existing trace context), and system-test expectations. ## Implementation details ### Which probes participate - A probe is coordinated when it is a full snapshot or has capture expressions (`ShouldCoordinateSampling`). - Everything else keeps its existing sampling path unchanged. ### Flow at probe entry (`ProbeProcessor.TryBeginProcess`) ```mermaid flowchart TD A[Probe hit] --> B{Has condition?} B -- yes --> C[No sampling yet.<br/>Capture span context, create snapshot creator] B -- no --> D{Coordinated probe<br/>and active trace?} D -- no --> E[Independent sampling:<br/>global limiter, then per-probe sampler] D -- yes --> F["traceContext.GetOrCreateDebuggerSamplingCoordinator()<br/>.TrySample(probeId, provider)"] F --> G{Decision} G -- Keep --> H[Capture span context,<br/>create snapshot creator, capture] G -- DropGlobal / DropProbe --> I[Skip before any capture work,<br/>record events.skipped] C --> J[Evaluate condition] J -- false --> K[Stop] J -- true --> F ``` - Unconditional probes are sampled before any capture work, so a dropped hit costs a lookup, not a capture. - Conditional probes evaluate their condition first and join coordination only when it is true. - The span context is captured at entry and used for the snapshot's `dd.trace_id` / `dd.span_id` and for the trace lookup, so a snapshot finalized after its span closed still correlates correctly. ### Per-trace coordinator (`DebuggerSamplingCoordinator`) ```mermaid stateDiagram-v2 [*] --> Undecided Undecided --> Keep: first hit's samplers accept (its probe ID claims a slot) Undecided --> DropGlobal: global limiter rejects Undecided --> DropProbe: per-probe sampler rejects Undecided --> Undecided: sampler threw (nothing published, waiting callers decide) Keep --> Keep: later hit claims its probe's slot once, further hits get DropProbe ``` - `TraceContext` gains one nullable field and a lazy `GetOrCreateDebuggerSamplingCoordinator()` accessor, the same pattern as the AppSec, IAST and feature-flag per-trace state. All sampling logic lives in the debugger. - The coordinator is allocated only for traces that hit a coordinated probe. - The coordinator stores a `DebuggerSamplingDecision` directly. `Undecided` is the enum's zero value, so a new coordinator starts undecided and an unassigned decision never means `Keep`. - The first hit consults the global limiter and its per-probe sampler once. Once the trace is kept, other probes bypass their samplers and are capped at one snapshot per probe for the trace. - A dropped trace is answered by a lock-free volatile read. Kept traces take a short monitor lock to claim the probe's slot in a `HashSet`. - Concurrent first hits wait on the lock for the in-flight decision. The decision is published only after the samplers return, so a sampler exception leaves the trace undecided and a waiting caller decides instead. - The lock is reentrant, and the samplers can run customer code on the deciding thread (for example a first-chance exception handler) that hits another coordinated probe in the same trace. `Monitor.IsEntered` detects that nested call and returns `DropProbe` instead of re-entering the samplers, which could otherwise recurse without bound. - A constrained struct provider (`IDebuggerSamplingDecisionProvider`) avoids delegate allocation and boxing on the sampling path. ### A claimed slot is never released - Once a probe claims its slot in a trace, the slot stays used whatever the outcome: a snapshot, a capture expression with no values, or a failed capture. - Once a trace is kept, samplers are no longer consulted, so the slot is the only throttle. Releasing it would let every later hit of the probe in that trace (loops, recursion) capture and evaluate again with no throttling. - A capture-expression result with no values and no errors depends on the probe definition, not on runtime values, so later hits would be empty too. ## Test coverage - Unit (`CoordinatedSamplingTests`): shared Keep/Drop decisions across snapshot and capture-expression probes, per-probe caps, conditions, independent sampling for template logs, decisions local to each trace, concurrent first hits, reentrancy, sampler exception recovery, empty capture expressions keeping the slot (including a mid-trace probe update), skipped-event telemetry, and entry-time span correlation. - End-to-end (`ProbesTests`): `CoordinatedSnapshotProbesEmitCompleteChains` (a complete three-probe chain across sampling windows) and `CoordinatedLineProbesEmitOncePerTrace` (one snapshot per line probe per trace). ## Other details - The global snapshot limiter is consulted once per participating trace; a kept trace may emit one snapshot for each participating probe. - Long-lived traces emit at most one snapshot per probe for the lifetime of their `TraceContext`. - Not supported: a probe updated mid-trace (same probe ID) doesn't emit again in that trace; it emits normally starting with the next trace. - Hits skipped by the per-trace cap are counted as `reason:rateLimitProbe` in `events.skipped`. - Condition-error diagnostic snapshots remain independently rate-limited. - Exception Replay stays independently sampled and keeps its captured correlation context. - `dd.trace_id` remains the existing 64-bit projection. - Enabling the .NET system-test manifest remains follow-up work. <!-- Fixes #{issue} -->
What Does This Do
Adds coordinated sampling for full debugger snapshots using the active Datadog context, using Context API.
State of Coordinated Sampling is stored into a Context attached to the root local span.
The first probe’s sampling decision controls related probes, avoiding fragmented snapshot sets.
Ensures each probe emits at most once per coordinated context. Log-only probes remain independent.
Encapsulates ProbeDefinition.probeId, updating callers to use getProbeId().
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: DEBUG-5829