feat(llmobs/export): offline LLM Obs span & evaluation export#4936
feat(llmobs/export): offline LLM Obs span & evaluation export#4936khanayan123 wants to merge 11 commits into
Conversation
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 3 jobs - 2 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 46ab716 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-09 17:11:35 Comparing candidate commit 46ab716 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 326 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
khanayan123
force-pushed
the
ayan.khan/llmobs-explicit-span-ids
branch
from
ae6ce8e to
a9279fb
Compare
khanayan123
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@codex review |
![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: 347bc287b0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80bc414ae0
ℹ️ 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".
| // Package exportutil holds small helpers shared by the offline export clients | ||
| // (llmobs/export and otlp/export): bounded response-body snippets and | ||
| // per-request failure aggregation. | ||
| package exportutil |
There was a problem hiding this comment.
The internal/AGENTS.md instructions require updating internal/README.md whenever a new internal package is introduced, including its name and functionality. This commit adds internal/exportutil, but internal/README.md still has no exportutil entry, so the new shared helper package is not documented for maintainers.
Useful? React with 👍 / 👎.
| DD: transport.DDAttributes{ | ||
| SpanID: e.SpanID, | ||
| TraceID: e.TraceID, | ||
| APMTraceID: e.APMTraceID, |
There was a problem hiding this comment.
When APMTraceID is not set, this still populates transport.DDAttributes with the zero value, and that shared type serializes apm_trace_id without omitempty. Offline spans that are not correlated to APM therefore send _dd.apm_trace_id: "" instead of omitting the optional field, which is distinguishable from absence and can make intake treat the correlation ID as malformed; use an export-specific _dd wire type or omit this field when empty.
Useful? React with 👍 / 👎.
| var agentURL *url.URL | ||
| if cfg.AgentURL != "" { | ||
| // Trim a trailing slash so the EVP proxy path is not doubled. | ||
| u, err := url.Parse(strings.TrimRight(cfg.AgentURL, "/")) |
There was a problem hiding this comment.
If a caller sets AgentURL to a common agent address like localhost:8126, url.Parse succeeds here, so New returns a client that later builds localhost:8126/evp_proxy/...; net/http then treats localhost as the scheme and every export fails with unsupported protocol scheme "localhost". Validate that the parsed Agent URL is an accepted absolute URL (or unix socket URL) and return the existing invalid-AgentURL error before constructing the client.
Useful? React with 👍 / 👎.
| Input string | ||
| Output string | ||
| // Metadata is free-form span metadata. | ||
| Metadata map[string]string |
There was a problem hiding this comment.
For offline spans reconstructed from the live LLM Obs API, numeric or nested metadata cannot be exported through this field because the live annotation path accepts arbitrary structured map[string]any metadata, while this new raw exporter narrows it to map[string]string. Those values must either be stringified or dropped before calling ExportSpans, so reconstructed spans no longer round-trip the metadata users can already record with live instrumentation.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c58770331
ℹ️ 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".
| MetricType: metricType, | ||
| TimestampMS: m.TimestampMS, | ||
| MLApp: mlApp, | ||
| Tags: m.Tags, |
There was a problem hiding this comment.
When ExportEvaluations is used without a caller-supplied ddtrace.version tag, or with a stale one, the metric is sent as-is because this path copies m.Tags directly. The live evaluation path explicitly removes any caller ddtrace.version tag and appends the current tracer version in internal/llmobs/llmobs.go (lines 956-962); evaluation payloads have no _dd.tracer_version field, so offline exports otherwise lose or misreport SDK-version attribution for these metrics.
Useful? React with 👍 / 👎.
|
|
||
| ### Export Utilities | ||
|
|
||
| Contains small helpers shared by the offline export clients ([llmobs/export](../llmobs/export/) and [otlp/export](../otlp/export/)): bounded, UTF-8-safe response-body snippets (`Snippet`), per-request failure aggregation (`Aggregate`), transient-vs-permanent retry classification (`Retriable`), and a bounded-retry engine over a POST closure (`Retry`). See [exportutil](./exportutil/). |
There was a problem hiding this comment.
This line documents Retriable and Retry as part of internal/exportutil and links to otlp/export, but this commit's internal/exportutil/exportutil.go only defines Snippet and Aggregate, and there is no otlp/export package in the tree. Anyone following the new internal package docs will look for helpers and a package that do not exist, so the README should either describe only the current API or land those helpers/package together.
Useful? React with 👍 / 👎.
| defaultSite = "datadoghq.com" | ||
| defaultSpanBatchSize = 50 | ||
| defaultEvalBatchSize = 1000 | ||
| defaultMaxSpanBytes = 5 << 20 // 5 MiB |
There was a problem hiding this comment.
With this default, encodeSpans only truncates input/output when the encoded chunk exceeds 5,242,880 bytes, but the LLM Obs sender uses sizeLimitEVPEvent = 5_000_000 and its size-based flushing test asserts HTTP payloads stay at or below that 5 MB limit. A default offline span chunk in the 5,000,001–5,242,880 byte range will therefore skip truncation and be posted over the Agent/EVP limit, causing large exports to be rejected instead of having IO dropped.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 776bb5d47d
ℹ️ 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".
| if len(body) <= c.maxSpanBytes { | ||
| return body, nil | ||
| } | ||
| for i := range p.Spans { |
There was a problem hiding this comment.
When the encoded body exceeds maxSpanBytes, this immediately drops input/output from every span in the batch. With the default 50-span batch size, a set of individually acceptable spans can exceed 5 MB only because they were grouped together; for example, 50 spans with ~120 KB inputs would all be sent with dropped_io even though smaller chunks would preserve the data. That permanently corrupts offline reconstructions, so oversized batches should be split before falling back to per-span I/O truncation.
Useful? React with 👍 / 👎.
| // Reuse the shared factory (proxy/pool/dialer). timeout=0 keeps the | ||
| // no-global-timeout behavior; per-request deadlines are set via context. | ||
| return internal.DefaultHTTPClient(0, false) |
There was a problem hiding this comment.
In the default agentless path this now returns internal.DefaultHTTPClient(0, false), whose transport installs a custom DialContext but leaves ForceAttemptHTTP2 unset. The net/http transport docs state that a custom dialer disables HTTP/2 unless ForceAttemptHTTP2 is true, so HTTPS LLM Obs intake traffic regresses from the previous HTTP/2-enabled transport to HTTP/1.1, which can hurt high-volume flushes. Preserve ForceAttemptHTTP2 when reusing the shared client.
Useful? React with 👍 / 👎.
| // MLApp is the default ml_app for evaluations when a metric omits it. | ||
| MLApp string |
There was a problem hiding this comment.
The live LLM Obs client rejects startup when MLApp is empty and stamps ml_app onto every span, but this offline config makes MLApp optional and only treats it as an evaluation default. A caller can therefore construct the documented Config{Site, APIKey, Service} and export spans without any ml_app: tag (or evaluations without ml_app), which produces payloads missing the ML app required for LLM Obs attribution. Make MLApp required here or use it to stamp spans and metrics by default.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a05f790a14
ℹ️ 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".
| if u.Scheme == "" || (u.Scheme != "unix" && u.Host == "") { | ||
| return nil, fmt.Errorf("llmobs/export: invalid AgentURL %q: must be an absolute URL (scheme://host) or a unix socket URL", cfg.AgentURL) | ||
| } |
There was a problem hiding this comment.
When AgentURL is a typo such as htt://localhost:8126 or another unsupported absolute URL like ftp://host, this check accepts it because Host is non-empty; every export then fails later in Post with unsupported protocol scheme after being retried as a transport error. The tracer's agent URL handling only supports http, https, and unix, so this constructor should reject other schemes up front.
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2826a98e49
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2826a98e49
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Add a public llmobs/export package: an offline client for exporting already-built LLM Observability spans and evaluations to Datadog intake (direct) or through the Agent EVP proxy. This is an export/transport API, not live instrumentation — it never starts the tracer, depends on the live LLM Obs lifecycle, or replays payloads through StartSpan/Finish. Implements the "Raw LLM Observability Export" part of the RFC. It lets offline reconstruction tools (e.g. Trajectory) depend on the SDK for the export mechanics — endpoint derivation, auth, HTTP, retry classification, size handling, and structured per-request results — while keeping reconstruction, projection, privacy, dedup and durable retry above the SDK. - Client.ExportSpans / ExportEvaluations over caller-built SpanEvent / EvaluationMetric payloads, lowered to the LLM Obs wire shape. - Caller-assigned trace/span/parent IDs preserved verbatim as opaque string payload fields; never routed into APM span IDs or sampling. - Chunking (50 spans, 1000 evals per request), best-effort 5MB size guard (truncates input/output), and per-request ExportResult with validation attribution. - Datadog route (DD-API-KEY) and Agent EVP route; reuses the internal LLM Obs transport for auth/routing/retry via a new transport.Post primitive. Multi-destination is modeled as one isolated Client per destination. A sibling otlp/export package is a stacked follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address reviewdog/golangci `modernize` findings (no behavior change): - chunk(): compute the slice end bound with min() (minmax) - appendUnique(): use slices.Contains (slicescontains) - export_test concurrency loop: range-over-int + sync.WaitGroup.Go (rangeint, waitgroup) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ExportSpans: wrap the span batch in a JSON array on the wire. The /api/v2/llmobs intake expects []PushSpanEventsRequest (see internal/llmobs/transport.PushSpanEvents); posting a bare object was the wrong top-level shape and would be rejected. (P1) - eval lower(): validate an explicit MetricType is one of categorical/score/boolean and matches the provided value kind, rejecting typos and mismatches as row-level ValidationErrors instead of letting the intake reject the whole chunk. (P2) - eval lower(): reject a non-nil but empty JSONValue, which json:omitempty would drop from the wire, producing a value-less metric. (P2) Tests updated for the array span shape and the new eval validations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reduce net-new duplication surfaced by a codebase reuse audit:
- Reuse internal/llmobs/transport.DDAttributes for the span _dd block and
transport.LLMObsMetric (+ PushMetricsRequest envelope, EvaluationSpanJoin/
EvaluationTagJoin) for evaluations, instead of parallel wire structs. Extends
the shared LLMObsMetric with the offline-only omitempty fields (json_value,
assessment, reasoning, metadata) so there is one eval-wire source of truth;
additive and safe for the live PushEvalMetrics path.
- Extract internal/exportutil {Snippet, Aggregate} and use it here (shared with
otlp/export in the stacked PR) to drop the byte-identical helpers.
- Point internal/llmobs/config.newHTTPClient at the shared internal.DefaultHTTPClient
(timeout=0 preserves the no-global-timeout behavior; per-request deadlines are
set via context), removing a 3rd copy of the HTTP client factory.
String-ID span links, the typed public SpanEvent, top-level service, the flat
meta."span.kind", chunking and the size guard stay custom (genuine shape
divergence from the live path).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Omit empty _dd.apm_trace_id: offline spans not correlated to APM must not send apm_trace_id:"" (distinct from absence for the optional field). Use an export-specific _dd wire type with omitempty instead of the shared transport.DDAttributes (which is non-omitempty to match the live path); the eval-metric reuse of transport.LLMObsMetric is unaffected. (P2) - Validate AgentURL is an absolute (or unix) URL in New, so a value like "localhost:8126" is rejected up front instead of failing every export at POST time with "unsupported protocol scheme". (P3) - Widen SpanEvent.Metadata to map[string]any so reconstructed spans can round-trip the structured metadata the live annotation path supports. (P2) - Document the new internal/exportutil package in internal/README.md as required by internal/AGENTS.md. (P2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Stamp ddtrace.version on exported evaluations: strip any caller-provided ddtrace.version tag and append the SDK's, matching the live eval path so intake can attribute the emitting SDK version. (P2) - Cap the default span-payload size at 5,000,000 bytes (the EVP event size limit, internal/llmobs sizeLimitEVPEvent) instead of 5 MiB, so the size guard cannot pass a chunk the intake would reject. (P2) - Trim the internal/README.md exportutil entry to the APIs that exist on this branch (Snippet, Aggregate) and reference only llmobs/export. (P3) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Split oversized span batches instead of dropping I/O from every span: when an encoded batch exceeds the size limit and has >1 span, bisect and recurse so individually-acceptable spans keep their input/output; only a lone span still over the limit has its I/O truncated. (P2) - Restore ForceAttemptHTTP2 on the LLM Obs HTTP client: reverts the reuse of internal.DefaultHTTPClient, whose custom dialer disabled HTTP/2 (the shared factory leaves ForceAttemptHTTP2 unset). (P3) - Stamp ml_app on exported spans from the client default when the caller did not supply an ml_app tag, matching the live client. (P2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Restrict AgentURL to http/https/unix schemes so a typo like "htt://host" or an unsupported scheme (ftp://) is rejected in New instead of failing every export at POST time. (P2) - Reject non-finite evaluation ScoreValue (NaN/Inf) as a row-level validation error; encoding/json cannot marshal it, so it would otherwise fail the whole chunk. (P2) - Stamp session_id as a span tag (from SpanEvent.SessionID) so evaluations can tag-join on session_id, matching the live LLM Obs path. (P2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reject non-encodable rows per-item instead of failing whole batches, and require ml_app + span kind up front: - ExportSpans now lowers, stamps, and trial-marshals each span; a span with a non-finite metric cost (or otherwise non-encodable value) is dropped as a ValidationError rather than poisoning the chunk's marshal. sendSpanBatch works on already-validated []wireSpan. - ExportSpans drops spans with an empty Kind (validateSpan). - ExportEvaluations trial-marshals each lowered metric so an unmarshalable JSONValue/Metadata is a row-level error, not a chunk failure. - New requires a non-empty MLApp, matching the live client and guaranteeing every exported span/evaluation carries an ml_app. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address two further Codex findings: - SpanMetrics dropped standard LLM Obs metric keys that had no field (ephemeral_1h/5m_input_tokens, billable_character_count, time_to_first_token) and could not carry custom keys at all. Add the missing standard fields plus an Extra map[string]float64 escape hatch with a merging MarshalJSON, so a reconstructed span never silently loses a metric. This mirrors the internal transport, which models span metrics as a flat key -> number map. - When a caller set SpanEvent.SessionID but also passed a stale session_id tag, the top-level session_id and the tag could disagree, so tag-joined evaluations attached to the wrong session. SessionID is now the source of truth and replaces any existing session_id tag (replaceTag), matching the live path which overwrites the session tag from the resolved session ID. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ics godoc From an adversarial self-review of the diff: - Add internal/exportutil unit tests for Snippet (pass-through, ASCII truncation, UTF-8 rune-boundary backoff) and Aggregate, which previously had no direct coverage. - Fix the SpanMetrics godoc: the flat wire type is internal/llmobs/transport.LLMObsSpanEvent.Metrics (there is no transport.Span), and clarify that only the token/count fields map to MetricKey* constants while the estimated_* fields carry cost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
khanayan123
force-pushed
the
ayan.khan/llmobs-explicit-span-ids
branch
from
2236774 to
46ab716
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
What
Adds a public
llmobs/exportpackage: an offline client for exporting already-built LLM Observability spans and evaluations to Datadog, either direct to intake or through the Agent EVP proxy.This is PR 1 of a stacked pair implementing the RFC "Raw LLM Observability Export, Multi-Destination Routing, and Caller-Assigned IDs in dd-trace-go." The sibling
otlp/exportpackage is the stacked follow-up (#4994).Why
Offline reconstruction tools (e.g. Trajectory) rebuild finished agent sessions and need to export the results without replaying them through live instrumentation. This moves the export mechanics into the SDK — endpoint derivation, auth, HTTP, retry classification, size handling, and structured per-request results — while reconstruction, projection, privacy filtering, deterministic IDs, dedup, and durable retry stay in the caller.
Design
StartSpan/Finish, never starts the tracer.trace_id/span_id/parent_id) are preserved verbatim as opaque string payload fields (decimaluint64strings are valid; hex not required) and are never routed into APM span IDs, APM trace IDs, or sampling. Empty parent →"undefined".{_dd.stage:"raw", event_type:"span", spans:[…]}; flatmeta."span.kind"; top-levelservice; string span-link IDs; optional_dd.apm_trace_id(omitted when unset). Eval join byspan_id+trace_idor tag; exactly one ofcategorical/score/boolean/json_value; optionalassessment/reasoning/metadata.ml_apprequired:Newrejects an emptyMLApp(matching the live client); every exported span/eval carries anml_app(client default, or per-span/metric override).span_id/trace_id/kind; each eval requires a valid join and exactly one finite value. Invalid rows are dropped and reported inExportResult.ValidationErrors— never failing a whole batch. Each row is trial-encoded, so a single non-JSON-encodable value (a non-finite metric cost, an unmarshalablejson_value/metadata) becomes a row-level error rather than a chunk failure.SpanMetricscarries the standard LLM Obs token/count/cost keys as named fields plus anExtra map[string]float64escape hatch, so a reconstructed span never silently loses a metric (mirrors the internal transport's flatmap[string]float64).session_id: the structuredSessionIDis the source of truth and overrides any stalesession_id:tag, matching the live path (so tag-joined evals attach to the right session).transport.Postprimitive on the internal LLM Obs transport provides auth/routing (agentless direct vs. Agent EVP) and retry (5xx/408/425/429 retriable, other 4xx permanent,Retry-Afterhonored on 429); the client owns chunking (50 spans, 1000 evals/req) and a best-effort 5 MB size guard (splits oversized batches; truncates input/output only as a last resort), and returns per-requestExportResultwith validation attribution.Tests
go test -race ./llmobs/export/... ./internal/exportutil/...— wire-shape/auth for both routes, chunking, size guard + batch-split, validation attribution (missing IDs/kind, non-finite metric, empty/unmarshalable json),ml_apprequirement + stamping,SpanMetricsExtra/named-field collision,session_idoverride, retry vs. permanent classification, context-cancellation, and a-racetest guarding caller-slice immutability; plusinternal/exportutilunit tests (Snippettruncation/rune-boundary,Aggregate). Internalinternal/llmobssuites still pass.golangci-lint(incl. themodernizeanalyzer) is clean.Review status
Reviewed clean by Codex on the current HEAD (no findings).
Open question (RFC §4)
Intake-contract re-confirm: the fields exercised here (top-level
service, string span-link IDs, opaquetrace_id, evalassessment/reasoning/metadata/json_value) are what Trajectory already sends in production — flagging for intake owners to confirm as a long-term public contract.🤖 Generated with Claude Code