feat(otlp/export): offline OTLP trace/metric/log export#4994
feat(otlp/export): offline OTLP trace/metric/log export#4994khanayan123 wants to merge 3 commits into
Conversation
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: d26661f | Docs | Datadog PR Page | Give us feedback! |
![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: 8657c2d694
ℹ️ 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 resp.StatusCode >= 200 && resp.StatusCode <= 299 { | ||
| return resp.StatusCode, respBody, nil |
There was a problem hiding this comment.
When a collector or Datadog-compatible endpoint returns HTTP 200 with an OTLP partial_success response that has rejected_spans, rejected_data_points, or rejected_log_records set, this branch reports the request as successful and ExportResult.OK() stays true even though data was dropped. Decode the signal-specific response proto on 2xx and return a non-retriable per-request error when the rejected count is non-zero so offline exporters do not silently lose telemetry.
Useful? React with 👍 / 👎.
BenchmarksBenchmark execution time: 2026-07-09 17:13:08 Comparing candidate commit d26661f 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/otlp-export
branch
from
8657c2d to
14a594f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
14a594f to
195f2e6
Compare
|
@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". |
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
195f2e6 to
9111a00
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9111a00ab0
ℹ️ 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".
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
9111a00 to
bbd3993
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
bbd3993 to
41cdd9b
Compare
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41cdd9bb72
ℹ️ 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".
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
2 times, most recently
from
33dd489 to
8a0f97b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a0f97b5a6
ℹ️ 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".
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) |
There was a problem hiding this comment.
When a collector or intake sends 2xx headers but the response body read fails, for example from a reset connection or mismatched Content-Length, this discards the read error and the following success branch reports the export as delivered. Because the body is what carries OTLP partial-success rejections, an unreadable response can hide dropped records; return an Attempt error when ReadAll fails so the request is surfaced as failed/retryable instead of successful.
Useful? React with 👍 / 👎.
| if vn < 0 { | ||
| return "" | ||
| } | ||
| return string(v) |
There was a problem hiding this comment.
If an OTLP endpoint returns a google.rpc.Status protobuf with a large or non-UTF-8 message field, this bypasses exportutil.Snippet and stores the decoded string directly in RequestResult.ResponseSnippet. That field is documented as a bounded, UTF-8-safe excerpt, so callers that log it can unexpectedly emit up to the full 1 MiB response limit or invalid text; run the decoded message through the same snippet path before returning it.
Useful? React with 👍 / 👎.
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
8a0f97b to
99af138
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
khanayan123
force-pushed
the
ayan.khan/llmobs-explicit-span-ids
branch
from
2236774 to
46ab716
Compare
Add a public otlp/export package: offline clients that POST already-built
OTLP collector proto requests (go.opentelemetry.io/proto/otlp/collector/
{trace,metrics,logs}/v1) as raw protobuf to Datadog's agentless OTLP intake
(otlp.<site>/v1/<signal>) or a caller-provided collector/Agent endpoint.
This is the OTLP half of the "Raw LLM Observability Export" RFC, stacked on
the llmobs/export PR. It uses a raw-proto transport and coexists with the
OTel-SDK-based exporters in ddtrace/opentelemetry (does not replace them).
- TraceClient/MetricClient/LogClient with ExportTraces/ExportMetrics/
ExportLogs; each input request is atomic (one request -> one POST -> one
indexed RequestResult); the SDK never merges or splits requests.
- Datadog route (Site+APIKey -> dd-api-key) or collector/Agent route
(Endpoint, no auth). Metrics on the Datadog route add dd-otel-metric-config
so exponential histograms render as distributions.
- Bounded retry (default 3 attempts; 5xx/408/429/network transient, other 4xx
permanent, caller-cancelled context not retriable), per-request results.
- Caller trace IDs/flags/tracestate preserved as-is; callers own proto
construction, projection, temporality, dedup, and durable retry.
Reuse (per the codebase reuse audit): the HTTP client comes from the shared
internal.DefaultHTTPClient, and retry/backoff/classification, response-body
snippets and failure aggregation live in internal/exportutil (shared with
llmobs/export) rather than being hand-rolled here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ages Address the Codex review for the OTLP transport: - Restrict retries to the OTLP/HTTP retryable set (429, 502, 503, 504, and network errors). A 500/501/505 or 408 is now permanent instead of burning every attempt. exportutil.Retry gains a pluggable Retriable predicate so the llmobs path keeps its own (broader) classification. - Honor Retry-After on throttling responses: doPost parses the header (delta-seconds or HTTP-date, clamped to 60s) and Retry waits at least that long before the next attempt instead of its 100ms backoff. - Surface decoded google.rpc.Status messages in ResponseSnippet for failed requests (extracted with protowire, no new dependency) instead of raw protobuf control bytes. Adds internal unit tests for the classifier, Retry-After parsing, and status decoding, plus behavioral tests for non-retryable 500 and the decoded snippet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s snippets Address the Codex review plus an adversarial self-review of the diff: - doPost now surfaces an io.ReadAll failure on a 2xx response as a transport- class (retryable) error instead of silently reporting success. The 2xx body carries OTLP partial-success rejections, so an unreadable body could otherwise hide dropped records. - Decoded google.rpc.Status messages now go through exportutil.Snippet, so ResponseSnippet stays bounded and UTF-8-safe even for a large/non-UTF-8 server message. - internal/README: correct the exportutil description — only Snippet/Aggregate are shared by both clients; Retry/Retriable are used by otlp/export (llmobs retries via the internal LLM Obs transport). Tests: add internal/exportutil retry_test.go (RetryAfter override, ctx-cancel mid-wait, MaxAttempts, non-retriable stop, custom classifier); doPost tests for the 2xx read-error and Retry-After header threading; and metric + log partial-success rejection tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
khanayan123
force-pushed
the
ayan.khan/otlp-export
branch
from
99af138 to
d26661f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d26661f73e
ℹ️ 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".
| return &http.Client{ | ||
| Transport: &http.Transport{ |
There was a problem hiding this comment.
When the configured endpoint or an intermediate proxy returns a 301/302/303 redirect, Go's default http.Client follows it by replaying the request as a GET with the body dropped, so a final 2xx page can be reported as a successful export even though the OTLP protobuf was never posted. The default redirect policy also forwards custom headers such as dd-api-key, so redirects can leak credentials to the redirected host. Please install a CheckRedirect policy for the default client, such as returning http.ErrUseLastResponse so 3xx responses are surfaced as failed exports instead of silent data loss.
Useful? React with 👍 / 👎.
| if resp.StatusCode >= 200 && resp.StatusCode <= 299 { | ||
| if readErr != nil { | ||
| // The 2xx body carries OTLP partial-success rejections; a failed read | ||
| // could hide dropped records, so surface it as a transport-class | ||
| // (retryable, status 0) failure instead of reporting full success. | ||
| return exportutil.Attempt{Body: respBody, Err: fmt.Errorf("otlp/export: read response body (HTTP %d): %w", resp.StatusCode, readErr)} | ||
| } | ||
| return exportutil.Attempt{Status: resp.StatusCode, Body: respBody} |
There was a problem hiding this comment.
OTLP/HTTP success and partial-success responses are HTTP 200 OK with a protobuf Export<Service>Response body, but this accepts every 2xx as delivered and later treats an unparsable body as zero rejections. If a proxy or misconfigured endpoint returns 202, 204, 206, or a 200 HTML/login page, Export* can return OK even though no OTLP response was received and partial rejections could not be inspected; please only report success after a 200 response whose body decodes as the expected OTLP response message.
Useful? React with 👍 / 👎.
1 participant
What
Adds a public
otlp/exportpackage: offline clients that POST already-built OTLP collector-proto requests as raw protobuf to Datadog's agentless OTLP intake or a caller-provided collector/Agent endpoint.The OTLP half of the RFC "Raw LLM Observability Export, Multi-Destination Routing, and Caller-Assigned IDs in dd-trace-go."
Design
go.opentelemetry.io/proto/otlp/collector/{trace,metrics,logs}/v1Export*ServiceRequestvalues (no new deps —proto/otlp+protobufare already direct deps).*Export*ServiceRequest→ one POST → one indexedRequestResult. The SDK never merges or splits requests.Site+APIKey→https://otlp.<site>/v1/<signal>withdd-api-key) or collector/Agent (Endpoint, no auth). Metrics on the Datadog route adddd-otel-metric-config: {"histograms":{"mode":"distributions"}}(metrics-only, Datadog-route-only).Retry-After(delta-seconds or HTTP-date, clamped to 60s) on throttling responses instead of the default backoff.rejected_{spans,data_points,log_records} > 0is reported as a failed request (a data-loss signal, not silent success). A failed read of a 2xx body is likewise surfaced as a retryable failure rather than reported delivered.google.rpc.Statusmessage intoRequestResult.ResponseSnippet(bounded/UTF-8-safe viaexportutil.Snippet) instead of raw protobuf control bytes.ddtrace/opentelemetry— it does not replace them. Retry mechanics live in a sharedinternal/exportutil.Retrywith a pluggable classifier;llmobs/exportkeeps its own broader classification via the internal LLM Obs transport.Tests
go test -race ./otlp/export/... ./internal/exportutil/...— endpoint derivation (Datadog vs collector, trailing-slash trim, schemeless/non-HTTP rejection), header injection (dd-api-key;dd-otel-metric-configmetrics+Datadog-route-only, incl. the Endpoint+APIKey override case), protobuf round-trip, per-request result rows, OTLP retry classification (503/502 retry, 500 permanent) with exact attempt counts, partial-success rejection for all three signals, decoded-status snippet,doPost2xx read-error +Retry-Afterheader threading, and theRetryengine directly (RetryAfter override, ctx-cancel mid-wait, MaxAttempts, custom classifier).golangci-lint(incl. themodernizeanalyzer) is clean.Review status
Reviewed clean by Codex on the current HEAD (no findings).
Open question (RFC §4)
Package placement (
otlp/export) and whether to expose/generalize the existingddtrace/tracerOTLP transport are@DataDog/apm-godesign calls; this PR colocates a new generalized raw-proto transport under the package. Intake-contract confirm for the agentless OTLPExport*ServiceRequestwrappers is flagged for intake owners.🤖 Generated with Claude Code