Fix scenario where dogstatsd tries to send to the wrong hostname

tracer/src/Datadog.Trace.Trimming/build/Datadog.Trace.Trimming.xml

@@ -827,6 +827,7 @@
       <type fullname="System.UriComponents" />
       <type fullname="System.UriCreationOptions" />
       <type fullname="System.UriFormat" />
+      <type fullname="System.UriHostNameType" />
       <type fullname="System.UriKind" />
       <type fullname="System.ValueTuple`1" />
       <type fullname="System.ValueTuple`2" />

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

@@ -14,6 +14,7 @@
 using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
+using Datadog.Trace.Vendors.StatsdClient.Transport;
 using MetricsTransportType = Datadog.Trace.Vendors.StatsdClient.Transport.TransportType;
 
 namespace Datadog.Trace.Configuration
@@ -34,10 +35,15 @@ public partial class ExporterSettings
         private Uri _agentUri;
 
         /// <summary>
-        /// The default port value for dogstatsd.
+        /// The default port value for dogstatsd when sending over UDP.
         /// </summary>
         internal const int DefaultDogstatsdPort = 8125;
 
+        /// <summary>
+        /// The default port value for dogstatsd when sending over UDP.
+        /// </summary>
+        internal const string DefaultDogstatsdHostname = "127.0.0.1";
+
         /// <summary>
         /// Default metrics UDS path.
         /// </summary>
@@ -153,7 +159,11 @@ internal Uri AgentUriInternal
                 // In the case the url was a UDS one, we do not change anything.
                 if (TracesTransport == TracesTransportType.Default)
                 {
+                    // This behaviour could be unexpected, but it's the existing behavior
+                    // If we expose a separate property for setting the stats URL we should consider
+                    // dropping this behaviour
                     MetricsTransport = MetricsTransportType.UDP;
+                    MetricsHostname = GetMetricsHostNameFromAgentUri(_agentUri);
                 }
             }
         }
@@ -284,31 +294,52 @@ private set
         /// </summary>
         [IgnoreForSnapshot] // We don't record this in telemetry currently, but if we do, we'll record it when we set its
         internal MetricsTransportType MetricsTransport { get; private set; }
+
+        /// <summary>
+        /// Gets or sets the agent host to use when <see cref="MetricsTransport"/> is <see cref="TransportType.UDP"/>
+        /// </summary>
+        [IgnoreForSnapshot] // We don't record this in telemetry currently, but if we do, we'll record it when we set its
+        internal string MetricsHostname { get; private set; }
 #pragma warning restore SA1624
 
         internal List<string> ValidationWarnings { get; }
 
         internal IConfigurationTelemetry Telemetry => _telemetry;
 
+        private static string GetMetricsHostNameFromAgentUri(Uri agentUri)
+        {
+            // If the customer has enabled UDS traces then the AgentUri will have
+            // the UDS path set for it, and the DnsSafeHost returns "".
+            var traceHostname = agentUri.DnsSafeHost;
+            return string.IsNullOrEmpty(traceHostname) ? DefaultDogstatsdHostname : traceHostname;
+        }
+
+        [MemberNotNull(nameof(MetricsHostname))]
         private void ConfigureMetricsTransport(string? traceAgentUrl, string? agentHost, int dogStatsdPort, string? metricsPipeName, string? metricsUnixDomainSocketPath)
         {
             // Agent port is set to zero in places like AAS where it's needed to prevent port conflict
-            // The agent will fail to start if it can not bind a port, so we need to override 8126 to prevent port conflict
+            // The agent will fail to start if it can not bind a port, so we need to override 8125 to prevent port conflict
             // Port 0 means it will pick some random available port
             if (dogStatsdPort < 0)
             {
                 ValidationWarnings.Add("The provided dogStatsD port isn't valid, it should be positive.");
             }
 
-            if (!string.IsNullOrWhiteSpace(traceAgentUrl) && !traceAgentUrl!.StartsWith(UnixDomainSocketPrefix) && Uri.TryCreate(traceAgentUrl, UriKind.Absolute, out var uri))
+            if (!string.IsNullOrWhiteSpace(traceAgentUrl)
+             && !traceAgentUrl!.StartsWith(UnixDomainSocketPrefix)
+             && Uri.TryCreate(traceAgentUrl, UriKind.Absolute, out var uri))
             {
-                // No need to set AgentHost, it is taken from the AgentUri and set in ConfigureTrace
                 MetricsTransport = MetricsTransportType.UDP;
+                MetricsHostname = GetMetricsHostNameFromAgentUri(uri);
             }
             else if (dogStatsdPort > 0 || agentHost != null)
             {
-                // No need to set AgentHost, it is taken from the AgentUri and set in ConfigureTrace
                 MetricsTransport = MetricsTransportType.UDP;
+                MetricsHostname = agentHost!; // assumes that agentHost is a valid hostname or IP address, just warn if it's not
+                if (Uri.CheckHostName(agentHost) is not (UriHostNameType.IPv4 or UriHostNameType.IPv6 or UriHostNameType.Dns))
+                {
+                    ValidationWarnings.Add($"The provided agent host '{agentHost}' is not a valid hostname or IP address.");
+                }
             }
             else if (!string.IsNullOrWhiteSpace(metricsPipeName))
             {
@@ -334,10 +365,12 @@ private void ConfigureMetricsTransport(string? traceAgentUrl, string? agentHost,
             else
             {
                 MetricsTransport = MetricsTransportType.UDP;
+                MetricsHostname = DefaultDogstatsdHostname;
                 DogStatsdPortInternal = DefaultDogstatsdPort;
             }
 
             DogStatsdPortInternal = dogStatsdPort > 0 ? dogStatsdPort : DefaultDogstatsdPort;
+            MetricsHostname ??= DefaultDogstatsdHostname;
         }
 
         private void RecordTraceTransport(string transport, ConfigurationOrigins origin = ConfigurationOrigins.Default)

tracer/src/Datadog.Trace/Configuration/ImmutableExporterSettings.cs

@@ -54,6 +54,7 @@ internal ImmutableExporterSettings(ExporterSettings settings, bool unused)
             TracesPipeTimeoutMsInternal = settings.TracesPipeTimeoutMsInternal;
 
             MetricsTransport = settings.MetricsTransport;
+            MetricsHostname = settings.MetricsHostname;
             MetricsPipeNameInternal = settings.MetricsPipeNameInternal;
             DogStatsdPortInternal = settings.DogStatsdPortInternal;
 
@@ -143,6 +144,11 @@ internal ImmutableExporterSettings(ExporterSettings settings, bool unused)
         /// </summary>
         internal Vendors.StatsdClient.Transport.TransportType MetricsTransport { get; }
 
+        /// <summary>
+        /// Gets the agent host to use when <see cref="MetricsTransport"/> is <see cref="Vendors.StatsdClient.Transport.TransportType.UDP"/>
+        /// </summary>
+        internal string MetricsHostname { get; }
+
         internal List<string> ValidationWarnings { get; }
     }
 }

tracer/src/Datadog.Trace/TracerManagerFactory.cs

@@ -323,25 +323,20 @@ internal static IDogStatsd CreateDogStatsdClient(ImmutableTracerSettings setting
                 switch (settings.ExporterInternal.MetricsTransport)
                 {
                     case MetricsTransportType.NamedPipe:
-                        Log.Information("Using windows named pipes for metrics transport.");
                         config.PipeName = settings.ExporterInternal.MetricsPipeNameInternal;
+                        Log.Information("Using windows named pipes for metrics transport: {PipeName}.", config.PipeName);
                         break;
 #if NETCOREAPP3_1_OR_GREATER
                     case MetricsTransportType.UDS:
-                        Log.Information("Using unix domain sockets for metrics transport.");
                         config.StatsdServerName = $"{ExporterSettings.UnixDomainSocketPrefix}{settings.ExporterInternal.MetricsUnixDomainSocketPathInternal}";
+                        Log.Information("Using unix domain sockets for metrics transport: {Socket}.", config.StatsdServerName);
                         break;
 #endif
                     case MetricsTransportType.UDP:
                     default:
-                        // 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.StatsdServerName = settings.ExporterInternal.MetricsHostname;
                         config.StatsdPort = settings.ExporterInternal.DogStatsdPortInternal;
+                        Log.Information<string, int>("Using UDP for metrics transport: {Hostname}:{Port}.", config.StatsdServerName, config.StatsdPort);
                         break;
                 }