Skip to content

Commit f7657ca

Browse files
Lewis-ELewis-E
committed
Nit-fixes for string handling, ResolveAndRecordPipeName from RecordCalculatedPipeName, and comment improvements. Updated expected ServerlessCompat version for NamedPipe instrumentation from 1.4 to 1.5
1 parent efe17b2 commit f7657ca

5 files changed

Lines changed: 94 additions & 26 deletions

File tree

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Serverless/CompatibilityLayer_CalculateDogStatsDPipeName_Integration.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ internal static CallTargetReturn<string> OnMethodEnd<TTarget>(
5050
}
5151

5252
// Use the tracer's configured pipe name, which in Azure Functions will be the
53-
// unique name generated by ExporterSettings. If the tracer hasn't configured a
54-
// pipe name, let the compat layer's own calculated name pass through unchanged.
53+
// unique name generated by SettingsManager.CreatePipeNames. If the tracer is
54+
// missing a pipe name, let the compat layer's own calculated name pass through.
5555
var tracerPipeName = Tracer.Instance.Settings.Manager.InitialExporterSettings.MetricsPipeName;
5656
if (tracerPipeName is null)
5757
{

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Serverless/CompatibilityLayer_CalculateTracePipeName_Integration.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ internal static CallTargetReturn<string> OnMethodEnd<TTarget>(
5050
}
5151

5252
// Use the tracer's configured pipe name, which in Azure Functions will be the
53-
// unique name generated by ExporterSettings. If the tracer hasn't configured a
54-
// pipe name, let the compat layer's own calculated name pass through unchanged.
53+
// unique name generated by SettingsManager.CreatePipeNames. If the tracer is
54+
// missing a pipe name, let the compat layer's own calculated name pass through.
5555
var tracerPipeName = Tracer.Instance.Settings.Manager.InitialExporterSettings.TracesPipeName;
5656
if (tracerPipeName is null)
5757
{

‎tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,8 @@ internal ExporterSettings(Raw rawSettings, Func<string, bool> fileExists, IConfi
8484

8585
ValidationWarnings = new List<string>();
8686

87-
// Customer-configured pipe names always take precedence; otherwise use whatever the
88-
// SettingsManager pre-computed for the Serverless Compat layer (may be null on
89-
// non-Windows / non-Azure-Function / missing-compat-layer runs).
90-
var tracesPipeName = !StringUtil.IsNullOrEmpty(rawSettings.TracesPipeName)
91-
? rawSettings.TracesPipeName
92-
: pipeNames.TracesPipeName;
93-
var metricsPipeName = !StringUtil.IsNullOrEmpty(rawSettings.MetricsPipeName)
94-
? rawSettings.MetricsPipeName
95-
: pipeNames.MetricsPipeName;
96-
97-
RecordCalculatedPipeName(ConfigurationKeys.TracesPipeName, tracesPipeName, rawSettings.TracesPipeName);
98-
RecordCalculatedPipeName(ConfigurationKeys.MetricsPipeName, metricsPipeName, rawSettings.MetricsPipeName);
87+
var tracesPipeName = ResolveAndRecordPipeName(ConfigurationKeys.TracesPipeName, pipeNames.TracesPipeName, rawSettings.TracesPipeName);
88+
var metricsPipeName = ResolveAndRecordPipeName(ConfigurationKeys.MetricsPipeName, pipeNames.MetricsPipeName, rawSettings.MetricsPipeName);
9989

10090
var traceSettings = GetTraceTransport(
10191
agentUri: rawSettings.TraceAgentUri,
@@ -334,17 +324,22 @@ private static string GetMetricsHostNameFromAgentUri(Uri agentUri)
334324
return string.IsNullOrEmpty(traceHostname) ? DefaultDogstatsdHostname : traceHostname;
335325
}
336326

337-
private void RecordCalculatedPipeName(string configKey, string? resolved, string? rawValue)
327+
private string? ResolveAndRecordPipeName(string configKey, string? pipeValue, string? rawValue)
338328
{
329+
// Customer-configured pipe names always take precedence; otherwise use whatever the
330+
// SettingsManager pre-computed for the Serverless Compat layer (may be null on
331+
// non-Windows / non-Azure-Function / missing-compat-layer runs).
332+
var resolved = !StringUtil.IsNullOrEmpty(rawValue) ? rawValue : pipeValue;
333+
339334
// Only record a Calculated telemetry entry when the final value came from the generator
340335
// (holder) rather than from customer config — the customer-set path is already recorded
341336
// by the ConfigurationBuilder read in Raw with its real origin.
342-
if (resolved is null || resolved == rawValue)
337+
if (resolved is not null && resolved != rawValue)
343338
{
344-
return;
339+
_telemetry.Record(configKey, resolved, recordValue: true, ConfigurationOrigins.Calculated);
345340
}
346341

347-
_telemetry.Record(configKey, resolved, recordValue: true, ConfigurationOrigins.Calculated);
342+
return resolved;
348343
}
349344

350345
private MetricsTransportSettings ConfigureMetricsTransport(string? metricsUrl, string? traceAgentUrl, string? agentHost, int dogStatsdPort, string? metricsPipeName, string? metricsUnixDomainSocketPath)

‎tracer/src/Datadog.Trace/Serverless/ServerlessCompatPipeNameHelper.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Datadog.Trace.Serverless
1414
{
1515
/// <summary>
1616
/// Checks whether the Datadog Serverless Compat layer is available and supports named-pipe
17-
/// transport. Pipe-name generation itself lives in <see cref="ServerlessCompatPipeNames"/>.
17+
/// transport. Pipe-name generation itself lives in SettingsManager.CreatePipeNames.
1818
/// </summary>
1919
internal static class ServerlessCompatPipeNameHelper
2020
{
@@ -46,10 +46,10 @@ internal static bool IsCompatLayerAvailableWithPipeSupport(
4646
// DD_SERVERLESS_COMPAT_PATH overrides the default binary location
4747
// (matches CompatibilityLayer.cs in datadog-serverless-compat-dotnet).
4848
const string defaultCompatBinaryPath = @"C:\home\site\wwwroot\datadog\bin\windows-amd64\datadog-serverless-compat.exe";
49-
var compatBinaryPath = !string.IsNullOrEmpty(compatPathOverride) ? compatPathOverride! : defaultCompatBinaryPath;
49+
var compatBinaryPath = !StringUtil.IsNullOrEmpty(compatPathOverride) ? compatPathOverride : defaultCompatBinaryPath;
5050

5151
// Check that the compat DLL exists and has a version that supports named pipes.
52-
// Named pipe support was added in compat version 1.4.0 (dev builds use 0.0.0).
52+
// Named pipe support was added in compat version 1.5.0 (dev builds use 0.0.0).
5353
var compatDllPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory ?? string.Empty, "Datadog.Serverless.Compat.dll");
5454
if (!fileExists(compatBinaryPath) || !fileExists(compatDllPath))
5555
{
@@ -65,17 +65,17 @@ internal static bool IsCompatLayerAvailableWithPipeSupport(
6565
return false;
6666
}
6767

68-
// Allow 0.0.0 (dev builds) or >= 1.4.0 (first release with pipe support)
68+
// Allow 0.0.0 (dev builds) or >= 1.5.0 (first release with pipe support)
6969
var isDevBuild = version.Major == 0 && version.Minor == 0 && version.Build == 0;
70-
var isSupported = version.Major > 1 || (version.Major == 1 && version.Minor >= 4);
70+
var isSupported = version.Major > 1 || (version.Major == 1 && version.Minor >= 5);
7171

7272
if (isDevBuild || isSupported)
7373
{
7474
Log.Debug("Compat layer version {Version} supports named pipes.", version);
7575
return true;
7676
}
7777

78-
Log.Debug("Compat layer version {Version} does not support named pipes (requires v1.4.0 or greater. Using fallback communication methods.)", version);
78+
Log.Debug("Compat layer version {Version} does not support named pipes (requires v1.5.0 or greater. Using fallback communication methods.)", version);
7979
return false;
8080
}
8181
catch (Exception ex)

‎tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsAzureFunctionsPipeTests.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
#nullable enable
77

88
using System.Collections.Specialized;
9+
using System.Linq;
910
using Datadog.Trace.Agent;
1011
using Datadog.Trace.Configuration;
1112
using Datadog.Trace.Configuration.Telemetry;
13+
using Datadog.Trace.Serverless;
1214
using FluentAssertions;
1315
using Xunit;
1416
using MetricsTransportType = Datadog.Trace.Vendors.StatsdClient.Transport.TransportType;
@@ -77,5 +79,76 @@ public void Constructor_SelectsCorrectMetricsPipeName(string? explicitPipeName,
7779
settings.MetricsPipeName.Should().BeNull();
7880
}
7981
}
82+
83+
// Cases below exercise the SettingsManager-injected pipe-name path. The 3-arg test ctor used
84+
// above defaults pipeNames to None, so it can't reach the auto-generated branch.
85+
86+
[Theory]
87+
[InlineData(null, "dd_trace_auto", "dd_trace_auto")] // raw unset → auto-gen wins
88+
[InlineData("", "dd_trace_auto", "dd_trace_auto")] // empty raw → auto-gen wins
89+
[InlineData("customer_pipe", "dd_trace_auto", "customer_pipe")] // customer raw beats auto-gen
90+
public void Constructor_TracesPipeName_AutoGenAndOverride(string? explicitPipeName, string autoGen, string expected)
91+
{
92+
var config = new NameValueCollection();
93+
if (explicitPipeName is not null)
94+
{
95+
config.Add(ConfigurationKeys.TracesPipeName, explicitPipeName);
96+
}
97+
98+
var source = new NameValueConfigurationSource(config);
99+
var telemetry = new ConfigurationTelemetry();
100+
var raw = new ExporterSettings.Raw(source, telemetry);
101+
var pipeNames = new ServerlessCompatPipeNames(TracesPipeName: autoGen, MetricsPipeName: "dd_dogstatsd_auto");
102+
var settings = new ExporterSettings(raw, _ => false, telemetry, pipeNames);
103+
104+
settings.TracesPipeName.Should().Be(expected);
105+
settings.TracesTransport.Should().Be(TracesTransportType.WindowsNamedPipe);
106+
}
107+
108+
[Theory]
109+
[InlineData(null, "dd_dogstatsd_auto", "dd_dogstatsd_auto")]
110+
[InlineData("", "dd_dogstatsd_auto", "dd_dogstatsd_auto")]
111+
[InlineData("customer_metrics_pipe", "dd_dogstatsd_auto", "customer_metrics_pipe")]
112+
public void Constructor_MetricsPipeName_AutoGenAndOverride(string? explicitPipeName, string autoGen, string expected)
113+
{
114+
var config = new NameValueCollection();
115+
if (explicitPipeName is not null)
116+
{
117+
config.Add(ConfigurationKeys.MetricsPipeName, explicitPipeName);
118+
}
119+
120+
var source = new NameValueConfigurationSource(config);
121+
var telemetry = new ConfigurationTelemetry();
122+
var raw = new ExporterSettings.Raw(source, telemetry);
123+
var pipeNames = new ServerlessCompatPipeNames(TracesPipeName: "dd_trace_auto", MetricsPipeName: autoGen);
124+
var settings = new ExporterSettings(raw, _ => false, telemetry, pipeNames);
125+
126+
settings.MetricsPipeName.Should().Be(expected);
127+
settings.MetricsTransport.Should().Be(MetricsTransportType.NamedPipe);
128+
}
129+
130+
[Fact]
131+
public void Constructor_RecordsCalculatedTelemetryOrigin_OnlyForAutoGenPipeNames()
132+
{
133+
// Customer set traces pipe explicitly; metrics pipe falls through to auto-gen.
134+
// Only the metrics record should be tagged ConfigurationOrigins.Calculated by ExporterSettings —
135+
// the customer-set traces value is recorded by the Raw ctor with its real origin.
136+
var config = new NameValueCollection();
137+
config.Add(ConfigurationKeys.TracesPipeName, "customer_traces_pipe");
138+
139+
var source = new NameValueConfigurationSource(config);
140+
var telemetry = new ConfigurationTelemetry();
141+
var raw = new ExporterSettings.Raw(source, telemetry);
142+
var pipeNames = new ServerlessCompatPipeNames(TracesPipeName: "dd_trace_auto", MetricsPipeName: "dd_dogstatsd_auto");
143+
_ = new ExporterSettings(raw, _ => false, telemetry, pipeNames);
144+
145+
var calculatedEntries = telemetry.GetQueueForTesting()
146+
.Where(e => e.Origin == ConfigurationOrigins.Calculated)
147+
.ToList();
148+
149+
calculatedEntries.Should().ContainSingle(e => e.Key == ConfigurationKeys.MetricsPipeName)
150+
.Which.StringValue.Should().Be("dd_dogstatsd_auto");
151+
calculatedEntries.Should().NotContain(e => e.Key == ConfigurationKeys.TracesPipeName);
152+
}
80153
}
81154
}

0 commit comments

Comments
 (0)