Repository navigation
fix(otel): unify OTLP endpoint and protocol resolution - #9505
Merged
Merged
Conversation
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. |
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
General OTLP endpoints containing a query string or fragment now send HTTP logs to the wrong path, breaking delivery to collectors requiring /v1/logs. A focused URI reproduction confirmed the regression and the suggested path-based replacement.
🤖 Bits Code Review · Commit e518313
![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: e518313e2c
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-10-30 10:12:31 Comparing candidate commit 547dade in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 8 known flaky benchmarks, 117 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9505) and master. ✅ No regressions detected |
vpellan
approved these changes
Oct 27, 2026
vpellan
reviewed
Oct 27, 2026
MilanGarnier
force-pushed
the
milan.garnier/otlp-unify-endpoint-protocol
branch
from
October 28, 2026 09:18
9fc6376 to
33e8851
Compare
Co-authored-by: Victor Pellan <37553749+vpellan@users.noreply.github.com>
anna-git
reviewed
Oct 29, 2026
anna-git
left a comment
anna-git
left a comment
Contributor
There was a problem hiding this comment.
Thanks for uniformizing! I left a few comments
…with it
Address review feedback:
- Resolve the general endpoint with a single TryGetOtlpUri call
- Like the OpenTelemetry .NET SDK, skip appending v1/{signal} when the endpoint already ends with it
- Name the signal in the default transport warning
- Add an explicit http/json arm to GetOtlpProtocolName
- Check that an inherited signal protocol is not recorded under the signal key
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read the signal protocol keys without their OTEL_EXPORTER_OTLP_PROTOCOL alias. GetOtlpTransport already falls back to the general protocol, and now records the inherited value under the signal key as calculated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anna-git
approved these changes
Oct 30, 2026
anna-git
left a comment
anna-git
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing all comments!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Traces, metrics and logs now resolve their OTLP endpoint and protocol through one code path,
ExporterSettings.GetOtlpTransport, which also records the resolved values in configuration telemetry. Logs used to be resolved separately inTracerSettings, with their own rules.Every signal now behaves the same way:
OTEL_EXPORTER_OTLP_{SIGNAL}_ENDPOINT) if valid. OtherwiseOTEL_EXPORTER_OTLP_ENDPOINT, used as-is for gRPC and withv1/<signal>appended for HTTP, keeping any base path as the OTLP/HTTP spec requires. Like the OpenTelemetry .NET SDK,v1/<signal>is not appended again if the endpoint already ends with it. Otherwisehttp://<DD_AGENT_HOST>:4317(gRPC) orhttp://<DD_AGENT_HOST>:4318/v1/<signal>(HTTP), based on the signal's protocol, withlocalhostas the default host. An empty value counts as unset, and an invalid one falls back to the next option with a validation warning.OTEL_EXPORTER_OTLP_PROTOCOL, otherwise the signal default (traceshttp/json, metricshttp/protobuf, logsgrpc). Values are matched case-insensitively, an empty value counts as unset, and an unsupported value falls back to the signal default with a validation warning. The logs exporter only serializes protobuf, sohttp/jsonbecomeshttp/protobufwith a warning.OTEL_EXPORTER_OTLP_{SIGNAL}_ENDPOINTand_PROTOCOLkey reports the value its exporter actually uses. When that value is not the one set on the key, its origin iscalculatedif derived from the general settings (or, for logs, ifhttp/jsonwas replaced) anddefaultotherwise.Reason for change
Bugs fixed by the shared code path:
OTEL_EXPORTER_OTLP_ENDPOINT:http://collector:4318/mycollectorsent logs tohttp://collector:4318/v1/logs(APMAPI-2620).http/protobufand no endpoint were sent to port4317unlessOTEL_EXPORTER_OTLP_PROTOCOLwas alsohttp/protobuf, because the port came from the general protocol.OTEL_EXPORTER_OTLP_PROTOCOL.http/jsonprotocol made logs fall back togrpc.v1/<signal>again to a general endpoint that already ended with it (http://collector:4318/v1/tracesbecamehttp://collector:4318/v1/traces/v1/traces).http://collector:4318/mycollectorv1/metrics), no endpoint for traces unless set explicitly, and protocols computed inTracerSettingsthat could differ from the ones the exporters used. The metrics protocol default was reported asgrpcinstead ofhttp/protobuf.http/protobuffor metrics configured withhttp/json, although the metrics exporter sends JSON.Implementation details
ExporterSettingsgetsGetOtlpLogsTransport, a sibling of the traces and metrics ones, and theOtlpLogsEndpointandOtlpLogsProtocolsettings. The logsOtlpExporternow takesExporterSettings, like the metrics exporter.GetOtlpTransportandGetOtlpUrirecord the resolved protocol and endpoint, skipping a value set explicitly on the signal key sinceConfigurationBuilderalready recorded it.RecordOtlpGeneralDefaultsrecords the general protocol and endpoint defaults.OTEL_EXPORTER_OTLP_PROTOCOLalias, through a newConfigurationBuilder.WithKeys(key, includeAliases: false)overload, becauseGetOtlpTransportapplies that fallback itself. An inherited protocol is then recorded under the signal key ascalculated, and a signal protocol set to the same value as the general one keeps its own origin. The aliases stay insupported-configurations.yaml, since the fallback still applies.TracerSettings: the OTLP endpoint and protocol settings are removed, along with the telemetry they recorded.TracerManager: the startup log no longer rewriteshttp/jsontohttp/protobufand logs the protocol resolved byExporterSettings.supported-configurations.yaml:OTEL_EXPORTER_OTLP_METRICS_PROTOCOLnow uses registry implementation B (defaulthttp/protobuf).grpc), like traces and metrics, instead of toOTEL_EXPORTER_OTLP_PROTOCOL. For example, withOTEL_EXPORTER_OTLP_LOGS_PROTOCOL=invalidandOTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf, logs are now sent over gRPC (port4317by default) instead of HTTP.Test coverage
ExporterSettingsTestscovers:http/jsonand a signal endpoint; for traces and metrics, the default and a base path; for every signal, a general endpoint that already ends withv1/<signal>The
TracerSettingsTestsfor the removed settings are deleted.DataDog/system-tests#8112 added base-path tests for logs, metrics and traces. The .NET logs test is marked
bug (APMAPI-2620)in the system-tests manifest until this change ships.Other details
unix://general endpoint is used as-is, without appendingv1/logs. The logs exporter does not support Unix sockets, before or after this PR.OTEL_*values are treated as unset at the configuration source level.GetOtlpTransportstill treats empty endpoints and protocols as unset as well, for sources that don't apply that check (e.g.DictionaryObjectConfigurationSource).