Fix bugs with DogStatsD when using named pipes or UDS

tracer/src/Datadog.Trace/TracerManagerFactory.cs

@@ -296,10 +296,8 @@ internal static IDogStatsd CreateDogStatsdClient(ImmutableTracerSettings setting
                 switch (settings.ExporterInternal.MetricsTransport)
                 {
                     case MetricsTransportType.NamedPipe:
-                        // Environment variables for windows named pipes are not explicitly passed to statsd.
-                        // They are retrieved within the vendored code, so there is nothing to pass.
-                        // Passing anything through StatsdConfig may cause bugs when windows named pipes should be used.
                         Log.Information("Using windows named pipes for metrics transport.");
+                        config.PipeName = settings.ExporterInternal.MetricsPipeNameInternal;
                         break;
 #if NETCOREAPP3_1_OR_GREATER
                     case MetricsTransportType.UDS:
@@ -309,7 +307,13 @@ internal static IDogStatsd CreateDogStatsdClient(ImmutableTracerSettings setting
 #endif
                     case MetricsTransportType.UDP:
                     default:
-                        config.StatsdServerName = settings.ExporterInternal.AgentUriInternal.DnsSafeHost;
+                        // If the customer has enabled UDS traces but _not_ UDS metrics, then the AgentUri will have
+                        // the UDS path set for it, and the DnsSafeHost returns "". Ideally, we would expose
+                        // a TracesAgentUri and MetricsAgentUri or something like that instead. This workaround
+                        // is a horrible hack, but I can't bear to touch ExporterSettings until it's had an
+                        // extensive tidy up, so this should do for now
+                        var traceHostname = settings.ExporterInternal.AgentUriInternal.DnsSafeHost;
+                        config.StatsdServerName = string.IsNullOrEmpty(traceHostname) ? "127.0.0.1" : traceHostname;
                         config.StatsdPort = settings.ExporterInternal.DogStatsdPortInternal;
                         break;
                 }

tracer/test/Datadog.Trace.TestHelpers/EnvironmentRestorerAttribute.cs

@@ -0,0 +1,44 @@
+// <copyright file="EnvironmentRestorerAttribute.cs" company="Datadog">
+// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
+// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
+// </copyright>
+
+using System;
+using System.Collections.Generic;
+using System.Reflection;
+using Xunit.Sdk;
+
+namespace Datadog.Trace.TestHelpers;
+
+[AttributeUsage(AttributeTargets.Class)]
+public class EnvironmentRestorerAttribute : BeforeAfterTestAttribute
+{
+    private readonly string[] _variables;
+    private readonly Dictionary<string, string> _originalVariables;
+
+    public EnvironmentRestorerAttribute(params string[] args)
+    {
+        _variables = args;
+        _originalVariables = new(args.Length);
+    }
+
+    public override void Before(MethodInfo methodUnderTest)
+    {
+        foreach (var variable in _variables)
+        {
+            _originalVariables[variable] = Environment.GetEnvironmentVariable(variable);
+            // clear variable
+            Environment.SetEnvironmentVariable(variable, null);
+        }
+
+        base.Before(methodUnderTest);
+    }
+
+    public override void After(MethodInfo methodUnderTest)
+    {
+        foreach (var variable in _originalVariables)
+        {
+            Environment.SetEnvironmentVariable(variable.Key, variable.Value);
+        }
+    }
+}

tracer/test/Datadog.Trace.Tests/DogStatsDTests.cs

@@ -5,18 +5,31 @@
 
 using System;
 using System.Collections.Immutable;
+using System.IO;
 using System.Linq;
 using System.Threading;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.DogStatsd;
 using Datadog.Trace.TestHelpers;
 using Datadog.Trace.Vendors.StatsdClient;
+using Datadog.Trace.Vendors.StatsdClient.Transport;
+using FluentAssertions;
 using Moq;
 using Xunit;
 using Xunit.Abstractions;
 
 namespace Datadog.Trace.Tests
 {
+    [Collection(nameof(EnvironmentVariablesTestCollection))]
+    [EnvironmentRestorer(
+        ConfigurationKeys.AgentHost,
+        ConfigurationKeys.AgentUri,
+        ConfigurationKeys.AgentPort,
+        ConfigurationKeys.DogStatsdPort,
+        ConfigurationKeys.TracesPipeName,
+        ConfigurationKeys.TracesUnixDomainSocketPath,
+        ConfigurationKeys.MetricsPipeName,
+        ConfigurationKeys.MetricsUnixDomainSocketPath)]
     public class DogStatsDTests
     {
         private readonly ITestOutputHelper _output;
@@ -97,6 +110,121 @@ public void Send_metrics_when_enabled()
             */
         }
 
+        [SkippableTheory]
+        [InlineData(null, null, null)] // Should default to udp
+        [InlineData("http://127.0.0.1:1234", null, null)]
+        [InlineData(null, "127.0.0.1", null)]
+        [InlineData(null, "127.0.0.1", "1234")]
+        public void CanCreateDogStatsD_UDP_FromTraceAgentSettings(string agentUri, string agentHost, string port)
+        {
+            SkipOn.Platform(SkipOn.PlatformValue.MacOs);
+
+            var settings = new TracerSettings(new NameValueConfigurationSource(new()
+            {
+                { ConfigurationKeys.AgentUri, agentUri },
+                { ConfigurationKeys.AgentHost, agentHost },
+                { ConfigurationKeys.DogStatsdPort, port },
+            })).Build();
+
+            settings.Exporter.MetricsTransport.Should().Be(TransportType.UDP);
+            var expectedPort = settings.Exporter.DogStatsdPort;
+
+            // Dogstatsd tries to actually contact the agent during creation, so need to have something listening
+            // No guarantees it's actually using the _right_ config here, but it's better than nothing
+            using var agent = MockTracerAgent.Create(_output, useStatsd: true, requestedStatsDPort: expectedPort);
+
+            var dogStatsD = TracerManagerFactory.CreateDogStatsdClient(settings, "test service", null);
+
+            // If there's an error during configuration, we get a no-op instance, so using this as a test
+            dogStatsD.Should()
+                     .NotBeNull()
+                     .And.BeOfType<DogStatsdService>();
+        }
+
+        [SkippableFact]
+        public void CanCreateDogStatsD_NamedPipes_FromTraceAgentSettings()
+        {
+            SkipOn.Platform(SkipOn.PlatformValue.MacOs);
+            SkipOn.Platform(SkipOn.PlatformValue.Linux);
+
+            using var agent = MockTracerAgent.Create(_output, new WindowsPipesConfig($"trace-{Guid.NewGuid()}", $"metrics-{Guid.NewGuid()}") { UseDogstatsD = true });
+
+            var settings = new TracerSettings(new NameValueConfigurationSource(new()
+            {
+                { ConfigurationKeys.MetricsPipeName, agent.StatsWindowsPipeName },
+            })).Build();
+
+            settings.Exporter.MetricsTransport.Should().Be(TransportType.NamedPipe);
+
+            // Dogstatsd tries to actually contact the agent during creation, so need to have something listening
+            // No guarantees it's actually using the _right_ config here, but it's better than nothing
+            var dogStatsD = TracerManagerFactory.CreateDogStatsdClient(settings, "test service", null);
+
+            // If there's an error during configuration, we get a no-op instance, so using this as a test
+            dogStatsD.Should()
+                     .NotBeNull()
+                     .And.BeOfType<DogStatsdService>();
+        }
+
+#if NETCOREAPP3_1_OR_GREATER
+        [SkippableFact]
+        public void CanCreateDogStatsD_UDS_FromTraceAgentSettings()
+        {
+            // UDP Datagrams over UDP are not supported on Windows
+            SkipOn.Platform(SkipOn.PlatformValue.Windows);
+            SkipOn.Platform(SkipOn.PlatformValue.MacOs);
+
+            var tracesPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            var metricsPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            using var agent = MockTracerAgent.Create(_output, new UnixDomainSocketConfig(tracesPath, metricsPath) { UseDogstatsD = true });
+
+            var settings = new TracerSettings(new NameValueConfigurationSource(new()
+            {
+                { ConfigurationKeys.AgentUri, $"unix://{tracesPath}" },
+                { ConfigurationKeys.MetricsUnixDomainSocketPath, $"unix://{metricsPath}" },
+            })).Build();
+
+            settings.Exporter.MetricsTransport.Should().Be(TransportType.UDS);
+
+            // Dogstatsd tries to actually contact the agent during creation, so need to have something listening
+            // No guarantees it's actually using the _right_ config here, but it's better than nothing
+            var dogStatsD = TracerManagerFactory.CreateDogStatsdClient(settings, "test service", null);
+
+            // If there's an error during configuration, we get a no-op instance, so using this as a test
+            dogStatsD.Should()
+                     .NotBeNull()
+                     .And.BeOfType<DogStatsdService>();
+        }
+
+        [SkippableFact]
+        public void CanCreateDogStatsD_UDS_FallsBackToUdp_FromTraceAgentSettings()
+        {
+            SkipOn.Platform(SkipOn.PlatformValue.MacOs);
+
+            var tracesPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            using var udsAgent = MockTracerAgent.Create(_output, new UnixDomainSocketConfig(tracesPath, null) { UseDogstatsD = false });
+            using var udpAgent = MockTracerAgent.Create(_output, useStatsd: true);
+
+            var settings = new TracerSettings(new NameValueConfigurationSource(new()
+            {
+                { ConfigurationKeys.AgentUri, $"unix://{tracesPath}" },
+            })).Build();
+
+            // If we're not using the "default" UDS path, then we fallback to UDP for stats
+            // Should fallback to the "default" stats location
+            settings.Exporter.MetricsTransport.Should().Be(TransportType.UDP);
+
+            // Dogstatsd tries to actually contact the agent during creation, so need to have something listening
+            // No guarantees it's actually using the _right_ config here, but it's better than nothing
+            var dogStatsD = TracerManagerFactory.CreateDogStatsdClient(settings, "test service", null);
+
+            // If there's an error during configuration, we get a no-op instance, so using this as a test
+            dogStatsD.Should()
+                     .NotBeNull()
+                     .And.BeOfType<DogStatsdService>();
+        }
+#endif
+
         private static IImmutableList<MockSpan> SendSpan(bool tracerMetricsEnabled, IDogStatsd statsd)
         {
             IImmutableList<MockSpan> spans;