Make ExporterSettings properly immutable and remove ImmutableExporterSettings (#6408)

## Summary of changes

Merge `ExporterSettings` with `ImmutableExporterSettings`

## Reason for change

This stack of PRs is about doing one-shot configuration instead of
mutation. We never mutate these in the tracer after creation, so there's
no need for the separate types.

## Implementation details

- Made the properties in `ExporterSettings` get-only. This required
quite a lot of work because we were doing a lot of mutating of the
settings in the "helper" functions.
- I only _lightly_ refactored those methods (as much as possible) to
avoid setting the properties in the functions and instead returning the
details to set later.
- These are prime candidates for some _much_ heavier refactoring later,
but I didn't want to get bogged down with that in this PR
- Replace all usages of `Immutable*` with `ExporterSettings`
- Replace usages of `AgentUriInternal` with `AgentUri`
- Move `Immutable*Tests` into appropriate file and tweak
- Replace mutations with initialization

## Test coverage

All covered by existing details

## Other details

Part of Stack
- #6370
- #6376
- #6385
- #6386
- #6397 
- #6399
- #6400
- #6405
- #6408 👈 This PR
- #6415

Author: andrewlock

tracer/src/Datadog.Trace.Tools.Runner/Utils.cs

@@ -425,7 +425,7 @@ public static string GetEnvironmentVariable(string key, string defaultValue = nu
             using (cts.Token.Register(
                        () =>
                        {
-                           WriteError($"Error connecting to the Datadog Agent at {tracerSettings.Exporter.AgentUriInternal}.");
+                           WriteError($"Error connecting to the Datadog Agent at {tracerSettings.Exporter.AgentUri}.");
                            tcs.TrySetResult(null);
                        }))
             {

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

@@ -21,8 +21,6 @@ public partial class ExporterSettings
     private readonly Func<string, bool> _fileExists = File.Exists;
     private readonly DummyTelemetry _telemetry = new();
 
-    private Uri _agentUri;
-
     /// <summary>
     /// Initializes a new instance of the <see cref="ExporterSettings"/> class.
     /// </summary>
@@ -37,7 +35,11 @@ public ExporterSettings(IConfigurationSource? configuration)
 
         int.TryParse(agentPortStr, out var agentPort);
 
-        ConfigureTraceTransport(agentUri, tracePipeName, agentHost, agentPort, unixDomainSocketPath);
+        var traceSettings = GetTraceTransport(agentUri, tracePipeName, agentHost, agentPort, unixDomainSocketPath);
+        TracesTransport = traceSettings.Transport;
+        TracesPipeName = traceSettings.PipeName;
+        TracesUnixDomainSocketPath = traceSettings.UdsPath;
+        AgentUri = traceSettings.AgentUri;
     }
 
     /// <summary>
@@ -46,23 +48,27 @@ public ExporterSettings(IConfigurationSource? configuration)
     /// <param name="agentUri">Agent URI</param>
     public ExporterSettings(string agentUri)
     {
-        ConfigureTraceTransport(agentUri, null, null, null, null);
+        var traceSettings = GetTraceTransport(agentUri, null, null, null, null);
+        TracesTransport = traceSettings.Transport;
+        TracesPipeName = traceSettings.PipeName;
+        TracesUnixDomainSocketPath = traceSettings.UdsPath;
+        AgentUri = traceSettings.AgentUri;
     }
 
     internal enum TelemetryErrorCode
     {
         PotentiallyInvalidUdsPath
     }
 
-    internal Uri AgentUri => _agentUri;
+    internal Uri AgentUri { get; }
 
-    private List<string> ValidationWarnings { get; set; } = new();
+    private List<string> ValidationWarnings { get; } = new();
 
-    internal TracesTransportType TracesTransport { get; private set; }
+    internal TracesTransportType TracesTransport { get;  }
 
-    internal string? TracesPipeName { get; private set; }
+    internal string? TracesPipeName { get; }
 
-    internal string? TracesUnixDomainSocketPath { get; private set; }
+    internal string? TracesUnixDomainSocketPath { get; }
 
     private static string? GetValue(IConfigurationSource? configuration, params string[] keys)
     {

tracer/src/Datadog.Trace/Agent/AgentTransportStrategy.cs

@@ -31,10 +31,10 @@ internal static class AgentTransportStrategy
     /// <param name="getHttpHeaderHelper">A func that returns an <see cref="HttpHeaderHelperBase"/> for use
     /// with <see cref="DatadogHttpClient"/></param>
     /// <param name="getBaseEndpoint">A func that returns the endpoint to send requests to for a given "base" endpoint.
-    /// The base endpoint will be <see cref="ImmutableExporterSettings.AgentUri" /> for TCP requests and
+    /// The base endpoint will be <see cref="ExporterSettings.AgentUri" /> for TCP requests and
     /// http://localhost/ for named pipes/UDS</param>
     public static IApiRequestFactory Get(
-        ImmutableExporterSettings settings,
+        ExporterSettings settings,
         string productName,
         TimeSpan? tcpTimeout,
         KeyValuePair<string, string>[] defaultAgentHeaders,

tracer/src/Datadog.Trace/Agent/DiscoveryService/DiscoveryService.cs

@@ -77,7 +77,7 @@ public DiscoveryService(
                 SupportedTracerFlareEndpoint,
             };
 
-        public static DiscoveryService Create(ImmutableExporterSettings exporterSettings)
+        public static DiscoveryService Create(ExporterSettings exporterSettings)
             => Create(
                 exporterSettings,
                 tcpTimeout: TimeSpan.FromSeconds(15),
@@ -86,7 +86,7 @@ public static DiscoveryService Create(ImmutableExporterSettings exporterSettings
                 recheckIntervalMs: 30_000);
 
         public static DiscoveryService Create(
-            ImmutableExporterSettings exporterSettings,
+            ExporterSettings exporterSettings,
             TimeSpan tcpTimeout,
             int initialRetryDelayMs,
             int maxRetryDelayMs,

tracer/src/Datadog.Trace/Agent/TracesTransportStrategy.cs

@@ -11,7 +11,7 @@ namespace Datadog.Trace.Agent
 {
     internal static class TracesTransportStrategy
     {
-        public static IApiRequestFactory Get(ImmutableExporterSettings settings)
+        public static IApiRequestFactory Get(ExporterSettings settings)
             => AgentTransportStrategy.Get(
                 settings,
                 productName: "trace",

tracer/src/Datadog.Trace/Ci/Agent/Payloads/EventPlatformPayload.cs

@@ -158,7 +158,7 @@ private void EnsureUrl()
                     break;
                 case TracesTransportType.Default:
                 default:
-                    builder = new UriBuilder(_settings.TracerSettings.Exporter.AgentUriInternal);
+                    builder = new UriBuilder(_settings.TracerSettings.Exporter.AgentUri);
                     break;
             }
 

tracer/src/Datadog.Trace/Ci/CIVisibility.cs

@@ -110,7 +110,7 @@ public static void Initialize()
                 else
                 {
                     discoveryService = DiscoveryService.Create(
-                        new ImmutableExporterSettings(settings.TracerSettings.Exporter, true),
+                        settings.TracerSettings.Exporter,
                         tcpTimeout: TimeSpan.FromSeconds(5),
                         initialRetryDelayMs: 10,
                         maxRetryDelayMs: 1000,

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/Configuration/TracerSettings/PopulateDictionaryIntegration.cs

@@ -45,7 +45,7 @@ internal static CallTargetState OnMethodBegin<TTarget>(Dictionary<string, object
     internal static void PopulateSettings(Dictionary<string, object?> values, Trace.Configuration.TracerSettings settings)
     {
         // record all the settings in the dictionary
-        values[TracerSettingKeyConstants.AgentUriKey] = settings.Exporter.AgentUriInternal;
+        values[TracerSettingKeyConstants.AgentUriKey] = settings.Exporter.AgentUri;
 #pragma warning disable CS0618 // Type or member is obsolete
         values[TracerSettingKeyConstants.AnalyticsEnabledKey] = settings.AnalyticsEnabled;
 #pragma warning restore CS0618 // Type or member is obsolete

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

@@ -20,12 +20,12 @@ namespace Datadog.Trace.Configuration
     public partial class ExporterSettings
     {
         /// <summary>
-        /// The default host value for <see cref="AgentUriInternal"/>.
+        /// The default host value for <see cref="AgentUri"/>.
         /// </summary>
         public const string DefaultAgentHost = "localhost";
 
         /// <summary>
-        /// The default port value for <see cref="AgentUriInternal"/>.
+        /// The default port value for <see cref="AgentUri"/>.
         /// </summary>
         public const int DefaultAgentPort = 8126;
 
@@ -39,8 +39,7 @@ public partial class ExporterSettings
         /// </summary>
         internal const string DefaultTracesUnixDomainSocket = "/var/run/datadog/apm.socket";
 
-        [MemberNotNull(nameof(_agentUri))]
-        private void ConfigureTraceTransport(string? agentUri, string? tracesPipeName, string? agentHost, int? agentPort, string? tracesUnixDomainSocketPath)
+        private TraceTransportSettings GetTraceTransport(string? agentUri, string? tracesPipeName, string? agentHost, int? agentPort, string? tracesUnixDomainSocketPath)
         {
             var origin = ConfigurationOrigins.Default; // default because only called from constructor
 
@@ -49,37 +48,37 @@ private void ConfigureTraceTransport(string? agentUri, string? tracesPipeName, s
             // 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.
             if (!string.IsNullOrWhiteSpace(agentUri))
             {
-                if (TrySetAgentUriAndTransport(agentUri!, origin))
+                if (TryGetAgentUriAndTransport(agentUri!, origin, out var settings))
                 {
-                    return;
+                    return settings;
                 }
             }
 
             if (!string.IsNullOrWhiteSpace(tracesPipeName))
             {
-                TracesTransport = TracesTransportType.WindowsNamedPipe;
-                TracesPipeName = tracesPipeName;
                 RecordTraceTransport(nameof(TracesTransportType.WindowsNamedPipe), origin);
 
                 // The Uri isn't needed anymore in that case, just populating it for retro compatibility.
                 if (!Uri.TryCreate($"http://{agentHost ?? DefaultAgentHost}:{agentPort ?? DefaultAgentPort}", UriKind.Absolute, out var uri))
                 {
-                    // fallback so _agentUri is always non-null
+                    // fallback so AgentUri is always non-null
                     uri = CreateDefaultUri();
                 }
 
-                SetAgentUriReplacingLocalhost(uri, origin);
-                return;
+                return new TraceTransportSettings(
+                    TracesTransportType.WindowsNamedPipe,
+                    GetAgentUriReplacingLocalhost(uri, origin),
+                    PipeName: tracesPipeName);
             }
 
             // This property shouldn't have been introduced. We need to remove it as part of 3.0
             // But while it's here, we need to handle it properly
             if (!string.IsNullOrWhiteSpace(tracesUnixDomainSocketPath))
             {
 #if NETCOREAPP3_1_OR_GREATER
-                if (TrySetAgentUriAndTransport(UnixDomainSocketPrefix + tracesUnixDomainSocketPath, origin))
+                if (TryGetAgentUriAndTransport(UnixDomainSocketPrefix + tracesUnixDomainSocketPath, origin, out var settings))
                 {
-                    return;
+                    return settings;
                 }
 #else
                 // .NET Core 2.1 and .NET FX don't support Unix Domain Sockets
@@ -99,9 +98,9 @@ private void ConfigureTraceTransport(string? agentUri, string? tracesPipeName, s
                 // The agent will fail to start if it can not bind a port, so we need to override 8126 to prevent port conflict
                 // Port 0 means it will pick some random available port
 
-                if (TrySetAgentUriAndTransport(agentHost ?? DefaultAgentHost, agentPort ?? DefaultAgentPort))
+                if (TryGetAgentUriAndTransport(agentHost ?? DefaultAgentHost, agentPort ?? DefaultAgentPort, out var settings))
                 {
-                    return;
+                    return settings;
                 }
             }
 
@@ -111,45 +110,44 @@ private void ConfigureTraceTransport(string? agentUri, string? tracesPipeName, s
             {
                 // setting the urls as well for retro compatibility in the almost impossible case where someone
                 // used this config and accessed the AgentUri property as well (to avoid a potential null ref)
-                // Using Set not TrySet because we know this is a valid Uri and ensures _agentUri is always non-null
-                SetAgentUriAndTransport(new Uri(UnixDomainSocketPrefix + DefaultTracesUnixDomainSocket), origin);
-                return;
+                // Using Get not TryGet because we know this is a valid Uri and ensures _agentUri is always non-null
+                return GetAgentUriAndTransport(new Uri(UnixDomainSocketPrefix + DefaultTracesUnixDomainSocket), origin);
             }
 #endif
 
             ValidationWarnings.Add("No transport configuration found, using default values");
 
             // we know this URL is valid so don't use TrySet, otherwise can't guarantee _agentUri is non null
-            SetAgentUriAndTransport(CreateDefaultUri(), origin);
+            return GetAgentUriAndTransport(CreateDefaultUri(), origin);
         }
 
-        [MemberNotNullWhen(true, nameof(_agentUri))]
-        private bool TrySetAgentUriAndTransport(string host, int port)
+        private bool TryGetAgentUriAndTransport(string host, int port, out TraceTransportSettings settings)
         {
-            return TrySetAgentUriAndTransport($"http://{host}:{port}", ConfigurationOrigins.Default); // default because only called from constructor
+            return TryGetAgentUriAndTransport($"http://{host}:{port}", ConfigurationOrigins.Default, out settings); // default because only called from constructor
         }
 
-        [MemberNotNullWhen(true, nameof(_agentUri))]
-        private bool TrySetAgentUriAndTransport(string url, ConfigurationOrigins origin)
+        private bool TryGetAgentUriAndTransport(string url, ConfigurationOrigins origin, out TraceTransportSettings settings)
         {
             if (!Uri.TryCreate(url, UriKind.Absolute, out var uri))
             {
                 ValidationWarnings.Add($"The Uri: '{url}' is not valid. It won't be taken into account to send traces. Note that only absolute urls are accepted.");
+                settings = default;
                 return false;
             }
 
-            SetAgentUriAndTransport(uri, ConfigurationOrigins.Default); // default because only called from constructor
+            settings = GetAgentUriAndTransport(uri, ConfigurationOrigins.Default); // default because only called from constructor
             return true;
         }
 
-        [MemberNotNull(nameof(_agentUri))]
-        private void SetAgentUriAndTransport(Uri uri, ConfigurationOrigins origin)
+        private TraceTransportSettings GetAgentUriAndTransport(Uri uri, ConfigurationOrigins origin)
         {
+            TracesTransportType transport;
+            string? udsPath;
             if (uri.OriginalString.StartsWith(UnixDomainSocketPrefix, StringComparison.OrdinalIgnoreCase))
             {
 #if NETCOREAPP3_1_OR_GREATER
-                TracesTransport = TracesTransportType.UnixDomainSocket;
-                TracesUnixDomainSocketPath = uri.PathAndQuery;
+                transport = TracesTransportType.UnixDomainSocket;
+                udsPath = uri.PathAndQuery;
 
                 var absoluteUri = uri.AbsoluteUri.Replace(UnixDomainSocketPrefix, string.Empty);
                 bool potentiallyInvalid = false;
@@ -184,39 +182,43 @@ private void SetAgentUriAndTransport(Uri uri, ConfigurationOrigins origin)
                     recordValue: true,
                     origin,
                     TelemetryErrorCode.UdsOnUnsupportedPlatform);
-                SetAgentUriAndTransport(CreateDefaultUri(), ConfigurationOrigins.Calculated);
-                return;
+                return GetAgentUriAndTransport(CreateDefaultUri(), ConfigurationOrigins.Calculated);
 #endif
             }
             else
             {
-                TracesTransport = TracesTransportType.Default;
+                transport = TracesTransportType.Default;
+                udsPath = null;
                 RecordTraceTransport(nameof(TracesTransportType.Default), origin);
             }
 
-            SetAgentUriReplacingLocalhost(uri, origin);
+            var agentUri = GetAgentUriReplacingLocalhost(uri, origin);
+            return new(transport, agentUri, UdsPath: udsPath);
         }
 
-        [MemberNotNull(nameof(_agentUri))]
-        private void SetAgentUriReplacingLocalhost(Uri uri, ConfigurationOrigins origin)
+        private Uri GetAgentUriReplacingLocalhost(Uri uri, ConfigurationOrigins origin)
         {
+            Uri agentUri;
             if (string.Equals(uri.Host, "localhost", StringComparison.OrdinalIgnoreCase))
             {
                 // Replace localhost with 127.0.0.1 to avoid DNS resolution.
                 // When ipv6 is enabled, localhost is first resolved to ::1, which fails
                 // because the trace agent is only bound to ipv4.
                 // This causes delays when sending traces.
                 var builder = new UriBuilder(uri) { Host = "127.0.0.1" };
-                _agentUri = builder.Uri;
+                agentUri = builder.Uri;
             }
             else
             {
-                _agentUri = uri;
+                agentUri = uri;
             }
 
-            _telemetry.Record(ConfigurationKeys.AgentUri, _agentUri.ToString(), recordValue: true, origin);
+            _telemetry.Record(ConfigurationKeys.AgentUri, agentUri.ToString(), recordValue: true, origin);
+            return agentUri;
         }
 
         private Uri CreateDefaultUri() => new Uri($"http://{DefaultAgentHost}:{DefaultAgentPort}");
+
+        private readonly record struct TraceTransportSettings(TracesTransportType Transport, Uri AgentUri, string? UdsPath = null, string? PipeName = null);
     }
 }