Add Multi-Azure Function Unique Named Pipe Configuration#8164
Conversation
37f021e to
1b6956c
Compare
BenchmarksBenchmark execution time: 2026-04-30 21:56:00 Comparing candidate commit c1dc086 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 60 known flaky benchmarks, 27 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8164) 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 (8164) - mean (73ms) : 70, 75
master - mean (73ms) : 69, 77
section Bailout
This PR (8164) - mean (77ms) : 75, 79
master - mean (78ms) : 75, 81
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,130ms) : 1085, 1174
master - mean (1,131ms) : 1082, 1180
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 (8164) - mean (114ms) : 110, 119
master - mean (115ms) : 110, 120
section Bailout
This PR (8164) - mean (120ms) : 114, 125
master - mean (120ms) : 114, 125
section CallTarget+Inlining+NGEN
This PR (8164) - mean (813ms) : 781, 844
master - mean (808ms) : 780, 837
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (101ms) : 97, 105
master - mean (105ms) : 100, 110
section Bailout
This PR (8164) - mean (106ms) : 99, 112
master - mean (106ms) : 101, 112
section CallTarget+Inlining+NGEN
This PR (8164) - mean (950ms) : 909, 990
master - mean (946ms) : 913, 979
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (101ms) : 98, 104
master - mean (101ms) : 97, 104
section Bailout
This PR (8164) - mean (104ms) : 99, 109
master - mean (103ms) : 98, 109
section CallTarget+Inlining+NGEN
This PR (8164) - mean (837ms) : 788, 886
master - mean (835ms) : 779, 891
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 (8164) - mean (203ms) : 193, 213
master - mean (203ms) : 195, 212
section Bailout
This PR (8164) - mean (207ms) : 198, 217
master - mean (208ms) : 200, 216
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,251ms) : 1201, 1301
master - mean (1,244ms) : 1197, 1292
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 (8164) - mean (294ms) : 281, 308
master - mean (295ms) : 282, 307
section Bailout
This PR (8164) - mean (295ms) : 282, 309
master - mean (293ms) : 285, 301
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,004ms) : 974, 1034
master - mean (998ms) : 973, 1024
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (288ms) : 275, 302
master - mean (285ms) : 275, 296
section Bailout
This PR (8164) - mean (288ms) : 276, 299
master - mean (286ms) : 274, 298
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,178ms) : 1126, 1230
master - mean (1,167ms) : 1122, 1211
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (286ms) : 273, 299
master - mean (284ms) : 273, 295
section Bailout
This PR (8164) - mean (287ms) : 275, 299
master - mean (288ms) : 275, 301
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,058ms) : 1001, 1115
master - mean (1,060ms) : 987, 1133
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
aaff9b5 to
08a6c72
Compare
2fcae54 to
edb4121
Compare
duncanpharvey
left a comment
There was a problem hiding this comment.
We check the presence of the compat layer + windows configs (using undocumented envvar DD_SERVERLESS_COMPAT_BINARY_PATH if necessary to adapt to Azure changes) to see if named pipes can be used or if we should rely on TCP
What is meant by this? I don't see DD_SERVERLESS_COMPAT_BINARY_PATH in any of the PR changes.
|
Should this dependency for |
|
@codex review |
| && !Util.EnvironmentHelpers.IsUsingAzureAppServicesSiteExtension() | ||
| && IsCompatLayerAvailableWithPipeSupport()) | ||
| { | ||
| var baseName = Util.EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.TracesPipeName); |
There was a problem hiding this comment.
This is only reading the "base" name from environment variables, but the tracer can be configured in other ways (e.g. config files), and those configs would be ignored there. Instead of reading from env vars directly, I think you can read the user setting from rawSettings.TracesPipeName, passed in via parameter.
There was a problem hiding this comment.
Agreed, but we also need to make sure we record the new "calculated" value in telemetry as well
There was a problem hiding this comment.
Looks like my comment is addressed (no reading env vars directly), but I'll leave this open for Andrew's comment (configuration telemetry).
andrewlock
left a comment
There was a problem hiding this comment.
LGTM in general, but there's some specific things to address before we merge it (mostly already highlighted by Lucas)
- We should only target existing major versions, and bump support later, otherwise we need strict backward and forward compatibility, which is very hard to maintain and very easy to screw up 😅
- We shouldn't have static state if we can help it. If we want to cache things, we typically handle that by:
- Move the calculating and storage of the value to a helper type as instance methods.
- Create a "singleton" instance of the helper
- Pass the singleton in to the constructor (or, if this gives too much blast radius, allow passing in a null value for the helper, and grab the singleton in the body. This isn't the preferred approach, but it's sometimes the practical one)
- We need to ensure we record the new calculated values in telemetry
- The instrumentation seems a little overly complex considering the behaviours we need and the failure cases
| && !Util.EnvironmentHelpers.IsUsingAzureAppServicesSiteExtension() | ||
| && IsCompatLayerAvailableWithPipeSupport()) | ||
| { | ||
| var baseName = Util.EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.TracesPipeName); |
There was a problem hiding this comment.
Agreed, but we also need to make sure we record the new "calculated" value in telemetry as well
c9b7aee to
1df4f5a
Compare
+ Maximum Version change Co-authored-by: Andrew Lock <andrewlock.net@gmail.com>
StringUtil.IsNullOrEmpty has [NotNullWhen(false)] so the compiler already knows the value is non-null in the false branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…SERVERLESS_COMPAT_PATH config key Update supported_calltargets.g.json and generated_calltargets.g.cpp to reflect MaximumVersion change from 2.*.* to 1.*.*. Add DD_SERVERLESS_COMPAT_PATH to supported-configurations.yaml and restore ConfigurationKeys usage in ServerlessCompatPipeNameHelper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Version 1.4.0 adds the CalculateTracePipeName and CalculateDogStatsDPipeName methods that the tracer instruments for named pipe coordination. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ests - Update version range comments from 2.*.* to 1.*.* in both integrations - Update DD_SERVERLESS_COMPAT_PATH documentation for linux default path - Rewrite ServerlessCompatIntegrationTests: remove reflection-based tests for removed cache fields, add Theory-based tests for exception passthrough and fallback behavior with exact value assertions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…typo, use SkipOn in tests - Move ServerlessCompatPipeNameHelper from ClrProfiler.AutoInstrumentation.Serverless to Datadog.Trace.Serverless to avoid Configuration depending on instrumentation code - Fix "compatability" -> "compatibility" typo in YAML documentation - Replace silent early-return platform guards with SkipOn/SkippableFact so Windows-only tests show as skipped instead of silently passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Compat 1.4.0 isn't published yet and has a dependency on these tracer changes. Keep the 1.2.0 dependency — the instrumentation silently skips when the target methods don't exist, and IsCompatLayerAvailableWithPipeSupport gates on >= 1.4.0 at runtime. The dependency will be bumped when compat 1.4.0 is published. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Master #8231 extracted serverless platform detection from EnvironmentHelpers into dedicated cached classes under Datadog.Trace.Serverless. Update the Azure-Functions pipe-name generation gate to call AzureInfo.Instance.IsAzureFunction and IsUsingSiteExtension instead of the removed EnvironmentHelpers methods. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses review feedback: a customer-set TracesPipeName/MetricsPipeName (from any configuration source) should pass through verbatim even when running in Azure Functions, even though this can cause pipe-name collisions across multiple function instances sharing a plan. We only fabricate a GUID-suffixed default pipe name when the customer has not configured one themselves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rebase Post-rebase reconciliation: master's trace_filter additions took conflict priority during rebase, dropping the ServerlessCompat integration_name tag entries. Re-add them in both integration_name tag groups and shift downstream indices by +2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous attempt missed `var index = N + ((int)tag1 * M) + (int)tag2;` forms. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Regenerated via Nuke (Restore + CompileManagedSrc). Both integration_name tag groups now reflect 85 entries instead of 84. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Customer code calling Tracer.Configure triggers ExporterSettings reconstruction. With the previous static-GUID generator, each rebuild produced a new pipe name that no longer matched what the Serverless Compat layer already bound to. SettingsManager now computes the gating (Windows + Azure Function + compat layer 1.4+) once at startup, generates GUIDs, and freezes them into an immutable ServerlessCompatPipeNames record passed to every ExporterSettings construction. DD_SERVERLESS_COMPAT_PATH moved into Raw via the telemetry-enabled ConfigurationBuilder so it shows up in config telemetry. Integration callbacks drop try/catch + null chains and short-circuit when no tracer pipe name is configured. Log levels aligned with Andrew's review (Debug for missing compat layer, Error for unexpected failures). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Andrew's review: unit tests should not depend on Tracer.Instance global state. The removed case relied on Tracer.Instance defaulting to unconfigured pipe names, which is unsafe — any prior test could have left the singleton in an unrelated state. The non-exception path is covered end-to-end by integration tests elsewhere. The exception-guard test stays since it short-circuits before reaching Tracer.Instance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The helper's only caller (SettingsManager.CreatePipeNames) already does the Windows short-circuit as the first gate in its condition chain. The internal RuntimeInformation.IsOSPlatform check was unreachable in practice and duplicated the rule in two places. Drops the System.Runtime.InteropServices import and the #if !NETFRAMEWORK guard. Tests that assumed a platform-gated behavior are now cross-platform, bringing 10 previously-skipped cases into every run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…lculatedPipeName, and comment improvements. Updated expected ServerlessCompat version for NamedPipe instrumentation from 1.4 to 1.5
f7657ca to
ad59f32
Compare
## Summary - Bumps the `Datadog.Serverless.Compat` `PackageReference` in `tracer/src/Datadog.AzureFunctions/Datadog.AzureFunctions.csproj` from `1.4.0` → `1.5.0`. - 1.5.0 ships multifunction named-pipe coordination for .NET on Azure Functions Premium plans. It bundles the [`datadog-serverless-compat/v0.24.0`](https://github.com/DataDog/serverless-components/releases/tag/datadog-serverless-compat%2Fv0.24.0) binary, which contains the dual TCP+pipe listener ([serverless-components#94](DataDog/serverless-components#94)) and best-effort TCP-8126 bind ([serverless-components#129](DataDog/serverless-components#129)) — both required to land the named-pipe path merged in [#8164](#8164). ## Test plan - [ ] CI green (csproj-only change; should be a no-op outside of `Datadog.AzureFunctions` packaging) - [ ] Confirm a built `Datadog.AzureFunctions` package resolves `Datadog.Serverless.Compat 1.5.0` from NuGet at restore time 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of changes
Reason for change
Implementation details
For Azure Functions, the tracer loads first, then the compat layer, and then the compat layer launches a rust binary to act as the agent. Additionally, the client may or may not be using the DogstatsD client, and it is also possible to have the compat layer and DogstatsD in place without the tracer. We need to coordinate named pipes across all three/four of these codebases, noting that the tracer will use DogstatsD's pipe for runtime metrics if available.
Given this order of operations, and that we want this to work with immutable configuration, the tracer should set pipe names for the compatibility layer via instrumentation if the tracer is present.
DD_SERVERLESS_COMPAT_BINARY_PATHif necessary to adapt to Azure changes) to see if named pipes can be used or if we should rely on TCPTest coverage