managed loader: refactor for testability, add unit tests (#7594)

## Summary of changes

Refactored the managed loader
(`Datadog.Trace.ClrProfiler.Managed.Loader`) to improve testability and
added comprehensive unit tests.

## Reason for change

The managed loader code was difficult to test due to tight coupling with
environment variables and filesystem access. This refactoring enables
better test coverage and makes the code more maintainable.

## Implementation details

- **New abstractions**: 
- Added `IEnvironmentVariableProvider` and `EnvironmentVariableProvider`
to abstract environment variable access, enabling dependency injection
for tests
- Implemented as `readonly struct` with generic type parameters to avoid
heap allocations and boxing
- **Refactored startup logic**:
- Moved TFM directory computation into dedicated methods that can be
tested independently
  - Separated directory existence checks from path computation
  - Improved error messages and logging consistency
- **Code improvements**:
  - Added nullable reference type annotations throughout
- Standardized Boolean environment variable parsing to match tracer
conventions
  - Cleaned up comments and improved code clarity

## Test coverage

Added 5 new test files with coverage of:
- Environment variable reading and Boolean parsing
(`EnvironmentVariableProviderTests`)
- Log directory resolution across platforms (`StartupLoggerTests`)
- TFM directory computation for .NET Core (`StartupNetCoreTests`)
- TFM directory computation for .NET Framework
(`StartupNetFrameworkTests`)
- Edge cases and error conditions
- Added `MockEnvironmentVariableProvider` for test isolation

## Other details

This refactoring is preparatory work for #7568 which makes
`DD_DOTNET_TRACER_HOME` optional, but the changes were hard to test.

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Andrew Lock <[email protected]>

Author: 3 people

tracer/build/_build/Build.Steps.cs

@@ -1366,6 +1366,7 @@ void PrepareMonitoringHomeLinuxForPackaging(AbsolutePath assetsDirectory, string
         .DependsOn(CopyNativeFilesForAppSecUnitTests)
         .DependsOn(CopyNativeFilesForTests)
         .DependsOn(CompileManagedTestHelpers)
+        .DependsOn(CompileManagedLoader)
         .Executes(() =>
         {
             DotnetBuild(TracerDirectory.GlobFiles("test/**/*.Tests.csproj"));

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/AssemblyInfo.cs

@@ -0,0 +1,8 @@
+// <copyright file="AssemblyInfo.cs" company="Datadog">
+// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
+// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
+// </copyright>
+
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Datadog.Trace.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010025b855c8bc41b1d47e777fc247392999ca6f553cdb030fac8e3bd010171ded9982540d988553935f44f7dd58cb4b17fbb92653d5c2dc5112696886665b317c6f92795bf64beab2405c501c8a30cb1b31b1541ed66e27d9823169ec2815b00ceeeecc8d5a1bf43db67d2961a3e9bea1397f043ec07491709649252f5565b756c5")]

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/EnvironmentVariableProvider.cs

@@ -0,0 +1,41 @@
+// <copyright file="EnvironmentVariableProvider.cs" company="Datadog">
+// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
+// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
+// </copyright>
+
+#nullable enable
+
+using System;
+
+namespace Datadog.Trace.ClrProfiler.Managed.Loader;
+
+/// <summary>
+/// Default implementation of <see cref="IEnvironmentVariableProvider"/> that uses System.Environment.
+/// </summary>
+internal readonly struct EnvironmentVariableProvider : IEnvironmentVariableProvider
+{
+    private readonly bool _logErrors;
+
+    public EnvironmentVariableProvider(bool logErrors)
+    {
+        _logErrors = logErrors;
+    }
+
+    /// <inheritdoc />
+    public string? GetEnvironmentVariable(string key)
+    {
+        try
+        {
+            return Environment.GetEnvironmentVariable(key);
+        }
+        catch (Exception ex)
+        {
+            if (_logErrors)
+            {
+                StartupLogger.Log(ex, "Error reading environment variable {0}", key);
+            }
+
+            return null;
+        }
+    }
+}

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/IEnvironmentVariableProvider.cs

@@ -0,0 +1,72 @@
+// <copyright file="IEnvironmentVariableProvider.cs" company="Datadog">
+// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
+// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
+// </copyright>
+
+#nullable enable
+
+using System;
+
+namespace Datadog.Trace.ClrProfiler.Managed.Loader;
+
+/// <summary>
+/// Interface for accessing environment variables. This abstraction allows for easier testing
+/// by enabling mocking of environment variable access.
+/// </summary>
+internal interface IEnvironmentVariableProvider
+{
+    /// <summary>
+    /// Gets the value of an environment variable.
+    /// </summary>
+    /// <param name="key">The name of the environment variable.</param>
+    /// <returns>The value of the environment variable, or null if it does not exist.</returns>
+    string? GetEnvironmentVariable(string key);
+}
+
+/// <summary>
+/// Extension methods for <see cref="IEnvironmentVariableProvider"/>.
+/// </summary>
+internal static class EnvironmentVariableProviderExtensions
+{
+    /// <summary>
+    /// Gets the boolean value of an environment variable.
+    /// </summary>
+    /// <typeparam name="TEnvVars">The type of environment variable provider.</typeparam>
+    /// <param name="provider">The environment variable provider.</param>
+    /// <param name="key">The name of the environment variable.</param>
+    /// <returns>A boolean value parsed from the environment variable, or the default value if parsing is not possible.</returns>
+    public static bool? GetBooleanEnvironmentVariable<TEnvVars>(this TEnvVars provider, string key)
+        where TEnvVars : IEnvironmentVariableProvider
+    {
+        var value = provider.GetEnvironmentVariable(key);
+
+        if (string.IsNullOrEmpty(value))
+        {
+            return null;
+        }
+
+        if (value!.Length == 1)
+        {
+            return value[0] switch
+                   {
+                       '1' or 'T' or 't' or 'Y' or 'y' => true,
+                       '0' or 'F' or 'f' or 'N' or 'n' => false,
+                       _ => null
+                   };
+        }
+
+        if (string.Equals(value, "TRUE", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(value, "YES", StringComparison.OrdinalIgnoreCase))
+        {
+            return true;
+        }
+
+        if (string.Equals(value, "FALSE", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(value, "NO", StringComparison.OrdinalIgnoreCase))
+        {
+            return false;
+        }
+
+        return null;
+    }
+}

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.NetCore.cs

@@ -4,6 +4,9 @@
 // </copyright>
 
 #if NETCOREAPP
+
+#nullable enable
+
 using System;
 using System.Collections.Generic;
 using System.IO;
@@ -16,58 +19,63 @@ namespace Datadog.Trace.ClrProfiler.Managed.Loader
     /// </summary>
     public partial class Startup
     {
-        private static CachedAssembly[] _assemblies;
+        private static readonly System.Runtime.Loader.AssemblyLoadContext DependencyLoadContext = new ManagedProfilerAssemblyLoadContext();
 
-        internal static System.Runtime.Loader.AssemblyLoadContext DependencyLoadContext { get; } = new ManagedProfilerAssemblyLoadContext();
+        private static CachedAssembly[]? _assemblies;
 
-        private static string ResolveManagedProfilerDirectory()
+        internal static string ComputeTfmDirectory(string tracerHomeDirectory)
         {
-            string tracerFrameworkDirectory = "netstandard2.0";
-
             var version = Environment.Version;
+            string managedLibrariesDirectory;
 
-            // Old versions of .net core have a major version of 4
-            if ((version.Major == 3 && version.Minor >= 1) || version.Major >= 5)
+            if (version.Major >= 6)
             {
-                tracerFrameworkDirectory = version.Major >= 6 ? "net6.0" : "netcoreapp3.1";
+                // version > 6.0
+                managedLibrariesDirectory = "net6.0";
             }
-
-            var tracerHomeDirectory = ReadEnvironmentVariable("DD_DOTNET_TRACER_HOME") ?? string.Empty;
-            var fullPath = Path.GetFullPath(Path.Combine(tracerHomeDirectory, tracerFrameworkDirectory));
-
-            if (!Directory.Exists(fullPath))
+            else if (version is { Major: 3, Minor: >= 1 } || version.Major == 5)
             {
-                StartupLogger.Log($"The tracer home directory cannot be found at '{fullPath}', based on the DD_DOTNET_TRACER_HOME value '{tracerHomeDirectory}' and current directory {Environment.CurrentDirectory}");
-                return null;
+                // version is 3.1 or 5.0
+                managedLibrariesDirectory = "netcoreapp3.1";
             }
-
-            // We use the List/Array approach due the number of files in the tracer home folder (7 in netstandard, 2 netcoreapp3.1+)
-            var assemblies = new List<CachedAssembly>();
-            foreach (var file in Directory.EnumerateFiles(fullPath, "*.dll", SearchOption.TopDirectoryOnly))
+            else
             {
-                assemblies.Add(new CachedAssembly(file, null));
+                // version < 3.1 (note: previous versions of .NET Core had major version 4)
+                managedLibrariesDirectory = "netstandard2.0";
             }
 
-            _assemblies = assemblies.ToArray();
-            StartupLogger.Debug("Total number of assemblies: {0}", _assemblies.Length);
+            var fullPath = Path.Combine(Path.GetFullPath(tracerHomeDirectory), managedLibrariesDirectory);
+
+            if (Directory.Exists(fullPath))
+            {
+                // We use the List/Array approach due to the number of files in the tracer home folder (7 in netstandard, 2 netcoreapp3.1+)
+                var assemblies = new List<CachedAssembly>();
+                foreach (var file in Directory.EnumerateFiles(fullPath, "*.dll", SearchOption.TopDirectoryOnly))
+                {
+                    assemblies.Add(new CachedAssembly(file, null));
+                }
+
+                _assemblies = [..assemblies];
+                StartupLogger.Debug("Total number of assemblies: {0}", _assemblies.Length);
+            }
 
             return fullPath;
         }
 
-        private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
+        private static Assembly? AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
         {
             return ResolveAssembly(args.Name);
         }
 
-        private static Assembly ResolveAssembly(string name)
+        private static Assembly? ResolveAssembly(string name)
         {
             var assemblyName = new AssemblyName(name);
 
             // On .NET Framework, having a non-US locale can cause mscorlib
             // to enter the AssemblyResolve event when searching for resources
             // in its satellite assemblies. This seems to have been fixed in
             // .NET Core in the 2.0 servicing branch, so we should not see this
-            // occur, but guard against it anyways. If we do see it, exit early
+            // occur but guard against it anyway. If we do see it, exit early
             // so we don't cause infinite recursion.
             if (string.Equals(assemblyName.Name, "System.Private.CoreLib.resources", StringComparison.OrdinalIgnoreCase) ||
                 string.Equals(assemblyName.Name, "System.Net.Http", StringComparison.OrdinalIgnoreCase))
@@ -76,9 +84,8 @@ private static Assembly ResolveAssembly(string name)
             }
 
             // WARNING: Logs must not be added _before_ we check for the above bail-out conditions
-            StartupLogger.Debug("Assembly Resolve event received for: {0}", name);
+            StartupLogger.Debug("Assembly Resolve event received for: {0}. Searching in: {1}", name, ManagedProfilerDirectory);
             var path = Path.Combine(ManagedProfilerDirectory, $"{assemblyName.Name}.dll");
-            StartupLogger.Debug("Looking for: {0}", path);
 
             if (IsDatadogAssembly(path, out var cachedAssembly))
             {
@@ -90,15 +97,15 @@ private static Assembly ResolveAssembly(string name)
                     return cachedAssembly;
                 }
 
-                // Only load the main profiler into the default Assembly Load Context.
-                // If Datadog.Trace or other libraries are provided by the NuGet package their loads are handled in the following two ways.
-                // 1) The AssemblyVersion is greater than or equal to the version used by Datadog.Trace, the assembly
+                // Only load the main profiler into the default AssemblyLoadContext.
+                // If the NuGet package provides Datadog.Trace or other libraries, loading them is handled in the following two ways:
+                // 1) If the AssemblyVersion is greater than or equal to the version used by Datadog.Trace, the assembly
                 //    will load successfully and will not invoke this resolve event.
-                // 2) The AssemblyVersion is lower than the version used by Datadog.Trace, the assembly will fail to load
+                // 2) If the AssemblyVersion is lower than the version used by Datadog.Trace, the assembly will fail to load
                 //    and invoke this resolve event. It must be loaded in a separate AssemblyLoadContext since the application will only
-                //    load the originally referenced version
-                StartupLogger.Debug("Loading {0} with DependencyLoadContext.LoadFromAssemblyPath", path);
-                var assembly = DependencyLoadContext.LoadFromAssemblyPath(path); // Load unresolved framework and third-party dependencies into a custom Assembly Load Context
+                //    load the originally referenced version.
+                StartupLogger.Debug("Calling DependencyLoadContext.LoadFromAssemblyPath(\"{0}\")", path);
+                var assembly = DependencyLoadContext.LoadFromAssemblyPath(path); // Load unresolved framework and third-party dependencies into a custom AssemblyLoadContext
                 SetDatadogAssembly(path, assembly);
                 return assembly;
             }
@@ -108,9 +115,9 @@ private static Assembly ResolveAssembly(string name)
             return null;
         }
 
-        private static bool IsDatadogAssembly(string path, out Assembly cachedAssembly)
+        private static bool IsDatadogAssembly(string path, out Assembly? cachedAssembly)
         {
-            for (var i = 0; i < _assemblies.Length; i++)
+            for (var i = 0; i < _assemblies!.Length; i++)
             {
                 var assembly = _assemblies[i];
                 if (assembly.Path == path)
@@ -126,22 +133,22 @@ private static bool IsDatadogAssembly(string path, out Assembly cachedAssembly)
 
         private static void SetDatadogAssembly(string path, Assembly cachedAssembly)
         {
-            for (var i = 0; i < _assemblies.Length; i++)
+            for (var i = 0; i < _assemblies!.Length; i++)
             {
                 if (_assemblies[i].Path == path)
                 {
                     _assemblies[i] = new CachedAssembly(path, cachedAssembly);
-                    break;
+                    return;
                 }
             }
         }
 
         private readonly struct CachedAssembly
         {
             public readonly string Path;
-            public readonly Assembly Assembly;
+            public readonly Assembly? Assembly;
 
-            public CachedAssembly(string path, Assembly assembly)
+            public CachedAssembly(string path, Assembly? assembly)
             {
                 Path = path;
                 Assembly = assembly;

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.NetFramework.cs

@@ -5,6 +5,8 @@
 
 #if NETFRAMEWORK
 
+#nullable enable
+
 using System;
 using System.IO;
 using System.Reflection;
@@ -16,20 +18,12 @@ namespace Datadog.Trace.ClrProfiler.Managed.Loader
     /// </summary>
     public partial class Startup
     {
-        private static string ResolveManagedProfilerDirectory()
+        internal static string ComputeTfmDirectory(string tracerHomeDirectory)
         {
-            var tracerHomeDirectory = ReadEnvironmentVariable("DD_DOTNET_TRACER_HOME") ?? string.Empty;
-            var fullPath = Path.GetFullPath(Path.Combine(tracerHomeDirectory, "net461"));
-            if (!Directory.Exists(fullPath))
-            {
-                StartupLogger.Log($"The tracer home directory cannot be found at '{fullPath}', based on the DD_DOTNET_TRACER_HOME value '{tracerHomeDirectory}' and current directory {Environment.CurrentDirectory}");
-                return null;
-            }
-
-            return fullPath;
+            return Path.Combine(Path.GetFullPath(tracerHomeDirectory), "net461");
         }
 
-        private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
+        private static Assembly? AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
         {
             try
             {
@@ -43,7 +37,7 @@ private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sende
             return null;
         }
 
-        private static Assembly ResolveAssembly(string name)
+        private static Assembly? ResolveAssembly(string name)
         {
             var assemblyName = new AssemblyName(name);
 
@@ -60,23 +54,23 @@ private static Assembly ResolveAssembly(string name)
 
             // WARNING: Logs must not be added _before_ we check for the above bail-out conditions
             var path = string.IsNullOrEmpty(ManagedProfilerDirectory) ? $"{assemblyName.Name}.dll" : Path.Combine(ManagedProfilerDirectory, $"{assemblyName.Name}.dll");
-            StartupLogger.Debug("  Looking for: '{0}'", path);
+            StartupLogger.Debug("Assembly Resolve event received for: {0}. Looking for: {1}", name, path);
 
             if (File.Exists(path))
             {
-                if (name.StartsWith("Datadog.Trace, Version=") && name != AssemblyName)
+                if (name.StartsWith("Datadog.Trace, Version=", StringComparison.Ordinal) && name != AssemblyName)
                 {
                     StartupLogger.Debug("  Trying to load '{0}' which does not match the expected version ('{1}'). [Path={2}]", name, AssemblyName, path);
                     return null;
                 }
 
-                StartupLogger.Debug("  Resolving '{0}', loading '{1}'", name, path);
+                StartupLogger.Debug("Calling Assembly.LoadFrom(\"{0}\")", path);
                 var assembly = Assembly.LoadFrom(path);
-                StartupLogger.Debug("Assembly '{0}' loaded.", assembly?.FullName ?? "(null)");
+                StartupLogger.Debug("Assembly loaded: {0}", assembly.FullName);
                 return assembly;
             }
 
-            StartupLogger.Debug("Assembly not found in path: '{0}'", path);
+            StartupLogger.Debug("Assembly not found in path: {0}", path);
             return null;
         }
     }