Repository navigation
fix(llmobs): mark artificially added gen_ai tags on APM spans - #20411
gh-worker-dd-mergequeue-cf854d[bot] merged 8 commits into
Conversation
Adds an internal _dd.llmobs.artificial_gen_ai_tags tag set alongside the gen_ai.* attributes written onto APM spans, so backend processors can distinguish these from tags a user set directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 1 Pipeline job failed
ℹ️ InfoNo other issues found (see more)🧪 All tests passed Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: d9638f9 | Docs | View more details | Give us feedback! |
Codeowners resolved asResolved from the full PR diff against No remaining files require a CODEOWNERS review. |
Circular import analysis
|
Dependency direction analysis
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 121091bce6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The new _dd.llmobs.artificial_gen_ai_tags tag was missing from _LLMOBS_SHADOW_IGNORES, breaking LLMObs-enabled snapshot comparisons. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BenchmarksBenchmark execution time: 2026-09-18 18:52:58 Comparing candidate commit d9638f9 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 590 metrics, 10 unstable metrics, 7 known flaky benchmarks, 17 flaky benchmarks without significant changes.
|
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the tracer-wide boolean tag convention (lowercase "true"/"false") instead of Python's str(bool) capitalization. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ncybul
left a comment
There was a problem hiding this comment.
LGTM besides a couple minor comments. Before we merge this, let's test this in conjunction with your backend PR.
Moves the marker constant next to the other LLMOBS_APM_SHADOW_* constants and renames it to LLMOBS_ARTIFICIAL_GEN_AI_TAGS_KEY. Drops the release note since the actual fix lives in the backend; this internal tag only makes that fix more reliable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
95c1aef
into
main
## Summary - When `set_gen_ai_apm_tags` writes `gen_ai.*` attributes onto an APM span, it now also sets an internal `_dd.llmobs.artificial_gen_ai_tags` tag. - This lets our backend processor distinguish gen_ai tags that ddtrace added artificially from tags a user set directly on the span. Uses the existing `_dd.` prefix convention for internal, non-user-searchable tags (see `_dd.llmobs.*` shadow tags in `ddtrace/llmobs/_constants.py`). Follow-up to #20083 / #20375. ## Testing plan - Extended `tests/llmobs/test_llmobs_gen_ai_apm_tags.py` to assert the new tag is set alongside the existing `gen_ai.*` tags. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: max.zhang <max.zhang@datadoghq.com>
* Reland "feat(llmobs): emit gen_ai.* attributes on APM spans (#10261)" This reverts commit 374e244, restoring #10261 verbatim. The follow-up commit adds the marker tag that addresses the reason for the revert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): mark tracer-emitted gen_ai APM tags The backend reads `gen_ai.*` tags on an APM span as user-set and builds a duplicate LLMObs span out of them, which is why #10261 was reverted. Write `_dd.llmobs.artificial_gen_ai_tags` alongside, matching dd-trace-py, so it can tell the two apart. Port of DataDog/dd-trace-py#20411. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Reland "feat(llmobs): emit gen_ai.* attributes on APM spans (#10261)" This reverts commit 374e244, restoring #10261 verbatim. The follow-up commit adds the marker tag that addresses the reason for the revert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): mark tracer-emitted gen_ai APM tags The backend reads `gen_ai.*` tags on an APM span as user-set and builds a duplicate LLMObs span out of them, which is why #10261 was reverted. Write `_dd.llmobs.artificial_gen_ai_tags` alongside, matching dd-trace-py, so it can tell the two apart. Port of DataDog/dd-trace-py#20411. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Reland "feat(llmobs): emit gen_ai.* attributes on APM spans (#10261)" This reverts commit 374e244, restoring #10261 verbatim. The follow-up commit adds the marker tag that addresses the reason for the revert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): mark tracer-emitted gen_ai APM tags The backend reads `gen_ai.*` tags on an APM span as user-set and builds a duplicate LLMObs span out of them, which is why #10261 was reverted. Write `_dd.llmobs.artificial_gen_ai_tags` alongside, matching dd-trace-py, so it can tell the two apart. Port of DataDog/dd-trace-py#20411. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…20479) ## Summary - Relands #20375's reverted behavior: `_apply_shadow_metrics` now calls `set_gen_ai_apm_tags` again when LLMObs is disabled, emitting `gen_ai.*` attributes on APM-only spans. - This is safe now because #20411 (base of this PR) makes `set_gen_ai_apm_tags` always mark the span as artificially tagged. The backend can use that marker to distinguish these APM-only `gen_ai.*` tags from a real LLM Observability span and avoid creating a duplicate LLMObs span, which was the original bug #20375 fixed by removing the tags entirely. ## Testing plan - Updated `tests/llmobs/test_llmobs.py::test_gen_ai_tags_marked_artificial_when_llmobs_disabled` (replaces the old `test_no_gen_ai_tags_when_llmobs_disabled`) to assert `gen_ai.*` tags and the artificial marker are both set when LLMObs is disabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: max.zhang <max.zhang@datadoghq.com>
…ed (#10437) * Reland "feat(llmobs): emit gen_ai.* attributes on APM spans (#10261)" This reverts commit 374e244, restoring #10261 verbatim. The follow-up commit adds the marker tag that addresses the reason for the revert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): mark tracer-emitted gen_ai APM tags The backend reads `gen_ai.*` tags on an APM span as user-set and builds a duplicate LLMObs span out of them, which is why #10261 was reverted. Write `_dd.llmobs.artificial_gen_ai_tags` alongside, matching dd-trace-py, so it can tell the two apart. Port of DataDog/dd-trace-py#20411. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(llmobs): emit gen_ai.* APM tags when LLM Observability is disabled LLMObs plugins used to disable themselves entirely when LLMObs was off, so the `gen_ai.*` APM attributes were only ever written for LLMObs-enabled applications. Keep the plugins subscribed instead: their handlers skip the LLMObs payload and emit only the APM attributes. The tagging moves out of `LLMObsSpanProcessor` into a shared `gen-ai-tags.js` so the plugins can reuse it, and the marker tag moves with it. Integrations resolve token usage and late model corrections through `getGenAiApmEndTags`. Covers anthropic, claude-agent-sdk, bedrock, google-genai, langchain, langgraph, openai, vertexai and both Vercel AI telemetry paths, each with a unit spec driving its diagnostic channels directly. Carries the enabled-path changes this path is built on: the Anthropic and LangChain usage helpers it reuses, the guards its Bedrock handler shares, and the Bedrock header-caching skip that keeps a response with no token counts from reporting zeros. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): address review on the gen_ai APM tag disabled path - Read streamed Bedrock usage through the shared provider-aware extractor instead of scanning for `amazon-bedrock-invocationMetrics`. Anthropic reports its counts on the message body and Amazon on plain body fields, so a streamed `invokeModel` on those models reported no usage at all. - Stop tagging `gen_ai.application.name`. ml_app is an LLM Observability concept, and with LLMObs off the only value left is the service name the span already carries. Matches dd-trace-py's reduced path. - Drop the in-process session fallback, whose only writer is the tagger and so is dead on this path. A propagated session is still inherited. - Apply the model and provider defaults when an integration promotes a span to a model-backed kind at the end of the operation, so a promotion can no longer leave an llm span with no model. - Cover the Model Context Protocol plugins, which inherit this path. - Drive the google-genai chunk channel in its spec rather than fabricating the field the chunk handler sets. - Document the tags in docs/API.md and both TypeScript surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(llmobs): trim the gen_ai APM tag documentation Drop the reference section and keep the `llmobs` option paragraph as it was, noting only that the option also turns off the basic `gen_ai.*` tags. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(llmobs): drop the API.md change Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(llmobs): trim the null-prototype comments Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): wire OpenAI Realtime into the reduced gen_ai path Two gaps left by the plugins staying subscribed when LLMObs is off: - The response plugin subscribes to `dd-trace:openai:realtime:audio` purely as a capability signal: the instrumentation retains a turn's audio only while something listens there. Staying subscribed with LLMObs off made it buffer megabytes per turn for a payload the reduced path never builds. Gate the subscription on LLMObs itself. - The turn's token usage is read only in `setLLMObsTags`, which the reduced path skips, so realtime spans reported no `gen_ai.usage.*`. Forward it through `getGenAiApmEndTags`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(llmobs): cover the claude-agent-sdk query session on the reduced path The query span learns its session only when the stream resolves, so it arrives through the end hook rather than the register options. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): bound Bedrock stream retention and follow LLMObs toggles - The disabled Bedrock path retained every streamed frame until the response completed so the shared extractor could read the provider's token counts. Keep only the frames whose bytes carry one of the fields that extractor reads, decided with a byte search over the raw frame, so generated text is dropped as it arrives. - The Realtime audio capability subscription was decided once, in the constructor. `LLMObs.enable()` and `disable()` flip `DD_LLMOBS_ENABLED` at runtime without reconfiguring plugins, so enabling later left Realtime events without audio. Re-evaluate it on configure and per turn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(llmobs): keep OpenAI Realtime off the reduced gen_ai path Realtime is the one integration where staying subscribed costs more than the tags are worth: the instrumentation retains a turn's audio only while these plugins are subscribed, and only the LLM Observability payload reads those bytes. Wiring it into the reduced path meant either buffering megabytes for no consumer or reaching into the plugin's subscription lifecycle, and the latter broke the enabled path's audio capture. Restore `realtime.js` to its original behavior and give `LLMObsPlugin` a `emitsGenAiApmTags` opt-out, which realtime sets to `false`: with LLM Observability off it is disabled outright, exactly as before these tags existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): anchor the Bedrock usage markers to JSON field names The byte search matched bare words, so a streamed response whose generated text mentioned usage or TokenCount kept those frames until the response completed. Quote the markers: a quote inside a JSON string arrives escaped, so prose cannot produce the byte sequence a field name does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): read Bedrock streamed usage through one shared table The reduced gen_ai path decided which streamed frames to retain with a byte search for four hand-written field names, then re-read them through the LLMObs stream extractor at `:complete:`. The list was maintained by hand against a switch in another file, so a renamed or unlisted field would have dropped every frame and emitted a span with no token counts and no error. Fold the counts into a running total as each frame arrives instead, the way the anthropic plugin and Bedrock's own Converse path already do. `mergeStreamedUsage` holds the per-provider field table and is shared with the stream extractor, so both paths read usage from one definition and nothing is retained past four numbers. That also fixes an overwrite in the extractor: Amazon reports its counts on a frame that carries text, and a later text-only frame reset them to undefined. `buildUsage` now returns undefined when a Converse response reports no counts, and `extractTokens` leaves an unreported count undefined rather than coercing it to zero, so the APM attributes omit what nothing measured. The LLMObs metrics keep reporting zero through `zeroFilled`. Also drops the dead `marked` bookkeeping in `setGenAiApmUsageMetrics`, which could not run unmarked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(llmobs): give the claude-agent-sdk gen_ai spec private channels The spec subscribed its plugin instances to the production prefixes, so its publishes shared diagnostic channels with the real tracer and with any other spec in the same process. Subclass both plugins with a test-only prefix, the way the ai and langgraph gen_ai specs already do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): complete the Bedrock usage sources on the reduced path Anthropic streams report the final output count on the closing `message_delta`, at the top level rather than under `message`, so reading only `message.usage` kept the provisional count from `message_start` whenever the trailing invocation metrics were absent. Read both locations. A non-streamed `invokeModel` whose response carried no correlatable token-count headers reported no usage at all, although AI21, Amazon and Meta bodies expose the counts and the enabled path already reads them. Fall back to the same extractor when the headers reported nothing, leaving the body unread otherwise. Both reported by codex on #10437. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): keep each operation on its start-time LLMObs mode `_llmobsEnabled` reads the live config flag, which `llmobs.enable()` and `llmobs.disable()` mutate while operations are in flight. An operation that started on the reduced path and finished after an enable asked the tagger to tag a span its start never registered, warning on every call, and skipped `getGenAiApmEndTags`, losing the model and token counts the response carries. The reverse order left the LLMObs parent store unrestored. Latch the mode on the operation context at first read and have every hook for that operation consult it, so start, end, asyncEnd and the streamed-chunk subscribers all take the branch the start took. `configure` keeps reading the live flag: subscription is a process-level decision, not a per-operation one. Reported by codex on #10437. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llmobs): merge both Bedrock token sources and keep measured zeros A non-streamed `invokeModel` can report some counts in its headers and the rest in its body, so gating the body read on the headers reporting nothing dropped the remainder. Read both and let `extractTokens` merge them field by field, as it already does per field. `resolveCount` also picked its source with `||`, which discarded a body-reported zero and let a header override the body it documents as authoritative. Select on presence instead, so a measured zero stays a measurement. The plugin-base toggle spec drove a start with LLMObs enabled, which enters the LLMObs storage; with no matching end the fake span leaked into every later test in the process and reparented the SDK spans. Start it without register options so no span is registered and the storage is untouched. Reported by codex on #10437. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
set_gen_ai_apm_tagswritesgen_ai.*attributes onto an APM span, it now also sets an internal_dd.llmobs.artificial_gen_ai_tagstag._dd.prefix convention for internal, non-user-searchable tags (see_dd.llmobs.*shadow tags inddtrace/llmobs/_constants.py).Follow-up to #20083 / #20375.
Testing plan
tests/llmobs/test_llmobs_gen_ai_apm_tags.pyto assert the new tag is set alongside the existinggen_ai.*tags.🤖 Generated with Claude Code