Skip to content

Conversation

@lucaspimentel
Copy link
Member

@lucaspimentel lucaspimentel commented Sep 30, 2025

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.

@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from a5aefab to 71d026c Compare September 30, 2025 21:30
@dd-trace-dotnet-ci-bot
Copy link

dd-trace-dotnet-ci-bot bot commented Sep 30, 2025

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 (7594) - mean (75ms)  : 72, 77
     .   : milestone, 75,
    master - mean (75ms)  : 72, 77
     .   : milestone, 75,

    section Baseline
    This PR (7594) - mean (73ms)  : 66, 79
     .   : milestone, 73,
    master - mean (71ms)  : 69, 73
     .   : milestone, 71,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (1,068ms)  : 1000, 1137
     .   : milestone, 1068,
    master - mean (1,074ms)  : 993, 1156
     .   : milestone, 1074,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (112ms)  : 109, 114
     .   : milestone, 112,
    master - mean (114ms)  : 105, 123
     .   : milestone, 114,

    section Baseline
    This PR (7594) - mean (110ms)  : 107, 114
     .   : milestone, 110,
    master - mean (110ms)  : 107, 114
     .   : milestone, 110,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (754ms)  : 728, 780
     .   : milestone, 754,
    master - mean (760ms)  : 733, 786
     .   : milestone, 760,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (99ms)  : 96, 102
     .   : milestone, 99,
    master - mean (98ms)  : 96, 100
     .   : milestone, 98,

    section Baseline
    This PR (7594) - mean (98ms)  : 95, 100
     .   : milestone, 98,
    master - mean (97ms)  : 94, 100
     .   : milestone, 97,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (716ms)  : 678, 754
     .   : milestone, 716,
    master - mean (728ms)  : 669, 788
     .   : milestone, 728,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (98ms)  : 96, 100
     .   : milestone, 98,
    master - mean (99ms)  : 94, 104
     .   : milestone, 99,

    section Baseline
    This PR (7594) - mean (97ms)  : 94, 99
     .   : milestone, 97,
    master - mean (97ms)  : 94, 100
     .   : milestone, 97,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (672ms)  : 657, 686
     .   : milestone, 672,
    master - mean (676ms)  : 658, 693
     .   : milestone, 676,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (204ms)  : 198, 210
     .   : milestone, 204,
    master - mean (196ms)  : 192, 199
     .   : milestone, 196,

    section Baseline
    This PR (7594) - mean (195ms)  : 190, 199
     .   : milestone, 195,
    master - mean (192ms)  : 189, 196
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (1,187ms)  : 1100, 1275
     .   : milestone, 1187,
    master - mean (1,170ms)  : 1103, 1236
     .   : milestone, 1170,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (283ms)  : 273, 293
     .   : milestone, 283,
    master - mean (279ms)  : 273, 284
     .   : milestone, 279,

    section Baseline
    This PR (7594) - mean (278ms)  : 273, 283
     .   : milestone, 278,
    master - mean (277ms)  : 272, 282
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (956ms)  : 907, 1006
     .   : milestone, 956,
    master - mean (942ms)  : 899, 985
     .   : milestone, 942,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (272ms)  : 266, 278
     .   : milestone, 272,
    master - mean (269ms)  : 266, 273
     .   : milestone, 269,

    section Baseline
    This PR (7594) - mean (271ms)  : 264, 278
     .   : milestone, 271,
    master - mean (270ms)  : 265, 275
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (930ms)  : 882, 978
     .   : milestone, 930,
    master - mean (932ms)  : 876, 989
     .   : milestone, 932,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7594) - mean (270ms)  : 264, 276
     .   : milestone, 270,
    master - mean (269ms)  : 266, 272
     .   : milestone, 269,

    section Baseline
    This PR (7594) - mean (270ms)  : 266, 275
     .   : milestone, 270,
    master - mean (268ms)  : 264, 272
     .   : milestone, 268,

    section CallTarget+Inlining+NGEN
    This PR (7594) - mean (856ms)  : 826, 886
     .   : milestone, 856,
    master - mean (854ms)  : 833, 874
     .   : milestone, 854,

Loading

@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch 2 times, most recently from f7a0f93 to f708780 Compare October 1, 2025 21:55
@lucaspimentel lucaspimentel changed the title managed loader cleanup and testability managed loader: refactoring for testability, add unit tests Oct 1, 2025
@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from f708780 to bf77e28 Compare October 1, 2025 23:30
@lucaspimentel lucaspimentel changed the title managed loader: refactoring for testability, add unit tests managed loader: refactor for testability, add unit tests Oct 2, 2025
@lucaspimentel lucaspimentel marked this pull request as ready for review October 2, 2025 02:53
@lucaspimentel lucaspimentel requested review from a team as code owners October 2, 2025 02:53
@datadog-datadog-prod-us1

This comment has been minimized.

Copy link
Member

@tonyredondo tonyredondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to do something else, I'll finish later.

@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from caaa220 to 7d426a8 Compare October 2, 2025 14:01
@lucaspimentel lucaspimentel requested a review from a team October 2, 2025 14:04
@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from 3a9626d to 2df2a24 Compare October 2, 2025 17:52
lucaspimentel added a commit that referenced this pull request Oct 3, 2025
## Summary of changes

- Add performance optimization patterns, testing guidelines, and logging
guidelines to `AGENTS.md`.
- Bonus: Fix noop pipeline path exclusions to properly skip CI for
documentation-only PRs.

## Reason for change

**Documentation:** Document reusable patterns that apply broadly across
the tracer codebase:
- Performance patterns for startup code and hot paths (from PR #7594)
- Testing patterns for testability and isolation (from PR #7594)
- Logging terminology to avoid customer confusion (from PR #7467)

**CI Optimization:** The noop pipeline was not triggering for
documentation-only PRs because it uses `StartsWith()` matching, not glob
patterns. The `**/CLAUDE.md` pattern never matched `.claude/CLAUDE.md`.

## Implementation details

**New documentation sections:**
- Performance Guidelines: Identifies performance-critical code paths
(bootstrap/startup and hot paths)
- Performance Optimization Patterns: Zero-allocation structs, logging
best practices, avoiding params arrays
- Testing Patterns: Abstraction-for-testability with examples
- Logging Guidelines: Consistent customer-facing terminology (Datadog
SDK, Instrumentation, Continuous Profiler, Datadog.Trace.dll)

**Pipeline fixes:**
- Changed `**/CLAUDE.md` glob to `.claude/` prefix (compatible with
`StartsWith()`)
- Added `.devcontainer/`, `.vscode/`, `LICENSE`, `NOTICE` to exclusions

**Other changes:**
- Fixed `.claude/CLAUDE.md` relative path reference
- Improved formatting in Coding Style section

## Test coverage

N/A - documentation and CI configuration only

---------

Co-authored-by: Claude <[email protected]>
igoragoli pushed a commit that referenced this pull request Oct 6, 2025
## Summary of changes

- Add performance optimization patterns, testing guidelines, and logging
guidelines to `AGENTS.md`.
- Bonus: Fix noop pipeline path exclusions to properly skip CI for
documentation-only PRs.

## Reason for change

**Documentation:** Document reusable patterns that apply broadly across
the tracer codebase:
- Performance patterns for startup code and hot paths (from PR #7594)
- Testing patterns for testability and isolation (from PR #7594)
- Logging terminology to avoid customer confusion (from PR #7467)

**CI Optimization:** The noop pipeline was not triggering for
documentation-only PRs because it uses `StartsWith()` matching, not glob
patterns. The `**/CLAUDE.md` pattern never matched `.claude/CLAUDE.md`.

## Implementation details

**New documentation sections:**
- Performance Guidelines: Identifies performance-critical code paths
(bootstrap/startup and hot paths)
- Performance Optimization Patterns: Zero-allocation structs, logging
best practices, avoiding params arrays
- Testing Patterns: Abstraction-for-testability with examples
- Logging Guidelines: Consistent customer-facing terminology (Datadog
SDK, Instrumentation, Continuous Profiler, Datadog.Trace.dll)

**Pipeline fixes:**
- Changed `**/CLAUDE.md` glob to `.claude/` prefix (compatible with
`StartsWith()`)
- Added `.devcontainer/`, `.vscode/`, `LICENSE`, `NOTICE` to exclusions

**Other changes:**
- Fixed `.claude/CLAUDE.md` relative path reference
- Improved formatting in Coding Style section

## Test coverage

N/A - documentation and CI configuration only

---------

Co-authored-by: Claude <[email protected]>
@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch 3 times, most recently from 58ec179 to ad16d72 Compare October 10, 2025 18:29
@pr-commenter
Copy link

pr-commenter bot commented Oct 10, 2025

Benchmarks

Benchmarks Report for benchmark platform 🐌

Benchmarks for #7594 compared to master:

  • 2 benchmarks are slower, with geometric mean 2.287
  • 4 benchmarks have fewer allocations
  • 5 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #7594

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑net472 6.02 KB 6.08 KB 60 B 1.00%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 10.3μs 16.7ns 57.9ns 0 0 0 5.52 KB
master StartStopWithChild netcoreapp3.1 13.6μs 70.6ns 323ns 0 0 0 5.71 KB
master StartStopWithChild net472 21.4μs 92.4ns 462ns 0.977 0.434 0.109 6.02 KB
#7594 StartStopWithChild net6.0 10.3μs 57ns 347ns 0 0 0 5.51 KB
#7594 StartStopWithChild netcoreapp3.1 13.7μs 70ns 305ns 0 0 0 5.71 KB
#7594 StartStopWithChild net472 22.2μs 116ns 544ns 0.88 0.33 0 6.08 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 939μs 182ns 706ns 0 0 0 2.71 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 1.01ms 159ns 614ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 1.22ms 66ns 247ns 0 0 0 3.31 KB
#7594 WriteAndFlushEnrichedTraces net6.0 914μs 145ns 561ns 0 0 0 2.71 KB
#7594 WriteAndFlushEnrichedTraces netcoreapp3.1 1.02ms 273ns 1.06μs 0 0 0 2.7 KB
#7594 WriteAndFlushEnrichedTraces net472 1.2ms 467ns 1.81μs 0 0 0 3.31 KB
Benchmarks.Trace.Asm.AppSecBodyBenchmark - Unknown 🤷 Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 N/A N/A N/A NaN NaN NaN 0 b
master AllCycleSimpleBody netcoreapp3.1 N/A N/A N/A NaN NaN NaN 0 b
master AllCycleSimpleBody net472 N/A N/A N/A NaN NaN NaN 0 b
master AllCycleMoreComplexBody net6.0 N/A N/A N/A NaN NaN NaN 0 b
master AllCycleMoreComplexBody netcoreapp3.1 N/A N/A N/A NaN NaN NaN 0 b
master AllCycleMoreComplexBody net472 N/A N/A N/A NaN NaN NaN 0 b
master ObjectExtractorSimpleBody net6.0 313ns 1.73ns 10.7ns 0 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 401ns 1.94ns 8.01ns 0 0 0 272 B
master ObjectExtractorSimpleBody net472 302ns 0.0194ns 0.0752ns 0.044 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 6.3μs 5.78ns 22.4ns 0 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 7.8μs 36.1ns 144ns 0 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 6.72μs 1.17ns 4.54ns 0.572 0 0 3.8 KB
#7594 AllCycleSimpleBody net6.0 N/A N/A N/A NaN NaN NaN 0 b
#7594 AllCycleSimpleBody netcoreapp3.1 N/A N/A N/A NaN NaN NaN 0 b
#7594 AllCycleSimpleBody net472 N/A N/A N/A NaN NaN NaN 0 b
#7594 AllCycleMoreComplexBody net6.0 N/A N/A N/A NaN NaN NaN 0 b
#7594 AllCycleMoreComplexBody netcoreapp3.1 N/A N/A N/A NaN NaN NaN 0 b
#7594 AllCycleMoreComplexBody net472 N/A N/A N/A NaN NaN NaN 0 b
#7594 ObjectExtractorSimpleBody net6.0 327ns 1.77ns 9.2ns 0 0 0 280 B
#7594 ObjectExtractorSimpleBody netcoreapp3.1 398ns 2.08ns 10.6ns 0 0 0 272 B
#7594 ObjectExtractorSimpleBody net472 296ns 0.0323ns 0.125ns 0.0433 0 0 281 B
#7594 ObjectExtractorMoreComplexBody net6.0 6.37μs 29.3ns 113ns 0 0 0 3.78 KB
#7594 ObjectExtractorMoreComplexBody netcoreapp3.1 7.87μs 39ns 170ns 0 0 0 3.69 KB
#7594 ObjectExtractorMoreComplexBody net472 6.82μs 4.37ns 16.9ns 0.574 0 0 3.8 KB
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EncodeArgs net6.0 77.4μs 236ns 881ns 0 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 97.3μs 231ns 896ns 0 0 0 32.4 KB
master EncodeArgs net472 109μs 7.78ns 30.1ns 4.88 0 0 32.51 KB
master EncodeLegacyArgs net6.0 144μs 15.9ns 55.1ns 0 0 0 2.15 KB
master EncodeLegacyArgs netcoreapp3.1 198μs 189ns 732ns 0 0 0 2.14 KB
master EncodeLegacyArgs net472 263μs 50.1ns 194ns 0 0 0 2.17 KB
#7594 EncodeArgs net6.0 79.3μs 318ns 1.23μs 0 0 0 32.4 KB
#7594 EncodeArgs netcoreapp3.1 97.3μs 101ns 376ns 0 0 0 32.4 KB
#7594 EncodeArgs net472 109μs 12.2ns 42.1ns 4.9 0 0 32.51 KB
#7594 EncodeLegacyArgs net6.0 144μs 20.9ns 78.2ns 0 0 0 2.15 KB
#7594 EncodeLegacyArgs netcoreapp3.1 197μs 137ns 529ns 0 0 0 2.14 KB
#7594 EncodeLegacyArgs net472 260μs 27.2ns 102ns 0 0 0 2.16 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #7594

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack‑netcoreapp3.1 2.459 297,119.51 730,751.30
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1 2.126 408,055.10 867,592.50

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWafRealisticBenchmark net6.0 398μs 221ns 857ns 0 0 0 4.56 KB
master RunWafRealisticBenchmark netcoreapp3.1 409μs 366ns 1.42μs 0 0 0 4.48 KB
master RunWafRealisticBenchmark net472 428μs 49.1ns 190ns 0 0 0 4.68 KB
master RunWafRealisticBenchmarkWithAttack net6.0 294μs 37.6ns 146ns 0 0 0 2.24 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 297μs 129ns 466ns 0 0 0 2.22 KB
master RunWafRealisticBenchmarkWithAttack net472 311μs 25ns 96.8ns 0 0 0 2.29 KB
#7594 RunWafRealisticBenchmark net6.0 391μs 63.7ns 247ns 0 0 0 4.55 KB
#7594 RunWafRealisticBenchmark netcoreapp3.1 827μs 13.1μs 131μs 0 0 0 4.48 KB
#7594 RunWafRealisticBenchmark net472 429μs 33.3ns 120ns 0 0 0 4.66 KB
#7594 RunWafRealisticBenchmarkWithAttack net6.0 284μs 33.4ns 129ns 0 0 0 2.24 KB
#7594 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 723μs 5.05μs 50.5μs 0 0 0 2.22 KB
#7594 RunWafRealisticBenchmarkWithAttack net472 309μs 27.1ns 101ns 0 0 0 2.29 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 61.7μs 37.2ns 144ns 0 0 0 14.52 KB
master SendRequest netcoreapp3.1 70.9μs 92.2ns 332ns 0 0 0 17.42 KB
master SendRequest net472 0.0016ns 0.001ns 0.00389ns 0 0 0 0 b
#7594 SendRequest net6.0 60.5μs 74.9ns 290ns 0 0 0 14.52 KB
#7594 SendRequest netcoreapp3.1 71.6μs 58.8ns 212ns 0 0 0 17.42 KB
#7594 SendRequest net472 0.00109ns 0.000788ns 0.00305ns 0 0 0 0 b
Benchmarks.Trace.CharSliceBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #7594

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0 1 B 5 B 4 B 400.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 2 B 4 B 2 B 100.00%

Fewer allocations 🎉 in #7594

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net472 73 B 0 b -73 B -100.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472 47 B 0 b -47 B -100.00%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master OriginalCharSlice net6.0 1.89ms 1.32μs 4.94μs 0 0 0 640.01 KB
master OriginalCharSlice netcoreapp3.1 2.14ms 2.23μs 8.35μs 0 0 0 640 KB
master OriginalCharSlice net472 2.64ms 115ns 398ns 100 0 0 641.95 KB
master OptimizedCharSlice net6.0 1.33ms 316ns 1.23μs 0 0 0 2 B
master OptimizedCharSlice netcoreapp3.1 1.71ms 734ns 2.84μs 0 0 0 1 B
master OptimizedCharSlice net472 1.95ms 231ns 866ns 0 0 0 73 B
master OptimizedCharSliceWithPool net6.0 858μs 33.7ns 130ns 0 0 0 1 B
master OptimizedCharSliceWithPool netcoreapp3.1 889μs 38.6ns 149ns 0 0 0 0 b
master OptimizedCharSliceWithPool net472 1.15ms 85.4ns 319ns 0 0 0 47 B
#7594 OriginalCharSlice net6.0 1.92ms 4.04μs 15.7μs 0 0 0 640.01 KB
#7594 OriginalCharSlice netcoreapp3.1 2.15ms 7.35μs 28.5μs 0 0 0 640 KB
#7594 OriginalCharSlice net472 2.74ms 120ns 464ns 100 0 0 641.95 KB
#7594 OptimizedCharSlice net6.0 1.43ms 273ns 1.06μs 0 0 0 4 B
#7594 OptimizedCharSlice netcoreapp3.1 1.65ms 417ns 1.61μs 0 0 0 1 B
#7594 OptimizedCharSlice net472 1.9ms 296ns 1.14μs 0 0 0 0 b
#7594 OptimizedCharSliceWithPool net6.0 836μs 39.9ns 154ns 0 0 0 5 B
#7594 OptimizedCharSliceWithPool netcoreapp3.1 811μs 168ns 651ns 0 0 0 0 b
#7594 OptimizedCharSliceWithPool net472 1.13ms 108ns 418ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 669μs 3.47μs 15.9μs 0 0 0 41.8 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 663μs 2.24μs 8.09μs 0 0 0 41.97 KB
master WriteAndFlushEnrichedTraces net472 827μs 3.44μs 13.3μs 4.46 0 0 55.83 KB
#7594 WriteAndFlushEnrichedTraces net6.0 703μs 593ns 2.29μs 0 0 0 41.79 KB
#7594 WriteAndFlushEnrichedTraces netcoreapp3.1 695μs 2.84μs 11μs 0 0 0 41.93 KB
#7594 WriteAndFlushEnrichedTraces net472 899μs 1.94μs 7.24μs 4.46 0 0 55.93 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.98μs 10.4ns 50.8ns 0 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 2.56μs 11.2ns 43.4ns 0 0 0 1.02 KB
master ExecuteNonQuery net472 2.83μs 2.01ns 7.52ns 0.155 0.0141 0 987 B
#7594 ExecuteNonQuery net6.0 1.82μs 9.63ns 50ns 0 0 0 1.02 KB
#7594 ExecuteNonQuery netcoreapp3.1 2.73μs 8.47ns 32.8ns 0 0 0 1.02 KB
#7594 ExecuteNonQuery net472 2.74μs 3.55ns 13.7ns 0.153 0.0139 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.72μs 9.14ns 46.6ns 0 0 0 1.03 KB
master CallElasticsearch netcoreapp3.1 2.27μs 7.89ns 30.6ns 0 0 0 1.03 KB
master CallElasticsearch net472 3.52μs 5.49ns 21.2ns 0.158 0 0 1.04 KB
master CallElasticsearchAsync net6.0 1.83μs 8.67ns 35.7ns 0 0 0 1.01 KB
master CallElasticsearchAsync netcoreapp3.1 2.38μs 11.8ns 51.4ns 0 0 0 1.08 KB
master CallElasticsearchAsync net472 3.72μs 3.04ns 11.8ns 0.168 0 0 1.1 KB
#7594 CallElasticsearch net6.0 1.72μs 8.68ns 38.8ns 0 0 0 1.03 KB
#7594 CallElasticsearch netcoreapp3.1 2.23μs 8.99ns 34.8ns 0 0 0 1.03 KB
#7594 CallElasticsearch net472 3.45μs 2.01ns 7.78ns 0.155 0 0 1.04 KB
#7594 CallElasticsearchAsync net6.0 1.81μs 0.682ns 2.64ns 0 0 0 1.01 KB
#7594 CallElasticsearchAsync netcoreapp3.1 2.37μs 7.06ns 27.3ns 0 0 0 1.08 KB
#7594 CallElasticsearchAsync net472 3.69μs 4.97ns 19.3ns 0.162 0 0 1.1 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.88μs 1.6ns 6.21ns 0 0 0 952 B
master ExecuteAsync netcoreapp3.1 2.32μs 8.61ns 33.4ns 0 0 0 952 B
master ExecuteAsync net472 2.74μs 4.55ns 17.6ns 0.137 0 0 915 B
#7594 ExecuteAsync net6.0 1.85μs 5.77ns 22.4ns 0 0 0 952 B
#7594 ExecuteAsync netcoreapp3.1 2.45μs 3.54ns 13.7ns 0 0 0 952 B
#7594 ExecuteAsync net472 2.56μs 3.18ns 11.9ns 0.141 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 7.06μs 14.8ns 53.3ns 0 0 0 2.36 KB
master SendAsync netcoreapp3.1 8.58μs 32.6ns 122ns 0 0 0 2.9 KB
master SendAsync net472 12.3μs 11.3ns 43.8ns 0.492 0 0 3.18 KB
#7594 SendAsync net6.0 6.84μs 4.68ns 17.5ns 0 0 0 2.36 KB
#7594 SendAsync netcoreapp3.1 8.22μs 19.6ns 76.1ns 0 0 0 2.9 KB
#7594 SendAsync net472 12.4μs 16.2ns 62.6ns 0.491 0 0 3.18 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #7594

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net6.0 43.17 KB 44.89 KB 1.72 KB 3.98%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 248.99 KB 255.94 KB 6.94 KB 2.79%

Fewer allocations 🎉 in #7594

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 44.75 KB 43.63 KB -1.12 KB -2.50%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 257.8 KB 250.9 KB -6.9 KB -2.68%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 44.4μs 272ns 2.52μs 0 0 0 43.17 KB
master StringConcatBenchmark netcoreapp3.1 49μs 349ns 3.33μs 0 0 0 44.75 KB
master StringConcatBenchmark net472 57.2μs 132ns 511ns 0 0 0 57.34 KB
master StringConcatAspectBenchmark net6.0 437μs 1.26μs 4.54μs 0 0 0 257.8 KB
master StringConcatAspectBenchmark netcoreapp3.1 497μs 1.69μs 5.84μs 0 0 0 248.99 KB
master StringConcatAspectBenchmark net472 421μs 1.9μs 13.2μs 0 0 0 278.53 KB
#7594 StringConcatBenchmark net6.0 45.6μs 265ns 1.98μs 0 0 0 44.89 KB
#7594 StringConcatBenchmark netcoreapp3.1 48.7μs 233ns 1.17μs 0 0 0 43.63 KB
#7594 StringConcatBenchmark net472 56.8μs 279ns 1.19μs 0 0 0 57.34 KB
#7594 StringConcatAspectBenchmark net6.0 459μs 2.21μs 8.86μs 0 0 0 250.9 KB
#7594 StringConcatAspectBenchmark netcoreapp3.1 510μs 1.98μs 8.15μs 0 0 0 255.94 KB
#7594 StringConcatAspectBenchmark net472 404μs 2.19μs 11.8μs 0 0 0 278.53 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.68μs 13.6ns 62.5ns 0 0 0 1.7 KB
master EnrichedLog netcoreapp3.1 3.57μs 18.2ns 85.3ns 0 0 0 1.7 KB
master EnrichedLog net472 3.87μs 2.97ns 11.5ns 0.252 0 0 1.64 KB
#7594 EnrichedLog net6.0 2.64μs 11.9ns 46.1ns 0 0 0 1.7 KB
#7594 EnrichedLog netcoreapp3.1 3.71μs 17.3ns 66.9ns 0 0 0 1.7 KB
#7594 EnrichedLog net472 3.97μs 4.08ns 15.8ns 0.258 0 0 1.64 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 122μs 59.5ns 222ns 0 0 0 4.31 KB
master EnrichedLog netcoreapp3.1 127μs 131ns 472ns 0 0 0 4.31 KB
master EnrichedLog net472 167μs 16.1ns 60.4ns 0 0 0 4.52 KB
#7594 EnrichedLog net6.0 122μs 52ns 195ns 0 0 0 4.31 KB
#7594 EnrichedLog netcoreapp3.1 126μs 246ns 852ns 0 0 0 4.31 KB
#7594 EnrichedLog net472 166μs 33.8ns 117ns 0 0 0 4.51 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 4.91μs 23.6ns 91.5ns 0 0 0 2.26 KB
master EnrichedLog netcoreapp3.1 6.61μs 30.7ns 119ns 0 0 0 2.26 KB
master EnrichedLog net472 7.66μs 7.03ns 27.2ns 0.306 0 0 2.08 KB
#7594 EnrichedLog net6.0 5.02μs 24.3ns 103ns 0 0 0 2.26 KB
#7594 EnrichedLog netcoreapp3.1 6.93μs 7.34ns 28.4ns 0 0 0 2.26 KB
#7594 EnrichedLog net472 7.63μs 9.2ns 35.6ns 0.304 0 0 2.08 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 2.06μs 10.9ns 54.6ns 0 0 0 1.2 KB
master SendReceive netcoreapp3.1 2.69μs 12.1ns 46.8ns 0 0 0 1.2 KB
master SendReceive net472 3.12μs 2.95ns 11.4ns 0.187 0 0 1.2 KB
#7594 SendReceive net6.0 2.06μs 3.2ns 12.4ns 0 0 0 1.2 KB
#7594 SendReceive netcoreapp3.1 2.55μs 11ns 45.2ns 0 0 0 1.2 KB
#7594 SendReceive net472 3.08μs 7.09ns 27.4ns 0.186 0 0 1.2 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 4.36μs 17.7ns 66.1ns 0 0 0 1.58 KB
master EnrichedLog netcoreapp3.1 5.7μs 7.13ns 26.7ns 0 0 0 1.63 KB
master EnrichedLog net472 6.71μs 4.91ns 19ns 0.302 0 0 2.03 KB
#7594 EnrichedLog net6.0 4.29μs 15.5ns 60.1ns 0 0 0 1.58 KB
#7594 EnrichedLog netcoreapp3.1 5.57μs 10.5ns 40.6ns 0 0 0 1.63 KB
#7594 EnrichedLog net472 6.71μs 10ns 38.8ns 0.303 0 0 2.03 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 821ns 0.286ns 1.07ns 0 0 0 576 B
master StartFinishSpan netcoreapp3.1 968ns 4.75ns 21.2ns 0 0 0 576 B
master StartFinishSpan net472 958ns 0.194ns 0.726ns 0.0912 0 0 578 B
master StartFinishScope net6.0 939ns 0.141ns 0.547ns 0 0 0 696 B
master StartFinishScope netcoreapp3.1 1.25μs 6.33ns 29ns 0 0 0 696 B
master StartFinishScope net472 1.16μs 0.118ns 0.457ns 0.0994 0 0 658 B
#7594 StartFinishSpan net6.0 785ns 3.64ns 14.1ns 0 0 0 576 B
#7594 StartFinishSpan netcoreapp3.1 957ns 5.07ns 20.9ns 0 0 0 576 B
#7594 StartFinishSpan net472 920ns 0.235ns 0.912ns 0.0876 0 0 578 B
#7594 StartFinishScope net6.0 942ns 4.66ns 17.4ns 0 0 0 696 B
#7594 StartFinishScope netcoreapp3.1 1.22μs 6.49ns 32.4ns 0 0 0 696 B
#7594 StartFinishScope net472 1.14μs 0.251ns 0.973ns 0.103 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 1.07μs 5.5ns 25.2ns 0 0 0 697 B
master RunOnMethodBegin netcoreapp3.1 1.41μs 5.5ns 20.6ns 0 0 0 696 B
master RunOnMethodBegin net472 1.42μs 1.06ns 4.1ns 0.0995 0 0 658 B
#7594 RunOnMethodBegin net6.0 1.07μs 3.55ns 13.7ns 0 0 0 696 B
#7594 RunOnMethodBegin netcoreapp3.1 1.46μs 6.92ns 26.8ns 0 0 0 696 B
#7594 RunOnMethodBegin net472 1.48μs 0.659ns 2.55ns 0.0972 0 0 658 B

@lucaspimentel lucaspimentel enabled auto-merge (squash) October 20, 2025 20:47
@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch 2 times, most recently from 6579b07 to 5f1a21a Compare October 21, 2025 17:59
lucaspimentel and others added 23 commits October 22, 2025 10:25
…use generic constraints

- Convert EnvironmentVariableProvider from class to readonly struct for zero allocation
- Add generic type parameter TEnvVars to extension methods to enable struct usage without boxing
- Update GetLogDirectoryFromEnvVars and GetDefaultLogDirectory to use generic constraints

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Co-authored-by: Andrew Lock <[email protected]>
@lucaspimentel lucaspimentel force-pushed the lpimentel/managed-loader-cleanup branch from 5f1a21a to 12d2d1b Compare October 22, 2025 14:29
@lucaspimentel lucaspimentel merged commit 0270676 into master Oct 22, 2025
150 of 151 checks passed
@lucaspimentel lucaspimentel deleted the lpimentel/managed-loader-cleanup branch October 22, 2025 15:38
@github-actions github-actions bot added this to the vNext-v3 milestone Oct 22, 2025
lucaspimentel added a commit that referenced this pull request Oct 23, 2025
~_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`/`CORECLR_PROFILER_PATH` and friends.

## Reason for change

One less env var for users to set manually. Easier onboarding, less
error-prone.

I recently worked on an escalation where `DD_DOTNET_TRACER_HOME` was set
to the wrong path so the tracer was not loading.

## 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:

  1. **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`)

2. **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

  3. **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`)

  4. **Architecture directory detection**:
- Maintains a `HashSet<string>` of known architecture directories
(`win-x64`, `win-x86`, `linux-x64`, `linux-arm64`, etc)

## 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 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
<!-- Fixes #{issue} -->

⚠️ Based on #7567, which was originally [generated by OpenAI
Codex](https://chatgpt.com/codex/tasks/task_b_68d59245fcec832180db94dd256130ba),
but I made substantial changes to clean up and refactor.


<!--  ⚠️ Note:

Where possible, please obtain 2 approvals prior to merging. Unless
CODEOWNERS specifies otherwise, for external teams it is typically best
to have one review from a team member, and one review from apm-dotnet.
Trivial changes do not require 2 reviews.

MergeQueue is NOT enabled in this repository. If you have write access
to the repo, the PR has 1-2 approvals (see above), and all of the
required checks have passed, you can use the Squash and Merge button to
merge the PR. If you don't have write access, or you need help, reach
out in the #apm-dotnet channel in Slack.
-->

---------

Co-authored-by: Claude <[email protected]>
@andrewlock andrewlock added area:tests unit tests, integration tests type:reliability labels Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tests unit tests, integration tests type:reliability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants