Skip to content

Add coordinated sampling for snapshot probes - #12452

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
jpbempel/logpoint-sync
Sep 14, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
jpbempel/logpoint-sync

Conversation

@jpbempel

@jpbempel jpbempel commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

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

Jira ticket: DEBUG-5829

@jpbempel
jpbempel requested a review from a team as a code owner September 10, 2026 15:44
@jpbempel
jpbempel requested review from evanchooly and removed request for a team September 10, 2026 15:44
@jpbempel jpbempel added comp: debugger Dynamic Instrumentation type: feature Enhancements and improvements labels Sep 10, 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: 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".

@datadog-prod-us1-4

This comment has been minimized.

@pr-commenter

pr-commenter Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Debugger benchmarks

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
ci_job_date 1789398848 1789399174
end_time 2026-09-14T15:15:35 2026-09-14T15:20:58
git_branch master jpbempel/logpoint-sync
git_commit_sha 71d8c05 6cdb1ce
start_time 2026-09-14T15:14:09 2026-09-14T15:19:35
See matching parameters
Baseline Candidate
ci_job_id 2040972862 2040972862
ci_pipeline_id 137283211 137283211
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
git_commit_date 1789397612 1789397612

Summary

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

scenario Δ mean agg_http_req_duration_min Δ mean agg_http_req_duration_p50 Δ mean agg_http_req_duration_p75 Δ mean agg_http_req_duration_p99 Δ mean throughput
scenario:basic unsure
[-14.342µs; -1.612µs] or [-4.768%; -0.536%]
unsure
[-16.019µs; -3.232µs] or [-4.932%; -0.995%]
better
[-16.798µs; -4.384µs] or [-5.035%; -1.314%]
unstable
[-383.802µs; -178.320µs] or [-33.887%; -15.744%]
better
[+23.861op/s; +203.025op/s] or [+1.026%; +8.730%]
See unchanged results
scenario Δ mean agg_http_req_duration_min Δ mean agg_http_req_duration_p50 Δ mean agg_http_req_duration_p75 Δ mean agg_http_req_duration_p99 Δ mean throughput
scenario:noprobe unstable
[-33.080µs; +11.185µs] or [-10.587%; +3.580%]
unstable
[-38.996µs; +28.599µs] or [-11.349%; +8.323%]
unstable
[-47.899µs; +38.649µs] or [-13.421%; +10.830%]
unstable
[-232.954µs; -21.259µs] or [-18.443%; -1.683%]
same
scenario:loop same same same same same
Request duration reports for reports
gantt
    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,
Loading
  • baseline results
Scenario Request median duration [CI 0.99]
noprobe 343.62 µs [313.39 µs, 373.85 µs]
basic 324.825 µs [318.666 µs, 330.983 µs]
loop 8.088 ms [8.025 ms, 8.151 ms]
  • candidate results
Scenario Request median duration [CI 0.99]
noprobe 338.421 µs [304.489 µs, 372.353 µs]
basic 315.199 µs [308.896 µs, 321.503 µs]
loop 8.085 ms [8.021 ms, 8.149 ms]

@dd-octo-sts

dd-octo-sts Bot commented Sep 10, 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 14.81 s 14.66 s [+0.0%; +2.0%] (maybe worse)
startup:insecure-bank:tracing:Agent 13.61 s 13.69 s [-1.3%; +0.2%] (no difference)
startup:petclinic:appsec:Agent 17.68 s 17.49 s [+0.1%; +2.1%] (maybe worse)
startup:petclinic:iast:Agent 17.53 s 17.62 s [-1.4%; +0.4%] (no difference)
startup:petclinic:profiling:Agent 17.44 s 17.23 s [-0.1%; +2.4%] (no difference)
startup:petclinic:sca:Agent 17.62 s 17.59 s [-0.7%; +1.1%] (no difference)
startup:petclinic:tracing:Agent 16.59 s 16.73 s [-1.8%; +0.0%] (no difference)

Commit: 6cdb1ce3 · 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.

@datadog-prod-us1-4 datadog-prod-us1-4 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

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.

Open Bits AI session

🤖 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().
@jpbempel
jpbempel force-pushed the jpbempel/logpoint-sync branch from f56387e to 94fbfeb Compare September 14, 2026 14:07
@jpbempel
jpbempel added this pull request to the merge queue Sep 14, 2026
@dd-octo-sts

dd-octo-sts Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-09-14 20:32:51 UTC ℹ️ Start processing command /merge


2026-09-14 20:33:06 UTC ℹ️ MergeQueue: Pull request is not mergeable yet

It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.

  • Run /code blockers to see what is blocking it.
  • Run /remove to cancel it.

2026-09-14 20:35:28 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in master is approximately 1h (p90).


2026-09-14 21:38:07 UTC ℹ️ MergeQueue: This merge request was merged

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 14, 2026
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit f4b1632 into master Sep 14, 2026
611 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the jpbempel/logpoint-sync branch September 14, 2026 21:38
@github-actions github-actions Bot added this to the 1.67.0 milestone Sep 14, 2026
dudikeleti added a commit to DataDog/dd-trace-dotnet that referenced this pull request Sep 25, 2026
## 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} -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: debugger Dynamic Instrumentation type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants