Initialize SymbolUploader only if DI is explicitly enabled

Author: dudikeleti

tracer/src/Datadog.Trace/Debugger/DebuggerManager.cs

@@ -41,13 +41,15 @@ internal class DebuggerManager
         private readonly TaskCompletionSource<bool> _processExit;
         private volatile bool _isDebuggerEndpointAvailable;
         private int _initialized;
+        private int _symDbInitialized;
         private volatile TaskCompletionSource<bool>? _diDebounceGate;
         private volatile DynamicInstrumentation? _dynamicInstrumentation;
         private int _diState; // 0 = disabled, 1 = initializing, 2 = initialized
 
         private DebuggerManager(DebuggerSettings debuggerSettings, ExceptionReplaySettings exceptionReplaySettings)
         {
             _initialized = 0;
+            _symDbInitialized = 0;
             _isDebuggerEndpointAvailable = false;
             DebuggerSettings = debuggerSettings;
             ExceptionReplaySettings = exceptionReplaySettings;
@@ -157,6 +159,8 @@ private Task UpdateProductsState(TracerSettings tracerSettings, DebuggerSettings
 
             OneTimeSetup(tracerSettings);
 
+            InitializeSymbolUploaderIfNeeded(tracerSettings, newDebuggerSettings);
+
             lock (_syncLock)
             {
                 if (_processExit.Task.IsCompleted)
@@ -181,19 +185,31 @@ private void OneTimeSetup(TracerSettings tracerSettings)
 
             LifetimeManager.Instance.AddShutdownTask(ShutdownTasks);
             SetGeneralConfig(tracerSettings, DebuggerSettings);
-            InitializeSymbolUploader(tracerSettings);
             if (tracerSettings.StartupDiagnosticLogEnabled)
             {
                 _ = Task.Run(WriteStartupDebuggerDiagnosticLog);
             }
         }
 
-        private void InitializeSymbolUploader(TracerSettings tracerSettings)
+        private void InitializeSymbolUploaderIfNeeded(TracerSettings tracerSettings, DebuggerSettings newDebuggerSettings)
         {
             try
             {
-                if (_processExit.Task.IsCompleted || !DebuggerSettings.SymbolDatabaseUploadEnabled)
+                if (_processExit.Task.IsCompleted)
+                {
+                    return;
+                }
+
+                if (Interlocked.CompareExchange(ref _symDbInitialized, 1, 0) != 0)
+                {
+                    // Once created, the symbol uploader persists even if DI is later disabled
+                    return;
+                }
+
+                if (!DebuggerSettings.SymbolDatabaseUploadEnabled
+                 || !newDebuggerSettings.DynamicInstrumentationCanBeEnabled)
                 {
+                    // explicitly disabled via local env var or DI can not be enabled
                     return;
                 }
 
@@ -203,14 +219,21 @@ private void InitializeSymbolUploader(TracerSettings tracerSettings)
                     return;
                 }
 
+                if (!newDebuggerSettings.DynamicInstrumentationEnabled
+                 && newDebuggerSettings.DynamicSettings.DynamicInstrumentationEnabled != true)
+                {
+                    return;
+                }
+
+                // initialize symbol database uploader only if DI is enabled locally or remotely
                 var tracerManager = TracerManager.Instance;
-                SymbolsUploader = DebuggerFactory.CreateSymbolsUploader(tracerManager.DiscoveryService, RcmSubscriptionManager.Instance, ServiceName, tracerSettings, DebuggerSettings, tracerManager.GitMetadataTagsProvider);
-                _ = SymbolsUploader.StartFlushingAsync()
-                                   .ContinueWith(
-                                        t => Log.Error(t?.Exception, "Failed to initialize symbol uploader"),
-                                        CancellationToken.None,
-                                        TaskContinuationOptions.OnlyOnFaulted,
-                                        TaskScheduler.Default);
+                this.SymbolsUploader = DebuggerFactory.CreateSymbolsUploader(tracerManager.DiscoveryService, RcmSubscriptionManager.Instance, this.ServiceName, tracerSettings, this.DebuggerSettings, tracerManager.GitMetadataTagsProvider);
+                _ = this.SymbolsUploader.StartFlushingAsync()
+                        .ContinueWith(
+                             t => Log.Error(t?.Exception, "Failed to initialize symbol uploader"),
+                             CancellationToken.None,
+                             TaskContinuationOptions.OnlyOnFaulted,
+                             TaskScheduler.Default);
             }
             catch (Exception ex)
             {
@@ -377,7 +400,7 @@ private bool ShouldSkipDiUpdate(bool requested, bool current, DebuggerSettings d
         {
             if (requested && !debuggerSettings.DynamicInstrumentationCanBeEnabled)
             {
-                Log.Debug("Code Origin can't be enabled because the local environment variable is set to false");
+                Log.Debug("Dynamic Instrumentation can't be enabled because the local environment variable is set to false");
                 return true;
             }
 

tracer/test/Datadog.Trace.Debugger.IntegrationTests/DebuggerManagerDynamicTests.cs

@@ -66,9 +66,15 @@ await RunDynamicConfigurationTest(
                 // Initially, no DI objects should exist
                 memoryAssertions.NoObjectsExist<DynamicInstrumentation>();
                 memoryAssertions.NoObjectsExist<LineProbeResolver>();
+                memoryAssertions.NoObjectsExist<Symbols.SymbolsUploader>();
             },
             remoteConfig: new { DD_DYNAMIC_INSTRUMENTATION_ENABLED = true },
-            DynamicInstrumentationEnabledLogEntry);
+            DynamicInstrumentationEnabledLogEntry,
+            finalMemoryAssertions: memoryAssertions =>
+            {
+                // After enabling DI via remote config, symbol uploader should be created
+                memoryAssertions.ObjectsExist<Symbols.SymbolsUploader>();
+            });
     }
 
     [SkippableFact]
@@ -107,7 +113,7 @@ public async Task DebuggerManager_CodeOrigin_StartDisabled_EnabledViaRemoteConfi
 
         SetEnvironmentVariable(ConfigurationKeys.Rcm.RemoteConfigurationEnabled, "true");
 
-        await this.RunDynamicConfigurationTest(
+        await RunDynamicConfigurationTest(
             false,
             initialMemoryAssertions: memoryAssertions =>
             {
@@ -140,6 +146,7 @@ await RunDynamicConfigurationTest(
                 memoryAssertions.NoObjectsExist<SpanCodeOrigin.SpanCodeOrigin>();
                 memoryAssertions.NoObjectsExist<SnapshotSink>();
                 memoryAssertions.NoObjectsExist<LineProbeResolver>();
+                memoryAssertions.NoObjectsExist<Symbols.SymbolsUploader>();
             },
             remoteConfig: new
             {
@@ -156,6 +163,7 @@ await RunDynamicConfigurationTest(
                 memoryAssertions.ObjectsExist<SpanCodeOrigin.SpanCodeOrigin>();
                 memoryAssertions.ObjectsExist<SnapshotSink>();
                 memoryAssertions.ObjectsExist<LineProbeResolver>();
+                memoryAssertions.ObjectsExist<Symbols.SymbolsUploader>();
             });
     }
 
@@ -169,6 +177,7 @@ public async Task DebuggerManager_DynamicInstrumentation_StartEnabled_DisabledVi
 #if NET8_0_OR_GREATER
         Skip.If(!EnvironmentTools.IsTestTarget64BitProcess());
 #endif
+
         // Start with DI enabled via environment variable
         SetEnvironmentVariable(ConfigurationKeys.Debugger.DynamicInstrumentationEnabled, "true");
         SetEnvironmentVariable(ConfigurationKeys.Rcm.RemoteConfigurationEnabled, "true");
@@ -181,9 +190,16 @@ await RunDynamicConfigurationTest(
                 memoryAssertions.ObjectsExist<DynamicInstrumentation>();
                 memoryAssertions.ObjectsExist<SnapshotSink>();
                 memoryAssertions.ObjectsExist<LineProbeResolver>();
+                memoryAssertions.ObjectsExist<Symbols.SymbolsUploader>();
             },
             remoteConfig: new { DD_DYNAMIC_INSTRUMENTATION_ENABLED = false },
-            $"Dynamic Instrumentation {DisabledByRemoteConfiguration}");
+            $"Dynamic Instrumentation {DisabledByRemoteConfiguration}",
+            finalMemoryAssertions: memoryAssertions =>
+            {
+                // After disabling DI, symbol uploader should still exist (it's not disposed when DI is disabled)
+                // This is because symbol uploader initialization is one-time only
+                memoryAssertions.ObjectsExist<Symbols.SymbolsUploader>();
+            });
     }
 
     [SkippableFact]
@@ -200,7 +216,7 @@ public async Task DebuggerManager_ExceptionReplay_StartEnabled_DisabledViaRemote
         SetEnvironmentVariable(ConfigurationKeys.Debugger.ExceptionReplayEnabled, "true");
         SetEnvironmentVariable(ConfigurationKeys.Rcm.RemoteConfigurationEnabled, "true");
 
-        await this.RunDynamicConfigurationTest(
+        await RunDynamicConfigurationTest(
             true,
             initialMemoryAssertions: memoryAssertions =>
             {
@@ -225,7 +241,7 @@ public async Task DebuggerManager_CodeOrigin_StartEnabled_DisabledViaRemoteConfi
         SetEnvironmentVariable(ConfigurationKeys.Rcm.RemoteConfigurationEnabled, "true");
         SetEnvironmentVariable(ConfigurationKeys.Debugger.CodeOriginForSpansEnabled, "true");
 
-        await this.RunDynamicConfigurationTest(
+        await RunDynamicConfigurationTest(
             true,
             initialMemoryAssertions: memoryAssertions =>
             {

tracer/test/Datadog.Trace.Debugger.IntegrationTests/DebuggerManagerTests.cs

@@ -46,6 +46,7 @@ public DebuggerManagerTests(ITestOutputHelper output)
     public async Task DebuggerManager_AllFeaturesByDefault_NoDebuggerObjectsCreated()
     {
 #if NET8_0_OR_GREATER
+
         // These tests often hang on x86 on .NET 8+. Needs investigation
         Skip.If(!EnvironmentTools.IsTestTarget64BitProcess());
 #endif
@@ -55,8 +56,8 @@ await RunDebuggerManagerTestWithMemoryAssertions(memoryAssertions =>
             memoryAssertions.NoObjectsExist<LineProbeResolver>();
             memoryAssertions.NoObjectsExist<DynamicInstrumentation>();
             memoryAssertions.NoObjectsExist<ExceptionAutoInstrumentation.ExceptionReplay>();
+            memoryAssertions.NoObjectsExist<Symbols.SymbolsUploader>();
             memoryAssertions.ObjectsExist<SpanCodeOrigin.SpanCodeOrigin>();
-            memoryAssertions.ObjectsExist<Symbols.SymbolsUploader>();
         });
     }