Fix manual instrumentation not reporting the correct settings

Author: andrewlock

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

@@ -36,17 +36,19 @@ internal static CallTargetState OnMethodBegin<TTarget>(TTarget instance, object?
         // but in 3.7.0 we don't need to instrument them
         if (automaticTracer is Datadog.Trace.Tracer tracer)
         {
-            PopulateSettings(values, tracer.Settings);
+            PopulateSettings(values, tracer);
         }
 
         return CallTargetState.GetDefault();
     }
 
-    internal static void PopulateSettings(Dictionary<string, object?> values, TracerSettings settings)
+    internal static void PopulateSettings(Dictionary<string, object?> values, Datadog.Trace.Tracer tracer)
     {
         // record all the settings in the dictionary
-        var mutableSettings = settings.Manager.InitialMutableSettings;
-        var exporterSettings = settings.Manager.InitialExporterSettings;
+        var mutableSettings = tracer.CurrentTraceSettings.Settings;
+        // TODO: This doesn't get the "current" exporter settings, if they've been changed for code.
+        // We don't currently provide a way to do that without subscribing to all changes, which would be overkill here.
+        var exporterSettings = tracer.Settings.Manager.InitialExporterSettings;
         // This key is used to detect if the settings have been populated _at all_, so should always be sent
         values[TracerSettingKeyConstants.AgentUriKey] = exporterSettings.AgentUri;
 #pragma warning disable CS0618 // Type or member is obsolete
@@ -64,7 +66,7 @@ internal static void PopulateSettings(Dictionary<string, object?> values, Tracer
         values[TracerSettingKeyConstants.ServiceNameKey] = mutableSettings.ServiceName;
         values[TracerSettingKeyConstants.ServiceVersionKey] = mutableSettings.ServiceVersion;
         values[TracerSettingKeyConstants.StartupDiagnosticLogEnabledKey] = mutableSettings.StartupDiagnosticLogEnabled;
-        values[TracerSettingKeyConstants.StatsComputationEnabledKey] = settings.StatsComputationEnabled;
+        values[TracerSettingKeyConstants.StatsComputationEnabledKey] = tracer.Settings.StatsComputationEnabled;
         values[TracerSettingKeyConstants.TraceEnabledKey] = mutableSettings.TraceEnabled;
         values[TracerSettingKeyConstants.TracerMetricsEnabledKey] = mutableSettings.TracerMetricsEnabled;
 

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

@@ -29,12 +29,22 @@ public class GetUpdatedImmutableTracerSettingsIntegration
 {
     internal static CallTargetState OnMethodBegin<TTarget>(TTarget instance, ref object? automaticTracer, ref object? automaticSettings)
     {
+        // In previous versions of Datadog.Trace (<3.30.0), we would replace the entire TracerSettings
+        // object whenever the settings changed, and use that to track whether we need to update the manual
+        // side. Now, TracerSettings is immutable for the lifetime of the application, and we instead
+        // update the MutableSettings and ExporterSettings whenever something changes. To keep roughly the same
+        // compatible behaviour here, we now store the current MutableSettings in `automaticSettings` to track
+        // whether things need to update. Note that this isn't _strictly_ correct, because if the customer updates
+        // only the exporter settings, we won't track that it's changed here. However, in PopulateSettings we _also_
+        // don't populate the latest exporter settings there, so that's ok! Setting the exporter settings in code is
+        // deprecated (as it's problematic for a bunch of reasons), but it's still possible, so this is a half-way
+        // house way to handle it.
         if (automaticTracer is Datadog.Trace.Tracer tracer
-         && (automaticSettings is null || !ReferenceEquals(tracer.Settings, automaticSettings)))
+         && (automaticSettings is null || !ReferenceEquals(tracer.CurrentTraceSettings.Settings, automaticSettings)))
         {
-            automaticSettings = tracer.Settings;
+            automaticSettings = tracer.CurrentTraceSettings.Settings;
             var dict = new Dictionary<string, object?>();
-            CtorIntegration.PopulateSettings(dict, tracer.Settings);
+            CtorIntegration.PopulateSettings(dict, tracer);
             return new CallTargetState(scope: null, state: dict);
         }
 

tracer/test/Datadog.Trace.Tests/ManualInstrumentation/SettingsInstrumentationTests.cs

@@ -8,12 +8,14 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation;
 using Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Configuration.TracerSettings;
 using Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Tracer;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.Configuration.ConfigurationSources;
 using Datadog.Trace.Telemetry.Metrics;
+using Datadog.Trace.TestHelpers.TestTracer;
 using FluentAssertions;
 using Xunit;
 using CtorIntegration = Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Tracer.CtorIntegration;
@@ -219,12 +221,13 @@ public void ManualToAutomatic_CustomSettingsAreTransferredCorrectly(bool useLega
     }
 
     [Fact]
-    public void AutomaticToManual_ImmutableSettingsAreTransferredCorrectly()
+    public async Task AutomaticToManual_ImmutableSettingsAreTransferredCorrectly()
     {
         var automatic = GetAndAssertAutomaticTracerSettings();
+        await using var tracer = TracerHelper.Create(automatic);
 
         Dictionary<string, object> serializedSettings = new();
-        CtorIntegration.PopulateSettings(serializedSettings, automatic);
+        CtorIntegration.PopulateSettings(serializedSettings, tracer);
 
         var manual = new ImmutableManualSettings(serializedSettings);