make `DD_DOTNET_TRACER_HOME` optional #7568

lucaspimentel · 2025-09-25T22:26:28Z

DO NOT MERGE YET. This PR is stacked on #7594. Merge that PR first.

Summary of changes

Make DD_DOTNET_TRACER_HOME optional. If not set, try to figure out the path from COR_PROFILER_PATH and friends.

Reason for change

One less env var for users to set manually = easier onboarding, less steps, less error-prone.

Implementation details

The implementation makes DD_DOTNET_TRACER_HOME optional by adding fallback logic to derive the tracer home path from the profiler path environment variables:

New method GetTracerHomePath (Startup.cs:150-178):
- First checks for explicit DD_DOTNET_TRACER_HOME setting (preserves backward compatibility)
- Falls back to computing the path from architecture-specific profiler path env vars (e.g., CORECLR_PROFILER_PATH_64, COR_PROFILER_PATH_64)
- If not found, tries the generic profiler path env var (e.g., CORECLR_PROFILER_PATH, COR_PROFILER_PATH)
New method ComputeTracerHomePathFromProfilerPath (Startup.cs:180-218):
- Takes a profiler path like C:\tracer\win-x64\Datadog.Trace.ClrProfiler.Native.dll
- Extracts the parent directory
- If the parent directory name matches a known architecture folder (e.g., win-x64, linux-arm64, osx), goes up one more level
- Returns the computed tracer home path
Platform-specific helper methods:
- GetProfilerPathEnvVarNameForArch(): Returns the architecture-specific env var name (CORECLR_PROFILER_PATH_64 on .NET Core x64, COR_PROFILER_PATH_32 on .NET Framework x86, etc.)
- GetProfilerPathEnvVarNameFallback(): Returns the generic env var name (CORECLR_PROFILER_PATH or COR_PROFILER_PATH)
Architecture directory detection:
- Maintains a HashSet<string> of known architecture directories (win-x64, win-x86, linux-x64, linux-arm64, linux-musl-x64, linux-musl-arm64, osx, osx-arm64, osx-x64)
- Uses case-insensitive comparison for robustness

Test coverage

Unit tests (tracer/test/Datadog.Trace.Tests/ClrProfiler/Managed/Loader/):

Environment variable reading and fallback logic (StartupNetCoreTests.cs, StartupNetFrameworkTests.cs)
GetTracerHomePath() with explicit DD_DOTNET_TRACER_HOME (with/without whitespace)
GetTracerHomePath() with architecture-specific profiler path fallback
GetTracerHomePath() with generic profiler path fallback
ComputeTracerHomePathFromProfilerPath() with all 10 architecture directories
Edge cases: empty values, whitespace, missing variables, null handling

Integration tests:

Added WhenOmittingTracerHome_InstrumentsApp() test in InstrumentationTests.cs that explicitly omits DD_DOTNET_TRACER_HOME and verifies instrumentation works via fallback logic
Modified all smoke tests (SmokeTests/SmokeTestBase.cs) to omit DD_DOTNET_TRACER_HOME by default, providing comprehensive real-world validation of the fallback behavior across multiple regression scenarios

Other details

⚠️ Based on #7567, which was originally generated by OpenAI Codex, but I made substantial changes to clean up and refactor.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

@codex fix this CI failure
@codex address that feedback

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

dd-trace-dotnet-ci-bot · 2025-09-25T23:41:41Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

Welch test with statistical test for significance of 5%
Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (72ms)  : 71, 73
     .   : milestone, 72,
    master - mean (72ms)  : 71, 73
     .   : milestone, 72,

    section Baseline
    This PR (7568) - mean (71ms)  : 60, 83
     .   : milestone, 71,
    master - mean (68ms)  : 66, 71
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (1,049ms)  : 995, 1103
     .   : milestone, 1049,
    master - mean (1,043ms)  : 1007, 1080
     .   : milestone, 1043,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (106ms)  : 105, 108
     .   : milestone, 106,
    master - mean (106ms)  : 105, 107
     .   : milestone, 106,

    section Baseline
    This PR (7568) - mean (105ms)  : 103, 108
     .   : milestone, 105,
    master - mean (105ms)  : 103, 108
     .   : milestone, 105,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (740ms)  : 721, 759
     .   : milestone, 740,
    master - mean (741ms)  : 722, 760
     .   : milestone, 741,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (94ms)  : 93, 95
     .   : milestone, 94,
    master - mean (94ms)  : 93, 95
     .   : milestone, 94,

    section Baseline
    This PR (7568) - mean (93ms)  : 91, 95
     .   : milestone, 93,
    master - mean (93ms)  : 91, 95
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (698ms)  : 673, 724
     .   : milestone, 698,
    master - mean (703ms)  : 683, 723
     .   : milestone, 703,

gantt
    title Execution time (ms) FakeDbCommand (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (93ms)  : 92, 94
     .   : milestone, 93,
    master - mean (93ms)  : 92, 94
     .   : milestone, 93,

    section Baseline
    This PR (7568) - mean (92ms)  : 90, 95
     .   : milestone, 92,
    master - mean (92ms)  : 90, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (654ms)  : 641, 667
     .   : milestone, 654,
    master - mean (664ms)  : 648, 680
     .   : milestone, 664,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (202ms)  : 196, 207
     .   : milestone, 202,
    master - mean (213ms)  : 209, 217
     .   : milestone, 213,

    section Baseline
    This PR (7568) - mean (199ms)  : 194, 203
     .   : milestone, 199,
    master - mean (207ms)  : 203, 212
     .   : milestone, 207,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (1,180ms)  : 1118, 1241
     .   : milestone, 1180,
    master - mean (1,229ms)  : 1145, 1313
     .   : milestone, 1229,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (284ms)  : 275, 293
     .   : milestone, 284,
    master - mean (300ms)  : 294, 305
     .   : milestone, 300,

    section Baseline
    This PR (7568) - mean (284ms)  : 277, 291
     .   : milestone, 284,
    master - mean (299ms)  : 292, 306
     .   : milestone, 299,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (952ms)  : 906, 998
     .   : milestone, 952,
    master - mean (988ms)  : 958, 1019
     .   : milestone, 988,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (276ms)  : 271, 281
     .   : milestone, 276,
    master - mean (291ms)  : 284, 298
     .   : milestone, 291,

    section Baseline
    This PR (7568) - mean (277ms)  : 272, 283
     .   : milestone, 277,
    master - mean (291ms)  : 283, 299
     .   : milestone, 291,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (931ms)  : 881, 981
     .   : milestone, 931,
    master - mean (977ms)  : 940, 1015
     .   : milestone, 977,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7568) - mean (275ms)  : 270, 279
     .   : milestone, 275,
    master - mean (291ms)  : 286, 296
     .   : milestone, 291,

    section Baseline
    This PR (7568) - mean (276ms)  : 271, 281
     .   : milestone, 276,
    master - mean (291ms)  : 281, 301
     .   : milestone, 291,

    section CallTarget+Inlining+NGEN
    This PR (7568) - mean (861ms)  : 837, 886
     .   : milestone, 861,
    master - mean (923ms)  : 838, 1007
     .   : milestone, 923,

andrewlock

LGTM, thanks! I just think we need

Potential some perf tweaks
Tests (unit and smoke/integration)

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

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

andrewlock · 2025-09-26T09:21:19Z

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

+        private static bool IsDatadogAssembly(string path, out Assembly? cachedAssembly)
        {
-            for (var i = 0; i < _assemblies.Length; i++)
+            if (_assemblies is not null)


We can (and probably should) restructure Startup so that _assemblies is non-nullable, which avoids all these superfluous checks (we know it can't be null here in practice)

At a quick glance I couldn't make it non-nullable without defaulting to = [], which allocates an array (only the first time, but this only runs once). Since we know they can't be null, I suppressed the warnings with ! instead.

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

andrewlock · 2025-09-26T09:23:52Z

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

+                }
+
+                // if the directory name is one of these, go one level higher
+                List<string> architectures = ["win-x64", "win-x86", "linux-x64", "linux-arm64", "linux-musl-x64", "linux-musl-arm64", "osx", "osx-arm64", "osx-x64"];


this list should probably be a static readonly list at the top of the class instead of buried here

osx? Is that a thing?! 😅

It'll be easy to miss adding a new arch here, I wonder how we can avoid that? 🤔

this list should probably be a static readonly list at the top of the class instead of buried here

Agreed. I'll fix it. (and I blame AI)

osx? Is that a thing?!

Yes, in Datadog.Trace.Bundle.

It'll be easy to miss adding a new arch here

Yeah, I had the same concern. A previous version would try all combinations of OS and arch, but that seems a bit wasteful since we know we don't support many of those (like win-arm64 or linux-x86).

On the other hand, we'll notice quickly if something is missing because the tracer won't work?

this list should probably be a static readonly list at the top of the class instead of buried here

fixed

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

@codex fix this CI failure
@codex address that feedback

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

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

… logic works

lucaspimentel changed the title ~~Lpimentel/remove tracer home dependency~~ make DD_DOTNET_TRACER_HOME optional Sep 25, 2025

lucaspimentel marked this pull request as ready for review September 25, 2025 22:45

lucaspimentel requested review from a team as code owners September 25, 2025 22:45

chatgpt-codex-connector bot reviewed Sep 25, 2025

View reviewed changes

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.cs Outdated Show resolved Hide resolved

andrewlock approved these changes Sep 26, 2025

View reviewed changes

lucaspimentel force-pushed the lpimentel/remove-tracer-home-dependency branch from 7a38278 to 8c6e15a Compare September 30, 2025 15:17

lucaspimentel marked this pull request as draft September 30, 2025 22:37

lucaspimentel mentioned this pull request Oct 1, 2025

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

Open

lucaspimentel force-pushed the lpimentel/remove-tracer-home-dependency branch from 01ee21f to 401e2b7 Compare October 2, 2025 02:39

lucaspimentel changed the base branch from master to lpimentel/managed-loader-cleanup October 2, 2025 02:41

lucaspimentel marked this pull request as ready for review October 2, 2025 03:05

chatgpt-codex-connector bot reviewed Oct 2, 2025

View reviewed changes

tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.cs Show resolved Hide resolved

lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from caaa220 to 7d426a8 Compare October 2, 2025 14:01

lucaspimentel requested a review from a team October 2, 2025 14:05

lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from 3a9626d to 2df2a24 Compare October 2, 2025 17:52

lucaspimentel added 5 commits October 2, 2025 13:53

use fallback if DD_DOTNET_TRACER_HOME is not set

d003b06

add unit tests

e40ad6f

use HashSet<string> instead of List<string>

569acd2

add comment

928a460

improve exception, remove comment

90cc70b

lucaspimentel force-pushed the lpimentel/remove-tracer-home-dependency branch from fc18f1d to 958c6b9 Compare October 2, 2025 17:53

support win-arm64

786176e

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

lucaspimentel force-pushed the lpimentel/remove-tracer-home-dependency branch from 958c6b9 to 786176e Compare October 2, 2025 18:04

lucaspimentel added 4 commits October 2, 2025 14:22

make log message actionable, remove reduntant log message

762e269

clean up code comments

737f0dc

remove "TFM" from log messages

d35fc5f

add an integration test that doesn't set DD_DOTNET_TRACER_HOME

86b62e0

lucaspimentel requested a review from a team as a code owner October 2, 2025 18:49

don't set DD_DOTNET_TRACER_HOME in smoke tests to verify the fallback…

427a860

… logic works

bouwkast added the status:do-not-merge Work is done. Can review, but do not merge yet. label Oct 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

make `DD_DOTNET_TRACER_HOME` optional #7568

make `DD_DOTNET_TRACER_HOME` optional #7568

Uh oh!

lucaspimentel commented Sep 25, 2025 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

dd-trace-dotnet-ci-bot bot commented Sep 25, 2025 •

edited

Loading

Uh oh!

andrewlock left a comment

Uh oh!

Uh oh!

Uh oh!

andrewlock Sep 26, 2025

Uh oh!

lucaspimentel Oct 2, 2025 •

edited

Loading

Uh oh!

Uh oh!

andrewlock Sep 26, 2025

Uh oh!

lucaspimentel Sep 29, 2025

Uh oh!

lucaspimentel Sep 30, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

Uh oh!

make DD_DOTNET_TRACER_HOME optional #7568

Are you sure you want to change the base?

make DD_DOTNET_TRACER_HOME optional #7568

Uh oh!

Conversation

lucaspimentel commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

dd-trace-dotnet-ci-bot bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution-Time Benchmarks Report ⏱️

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andrewlock Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

lucaspimentel Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewlock Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

lucaspimentel Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

lucaspimentel Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

make `DD_DOTNET_TRACER_HOME` optional #7568

make `DD_DOTNET_TRACER_HOME` optional #7568

lucaspimentel commented Sep 25, 2025 •

edited

Loading

dd-trace-dotnet-ci-bot bot commented Sep 25, 2025 •

edited

Loading

lucaspimentel Oct 2, 2025 •

edited

Loading