Add Multi-Azure Function Unique Named Pipe Configuration#8164
Open
Add Multi-Azure Function Unique Named Pipe Configuration#8164
Conversation
Lewis-E
force-pushed
the
lewis/SVLS-8244/add-multifunction-named-pipes
branch
from
37f021e to
1b6956c
Compare
BenchmarksBenchmark execution time: 2026-04-21 20:41:26 Comparing candidate commit b920296 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 25 metrics, 0 unstable metrics, 87 known flaky benchmarks.
|
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 (75ms) : 70, 80
master - mean (75ms) : 70, 79
section Bailout
This PR (8164) - mean (78ms) : 74, 81
master - mean (77ms) : 75, 80
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,084ms) : 1038, 1129
master - mean (1,082ms) : 1029, 1135
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 (116ms) : 110, 121
master - mean (116ms) : 111, 121
section Bailout
This PR (8164) - mean (114ms) : 110, 118
master - mean (116ms) : 113, 119
section CallTarget+Inlining+NGEN
This PR (8164) - mean (776ms) : 747, 806
master - mean (782ms) : 748, 816
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 (102ms) : 97, 106
section Bailout
This PR (8164) - mean (102ms) : 99, 104
master - mean (102ms) : 100, 105
section CallTarget+Inlining+NGEN
This PR (8164) - mean (940ms) : 902, 979
master - mean (941ms) : 903, 979
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (103ms) : 97, 110
master - mean (105ms) : 100, 110
section Bailout
This PR (8164) - mean (102ms) : 97, 107
master - mean (102ms) : 98, 106
section CallTarget+Inlining+NGEN
This PR (8164) - mean (825ms) : 790, 859
master - mean (829ms) : 789, 870
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 (200ms) : 194, 206
master - mean (205ms) : 194, 217
section Bailout
This PR (8164) - mean (203ms) : 198, 207
master - mean (210ms) : 201, 219
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,184ms) : 1126, 1242
master - mean (1,214ms) : 1156, 1273
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 (286ms) : 276, 296
master - mean (296ms) : 281, 311
section Bailout
This PR (8164) - mean (286ms) : 279, 293
master - mean (297ms) : 280, 315
section CallTarget+Inlining+NGEN
This PR (8164) - mean (936ms) : 896, 976
master - mean (967ms) : 934, 999
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (278ms) : 269, 287
master - mean (289ms) : 275, 304
section Bailout
This PR (8164) - mean (279ms) : 267, 291
master - mean (295ms) : 275, 314
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,142ms) : 1099, 1186
master - mean (1,165ms) : 1119, 1210
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8164) - mean (277ms) : 268, 286
master - mean (291ms) : 268, 314
section Bailout
This PR (8164) - mean (277ms) : 268, 286
master - mean (289ms) : 275, 304
section CallTarget+Inlining+NGEN
This PR (8164) - mean (1,034ms) : 984, 1084
master - mean (1,071ms) : 978, 1164
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Lewis-E
force-pushed
the
lewis/SVLS-8244/add-multifunction-named-pipes
branch
2 times, most recently
from
aaff9b5 to
08a6c72
Compare
Lewis-E
force-pushed
the
lewis/SVLS-8244/add-multifunction-named-pipes
branch
3 times, most recently
from
2fcae54 to
edb4121
Compare
duncanpharvey
approved these changes
Mar 13, 2026
Contributor
duncanpharvey
left a comment
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.
Contributor
|
Should this dependency for |
Member
|
@codex review |
| && !Util.EnvironmentHelpers.IsUsingAzureAppServicesSiteExtension() | ||
| && IsCompatLayerAvailableWithPipeSupport()) | ||
| { | ||
| var baseName = Util.EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.TracesPipeName); |
Member
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.
Member
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
Member
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
reviewed
Mar 24, 2026
Member
andrewlock
left a comment
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); |
Member
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
Lewis-E
force-pushed
the
lewis/SVLS-8244/add-multifunction-named-pipes
branch
from
c9b7aee to
1df4f5a
Compare
…ions - ServerlessCompatPipeNameHelper: format, truncation, uniqueness - Integration OnMethodEnd: exception passthrough, caching, override behavior - ExporterSettings: constructor pipe name selection, generated name format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check for the compat binary and DLL on disk before generating pipe names in ExporterSettings static init. This prevents the tracer from switching to pipe transport when no compat layer is deployed to listen on the pipe. The version check allows 0.0.0 (dev builds) or >= 1.4.0 (first release with named pipe support). Also fix integration fallbacks to pass through the compat layer's return value instead of generating a new random GUID when no pre-generated pipe name exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wrap RuntimeInformation.IsOSPlatform in #if !NETFRAMEWORK (net461 doesn't have it) - Add null-coalescing for AppDomain.CurrentDomain.BaseDirectory (nullable warning) - Remove unused argument from Log.Debug call (DDLOG003 analyzer error) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…exists When ExporterSettings hasn't pre-generated a pipe name, the integrations now generate one via ServerlessCompatPipeNameHelper instead of falling back to the compat layer's value. Also use realistic GUID-format pipe names in test fixtures to match actual compat layer behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix "Compatability" → "Compatibility" typo (3 occurrences) - Use `is null` / `is not null` instead of `== null` / `!= null` - Use `StringUtil.IsNullOrEmpty()` instead of `string.IsNullOrEmpty()` - Remove duplicate test file in Datadog.Trace.Tests - Remove tests already covered by ServerlessCompatPipeNameHelperTests - Fix SA1508 blank line before closing brace in PlatformKeys Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nique pipe generation Instead of skipping pipe generation when an explicit pipe name is configured, use the configured value as the base name with a GUID suffix appended. This allows users to customize the pipe base name while still getting per-instance isolation on shared Azure Functions hosting plans. If no explicit pipe name is set, falls back to "dd_trace" / "dd_dogstatsd". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… integrations CalculateDogStatsDPipeName and CalculateTracePipeName are each only called once during Start(), so caching is unnecessary. The fallback to generate a new unique name is also removed since ExporterSettings always pre-generates the pipe name before these instrumentations fire, and generating a name the tracer doesn't know about is pointless — fall back to the compat layer's own value instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e constructor Replace static field initializers that read env vars directly with instance methods that use rawSettings.TracesPipeName/MetricsPipeName, respecting all config sources (env vars, config files, etc.) instead of only environment variables. Record the calculated pipe names in telemetry. Integrations now read pipe names from Tracer.Instance.Settings.Exporter instead of removed static properties. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e generation Check the Azure Functions/compat layer condition once in the constructor instead of duplicating it in two separate methods. Consolidate the two per-pipe methods into a single GenerateUniquePipeName overload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eHelper and make testable Move compat layer detection out of ExporterSettings into ServerlessCompatPipeNameHelper alongside the related pipe name logic. Extract I/O dependencies (file existence, assembly version) as delegates so version-checking logic can be fully unit tested. Add support for DD_SERVERLESS_COMPAT_PATH env var to override the default binary path, matching the compat layer's own behavior. Add comprehensive tests for all branches: version checks, file missing, null version, exceptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… fix ExporterSettings access path - Add DD_SERVERLESS_COMPAT_PATH to supported-configurations.yaml so the source generator creates ConfigurationKeys.ServerlessCompatPath - Fix integration classes to use the correct property path: Tracer.Instance.Settings.Manager.InitialExporterSettings - Restore using Datadog.Trace.Configuration (needed for IntegrationId) - Remove unused System.Reflection/InteropServices from ExporterSettings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
+ 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>
Lewis-E
force-pushed
the
lewis/SVLS-8244/add-multifunction-named-pipes
branch
from
4a4b1ba to
0587f49
Compare
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
4 participants
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
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