fix(llmobs): aggregate span.finished telemetry counts per interval - #12136
Conversation
LLMObsMetricCollector enqueued one raw metric of value 1 per finished span and left prepareMetrics() as a no-op, so the count was never aggregated in-process. Two problems followed: - Metric timestamps are second-granularity, so all points a series emits within the same second collapse to one value at the metrics intake. The reported rate was pinned at ~1/s per series regardless of the real span rate. - The raw queue holds RAW_QUEUE_SIZE (1024) entries per 10s metrics interval, silently dropping anything above ~102 spans/s per JVM. Count per tag combination with a LongAdder and emit one metric carrying the summed value in prepareMetrics(), matching CoreMetricCollector and the other tracers. The queue now holds one entry per tag combination per interval instead of one per span. A counter whose metric cannot be staged keeps its count for a later interval rather than losing it, and the number of tracked tag combinations is bounded. Also migrates the two affected Groovy tests to JUnit 5 / Java per the repo test convention. The previous "test aggregation of identical metrics" case asserted the buggy shape (three points of value 1); it is replaced by cases asserting a single point carrying the summed count, including one well above RAW_QUEUE_SIZE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Yun-Kim
added
tag: ai generated
This comment has been minimized.
This comment has been minimized.
🟢 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. |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Yun-Kim
changed the title
The three span.finished telemetry tests in DDLLMObsSpanTest asserted that finish() alone puts a metric in the queue. Recording now only increments a LongAdder, so drain() returned empty and the tests failed on every JVM. Reset the collector in the shared setup() fixture (a bare drain() no longer isolates counter state) and call prepareMetrics() before the assertion drain. Also from review: - drop the historical rationale from the class and field javadoc - reword the MAX_TAG_COMBINATIONS javadoc and raise the bound to 512, above the cardinality the 8-entry tag caches imply and clear of RAW_QUEUE_SIZE - guard the tag-limit debug log with isDebugEnabled() - annotate resetForTesting() with @VisibleForTesting Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflict was in LLMObsMetricCollector imports only: master added javax.annotation.Nullable for recordFeedbackSubmitted while this branch added ConcurrentHashMap/LongAdder for the span.finished aggregation. Kept both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![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: f3d305cda9
ℹ️ 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-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The new counters keep span totals across intervals and queue saturation. The static source review found no reportable regression.
🤖 Datadog Autotest · Commit f3d305c · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
ncybul
left a comment
ncybul
left a comment
There was a problem hiding this comment.
Looks good to me, but I wonder if you think the bot's comment is worth addressing.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 9d31521: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
3 participants
What Does This Do
Makes
LLMObsMetricCollectoraggregatemlobs.span.finishedcounts per tag combination in-process and emit one metric carrying the summed value per metrics interval, instead of enqueuing one raw metric of value1per finished span.Why
dd.instrumentation_telemetry_data.mlobs.span.finishedseverely underreports on the spans that are received by Datadog LLM Observability backend.recordSpanFinishedenqueued oneLLMObsMetricof value1per span, andprepareMetrics()was a no-op, so no summation ever happened. MetricCollector.java reduces each object stamped to second-granularity timestamp. This means every point a series emits within the same second collapses to a single value at the metrics intake, so the reported rate is pinned at ~1/s per series no matter the real span rate.I confirmed on the affected org:
max:dd.instrumentation_telemetry_data.mlobs.span.finished{org_id:...}.as_rate()sits flat at exactly 1.0/s, and the org-widesumof ~1k/min is just the ~17 active tag combinations each contributing 1/s.How It Works
recordSpanFinishedincrements aLongAdderkeyed by tag combo. stop enqueuing to metrics queue directlyprepareMetrics()drains each counter withsumThenReset()and stages one summed metric per tag comboVerification
Built and tested locally on JDK 21.
Same scratch test, same input (5000 identical spans inside one metrics interval), before vs after:
emitted 1024 metric entries, distinct values=[1], total reported=1024 out of 5000 spans5000Those "before" numbers are the two failure modes made concrete: the count pinned at exactly
RAW_QUEUE_SIZE(1024), and every emitted point carrying value1- the shape that then collapses to ~1/s per series at the metrics intake.Manual Validation - tested twice, sending 500 spans per instance with the fix and
master: