feat(tracer): add OTLP metrics HTTP exporter for span metrics#4899
feat(tracer): add OTLP metrics HTTP exporter for span metrics#4899rachelyangdog wants to merge 14 commits into
Conversation
|
![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: 3bab5e0551
ℹ️ 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".
3bab5e0 to
60f00dc
Compare
BenchmarksBenchmark execution time: 2026-07-24 16:06:18 Comparing candidate commit 9c10faa 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.
|
rachelyangdog
changed the title
73fa14b to
8b7e127
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b7e127f92
ℹ️ 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".
0591498 to
b6998bf
Compare
b6998bf to
053dd7f
Compare
d29a1f7 to
5825b24
Compare
053dd7f to
98e8123
Compare
5825b24 to
5ee761f
Compare
98e8123 to
167f231
Compare
5ee761f to
d09448e
Compare
167f231 to
6050a97
Compare
1173824 to
d1699f0
Compare
05cb3d2 to
4a7fb1d
Compare
9c0fc21 to
1eca7a0
Compare
5124a8e to
842ce96
Compare
1eca7a0 to
faba486
Compare
842ce96 to
20afb56
Compare
mtoffl01
left a comment
mtoffl01
left a comment
There was a problem hiding this comment.
Mostly nits, but overall I'd say - lets try to introduce as little new code as possible. Some of this logic already exists, or already exists in some other form, and can be modified in order to be shared across the repo rather than recreated
| cfg *internalconfig.Config | ||
| } | ||
|
|
||
| func newOTLPMetricsExporter(cfg *internalconfig.Config) *otlpMetricsExporter { |
There was a problem hiding this comment.
Does newOTLPMetricsExporter need all of cfg? Can't it accept the parameters it needs explicitly/only?
There was a problem hiding this comment.
cfg is used across both construction and the export() call path. there are 7 fields in total: AgentTimeout, OTLPMetricsURL, OTLPMetricsHeaders, OTLPMetricsProtocol at construction, plus OTelSemanticsEnabled, ReportHostname, Hostname inside buildOTLPMetricsRequest. Passing those explicitly would produce a 7-parameter constructor, which is harder to read and maintain than the single cfg argument. Storing cfg on the struct also avoids threading those values through separately on every export() call.
This matches how newOTLPTraceWriter is structured. it also takes the full config rather than individual fields.
| body, err = marshalExportRequestJSON(rms) | ||
| contentType = otlpMetricsContentTypeJSON | ||
| } else { | ||
| body, err = marshalExportRequestProto(rms) |
There was a problem hiding this comment.
We're implicitly defaulting to protobuf unless "http/json" was specified. Does internalconfig already inform the user that anything other than json or protobuf will be dropped/default to protobuf?
There was a problem hiding this comment.
Yes, internalconfig already handles this. validateOTLPProtocol is called at config load time for both OTEL_EXPORTER_OTLP_METRICS_PROTOCOL and (now) OTEL_EXPORTER_OTLP_PROTOCOL, and logs a warning naming the specific env var when an unsupported value is set, then falls back to the default. So by the time OTLPMetricsProtocol() is read in the exporter, it's guaranteed to be either "http/json" or "http/protobuf".
Also worth noting: the default was changed to "http/json" in this PR to align with the spec's transport section ("HTTP/JSON as the fallback protocol"), so the implicit behavior now favors JSON rather than protobuf.
There was a problem hiding this comment.
Right. it feels like this block is duplicating some of that.
| // gs.Name (the Datadog operation name, e.g. "web.request") is intentionally | ||
| // omitted in OTel mode — it has no OTel semconv equivalent and is only | ||
| // emitted as datadog.operation.name in default mode below. | ||
| if gs.Resource != "" { |
There was a problem hiding this comment.
This looks similar to the logic in span_to_otlp.go. is there any logic we can deduplicate - define once and share?
There was a problem hiding this comment.
buildDataPointAttributes and convertSpanAttributes (or the span attribute logic in span_to_otlp.go) operate on different input types: buildDataPointAttributes takes a *pb.ClientGroupedStats (pre-aggregated stats from the concentrator, with typed fields like HTTPMethod, HTTPStatusCode, GRPCStatusCode), while the span conversion takes a *Span with a raw meta/metrics map. There's no shared field access or control flow that would survive extraction into a shared helper.
The deduplication that was possible like the resource attribute construction (service.name, telemetry.sdk.*, deployment.environment.name, service.version) was already extracted into buildBaseResourceAttrs in this PR and is shared between buildMetricsResource and buildResource in span_to_otlp.go.
Adds otlpMetricsExporter which converts a ClientStatsPayload to an ExportMetricsServiceRequest, marshals it as HTTP/JSON (protojson) or HTTP/protobuf based on OTEL_EXPORTER_OTLP_METRICS_PROTOCOL, and POSTs it to the configured OTLP metrics endpoint. The protocol config field defaults to http/protobuf per the OTel spec; system tests set http/json explicitly via OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=http/json. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cs/v1 Importing go.opentelemetry.io/proto/otlp/collector/metrics/v1 in the test binary triggered the grpc-gateway -> genproto ambiguous-import error when confluent-kafka-go (which requires the old monolithic genproto) was in the same build graph. Replace the collector import with a protowire decode of ExportMetricsServiceRequest field 1 directly into the already-imported ResourceMetrics type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Request Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
otlpMetricsExporter now holds a *otlpTransport instead of duplicating the HTTP client/send logic. otlpTransport.send accepts a contentType parameter so the metrics exporter can use it for both JSON and protobuf. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… helpers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…thValidator + OTLP_PROTOCOL fallback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7e1dd9d to
5df63a6
Compare
…and resolveOTLPTraceURL Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ng literal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rce attribute logic Both buildResource (trace exports) and buildMetricsResource (metrics exports) built the same telemetry.sdk.* + service.* baseline attributes. Extract a shared buildBaseResourceAttrs helper in span_to_otlp.go and have both callers use it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nsport.go Move otlpContentTypeJSON and otlpContentTypeProto to otlp_transport.go (where otlpTransport.send takes the content type) and remove the duplicate definitions from otlp_metrics_exporter.go. Update all callers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Config Audit |
14124c6 to
bbdfee2
Compare
- Use internal.DefaultHTTPClient instead of bare &http.Client{} in
newOTLPMetricsExporter for connection pooling and proxy support
- Validate OTEL_EXPORTER_OTLP_PROTOCOL fallback through validateOTLPProtocol;
pass env var name to warning message so the right variable is named
- Change default OTLP metrics protocol to http/json per spec transport section
- Remove rpc.method attribute (removed from RFC spec June 25)
- Rename _DD_TRACE_METRICS_OTEL_FLUSH_INTERVAL to _DD_TRACE_STATS_INTERVAL
to match RFC-specified name used across all 5 SDK implementations
- Document intentional no-retry behavior on metrics export
- Document datadog.origin fidelity gap (proto only has Synthetics bool)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aligns the OTLPMetricsProtocol fallback with dd-trace-java's hardcoded production default and the RFC's revised FR10 (protobuf now satisfies the transport requirement; JSON support is no longer mandatory). The env vars (OTEL_EXPORTER_OTLP_METRICS_PROTOCOL, OTEL_EXPORTER_OTLP_PROTOCOL) remain fully configurable to either value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-metrics-transport
| body, err = marshalExportRequestJSON(rms) | ||
| contentType = otlpMetricsContentTypeJSON | ||
| } else { | ||
| body, err = marshalExportRequestProto(rms) |
There was a problem hiding this comment.
Right. it feels like this block is duplicating some of that.
2 participants
OTLP Span Metrics — Part 3 of 4: HTTP Exporter
Adds the HTTP transport that sends OTLP metrics payloads to the configured endpoint.
Stack order (merge in sequence):
otlp-span-metrics-config— configuration (PR feat(config): add OTLP span metrics config #4895)otlp-span-metrics-conversion— ClientStatsPayload → OTLP protobuf conversion (PR feat(tracer): stats to OTLP histogram conversion for OTLP span metrics #4897)otlp-span-metrics-wireup— tracer integration and system-test fixesWhat this PR does
Adds
ddtrace/tracer/otlp_metrics_exporter.go, a thin HTTP client that:*otlpMetricsExporterconfigured frominternal/config(endpoint URL, headers, protocol)application/x-protobufandapplication/jsonprotocols, controlled byOTEL_EXPORTER_OTLP_PROTOCOLBuildOTLPMetricsRequest(from PR 2) to convert the payload, then POSTs itOTLPMetricsURL,OTLPMetricsHeaders, andOTLPMetricsProtocolTesting
otlp_metrics_exporter_test.gowith a local HTTP server verifying correct content-type, headers, and payload encoding for both protocolsMotivation
Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Unsure? Have a question? Request a review!