Skip to content

feat(llmobs): agent attribution on spans (Go SDK)#5034

Open
yahya-mouman wants to merge 6 commits into
mainfrom
yahya/llmobs-agent-attribution
Open

feat(llmobs): agent attribution on spans (Go SDK)#5034
yahya-mouman wants to merge 6 commits into
mainfrom
yahya/llmobs-agent-attribution

Conversation

@yahya-mouman

Copy link
Copy Markdown

Description

Ports LLMObs agent attribution to the Go tracer, on par with dd-trace-py#18788 and dd-trace-js#9175.

Every LLMObs span that has an agent ancestor now carries:

"meta": {
  "agent_attribution": {
    "pagent_name":    "<nearest agent ancestor name>",
    "pagent_span_id": "<that agent span's id>"
  }
}

Spans with no agent ancestor omit the block entirely.

What's implemented

  • Resolve + store (internal/llmobs/llmobs.go): nearest agent resolved once at StartSpan via a one-level lookup that inherits the parent's already-resolved attribution (O(1), no ancestor walk). An agent never attributes itself; a sub-agent attributes to its enclosing agent.
  • Serialize (internal/llmobs/llmobs.go): surfaced as meta.agent_attribution at span finish, gated on presence. pagent_name is JSON null (key present, not absent) when id-only — matching the Python/Node wire shape.
  • Wire safety (internal/llmobs/util.go): agentNameWireSafe — byte-iterating, rejects names > 256 bytes or containing bytes outside [0x20, 0x7E] or comma. = is legal in tagset values and is not rejected.
  • Distributed propagation (internal/llmobs/propagation.go, llmobs/llmobs.go): new llmobs.InjectContext / llmobs.ExtractContext API. Inject writes _dd.p.llmobs_pagent_span_id (always) and _dd.p.llmobs_pagent_name (only when wire-safe and within 512-byte x-datadog-tags budget) into the carrier. Extract reads them back into PropagatedLLMSpan without overwriting existing ML app / trace id / span id fields. Users call both alongside the APM tracer's inject/extract.

Ownership boundary

All changes stay in internal/llmobs/ and llmobs/. No changes to ddtrace/tracer/, the APMSpan interface, or any non-LLMObs package.

Testing

  • In-process: tool under agent, indirect nesting (agent → workflow → tool), sub-agent under agent (nearest wins), top-level agent/LLM omit block, no-agent-in-chain omits block
  • Distributed: inject from agent, inject from tool (inherited), no agent → nothing written, unsafe name (comma) → id-only, name with = propagates, oversized name → id-only, full round-trip with name, full round-trip id-only → null name, extract preserves existing PropagatedLLMSpan fields
  • Full suite green: go test ./internal/llmobs/... ./llmobs/..., go vet clean

Follow-up (minor, non-blocking)

  • Add nil-map guard or doc note to InjectContext for nil carrier
  • Update CONTRIBUTING.md for the new public API
  • Add test for 512-byte budget ceiling with pre-existing x-datadog-tags

🤖 Generated with Claude Code

yahya-mouman and others added 5 commits July 16, 2026 12:58
Adds parentAgentName/parentAgentSpanID fields to Span, ParentAgentName/
ParentAgentSpanID fields to PropagatedLLMSpan, and wires resolveParentAgent
into StartSpan so every span records its nearest agent ancestor in O(1).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: yahya-mouman <yahya.mouman@datadoghq.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: yahya-mouman <yahya.mouman@datadoghq.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: yahya-mouman <yahya.mouman@datadoghq.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: yahya-mouman <yahya.mouman@datadoghq.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: yahya-mouman <yahya.mouman@datadoghq.com>
@yahya-mouman
yahya-mouman requested review from a team as code owners July 16, 2026 11:26
@yahya-mouman
yahya-mouman requested review from rarguelloF and removed request for a team July 16, 2026 11:26

@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: 56f640c165

ℹ️ 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".

Comment thread internal/llmobs/propagation.go Outdated
Comment thread internal/llmobs/propagation.go Outdated
Comment thread llmobs/llmobs.go Outdated
Comment thread internal/llmobs/propagation.go Outdated
Comment thread internal/llmobs/propagation.go Outdated
Comment thread internal/llmobs/propagation.go Outdated
@pr-commenter

pr-commenter Bot commented Jul 16, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-20 15:03:37

Comparing candidate commit 6ba23ee in PR branch yahya/llmobs-agent-attribution with baseline commit 104928a in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 325 metrics, 1 unstable metrics, 1 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

Known flaky benchmarks without significant changes:

  • scenario:BenchmarkOTLPTraceWriterFlush

@@ -0,0 +1,94 @@
// Unless explicitly stated otherwise all files in this repository are licensed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no need to create separate test files for these new tests, they can go into the existing ones based on where the tested functionality file name exists.

@yahya-mouman yahya-mouman Jul 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, consolidated both test files into one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unresolving as this test file still exists.

Comment thread internal/llmobs/resolve_parent_agent_test.go Outdated
Comment thread internal/llmobs/util.go Outdated
Comment thread internal/llmobs/propagation.go Outdated
Defer distributed agent attribution propagation to a follow-up PR.
The in-process attribution (resolveParentAgent + meta.agent_attribution
serialization) is self-contained and ships without the distributed API.

- Delete propagation.go and its tests (InjectContext, ExtractContext,
  injectAgentAttribution, extractAgentAttribution, agentNameWireSafe)
- Remove InjectContext/ExtractContext from the public llmobs package
- Merge resolve_parent_agent_test.go into start_span_agent_test.go

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@yahya-mouman
yahya-mouman requested a review from rarguelloF July 20, 2026 15:32

@rarguelloF rarguelloF left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the newest changes there's no cross-process propagation of these new fields. Please check how things are currently being automatically propagated, it should be a simple addition in those places.

@@ -0,0 +1,94 @@
// Unless explicitly stated otherwise all files in this repository are licensed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unresolving as this test file still exists.

Comment on lines +17 to +27
type testAPMSpan struct {
spanID string
traceID string
}

func (s testAPMSpan) Finish(_ FinishAPMSpanConfig) {}
func (s testAPMSpan) AddLink(_ SpanLink) {}
func (s testAPMSpan) SpanID() string { return s.spanID }
func (s testAPMSpan) TraceID() string { return s.traceID }
func (s testAPMSpan) SetBaggageItem(_ string, _ string) {}
func (s testAPMSpan) BaggageItem(_ string) string { return "" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a need to test using mock spans. Prefer to not use mocks unless its absolutely necessary.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants