Skip to content

Commit 0270676

Browse files
lucaspimentelclaudeandrewlock
authored
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]>
1 parent 7a823fe commit 0270676

16 files changed

+643
-206
lines changed

‎tracer/build/_build/Build.Steps.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,7 @@ void PrepareMonitoringHomeLinuxForPackaging(AbsolutePath assetsDirectory, string
13661366
.DependsOn(CopyNativeFilesForAppSecUnitTests)
13671367
.DependsOn(CopyNativeFilesForTests)
13681368
.DependsOn(CompileManagedTestHelpers)
1369+
.DependsOn(CompileManagedLoader)
13691370
.Executes(() =>
13701371
{
13711372
DotnetBuild(TracerDirectory.GlobFiles("test/**/*.Tests.csproj"));

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

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

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// <copyright file="EnvironmentVariableProvider.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#nullable enable
7+
8+
using System;
9+
10+
namespace Datadog.Trace.ClrProfiler.Managed.Loader;
11+
12+
/// <summary>
13+
/// Default implementation of <see cref="IEnvironmentVariableProvider"/> that uses System.Environment.
14+
/// </summary>
15+
internal readonly struct EnvironmentVariableProvider : IEnvironmentVariableProvider
16+
{
17+
private readonly bool _logErrors;
18+
19+
public EnvironmentVariableProvider(bool logErrors)
20+
{
21+
_logErrors = logErrors;
22+
}
23+
24+
/// <inheritdoc />
25+
public string? GetEnvironmentVariable(string key)
26+
{
27+
try
28+
{
29+
return Environment.GetEnvironmentVariable(key);
30+
}
31+
catch (Exception ex)
32+
{
33+
if (_logErrors)
34+
{
35+
StartupLogger.Log(ex, "Error reading environment variable {0}", key);
36+
}
37+
38+
return null;
39+
}
40+
}
41+
}

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// <copyright file="IEnvironmentVariableProvider.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#nullable enable
7+
8+
using System;
9+
10+
namespace Datadog.Trace.ClrProfiler.Managed.Loader;
11+
12+
/// <summary>
13+
/// Interface for accessing environment variables. This abstraction allows for easier testing
14+
/// by enabling mocking of environment variable access.
15+
/// </summary>
16+
internal interface IEnvironmentVariableProvider
17+
{
18+
/// <summary>
19+
/// Gets the value of an environment variable.
20+
/// </summary>
21+
/// <param name="key">The name of the environment variable.</param>
22+
/// <returns>The value of the environment variable, or null if it does not exist.</returns>
23+
string? GetEnvironmentVariable(string key);
24+
}
25+
26+
/// <summary>
27+
/// Extension methods for <see cref="IEnvironmentVariableProvider"/>.
28+
/// </summary>
29+
internal static class EnvironmentVariableProviderExtensions
30+
{
31+
/// <summary>
32+
/// Gets the boolean value of an environment variable.
33+
/// </summary>
34+
/// <typeparam name="TEnvVars">The type of environment variable provider.</typeparam>
35+
/// <param name="provider">The environment variable provider.</param>
36+
/// <param name="key">The name of the environment variable.</param>
37+
/// <returns>A boolean value parsed from the environment variable, or the default value if parsing is not possible.</returns>
38+
public static bool? GetBooleanEnvironmentVariable<TEnvVars>(this TEnvVars provider, string key)
39+
where TEnvVars : IEnvironmentVariableProvider
40+
{
41+
var value = provider.GetEnvironmentVariable(key);
42+
43+
if (string.IsNullOrEmpty(value))
44+
{
45+
return null;
46+
}
47+
48+
if (value!.Length == 1)
49+
{
50+
return value[0] switch
51+
{
52+
'1' or 'T' or 't' or 'Y' or 'y' => true,
53+
'0' or 'F' or 'f' or 'N' or 'n' => false,
54+
_ => null
55+
};
56+
}
57+
58+
if (string.Equals(value, "TRUE", StringComparison.OrdinalIgnoreCase) ||
59+
string.Equals(value, "YES", StringComparison.OrdinalIgnoreCase))
60+
{
61+
return true;
62+
}
63+
64+
if (string.Equals(value, "FALSE", StringComparison.OrdinalIgnoreCase) ||
65+
string.Equals(value, "NO", StringComparison.OrdinalIgnoreCase))
66+
{
67+
return false;
68+
}
69+
70+
return null;
71+
}
72+
}

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

Lines changed: 47 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
// </copyright>
55

66
#if NETCOREAPP
7+
8+
#nullable enable
9+
710
using System;
811
using System.Collections.Generic;
912
using System.IO;
@@ -16,58 +19,63 @@ namespace Datadog.Trace.ClrProfiler.Managed.Loader
1619
/// </summary>
1720
public partial class Startup
1821
{
19-
private static CachedAssembly[] _assemblies;
22+
private static readonly System.Runtime.Loader.AssemblyLoadContext DependencyLoadContext = new ManagedProfilerAssemblyLoadContext();
2023

21-
internal static System.Runtime.Loader.AssemblyLoadContext DependencyLoadContext { get; } = new ManagedProfilerAssemblyLoadContext();
24+
private static CachedAssembly[]? _assemblies;
2225

23-
private static string ResolveManagedProfilerDirectory()
26+
internal static string ComputeTfmDirectory(string tracerHomeDirectory)
2427
{
25-
string tracerFrameworkDirectory = "netstandard2.0";
26-
2728
var version = Environment.Version;
29+
string managedLibrariesDirectory;
2830

29-
// Old versions of .net core have a major version of 4
30-
if ((version.Major == 3 && version.Minor >= 1) || version.Major >= 5)
31+
if (version.Major >= 6)
3132
{
32-
tracerFrameworkDirectory = version.Major >= 6 ? "net6.0" : "netcoreapp3.1";
33+
// version > 6.0
34+
managedLibrariesDirectory = "net6.0";
3335
}
34-
35-
var tracerHomeDirectory = ReadEnvironmentVariable("DD_DOTNET_TRACER_HOME") ?? string.Empty;
36-
var fullPath = Path.GetFullPath(Path.Combine(tracerHomeDirectory, tracerFrameworkDirectory));
37-
38-
if (!Directory.Exists(fullPath))
36+
else if (version is { Major: 3, Minor: >= 1 } || version.Major == 5)
3937
{
40-
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}");
41-
return null;
38+
// version is 3.1 or 5.0
39+
managedLibrariesDirectory = "netcoreapp3.1";
4240
}
43-
44-
// We use the List/Array approach due the number of files in the tracer home folder (7 in netstandard, 2 netcoreapp3.1+)
45-
var assemblies = new List<CachedAssembly>();
46-
foreach (var file in Directory.EnumerateFiles(fullPath, "*.dll", SearchOption.TopDirectoryOnly))
41+
else
4742
{
48-
assemblies.Add(new CachedAssembly(file, null));
43+
// version < 3.1 (note: previous versions of .NET Core had major version 4)
44+
managedLibrariesDirectory = "netstandard2.0";
4945
}
5046

51-
_assemblies = assemblies.ToArray();
52-
StartupLogger.Debug("Total number of assemblies: {0}", _assemblies.Length);
47+
var fullPath = Path.Combine(Path.GetFullPath(tracerHomeDirectory), managedLibrariesDirectory);
48+
49+
if (Directory.Exists(fullPath))
50+
{
51+
// We use the List/Array approach due to the number of files in the tracer home folder (7 in netstandard, 2 netcoreapp3.1+)
52+
var assemblies = new List<CachedAssembly>();
53+
foreach (var file in Directory.EnumerateFiles(fullPath, "*.dll", SearchOption.TopDirectoryOnly))
54+
{
55+
assemblies.Add(new CachedAssembly(file, null));
56+
}
57+
58+
_assemblies = [..assemblies];
59+
StartupLogger.Debug("Total number of assemblies: {0}", _assemblies.Length);
60+
}
5361

5462
return fullPath;
5563
}
5664

57-
private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
65+
private static Assembly? AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
5866
{
5967
return ResolveAssembly(args.Name);
6068
}
6169

62-
private static Assembly ResolveAssembly(string name)
70+
private static Assembly? ResolveAssembly(string name)
6371
{
6472
var assemblyName = new AssemblyName(name);
6573

6674
// On .NET Framework, having a non-US locale can cause mscorlib
6775
// to enter the AssemblyResolve event when searching for resources
6876
// in its satellite assemblies. This seems to have been fixed in
6977
// .NET Core in the 2.0 servicing branch, so we should not see this
70-
// occur, but guard against it anyways. If we do see it, exit early
78+
// occur but guard against it anyway. If we do see it, exit early
7179
// so we don't cause infinite recursion.
7280
if (string.Equals(assemblyName.Name, "System.Private.CoreLib.resources", StringComparison.OrdinalIgnoreCase) ||
7381
string.Equals(assemblyName.Name, "System.Net.Http", StringComparison.OrdinalIgnoreCase))
@@ -76,9 +84,8 @@ private static Assembly ResolveAssembly(string name)
7684
}
7785

7886
// WARNING: Logs must not be added _before_ we check for the above bail-out conditions
79-
StartupLogger.Debug("Assembly Resolve event received for: {0}", name);
87+
StartupLogger.Debug("Assembly Resolve event received for: {0}. Searching in: {1}", name, ManagedProfilerDirectory);
8088
var path = Path.Combine(ManagedProfilerDirectory, $"{assemblyName.Name}.dll");
81-
StartupLogger.Debug("Looking for: {0}", path);
8289

8390
if (IsDatadogAssembly(path, out var cachedAssembly))
8491
{
@@ -90,15 +97,15 @@ private static Assembly ResolveAssembly(string name)
9097
return cachedAssembly;
9198
}
9299

93-
// Only load the main profiler into the default Assembly Load Context.
94-
// If Datadog.Trace or other libraries are provided by the NuGet package their loads are handled in the following two ways.
95-
// 1) The AssemblyVersion is greater than or equal to the version used by Datadog.Trace, the assembly
100+
// Only load the main profiler into the default AssemblyLoadContext.
101+
// If the NuGet package provides Datadog.Trace or other libraries, loading them is handled in the following two ways:
102+
// 1) If the AssemblyVersion is greater than or equal to the version used by Datadog.Trace, the assembly
96103
// will load successfully and will not invoke this resolve event.
97-
// 2) The AssemblyVersion is lower than the version used by Datadog.Trace, the assembly will fail to load
104+
// 2) If the AssemblyVersion is lower than the version used by Datadog.Trace, the assembly will fail to load
98105
// and invoke this resolve event. It must be loaded in a separate AssemblyLoadContext since the application will only
99-
// load the originally referenced version
100-
StartupLogger.Debug("Loading {0} with DependencyLoadContext.LoadFromAssemblyPath", path);
101-
var assembly = DependencyLoadContext.LoadFromAssemblyPath(path); // Load unresolved framework and third-party dependencies into a custom Assembly Load Context
106+
// load the originally referenced version.
107+
StartupLogger.Debug("Calling DependencyLoadContext.LoadFromAssemblyPath(\"{0}\")", path);
108+
var assembly = DependencyLoadContext.LoadFromAssemblyPath(path); // Load unresolved framework and third-party dependencies into a custom AssemblyLoadContext
102109
SetDatadogAssembly(path, assembly);
103110
return assembly;
104111
}
@@ -108,9 +115,9 @@ private static Assembly ResolveAssembly(string name)
108115
return null;
109116
}
110117

111-
private static bool IsDatadogAssembly(string path, out Assembly cachedAssembly)
118+
private static bool IsDatadogAssembly(string path, out Assembly? cachedAssembly)
112119
{
113-
for (var i = 0; i < _assemblies.Length; i++)
120+
for (var i = 0; i < _assemblies!.Length; i++)
114121
{
115122
var assembly = _assemblies[i];
116123
if (assembly.Path == path)
@@ -126,22 +133,22 @@ private static bool IsDatadogAssembly(string path, out Assembly cachedAssembly)
126133

127134
private static void SetDatadogAssembly(string path, Assembly cachedAssembly)
128135
{
129-
for (var i = 0; i < _assemblies.Length; i++)
136+
for (var i = 0; i < _assemblies!.Length; i++)
130137
{
131138
if (_assemblies[i].Path == path)
132139
{
133140
_assemblies[i] = new CachedAssembly(path, cachedAssembly);
134-
break;
141+
return;
135142
}
136143
}
137144
}
138145

139146
private readonly struct CachedAssembly
140147
{
141148
public readonly string Path;
142-
public readonly Assembly Assembly;
149+
public readonly Assembly? Assembly;
143150

144-
public CachedAssembly(string path, Assembly assembly)
151+
public CachedAssembly(string path, Assembly? assembly)
145152
{
146153
Path = path;
147154
Assembly = assembly;

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

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
#if NETFRAMEWORK
77

8+
#nullable enable
9+
810
using System;
911
using System.IO;
1012
using System.Reflection;
@@ -16,20 +18,12 @@ namespace Datadog.Trace.ClrProfiler.Managed.Loader
1618
/// </summary>
1719
public partial class Startup
1820
{
19-
private static string ResolveManagedProfilerDirectory()
21+
internal static string ComputeTfmDirectory(string tracerHomeDirectory)
2022
{
21-
var tracerHomeDirectory = ReadEnvironmentVariable("DD_DOTNET_TRACER_HOME") ?? string.Empty;
22-
var fullPath = Path.GetFullPath(Path.Combine(tracerHomeDirectory, "net461"));
23-
if (!Directory.Exists(fullPath))
24-
{
25-
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}");
26-
return null;
27-
}
28-
29-
return fullPath;
23+
return Path.Combine(Path.GetFullPath(tracerHomeDirectory), "net461");
3024
}
3125

32-
private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
26+
private static Assembly? AssemblyResolve_ManagedProfilerDependencies(object sender, ResolveEventArgs args)
3327
{
3428
try
3529
{
@@ -43,7 +37,7 @@ private static Assembly AssemblyResolve_ManagedProfilerDependencies(object sende
4337
return null;
4438
}
4539

46-
private static Assembly ResolveAssembly(string name)
40+
private static Assembly? ResolveAssembly(string name)
4741
{
4842
var assemblyName = new AssemblyName(name);
4943

@@ -60,23 +54,23 @@ private static Assembly ResolveAssembly(string name)
6054

6155
// WARNING: Logs must not be added _before_ we check for the above bail-out conditions
6256
var path = string.IsNullOrEmpty(ManagedProfilerDirectory) ? $"{assemblyName.Name}.dll" : Path.Combine(ManagedProfilerDirectory, $"{assemblyName.Name}.dll");
63-
StartupLogger.Debug(" Looking for: '{0}'", path);
57+
StartupLogger.Debug("Assembly Resolve event received for: {0}. Looking for: {1}", name, path);
6458

6559
if (File.Exists(path))
6660
{
67-
if (name.StartsWith("Datadog.Trace, Version=") && name != AssemblyName)
61+
if (name.StartsWith("Datadog.Trace, Version=", StringComparison.Ordinal) && name != AssemblyName)
6862
{
6963
StartupLogger.Debug(" Trying to load '{0}' which does not match the expected version ('{1}'). [Path={2}]", name, AssemblyName, path);
7064
return null;
7165
}
7266

73-
StartupLogger.Debug(" Resolving '{0}', loading '{1}'", name, path);
67+
StartupLogger.Debug("Calling Assembly.LoadFrom(\"{0}\")", path);
7468
var assembly = Assembly.LoadFrom(path);
75-
StartupLogger.Debug("Assembly '{0}' loaded.", assembly?.FullName ?? "(null)");
69+
StartupLogger.Debug("Assembly loaded: {0}", assembly.FullName);
7670
return assembly;
7771
}
7872

79-
StartupLogger.Debug("Assembly not found in path: '{0}'", path);
73+
StartupLogger.Debug("Assembly not found in path: {0}", path);
8074
return null;
8175
}
8276
}

0 commit comments

Comments
 (0)