Repository navigation
[OTLP] Default OTLP protocols to http/protobuf - #9506
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[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new gRPC metrics default exposes two incompatible paths: span metrics still send HTTP protobuf, and regular metrics bypass configured request timeouts. These can cause lost span metrics and prolonged exports when a collector stalls.
🤖 Bits Code Review · Commit f2d560a
vpellan
changed the title
![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: f2d560a1f2
ℹ️ 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".
vpellan
added
AI Generated
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9506) and master. ✅ No regressions detected |
BenchmarksBenchmark execution time: 2026-10-30 16:37:13 Comparing candidate commit 9d49b53 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 71 metrics, 1 unstable metrics, 7 known flaky benchmarks, 119 flaky benchmarks without significant changes.
|
vpellan
force-pushed
the
vpellan/otlp-default-protocol
branch
from
October 28, 2026 10:56
ffea4f3 to
e37b182
Compare
anna-git
approved these changes
Oct 29, 2026
anna-git
left a comment
anna-git
left a comment
Contributor
There was a problem hiding this comment.
LGTM, approving so this isn't blocked. Please merge only once it's rebased on the latest #9505, CI is green, and the system-tests PR is linked.
For system-tests: the logs failures come from default_protocol() in tests/parametric/otel_env_vars/utils.py, which hardcodes dotnet + logs → grpc. So it needs a code change there (version-gated, since older .NET releases keep gRPC), not only a manifest update. The traces default tests also XPASS now (APMAPI-2531). Could you link the system-tests PR here before merging?
Base automatically changed from
milan.garnier/otlp-unify-endpoint-protocol
to
master
October 30, 2026 12:23
There was a problem hiding this comment.
Configuring only the general OTLP protocol omits the inherited signal-protocol telemetry entries, causing the corresponding new telemetry assertions to fail.
🤖 Bits Code Review · Commit e37b182
Contributor
Author
|
@anna-git system-tests should be fixed once DataDog/system-tests#8256 is merged |
Default OTEL_EXPORTER_OTLP_PROTOCOL and its traces, metrics and logs variants to http/protobuf, through a single DefaultOtlpProtocol constant instead of a default per signal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ral endpoint Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vpellan
force-pushed
the
vpellan/otlp-default-protocol
branch
from
October 30, 2026 15:39
e37b182 to
9d49b53
Compare
bouwkast
pushed a commit
that referenced
this pull request
Nov 3, 2026
…fault (#9604) ## Summary of changes Updates `ExporterSettingsTests.TracesEncoding_DefaultsToOtlpWhenOtelSemanticsEnabled` to expect `TracesEncoding.OtlpProtobuf` instead of `TracesEncoding.OtlpJson` when traces are exported over OTLP. ## Reason for change The test fails on master. #9544 added it while the default OTLP traces protocol was still `http/json`. #9506, which was merged right after, changed that default to `http/protobuf`. The two PRs had no textual conflict, but together they break the test's expectations. ## Implementation details Test-only change: three `InlineData` rows now expect `OtlpProtobuf`. The product behavior is correct and matches `ManagedApiOtlpTests.ExporterSettings_WithoutValidTracesProtocol_DefaultsToHttpProtobuf`, added in #9506. ## Test coverage Ran `ExporterSettingsTests` and `ManagedApiOtlpTests` locally on net11.0. All pass. ## Other details Related: #9506, #9544 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
Default
OTEL_EXPORTER_OTLP_PROTOCOLand its traces, metrics and logs variants tohttp/protobuf, on every .NET version.Reason for change
Defaults currently differ per signal: traces
http/json, metricshttp/protobuf, logsgrpc.http/protobufis the default recommended by the OTLP exporter specification, and is supported by every signal on every .NET version.Implementation details
DefaultOtlpProtocolconstant replaces the per-signal defaults passed toGetOtlpTransportand the separate general default recorded in telemetry.supported-configurations.yaml: protocol defaults updated, andOTEL_EXPORTER_OTLP_ENDPOINTnow defaults tonull(like the signal endpoints in fix(otel): unify OTLP endpoint and protocol resolution #9505), since it depends on the protocol.OTEL_EXPORTER_OTLP_TRACES_PROTOCOLneeds a new registry implementation (C,http/protobuf).http/protobufinstead ofgrpc, and traces tohttp/protobufinstead ofhttp/json. An app that only setsOTEL_EXPORTER_OTLP_ENDPOINT=http://collector:4317now sends logs over HTTP tohttp://collector:4317/v1/logs, which fails against a receiver that only accepts gRPC. SetOTEL_EXPORTER_OTLP_LOGS_PROTOCOL=grpcto keep sending logs over gRPC, orOTEL_EXPORTER_OTLP_TRACES_PROTOCOL=http/jsonto keep the JSON traces encoding.Test coverage
ExporterSettingsTestsandManagedApiOtlpTestscheck the defaults, including logs sent over HTTP when onlyOTEL_EXPORTER_OTLP_ENDPOINTis set.Other details
System-tests manifest updates to follow.
🤖 Generated with Claude Code