Support relative path in `DD_DOTNET_TRACER_HOME` #6434

vandonr · 2024-12-13T11:39:03Z

Summary of changes

Resolve the path read from DD_DOTNET_TRACER_HOME to an absolute path before trying to load an assembly from there.

Reason for change

When writing a sample app and adding Datadog.Trace.Bundle as a dependency, it can be natural to set those variables to something like this :
CORECLR_PROFILER_PATH=./datadog/osx/Datadog.Trace.ClrProfiler.Native.dylib
DD_DOTNET_TRACER_HOME=./datadog

In recent versions of .NET, CORECLR_PROFILER_PATH supports a relative path, but DD_DOTNET_TRACER_HOME doesn't and never has, and throws an ArgumentException as documented when you try this with v3 of the packages.

However, in v2 of the library, if you referenced Datadog.Trace.Bundle or Datadog.Trace, the value of DD_DOTNET_TRACER_HOME was never actually used. That meant that it appeared as though relative paths worked.

In actual fact, what was happening is that referencing one of these NuGet packages caused the Datadog.Trace.dll file to be copied to the bin/publish output folder. As this dll was always next to the .exe, it was loaded automatically by the runtime, and never invoked the AssemblyResolve event code in the managed loader to load from monitoring home.

In v3, we no longer ship Datadog.Trace.dll in the NuGet packages, instead shipping Datadog.Trace.Manual.dll. That means that the DD_DOTNET_TRACER_HOME variable is used with v3, even when you reference the bundle, and gives the appearance that "relative paths are broken". To work around that, this PR adds support for relative paths.

To be clear, we still don't recommend using relative paths for these variables in general. The difficulty in understanding what the path is relative to, means that it will be often be more hassle than it's worth. That said, it can make sense in the bundle scenario described above, hence this PR.

Implementation details

Call GetFullPath() on the value provided in DD_DOTNET_TRACER_HOME. This uses a native call (e.g. Interop.Sys.GetCwd() on linux, GetFullPathNameW() on Windows) to determine the full path given a (potentially) relative path.

Test coverage

Added a couple of tests to confirm relative paths work.
Added a shim implementation of Path.GetRelativePath() for .NET Framework based on the .NET Core implementation.

Other details

Interestingly, getting the correct relative path in the integration tests is a massive pain. The problem is that the relative path you need is not the path relative to the executable, it's relative to "the current directory", which could be anywhere, depending on how you run the application. In the integration tests, for example, the current directory is the directory containing the integration test executable 💀

datadog-ddstaging · 2024-12-13T11:58:08Z

Datadog Report

Branch report: vandonr/nit
Commit report: e9e7a4b
Test service: dd-trace-dotnet

✅ 0 Failed, 231435 Passed, 2094 Skipped, 18h 28m 43.63s Total Time

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

andrewlock · 2024-12-13T12:41:52Z

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.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6434) - mean (69ms)  : 65, 72
     .   : milestone, 69,
    master - mean (69ms)  : 65, 73
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (980ms)  : 959, 1001
     .   : milestone, 980,
    master - mean (980ms)  : 953, 1007
     .   : milestone, 980,

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

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (677ms)  : 662, 692
     .   : milestone, 677,
    master - mean (678ms)  : 661, 695
     .   : milestone, 678,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6434) - mean (91ms)  : 89, 93
     .   : milestone, 91,
    master - mean (92ms)  : 89, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (635ms)  : 615, 654
     .   : milestone, 635,
    master - mean (633ms)  : 614, 653
     .   : milestone, 633,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6434) - mean (194ms)  : 189, 199
     .   : milestone, 194,
    master - mean (195ms)  : 190, 199
     .   : milestone, 195,

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (1,105ms)  : 1075, 1136
     .   : milestone, 1105,
    master - mean (1,099ms)  : 1071, 1126
     .   : milestone, 1099,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6434) - mean (280ms)  : 274, 286
     .   : milestone, 280,
    master - mean (278ms)  : 274, 281
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (870ms)  : 848, 892
     .   : milestone, 870,
    master - mean (871ms)  : 839, 903
     .   : milestone, 871,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6434) - mean (266ms)  : 262, 270
     .   : milestone, 266,
    master - mean (267ms)  : 262, 273
     .   : milestone, 267,

    section CallTarget+Inlining+NGEN
    This PR (6434) - mean (851ms)  : 822, 880
     .   : milestone, 851,
    master - mean (852ms)  : 817, 887
     .   : milestone, 852,

vandonr · 2024-12-13T13:23:03Z

🫢 I think it broke the installer smoke tests

andrewlock · 2024-12-13T14:05:22Z

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6434) (11.053M)   : 0, 11052996
    master (11.099M)   : 0, 11099434
    benchmarks/2.9.0 (11.144M)   : 0, 11143547

    section Automatic
    This PR (6434) (7.329M)   : 0, 7328613
    master (7.256M)   : 0, 7256080
    benchmarks/2.9.0 (7.982M)   : 0, 7981525

    section Trace stats
    master (7.515M)   : 0, 7515192

    section Manual
    master (11.082M)   : 0, 11081778

    section Manual + Automatic
    This PR (6434) (6.806M)   : 0, 6805809
    master (6.731M)   : 0, 6730853

    section DD_TRACE_ENABLED=0
    master (10.311M)   : 0, 10310558

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6434) (9.691M)   : 0, 9690908
    benchmarks/2.9.0 (9.621M)   : 0, 9621397

    section Automatic
    This PR (6434) (6.297M)   : 0, 6296599

    section Manual + Automatic
    This PR (6434) (6.003M)   : 0, 6003168

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6434) (9.962M)   : 0, 9961805
    master (10.331M)   : 0, 10331051

    section Automatic
    This PR (6434) (6.565M)   : 0, 6564654
    master (6.739M)   : 0, 6739399

    section Trace stats
    master (7.175M)   : 0, 7175131

    section Manual
    master (9.252M)   : 0, 9251665

    section Manual + Automatic
    This PR (6434) (6.152M)   : 0, 6152210
    master (5.870M)   : 0, 5869714

    section DD_TRACE_ENABLED=0
    master (8.916M)   : 0, 8916147

andrewlock · 2024-12-13T15:25:05Z

Benchmarks Report for tracer 🐌

Benchmarks for #6434 compared to master:

1 benchmarks are faster, with geometric mean 1.229
All benchmarks have the same 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 ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	8.3μs	47.3ns	335ns	0.0193	0.00773	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.2μs	55.2ns	292ns	0.0154	0.00514	0	5.8 KB
master	`StartStopWithChild`	net472	16.6μs	65.3ns	253ns	1.04	0.296	0.0905	6.22 KB
#6434	`StartStopWithChild`	net6.0	7.9μs	43.6ns	261ns	0.0151	0.00755	0	5.61 KB
#6434	`StartStopWithChild`	netcoreapp3.1	10.3μs	56.6ns	362ns	0.0202	0.0101	0	5.8 KB
#6434	`StartStopWithChild`	net472	16.4μs	54.3ns	210ns	1.05	0.314	0.107	6.21 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	483μs	409ns	1.58μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	688μs	233ns	873ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	857μs	484ns	1.81μs	0.431	3.3 KB
#6434	`WriteAndFlushEnrichedTraces`	net6.0	475μs	450ns	1.74μs	0	2.7 KB
#6434	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	648μs	467ns	1.75μs	0	2.7 KB
#6434	`WriteAndFlushEnrichedTraces`	net472	859μs	452ns	1.69μs	0.428	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	154μs	935ns	9.25μs	0.168	14.47 KB
master	`SendRequest`	netcoreapp3.1	186μs	1.78μs	17.7μs	0.175	17.27 KB
master	`SendRequest`	net472	0.00291ns	0.00119ns	0.00462ns	0	0 b
#6434	`SendRequest`	net6.0	145μs	829ns	6.31μs	0.142	14.47 KB
#6434	`SendRequest`	netcoreapp3.1	176μs	1.13μs	11μs	0.165	17.27 KB
#6434	`SendRequest`	net472	0.0022ns	0.00105ns	0.00405ns	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	587μs	3.32μs	21.8μs	0.291	0	0	41.66 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	672μs	3.63μs	19.5μs	0.388	0	0	41.81 KB
master	`WriteAndFlushEnrichedTraces`	net472	834μs	3.96μs	15.3μs	8.45	2.53	0.422	53.28 KB
#6434	`WriteAndFlushEnrichedTraces`	net6.0	588μs	3.17μs	17.1μs	0.566	0	0	41.73 KB
#6434	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	693μs	3.23μs	13.7μs	0.338	0	0	41.93 KB
#6434	`WriteAndFlushEnrichedTraces`	net472	828μs	3.82μs	15.3μs	8.28	2.48	0.414	53.27 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.3μs	1.36ns	5.28ns	0.0144	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.71μs	2.06ns	7.71ns	0.0133	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.1μs	2.58ns	10ns	0.156	0.00106	987 B
#6434	`ExecuteNonQuery`	net6.0	1.34μs	1.33ns	5.16ns	0.0141	0	1.02 KB
#6434	`ExecuteNonQuery`	netcoreapp3.1	1.82μs	1.27ns	4.41ns	0.0136	0	1.02 KB
#6434	`ExecuteNonQuery`	net472	2.11μs	2.37ns	9.18ns	0.156	0.00106	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.3μs	1.81ns	6.77ns	0.0134	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.52μs	2.42ns	9.36ns	0.013	976 B
master	`CallElasticsearch`	net472	2.47μs	1.18ns	4.43ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.3μs	0.441ns	1.65ns	0.013	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.6μs	0.577ns	2.16ns	0.0136	1.02 KB
master	`CallElasticsearchAsync`	net472	2.6μs	0.815ns	2.82ns	0.167	1.05 KB
#6434	`CallElasticsearch`	net6.0	1.22μs	0.477ns	1.85ns	0.0135	976 B
#6434	`CallElasticsearch`	netcoreapp3.1	1.51μs	0.551ns	1.98ns	0.013	976 B
#6434	`CallElasticsearch`	net472	2.49μs	2.46ns	9.52ns	0.157	995 B
#6434	`CallElasticsearchAsync`	net6.0	1.24μs	0.319ns	1.15ns	0.0131	952 B
#6434	`CallElasticsearchAsync`	netcoreapp3.1	1.62μs	1.02ns	3.95ns	0.0139	1.02 KB
#6434	`CallElasticsearchAsync`	net472	2.66μs	1.84ns	6.88ns	0.166	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.37μs	0.395ns	1.42ns	0.013	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.65μs	0.859ns	3.33ns	0.0124	952 B
master	`ExecuteAsync`	net472	1.82μs	0.896ns	3.47ns	0.145	915 B
#6434	`ExecuteAsync`	net6.0	1.26μs	0.867ns	3.12ns	0.0132	952 B
#6434	`ExecuteAsync`	netcoreapp3.1	1.58μs	1.11ns	4.15ns	0.0127	952 B
#6434	`ExecuteAsync`	net472	1.84μs	0.577ns	2.24ns	0.145	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.46μs	1.65ns	6.37ns	0.0314	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.4μs	1.84ns	7.13ns	0.0379	2.85 KB
master	`SendAsync`	net472	7.61μs	1.62ns	6.29ns	0.493	3.12 KB
#6434	`SendAsync`	net6.0	4.55μs	2.19ns	8.2ns	0.0318	2.31 KB
#6434	`SendAsync`	netcoreapp3.1	5.29μs	1.36ns	5.25ns	0.0374	2.85 KB
#6434	`SendAsync`	net472	7.44μs	2.58ns	9.99ns	0.495	3.12 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.48μs	0.708ns	2.65ns	0.0229	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.2μs	1.25ns	4.69ns	0.022	1.64 KB
master	`EnrichedLog`	net472	2.65μs	2.56ns	9.93ns	0.249	1.57 KB
#6434	`EnrichedLog`	net6.0	1.45μs	0.52ns	1.94ns	0.0234	1.64 KB
#6434	`EnrichedLog`	netcoreapp3.1	2.29μs	0.912ns	3.53ns	0.0216	1.64 KB
#6434	`EnrichedLog`	net472	2.75μs	1.7ns	6.37ns	0.25	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	119μs	135ns	525ns	0.059	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	121μs	157ns	607ns	0	0	4.28 KB
master	`EnrichedLog`	net472	150μs	114ns	426ns	0.677	0.226	4.46 KB
#6434	`EnrichedLog`	net6.0	118μs	248ns	962ns	0.0589	0	4.28 KB
#6434	`EnrichedLog`	netcoreapp3.1	122μs	284ns	1.1μs	0	0	4.28 KB
#6434	`EnrichedLog`	net472	151μs	173ns	669ns	0.684	0.228	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.91μs	0.908ns	3.52ns	0.0306	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.36μs	2.3ns	8.61ns	0.0284	2.2 KB
master	`EnrichedLog`	net472	4.86μs	1.99ns	7.69ns	0.321	2.02 KB
#6434	`EnrichedLog`	net6.0	2.91μs	1.17ns	4.54ns	0.0305	2.2 KB
#6434	`EnrichedLog`	netcoreapp3.1	4.12μs	2.05ns	7.94ns	0.0288	2.2 KB
#6434	`EnrichedLog`	net472	4.97μs	1.07ns	4.13ns	0.321	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.41μs	0.66ns	2.56ns	0.0162	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.73μs	0.74ns	2.86ns	0.0156	1.14 KB
master	`SendReceive`	net472	2.09μs	0.989ns	3.83ns	0.184	1.16 KB
#6434	`SendReceive`	net6.0	1.42μs	0.546ns	2.11ns	0.0163	1.14 KB
#6434	`SendReceive`	netcoreapp3.1	1.81μs	0.981ns	3.8ns	0.0154	1.14 KB
#6434	`SendReceive`	net472	2.25μs	0.991ns	3.57ns	0.183	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.67μs	0.721ns	2.79ns	0.0227	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.84μs	0.924ns	3.2ns	0.0211	1.65 KB
master	`EnrichedLog`	net472	4.39μs	1.53ns	5.71ns	0.323	2.04 KB
#6434	`EnrichedLog`	net6.0	2.79μs	0.712ns	2.57ns	0.0225	1.6 KB
#6434	`EnrichedLog`	netcoreapp3.1	4μs	2.08ns	8.06ns	0.0219	1.65 KB
#6434	`EnrichedLog`	net472	4.38μs	3.81ns	14.2ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6434

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1	1.229	808.11	657.35

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	399ns	0.229ns	0.887ns	0.00804	576 B
master	`StartFinishSpan`	netcoreapp3.1	610ns	0.455ns	1.7ns	0.00768	576 B
master	`StartFinishSpan`	net472	742ns	0.301ns	1.17ns	0.0915	578 B
master	`StartFinishScope`	net6.0	481ns	0.279ns	1.08ns	0.00991	696 B
master	`StartFinishScope`	netcoreapp3.1	808ns	0.536ns	2.08ns	0.00929	696 B
master	`StartFinishScope`	net472	961ns	0.532ns	2.06ns	0.104	658 B
#6434	`StartFinishSpan`	net6.0	397ns	0.29ns	1.12ns	0.00801	576 B
#6434	`StartFinishSpan`	netcoreapp3.1	625ns	0.532ns	2.06ns	0.00773	576 B
#6434	`StartFinishSpan`	net472	675ns	0.549ns	2.13ns	0.0918	578 B
#6434	`StartFinishScope`	net6.0	479ns	0.188ns	0.676ns	0.00986	696 B
#6434	`StartFinishScope`	netcoreapp3.1	657ns	0.463ns	1.79ns	0.00936	696 B
#6434	`StartFinishScope`	net472	872ns	0.558ns	2.16ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	588ns	0.358ns	1.38ns	0.00986	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	858ns	0.299ns	1.12ns	0.00947	696 B
master	`RunOnMethodBegin`	net472	1.14μs	0.509ns	1.97ns	0.104	658 B
#6434	`RunOnMethodBegin`	net6.0	646ns	0.557ns	2.16ns	0.00976	696 B
#6434	`RunOnMethodBegin`	netcoreapp3.1	951ns	2.56ns	9.91ns	0.00968	696 B
#6434	`RunOnMethodBegin`	net472	1.1μs	0.735ns	2.85ns	0.104	658 B

vandonr · 2024-12-16T14:04:44Z

(force-pushed to before the change to Path.GetFullPath to see if installer smoke tests break too)

andrewlock · 2024-12-17T11:07:25Z

Personally I would rather we remove the variable entirely. It's mostly unnecessary I think, given that :

We have to set the profiler path, and use one of two standard layouts, so we should be able to infer the location anyway
We set an env var in the profiler to say where it was found

The downside is that you can't point them at different places, but I think it's very unlikely anyone does that anyway, we can still respect it if it is provided, plus it seems "safer" as there's the fundamental question of "relative to what", right?

vandonr · 2024-12-17T13:20:27Z

Personally I would rather we remove the variable entirely.

I like it

there's the fundamental question of "relative to what", right?

hmm, relative to the location of the executable, but I guess that can eventually get confusing.

andrewlock

I'm still undecided whether we should support this, but if we do, we should add a simple integration test to confirm this works with a relative path (and that it breaks without the fix)

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

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

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs

…tCore.cs Co-authored-by: Andrew Lock <[email protected]>

…tFramework.cs Co-authored-by: Andrew Lock <[email protected]>

…entationTests.cs Co-authored-by: Andrew Lock <[email protected]>

…managed loader for debugging purposes

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs

vandonr · 2025-01-03T13:23:08Z

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/Helpers/PathUtil.cs

+        path = Path.GetFullPath(path);
+
+        // Need to check if the roots are different- if they are we need to return the "to" path.
+        if (!AreRootsEqual(relativeTo, path, comparisonType))


isn't this redundant with the check below on common length ?

Honestly, I didn't look to hard, I just copied from the .NET Core implementation 😄

Ah ok, I thought it was a lot of work just for a test ^^
Maybe worth a comment at the top to say where it's copy/pasted from ?

+ // based on https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs#L843

😃

oh yeah right, it's just that function, not the whole file.

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs

vandonr requested a review from a team as a code owner December 13, 2024 11:39

robertpi reviewed Dec 13, 2024

View reviewed changes

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

vandonr force-pushed the vandonr/nit branch from 7729c07 to c7c2156 Compare December 16, 2024 14:03

andrewlock approved these changes Dec 31, 2024

View reviewed changes

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

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

andrewlock self-requested a review December 31, 2024 12:24

vandonr requested a review from a team as a code owner January 2, 2025 14:04

andrewlock approved these changes Jan 2, 2025

View reviewed changes

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs Outdated Show resolved Hide resolved

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs Outdated Show resolved Hide resolved

andrewlock added type:bug area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) labels Jan 3, 2025

andrewlock changed the title ~~support relative path in DD_DOTNET_TRACER_HOME~~ Support relative path in DD_DOTNET_TRACER_HOME Jan 3, 2025

vandonr and others added 10 commits January 3, 2025 13:00

support relative path in DD_DOTNET_TRACER_HOME

fe1cb13

Update tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.Ne…

59d439b

…tCore.cs Co-authored-by: Andrew Lock <[email protected]>

Update tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.Ne…

f994b1b

…tFramework.cs Co-authored-by: Andrew Lock <[email protected]>

get full path earlier

d87cfa1

add tests with relative paths

c8efb77

Update tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/Instrum…

af46a28

…entationTests.cs Co-authored-by: Andrew Lock <[email protected]>

remove endif

7de2196

Add PathUtil.GetRelativePath helper (for .NET Framework)

bfd0697

Record current directory when failing to load the tracer home in the …

97c36fb

…managed loader for debugging purposes

Fix integration tests

6f6e684

andrewlock force-pushed the vandonr/nit branch from 0c7c2ce to 6f6e684 Compare January 3, 2025 13:00

robertpi reviewed Jan 3, 2025

View reviewed changes

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs Outdated Show resolved Hide resolved

vandonr commented Jan 3, 2025

View reviewed changes

robertpi reviewed Jan 3, 2025

View reviewed changes

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/InstrumentationTests.cs Outdated Show resolved Hide resolved

vandonr added 2 commits January 3, 2025 14:31

remove debug env variable

cfdacf1

remove irrelevant comment

e9e7a4b

robertpi approved these changes Jan 3, 2025

View reviewed changes

kevingosse approved these changes Jan 3, 2025

View reviewed changes

bouwkast approved these changes Jan 3, 2025

View reviewed changes

vandonr merged commit acd9908 into master Jan 3, 2025
100 of 102 checks passed

vandonr deleted the vandonr/nit branch January 3, 2025 16:15

github-actions bot added this to the vNext-v3 milestone Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support relative path in `DD_DOTNET_TRACER_HOME` #6434

Support relative path in `DD_DOTNET_TRACER_HOME` #6434

vandonr commented Dec 13, 2024 •

edited by andrewlock

Loading

datadog-ddstaging bot commented Dec 13, 2024 •

edited

Loading

andrewlock commented Dec 13, 2024 •

edited

Loading

vandonr commented Dec 13, 2024

andrewlock commented Dec 13, 2024 •

edited

Loading

andrewlock commented Dec 13, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6434

Raw results

Raw results

vandonr commented Dec 16, 2024

andrewlock commented Dec 17, 2024

vandonr commented Dec 17, 2024

andrewlock left a comment

vandonr Jan 3, 2025

andrewlock Jan 3, 2025

vandonr Jan 3, 2025

andrewlock Jan 3, 2025

vandonr Jan 3, 2025

Support relative path in DD_DOTNET_TRACER_HOME #6434

Support relative path in DD_DOTNET_TRACER_HOME #6434

Conversation

vandonr commented Dec 13, 2024 • edited by andrewlock Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

datadog-ddstaging bot commented Dec 13, 2024 • edited Loading

Datadog Report

andrewlock commented Dec 13, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

vandonr commented Dec 13, 2024

andrewlock commented Dec 13, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Dec 13, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6434

Raw results

Raw results

vandonr commented Dec 16, 2024

andrewlock commented Dec 17, 2024

vandonr commented Dec 17, 2024

andrewlock left a comment

Choose a reason for hiding this comment

vandonr Jan 3, 2025

Choose a reason for hiding this comment

andrewlock Jan 3, 2025

Choose a reason for hiding this comment

vandonr Jan 3, 2025

Choose a reason for hiding this comment

andrewlock Jan 3, 2025

Choose a reason for hiding this comment

vandonr Jan 3, 2025

Choose a reason for hiding this comment

Support relative path in `DD_DOTNET_TRACER_HOME` #6434

Support relative path in `DD_DOTNET_TRACER_HOME` #6434

vandonr commented Dec 13, 2024 •

edited by andrewlock

Loading

datadog-ddstaging bot commented Dec 13, 2024 •

edited

Loading

andrewlock commented Dec 13, 2024 •

edited

Loading

andrewlock commented Dec 13, 2024 •

edited

Loading

andrewlock commented Dec 13, 2024 •

edited

Loading