Skip to content

Commit d5726e6

Browse files
andrewlockandrewlock
authored
Add fix for whitespace in exporter settings and metrics URL (#7747)
## Summary of changes - Trim whitespace in the exporter settings values before trying to use them - Fix a bug where having a whitespace in the `DD_DOGSTATSD_SOCKET` variable would cause incorrect (and invalid) UDS config ## Reason for change This manifested in a different test in a branch, where an empty string passed in `DD_DOGSTATSD_SOCKET` was causing us to use UDS for metrics even through it's not available and not _really_ set. ## Implementation details - `Trim()` the string variables (I don't think there's a good reason not to?) - As we're trimming, we can switch to `IsNullOrEmpty` instead of `IsWhitespace` - Treat empty `DD_DOGSTATSD_SOCKET` as not set Alternatively, we could not trim, stick to using `IsWhitespace`, and just update the check I fixed to use `IsWhitespace` 🤷‍♂️ ## Test coverage Added a couple of unit tests. Only one test is for this specific issue, the others were just part of my investigation, and seemed reasonable. ## Other details Discovered as part of #7724 and https://datadoghq.atlassian.net/browse/LANGPLAT-819
1 parent 2772023 commit d5726e6

File tree

4 files changed

+53
-16
lines changed

4 files changed

+53
-16
lines changed

‎tracer/src/Datadog.Trace.Tools.dd_dotnet/ExporterSettings.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ internal enum TelemetryErrorCode
8989
{
9090
foreach (var key in keys)
9191
{
92-
var value = configuration?.GetString(key);
92+
var value = configuration?.GetString(key)?.Trim();
9393

9494
if (value != null)
9595
{

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ private TraceTransportSettings GetTraceTransport(string? agentUri, string? trace
4646
// Check the parameters in order of precedence
4747
// For some cases, we allow falling back on another configuration (eg invalid url as the application will need to be restarted to fix it anyway).
4848
// For other cases (eg a configured unix domain socket path not found), we don't fallback as the problem could be fixed outside the application.
49-
if (!string.IsNullOrWhiteSpace(agentUri))
49+
if (!string.IsNullOrEmpty(agentUri))
5050
{
5151
if (TryGetAgentUriAndTransport(agentUri!, origin, out var settings))
5252
{
5353
return settings;
5454
}
5555
}
5656

57-
if (!string.IsNullOrWhiteSpace(tracesPipeName))
57+
if (!string.IsNullOrEmpty(tracesPipeName))
5858
{
5959
RecordTraceTransport(nameof(TracesTransportType.WindowsNamedPipe), origin);
6060

@@ -73,7 +73,7 @@ private TraceTransportSettings GetTraceTransport(string? agentUri, string? trace
7373

7474
// This property shouldn't have been introduced. We need to remove it as part of 3.0
7575
// But while it's here, we need to handle it properly
76-
if (!string.IsNullOrWhiteSpace(tracesUnixDomainSocketPath))
76+
if (!string.IsNullOrEmpty(tracesUnixDomainSocketPath))
7777
{
7878
#if NETCOREAPP3_1_OR_GREATER
7979
if (TryGetAgentUriAndTransport(UnixDomainSocketPrefix + tracesUnixDomainSocketPath, origin, out var settings))

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ private static string GetMetricsHostNameFromAgentUri(Uri agentUri)
234234

235235
private MetricsTransportSettings ConfigureMetricsTransport(string? metricsUrl, string? traceAgentUrl, string? agentHost, int dogStatsdPort, string? metricsPipeName, string? metricsUnixDomainSocketPath)
236236
{
237-
if (!string.IsNullOrWhiteSpace(metricsUrl) && TryGetMetricsUriAndTransport(metricsUrl!, out var settingsFromUri))
237+
if (!string.IsNullOrEmpty(metricsUrl) && TryGetMetricsUriAndTransport(metricsUrl!, out var settingsFromUri))
238238
{
239239
return settingsFromUri;
240240
}
@@ -252,7 +252,7 @@ private MetricsTransportSettings ConfigureMetricsTransport(string? metricsUrl, s
252252

253253
MetricsTransportSettings settings;
254254

255-
if (!string.IsNullOrWhiteSpace(traceAgentUrl)
255+
if (!string.IsNullOrEmpty(traceAgentUrl)
256256
&& !traceAgentUrl!.StartsWith(UnixDomainSocketPrefix)
257257
&& Uri.TryCreate(traceAgentUrl, UriKind.Absolute, out var tcpUri))
258258
{
@@ -273,11 +273,11 @@ private MetricsTransportSettings ConfigureMetricsTransport(string? metricsUrl, s
273273
portSource: dogStatsDPortSource,
274274
out settings);
275275
}
276-
else if (!string.IsNullOrWhiteSpace(metricsPipeName))
276+
else if (!string.IsNullOrEmpty(metricsPipeName))
277277
{
278278
settings = new MetricsTransportSettings(TransportType.NamedPipe, PipeName: metricsPipeName);
279279
}
280-
else if (metricsUnixDomainSocketPath != null)
280+
else if (!string.IsNullOrEmpty(metricsUnixDomainSocketPath))
281281
{
282282
#if NETCOREAPP3_1_OR_GREATER
283283
SetUds(metricsUnixDomainSocketPath, metricsUnixDomainSocketPath, metricsUnixDomainSocketPath, ConfigurationKeys.MetricsUnixDomainSocketPath, out settings);
@@ -447,22 +447,22 @@ public Raw(IConfigurationSource source, IConfigurationTelemetry telemetry)
447447
// Get values from the config
448448
var config = new ConfigurationBuilder(source, telemetry);
449449
// NOTE: Keep this in sync with CreateUpdatedFromManualConfig below
450-
TraceAgentUri = config.WithKeys(ConfigurationKeys.AgentUri).AsString();
451-
TracesPipeName = config.WithKeys(ConfigurationKeys.TracesPipeName).AsString();
452-
TracesUnixDomainSocketPath = config.WithKeys(ConfigurationKeys.TracesUnixDomainSocketPath).AsString();
450+
TraceAgentUri = config.WithKeys(ConfigurationKeys.AgentUri).AsString()?.Trim();
451+
TracesPipeName = config.WithKeys(ConfigurationKeys.TracesPipeName).AsString()?.Trim();
452+
TracesUnixDomainSocketPath = config.WithKeys(ConfigurationKeys.TracesUnixDomainSocketPath).AsString()?.Trim();
453453

454454
TraceAgentHost = config
455455
.WithKeys(ConfigurationKeys.AgentHost, "DD_TRACE_AGENT_HOSTNAME", "DATADOG_TRACE_AGENT_HOSTNAME")
456-
.AsString();
456+
.AsString()?.Trim();
457457

458458
TraceAgentPort = config
459459
.WithKeys(ConfigurationKeys.AgentPort, "DATADOG_TRACE_AGENT_PORT")
460460
.AsInt32();
461461

462-
MetricsUrl = config.WithKeys(ConfigurationKeys.MetricsUri).AsString();
462+
MetricsUrl = config.WithKeys(ConfigurationKeys.MetricsUri).AsString()?.Trim();
463463
DogStatsdPort = config.WithKeys(ConfigurationKeys.DogStatsdPort).AsInt32(0);
464-
MetricsPipeName = config.WithKeys(ConfigurationKeys.MetricsPipeName).AsString();
465-
MetricsUnixDomainSocketPath = config.WithKeys(ConfigurationKeys.MetricsUnixDomainSocketPath).AsString();
464+
MetricsPipeName = config.WithKeys(ConfigurationKeys.MetricsPipeName).AsString()?.Trim();
465+
MetricsUnixDomainSocketPath = config.WithKeys(ConfigurationKeys.MetricsUnixDomainSocketPath).AsString()?.Trim();
466466

467467
TracesPipeTimeoutMs = config
468468
.WithKeys(ConfigurationKeys.TracesPipeTimeoutMs)

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,43 @@ public void Metrics_DogStatsdUrl_UdsOnLinux()
9292
AssertMetricsUdsIsConfigured(settingsFromSource, socketPath);
9393
settingsFromSource.ValidationWarnings.Should().BeEmpty();
9494
}
95+
96+
[Theory]
97+
[InlineData("""C:\Users\andrew.lock\AppData\Local\Temp\nltdjakv.cbs""")]
98+
[InlineData("/some/socket.soc")]
99+
public void Metrics_UdsTraceAgent_UsesDefaultUdp(string socket)
100+
{
101+
var config = Setup(FileExistsMock(socket.Replace('\\', '/')), $"DD_TRACE_AGENT_URL:{ExporterSettings.UnixDomainSocketPrefix}{socket}");
102+
AssertUdsIsConfigured(config, socket.Replace('\\', '/'));
103+
AssertMetricsUdpIsConfigured(config);
104+
config.ValidationWarnings.Should().BeEmpty();
105+
}
106+
107+
[Theory]
108+
[InlineData(null)]
109+
[InlineData("")]
110+
[InlineData(" ")]
111+
public void Metrics_UdsTraceAgent_EmptyDogStatsdUrl_UsesDefaultUdp(string metricsSocket)
112+
{
113+
var socket = "/some/socket.soc";
114+
var config = Setup(FileExistsMock(socket.Replace('\\', '/')), $"DD_DOGSTATSD_URL:{metricsSocket}", $"DD_TRACE_AGENT_URL:{ExporterSettings.UnixDomainSocketPrefix}{socket}");
115+
AssertUdsIsConfigured(config, socket);
116+
AssertMetricsUdpIsConfigured(config);
117+
config.ValidationWarnings.Should().BeEmpty();
118+
}
119+
120+
[Theory]
121+
[InlineData(null)]
122+
[InlineData("")]
123+
[InlineData(" ")]
124+
public void Metrics_UdsTraceAgent_EmptyDogStatsdSocket_UsesDefaultUdp(string metricsSocket)
125+
{
126+
var socket = "/some/socket.soc";
127+
var config = Setup(FileExistsMock(socket.Replace('\\', '/')), $"DD_DOGSTATSD_SOCKET:{metricsSocket}", $"DD_TRACE_AGENT_URL:{ExporterSettings.UnixDomainSocketPrefix}{socket}");
128+
AssertUdsIsConfigured(config, socket);
129+
AssertMetricsUdpIsConfigured(config);
130+
config.ValidationWarnings.Should().BeEmpty();
131+
}
95132
#else
96133
[Fact]
97134
public void Metrics_DogStatsdUrl_UdsUnsupported_UsesDefaultUdpOnWindows()
@@ -104,7 +141,7 @@ public void Metrics_DogStatsdUrl_UdsUnsupported_UsesDefaultUdpOnWindows()
104141
}
105142

106143
[Fact]
107-
public void Metrics_DogStatsdUrl_UdsUnsupported_UsesDefaultUdsOnLinux()
144+
public void Metrics_DogStatsdUrl_UdsUnsupported_UsesDefaultUdpOnLinux()
108145
{
109146
var socketPath = @"/some/socket.soc";
110147
var settings = Setup(FileExistsMock(socketPath), $"DD_DOGSTATSD_URL:unix://{socketPath}", "DD_TRACE_AGENT_URL:http://localhost:8126");

0 commit comments

Comments
 (0)