fix(otel): Submit OTLP traces to correct endpoint when APM UDS socket is set#8668
Conversation
- Rename enum TracesTransportType to AgentTransportType (still used by ExporterSettings.TracesTransport) - Add new AgentTransport property (type=AgentTransportType) to the ExporterSettings to separate logical traces endpoint vs agent endpoint - Change some usage of ExporterSettings.TracesTransport to ExporterSettings.AgentTransport
…ing an IApiRequestFactory
…ces HTTP client FOR ALL TRANSPORTS
… as a regression test
- Do not set OTLP trace headers twice. - Reset the traces transport back to the Datadog Agent transport if the OTLP protocol is unsupported
…lpTraces as a regression test" This reverts commit 4243173.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8668) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (75ms) : 70, 80
master - mean (74ms) : 70, 79
section Bailout
This PR (8668) - mean (78ms) : 75, 80
master - mean (77ms) : 75, 79
section CallTarget+Inlining+NGEN
This PR (8668) - mean (1,110ms) : 1055, 1165
master - mean (1,115ms) : 1048, 1181
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (113ms) : 109, 118
master - mean (114ms) : 109, 120
section Bailout
This PR (8668) - mean (115ms) : 112, 117
master - mean (113ms) : 111, 116
section CallTarget+Inlining+NGEN
This PR (8668) - mean (794ms) : 770, 817
master - mean (794ms) : 769, 820
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (101ms) : 98, 105
master - mean (101ms) : 98, 104
section Bailout
This PR (8668) - mean (106ms) : 100, 112
master - mean (106ms) : 100, 111
section CallTarget+Inlining+NGEN
This PR (8668) - mean (950ms) : 910, 991
master - mean (952ms) : 912, 991
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (100ms) : 95, 105
master - mean (99ms) : 96, 102
section Bailout
This PR (8668) - mean (101ms) : 98, 105
master - mean (101ms) : 98, 103
section CallTarget+Inlining+NGEN
This PR (8668) - mean (830ms) : 782, 878
master - mean (826ms) : 791, 862
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (197ms) : 191, 204
master - mean (198ms) : 191, 206
section Bailout
This PR (8668) - mean (201ms) : 197, 206
master - mean (203ms) : 199, 208
section CallTarget+Inlining+NGEN
This PR (8668) - mean (1,191ms) : 1150, 1232
master - mean (1,199ms) : 1156, 1241
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (285ms) : 279, 291
master - mean (286ms) : 279, 293
section Bailout
This PR (8668) - mean (287ms) : 281, 293
master - mean (287ms) : 280, 294
section CallTarget+Inlining+NGEN
This PR (8668) - mean (961ms) : 944, 978
master - mean (961ms) : 934, 987
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (278ms) : 271, 285
master - mean (278ms) : 269, 286
section Bailout
This PR (8668) - mean (278ms) : 273, 283
master - mean (278ms) : 272, 285
section CallTarget+Inlining+NGEN
This PR (8668) - mean (1,157ms) : 1124, 1191
master - mean (1,165ms) : 1121, 1209
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8668) - mean (277ms) : 270, 284
master - mean (278ms) : 272, 283
section Bailout
This PR (8668) - mean (277ms) : 271, 283
master - mean (276ms) : 271, 282
section CallTarget+Inlining+NGEN
This PR (8668) - mean (1,038ms) : 1000, 1076
master - mean (1,043ms) : 1005, 1080
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2494b6e346
ℹ️ 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".
…ring relativePath) returns the base endpoint if the argument is null.
BenchmarksBenchmark execution time: 2026-06-02 22:09:25 Comparing candidate commit 6419f4e in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 8 performance improvements and 13 performance regressions! Performance is the same for 43 metrics, 8 unstable metrics, 89 known flaky benchmarks, 37 flaky benchmarks without significant changes.
|
link04
left a comment
There was a problem hiding this comment.
Just a couple of nits but looks good TY! I'll try looking into adding UDS support to the test Agent so we can rely on it.
- Reuse ExporterSettings.UnixDomainSocketPrefix - Remove timeout from log message because it is unused
Summary of changes
Fixes a bug where setting the APM UDS socket would inadvertently set the HttpClient used by OTLP traces export, which led to trace submission issues. This is accomplished by having the OTLP traces exporter use a new code path to configure the underlying HTTP client.
Reason for change
The primary motivation was to resolve the following OTLP traces export issue: When OTLP traces export is enabled through the configuration OTEL_TRACES_EXPORTER=otlp, the endpoint defaults to TCP/IP communication with endpoint http://localhost:4317 or http://localhost:4318. However, when DD_APM_RECEIVER_SOCKET is configured and its socket file is present, the underlying HTTP client for the OTLP traces exporter is erroneously being configured for UDS communication rather than TCP/IP.
Implementation details
OtlpTransportStrategythat is a factory for OTLP traces HTTP clients. This supports TCP/IP endpoints and UDS endpoints (though UDS is not currently tested)ManagedApiOtlpgets an HTTP client fromOtlpTransportStrategy.GetTracesrather thanTracesTransportStrategy.GetOtlpHeaderHelperinstance to set default headers for all OTLP traces HTTP requestsTest coverage
Adds unit tests in
ManagedApiOtlpTests. Note: UDS is supported but currently untested.Other details
Replaces #8614 and #8460