Skip to content

Guard core-metric telemetry drain against queue-full delta loss - #12081

Open
dougqh wants to merge 2 commits into
masterfrom
dougqh/coremetric-queue-full-delta-loss
Open

Guard core-metric telemetry drain against queue-full delta loss#12081
dougqh wants to merge 2 commits into
masterfrom
dougqh/coremetric-queue-full-delta-loss

Conversation

@dougqh

@dougqhdougqh commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What Does This Do?

Stops the core-metric telemetry drain early when its queue is full instead of continuing to read counters it can't enqueue. Concretely: check remainingCapacity() before reading each counter in the span-metrics and baggage loops, and break before the getValueAndReset() + CoreMetric allocation when the queue is full. Behavior is unchanged in the healthy (non-full) case.

This does two things under overload:

  • Avoids wasted allocation — no longer builds CoreMetric objects (and their tag strings) only to drop them on a failed offer().
  • Preserves the deltagetValueAndReset() resets the counter's baseline, so a value read but not enqueued was lost permanently. Untouched counters now keep their accumulated delta and are collected on the next cycle.

Motivation

Both effects matter under backpressure: when the Datadog agent is unreachable, telemetry backs up and the queue fills. The original goal was to cut the per-counter allocation while the system is already overloaded. In doing that it became clear there's also a real data-loss bug — the counts we most want (the ones describing the degraded state) are exactly the ones the old code silently discarded.

Additional Notes

CoreMetricCollector.prepareMetrics() drains each core counter with getValueAndReset() and then offer()s the resulting CoreMetric onto a fixed ArrayBlockingQueue. Because getValueAndReset() resets the counter's delta baseline, if the queue is already full the reset has happened but the value is dropped — that delta is lost permanently, not deferred to the next cycle.

Sole-producer invariant: prepareMetrics() is the only writer to the queue (drain() only removes), so a remainingCapacity() == 0 check followed by offer() cannot race into a lost delta.

Scope / relation to #12070

The identical guard for the client-side trace-stats collapse loop ships in #12070 (which introduced that loop). This PR fixes the two pre-existing loops (span metrics, baggage) that share the same pattern.

🤖 Generated with Claude Code

getValueAndReset() resets a counter's delta baseline, so calling it before the
offer() succeeds means a full metricsQueue silently drops that delta forever --
worst under telemetry backpressure when the agent is unreachable. In both the
span-metrics and baggage collection loops, check remainingCapacity() before
reading each counter and stop early instead; the untouched counters are
collected on the next cycle.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqhdougqh added comp: telemetry Telemetry tag: ai generated Largely based on code generated by an AI or LLM type: bug fix Bug fix labels Jul 27, 2026
@datadog-prod-us1-4

This comment has been minimized.

@dd-octo-sts

dd-octo-stsBot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

SuiteStatus
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
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent13.98 s13.85 s[+0.0%; +1.7%] (maybe worse)
startup:insecure-bank:tracing:Agent12.91 s13.03 s[-1.7%; -0.2%] (maybe better)
startup:petclinic:appsec:Agent17.52 s17.20 s[+0.8%; +2.9%] (maybe worse)
startup:petclinic:iast:Agent17.48 s17.58 s[-1.5%; +0.4%] (no difference)
startup:petclinic:profiling:Agent17.23 s17.37 s[-2.3%; +0.6%] (no difference)
startup:petclinic:sca:Agent17.57 s17.57 s[-0.9%; +0.9%] (no difference)
startup:petclinic:tracing:Agent16.64 s16.59 s[-0.8%; +1.3%] (no difference)

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

Fills CoreMetricCollector's drain past its ArrayBlockingQueue capacity and
verifies every counter's delta is reported exactly once across cycles. Fails
against the pre-fix code (counters read-and-reset while the queue was full were
dropped) and passes with the remainingCapacity() guard.
Robust to the shared SpanMetricRegistryImpl singleton: tags its own
instrumentations with a unique prefix, counts only those, and drains
cycle-by-cycle until the queue is empty.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh
dougqh marked this pull request as ready for review July 28, 2026 21:54
@dougqh
dougqh requested a review from a team as a code ownerJuly 28, 2026 21:54

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

The capacity checks preserve both span-metric and baggage deltas when the telemetry queue is full: a compiled harness retained 1,224 span deltas and two baggage deltas across the overflow cycle, with no unexpected output or loss observed. The repository Gradle test could not start because the sandbox lacks the required Java 25 toolchain and cannot download it.

Was this helpful? React 👍 or 👎

📊 Validated against 2 scenarios · Open Bits AI session

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

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

Labels

comp: telemetryTelemetrytag: ai generatedLargely based on code generated by an AI or LLMtype: bug fixBug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@dougqh