Add coordinated sampling for snapshot probes - #12452
Conversation
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().
jpbempel
added
comp: debugger
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
| boolean sampled = | ||
| !logStatus.getDebugSessionStatus().isDisabled() | ||
| && ProbeRateLimiter.tryProbe(localSampler, isFullSnapshot()); | ||
| boolean sampled = !logStatus.getDebugSessionStatus().isDisabled() && trySample(localSampler); |
There was a problem hiding this comment.
When a full-snapshot probe tagged for an active debug session reaches this path, such as a conditioned probe, trySample can cache DROP even though LogStatus.shouldSend() later emits that probe unconditionally because the trigger already sampled the session. Every ordinary snapshot probe later in the same local trace then observes DROP and is suppressed, producing the fragmented snapshot set this coordination is meant to prevent; conversely, an active probe still emits after an ordinary probe cached DROP. Bypass or update the coordinated state for active-session probes rather than recording a decision that shouldSend() ignores.
Useful? React with 👍 / 👎.
| boolean sampled = | ||
| !logStatus.getDebugSessionStatus().isDisabled() | ||
| && ProbeRateLimiter.tryProbe(localSampler, isFullSnapshot()); | ||
| boolean sampled = !logStatus.getDebugSessionStatus().isDisabled() && trySample(localSampler); |
There was a problem hiding this comment.
When a full-snapshot probe tagged for an active debug session reaches this path (for example, a conditioned probe), trySample can cache DROP even though LogStatus.shouldSend() later emits that probe unconditionally because the trigger already sampled the session. Every ordinary snapshot probe later in the same local trace then observes DROP and is suppressed, producing exactly the fragmented snapshot set this coordination is meant to prevent; if an ordinary probe cached DROP first, the active probe still emits with the same inconsistency. Bypass or update the coordinated state for active-session probes rather than recording a decision that shouldSend() ignores.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| } | ||
| return state.tryEmit(getProbeId().getEncodedId()); |
There was a problem hiding this comment.
When an EMIT trace reaches two or more full-snapshot probes, only the first probe invokes ProbeRateLimiter.tryProbe; every later probe returns true here without consulting either its own sampler or GLOBAL_SNAPSHOT_SAMPLER. This lets followers exceed their documented snapshotsPerSecond maximum when probes have different rates, and it lets the configured global snapshot cap be exceeded by up to the number of matching probes per trace. Use a coordinated group decision that still enforces the applicable per-probe and global caps rather than bypassing both samplers for followers.
Useful? React with 👍 / 👎.
|
🎯 Code Coverage (details) 🔗 Commit SHA: f56387e | Docs | View more details | Give us feedback! |
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 4 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 (356.461 µs) : 333, 380
. : milestone, 356,
basic (336.266 µs) : 329, 343
. : milestone, 336,
loop (8.118 ms) : 8055, 8181
. : milestone, 8118,
section candidate
noprobe (369.974 µs) : 346, 394
. : milestone, 370,
basic (341.009 µs) : 336, 346
. : milestone, 341,
loop (8.117 ms) : 8053, 8181
. : milestone, 8117,
|
🟢 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. |
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
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
| } | ||
|
|
||
| private boolean trySample(Sampler sampler) { | ||
| if (!isFullSnapshot()) { |
There was a problem hiding this comment.
One feature can stop the other feature or let it emit without its own sampling decision.
Assertion details
- Input: Enable exception debugging and an ordinary snapshot probe on the same local root span.
- Expected:
Exception probes must use their exception sampling flow. They must not share the coordinated state of ordinary snapshot probes. - Actual:
The first full-snapshot probe stores its decision on the local root span. An ExceptionProbe uses the same state because it extends LogProbe and is a full-snapshot probe.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
2 participants
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