Rebuild MutableSettings on dynamic config changes

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/Tracer/ConfigureIntegration.cs

@@ -11,6 +11,7 @@
 using Datadog.Trace.Configuration.ConfigurationSources;
 using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
 using Datadog.Trace.Configuration.Telemetry;
+using Datadog.Trace.Logging;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 
@@ -32,6 +33,8 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Tr
 [EditorBrowsable(EditorBrowsableState.Never)]
 public class ConfigureIntegration
 {
+    private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<ConfigureIntegration>();
+
     internal static CallTargetState OnMethodBegin<TTarget>(Dictionary<string, object?> values)
     {
         ConfigureSettingsWithManualOverrides(values, useLegacySettings: false);
@@ -47,18 +50,74 @@ internal static void ConfigureSettingsWithManualOverrides(Dictionary<string, obj
         var isFromDefaults = values.TryGetValue(TracerSettingKeyConstants.IsFromDefaultSourcesKey, out var value) && value is true;
 
         // Build the configuration sources, including our manual instrumentation values
-        ManualInstrumentationConfigurationSourceBase manualConfigSource =
+        ManualInstrumentationConfigurationSourceBase manualConfig =
             useLegacySettings
                 ? new ManualInstrumentationLegacyConfigurationSource(values, isFromDefaults)
                 : new ManualInstrumentationConfigurationSource(values, isFromDefaults);
 
-        IConfigurationSource source = isFromDefaults
-                                          ? new CompositeConfigurationSource([manualConfigSource, GlobalConfigurationSource.Instance])
-                                          : manualConfigSource;
+        // We need to save this immediately, even if there's no manifest changes in the final settings
+        GlobalConfigurationSource.UpdateManualConfigurationSource(manualConfig);
+
+        var tracerSettings = Datadog.Trace.Tracer.Instance.Settings;
+        var dynamicConfig = GlobalConfigurationSource.DynamicConfigurationSource;
+        var initialSettings = isFromDefaults
+                                  ? tracerSettings.InitialMutableSettings
+                                  : MutableSettings.CreateWithoutDefaultSources(tracerSettings);
+
+        // TODO: these will eventually live elsewhere
+        var currentSettings = tracerSettings.MutableSettings;
+
+        var manualTelemetry = new ConfigurationTelemetry();
+        var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
+            dynamicConfig,
+            manualConfig,
+            initialSettings,
+            tracerSettings,
+            manualTelemetry,
+            new OverrideErrorLog()); // TODO: We'll later report these
+
+        var isSameMutableSettings = currentSettings.Equals(newMutableSettings);
+
+        // The only exporter setting we currently _allow_ to change is the AgentUri, but if that does change,
+        // it can mean that _everything_ about the exporter settings changes. To minimize the work to do, and
+        // to simplify comparisons, we try to read the agent url from the manual setting. If it's missing, not
+        // set, or unchanged, there's no need to update the exporter settings. In the future, ExporterSettings
+        // will live separate from TracerSettings entirely.
+        var exporterTelemetry = new ConfigurationTelemetry();
+        var newRawExporterSettings = ExporterSettings.Raw.CreateUpdatedFromManualConfig(
+            tracerSettings.Exporter.RawSettings,
+            manualConfig,
+            exporterTelemetry,
+            isFromDefaults);
+        var isSameExporterSettings = tracerSettings.Exporter.RawSettings.Equals(newRawExporterSettings);
+
+        if (isSameMutableSettings && isSameExporterSettings)
+        {
+            Log.Debug("No changes detected in the new configuration in code");
+            // Even though there were no "real" changes, there may be _effective_ changes in telemetry that
+            // need to be recorded (e.g. the customer set the value in code but it was already set via
+            // env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
+            // which we'll resolve later anyway, so just have that gap for now (it's very niche).
+            // If there are changes, they're recorded automatically in ConfigureInternal
+            manualTelemetry.CopyTo(TelemetryFactory.Config);
+            return;
+        }
 
-        var settings = new TracerSettings(source, new ConfigurationTelemetry(), new OverrideErrorLog());
+        Log.Information("Applying new configuration in code");
+        TracerSettings newSettings;
+        if (isSameExporterSettings)
+        {
+            newSettings = tracerSettings with { MutableSettings = newMutableSettings };
+        }
+        else
+        {
+            var exporterSettings = new ExporterSettings(newRawExporterSettings, exporterTelemetry);
+            newSettings = isSameMutableSettings
+                              ? tracerSettings with { Exporter = exporterSettings }
+                              : tracerSettings with { MutableSettings = newMutableSettings, Exporter = exporterSettings };
+        }
 
         // Update the global instance
-        Trace.Tracer.Configure(settings);
+        Trace.Tracer.Configure(newSettings);
     }
 }

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/GlobalConfigurationSource.cs

@@ -6,8 +6,10 @@
 #nullable enable
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
+using System.Threading;
 using Datadog.Trace.Configuration.ConfigurationSources;
 using Datadog.Trace.Configuration.Telemetry;
 using Datadog.Trace.LibDatadog.HandsOffConfiguration;
@@ -20,13 +22,20 @@ namespace Datadog.Trace.Configuration;
 /// </summary>
 internal class GlobalConfigurationSource
 {
+    private static IConfigurationSource _dynamicConfigConfigurationSource = NullConfigurationSource.Instance;
+    private static ManualInstrumentationConfigurationSourceBase _manualConfigurationSource = new ManualInstrumentationConfigurationSource(new Dictionary<string, object?>(), useDefaultSources: true);
+
     /// <summary>
     /// Gets the configuration source instance.
     /// </summary>
     internal static IConfigurationSource Instance => CreationResult.ConfigurationSource;
 
     internal static GlobalConfigurationSourceResult CreationResult { get; private set; } = CreateDefaultConfigurationSource();
 
+    internal static IConfigurationSource DynamicConfigurationSource => _dynamicConfigConfigurationSource;
+
+    internal static ManualInstrumentationConfigurationSourceBase ManualConfigurationSource => _manualConfigurationSource;
+
     /// <summary>
     /// Creates a <see cref="IConfigurationSource"/> by combining environment variables,
     /// Precedence is as follows:
@@ -133,4 +142,14 @@ private static string GetCurrentDirectory()
     {
         return AppDomain.CurrentDomain.BaseDirectory ?? Directory.GetCurrentDirectory();
     }
+
+    public static void UpdateDynamicConfigConfigurationSource(IConfigurationSource dynamic)
+    {
+        Interlocked.Exchange(ref _dynamicConfigConfigurationSource, dynamic);
+    }
+
+    public static void UpdateManualConfigurationSource(ManualInstrumentationConfigurationSourceBase manual)
+    {
+        Interlocked.Exchange(ref _manualConfigurationSource, manual);
+    }
 }

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

@@ -13,6 +13,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Datadog.Trace.Configuration.ConfigurationSources;
+using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
 using Datadog.Trace.Configuration.Telemetry;
 using Datadog.Trace.Debugger;
 using Datadog.Trace.Debugger.Configurations;
@@ -30,14 +31,12 @@ internal class DynamicConfigurationManager : IDynamicConfigurationManager
         private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<DynamicConfigurationManager>();
 
         private readonly IRcmSubscriptionManager _subscriptionManager;
-        private readonly ConfigurationTelemetry _configurationTelemetry;
         private readonly Dictionary<string, RemoteConfiguration> _activeConfigurations = new();
         private ISubscription? _subscription;
 
         public DynamicConfigurationManager(IRcmSubscriptionManager subscriptionManager)
         {
             _subscriptionManager = subscriptionManager;
-            _configurationTelemetry = new ConfigurationTelemetry();
         }
 
         public void Start()
@@ -68,50 +67,67 @@ public void Dispose()
             }
         }
 
-        internal static void OnlyForTests_ApplyConfiguration(ConfigurationBuilder settings)
+        internal static void OnlyForTests_ApplyConfiguration(IConfigurationSource dynamicConfig)
         {
-            OnConfigurationChanged(settings);
+            OnConfigurationChanged(dynamicConfig);
         }
 
-        private static void OnConfigurationChanged(ConfigurationBuilder settings)
+        private static void OnConfigurationChanged(IConfigurationSource dynamicConfig)
         {
-            var oldSettings = Tracer.Instance.Settings;
-
-            var headerTags = MutableSettings.InitializeHeaderTags(settings, ConfigurationKeys.HeaderTags, headerTagsNormalizationFixEnabled: true);
-            // var serviceNameMappings = TracerSettings.InitializeServiceNameMappings(settings, ConfigurationKeys.ServiceNameMappings);
-
-            var globalTags = settings.WithKeys(ConfigurationKeys.GlobalTags).AsDictionary();
-
-            var dynamicSettings = new ImmutableDynamicSettings
-            {
-                TraceEnabled = settings.WithKeys(ConfigurationKeys.TraceEnabled).AsBool(),
-                // RuntimeMetricsEnabled = settings.WithKeys(ConfigurationKeys.RuntimeMetricsEnabled).AsBool(),
-                // DataStreamsMonitoringEnabled = settings.WithKeys(ConfigurationKeys.DataStreamsMonitoring.Enabled).AsBool(),
-                // Note: Calling GetAsClass<string>() here instead of GetAsString() as we need to get the
-                // "serialized JToken", which in JsonConfigurationSource is different, as it allows for non-string tokens
-                SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAsClass<string>(validator: null, converter: s => s),
-                GlobalSamplingRate = settings.WithKeys(ConfigurationKeys.GlobalSamplingRate).AsDouble(),
-                // SpanSamplingRules = settings.WithKeys(ConfigurationKeys.SpanSamplingRules).AsString(),
-                LogsInjectionEnabled = settings.WithKeys(ConfigurationKeys.LogsInjectionEnabled).AsBool(),
-                HeaderTags = headerTags,
-                // ServiceNameMappings = serviceNameMappings == null ? null : new ReadOnlyDictionary<string, string>(serviceNameMappings)
-                GlobalTags = globalTags == null ? null : new ReadOnlyDictionary<string, string>(globalTags)
-            };
+            var tracerSettings = Tracer.Instance.Settings;
+            var manualSource = GlobalConfigurationSource.ManualConfigurationSource;
+            var mutableSettings = manualSource.UseDefaultSources
+                                      ? tracerSettings.InitialMutableSettings
+                                      : MutableSettings.CreateWithoutDefaultSources(tracerSettings);
+
+            // We save this immediately, even if there's no manifest changes in the final settings
+            GlobalConfigurationSource.UpdateDynamicConfigConfigurationSource(dynamicConfig);
+
+            OnConfigurationChanged(
+                dynamicConfig,
+                manualSource,
+                mutableSettings,
+                tracerSettings,
+                // TODO: In the future this will 'live' elsewhere
+                currentSettings: tracerSettings.MutableSettings,
+                new ConfigurationTelemetry(),
+                new OverrideErrorLog()); // TODO: We'll later report these
+        }
 
-            // Needs to be done before returning, to feed the value to the telemetry
-            // var debugLogsEnabled = settings.WithKeys(ConfigurationKeys.DebugEnabled).AsBool();
+        private static void OnConfigurationChanged(
+            IConfigurationSource dynamicConfig,
+            ManualInstrumentationConfigurationSourceBase manualConfig,
+            MutableSettings initialSettings,
+            TracerSettings tracerSettings,
+            MutableSettings currentSettings,
+            ConfigurationTelemetry telemetry,
+            OverrideErrorLog errorLog)
+        {
+            var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
+                dynamicConfig,
+                manualConfig,
+                initialSettings,
+                tracerSettings,
+                telemetry,
+                errorLog);
 
             TracerSettings newSettings;
-            if (dynamicSettings.Equals(oldSettings.DynamicSettings))
+            if (currentSettings.Equals(newMutableSettings))
             {
                 Log.Debug("No changes detected in the new dynamic configuration");
-                newSettings = oldSettings;
+                // Even though there were no "real" changes, there may be _effective_ changes in telemetry that
+                // need to be recorded (e.g. the customer set the value in code but it was already set via
+                // env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
+                // which we'll resolve later anyway, so just have that gap for now (it's very niche).
+                // If there are changes, they're recorded automatically in Tracer.Configure()
+                telemetry.CopyTo(TelemetryFactory.Config);
+                newSettings = tracerSettings;
             }
             else
             {
                 Log.Information("Applying new dynamic configuration");
 
-                newSettings = oldSettings with { DynamicSettings = dynamicSettings };
+                newSettings = tracerSettings with { MutableSettings = newMutableSettings };
 
                 /*
                 if (debugLogsEnabled != null && debugLogsEnabled.Value != GlobalSettings.Instance.DebugEnabled)
@@ -126,6 +142,9 @@ private static void OnConfigurationChanged(ConfigurationBuilder settings)
                 Tracer.Configure(newSettings);
             }
 
+            // TODO: This might not record the config in the correct order in future, but would require
+            // a big refactoring of debugger settings to resolve
+            var settings = new ConfigurationBuilder(dynamicConfig, TelemetryFactory.Config);
             var dynamicDebuggerSettings = new ImmutableDynamicDebuggerSettings
             {
                 DynamicInstrumentationEnabled = settings.WithKeys(ConfigurationKeys.Debugger.DynamicInstrumentationEnabled).AsBool(),
@@ -238,12 +257,8 @@ private void ApplyMergedConfiguration(List<RemoteConfiguration> remoteConfigurat
                 environment);
 
             var configurationSource = new DynamicConfigConfigurationSource(mergedConfigJToken, ConfigurationOrigins.RemoteConfig);
-            var configurationBuilder = new ConfigurationBuilder(configurationSource, _configurationTelemetry);
-
-            OnConfigurationChanged(configurationBuilder);
 
-            _configurationTelemetry.CopyTo(TelemetryFactory.Config);
-            _configurationTelemetry.Clear();
+            OnConfigurationChanged(configurationSource);
         }
     }
 }