Fix dlsym issue #6048

gleocadie · 2024-09-18T06:28:02Z

Summary of changes

This PR addresses the issue #6045

Reason for change

When using the dlsym function, the compiler adds in the import symbols table that we need the dlsym symbol.
Before being a universal binary (same binary used for glibc-based linux and musl-libc-based linux) and the compiler added in a DT_NEEDED section the library libdl.so (the library containing dlsym). When the wrapper is loaded, it will look through all the DT_NEEDED sections to find a library that contains the dlsym symbol.
Since being a universal binary, the DT_NEEDED sections are removed (part of being universal) and we have to resolve by hand needed symbols (dlsym, pthread_once ..).
If we use dlsym (or other symbol), we will hit this issue.

Implementation details

use __dd_dlsym instead

Test coverage

Added a snapshot test using nm that verifies that the undefined symbols in the universal binary haven't changed. It's equivalent to running

nm -D Datadog.Linux.ApiWrapper.x64.so | grep ' U ' | awk '{print $2}' | sed 's/@.*//' | sort

but done using Nuke instead. It would probably make sense for this to be a "normal" test in the native tests, but given it has a dependency on nm, which is definitely available in the universal build dockerfile it was quicker and easier to get this up and running directly. When it fails, it prints the diff and throws an exception, e.g.

System.Exception: Found differences in undefined symbols (dlsym) in the Native Wrapper library. Verify that these changes are expected, and will not cause problems. Removing symbols is generally a safe operation, but adding them could cause crashes. If the new symbols are safe to add, update the snapshot file at C:\repos\dd-trace-dotnet\tracer\test\snapshots\native-wrapper-symbols-x64.verified.txt with the new values

Other details

This will be hotfixed onto 3.3.1 ~~and 2.59.1~~
note: backporting to 2.x was not required, see #6051

andrewlock · 2024-09-18T06:55:43Z

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 (6048) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (70ms)  : 66, 74
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (1,103ms)  : 1078, 1128
     .   : milestone, 1103,
    master - mean (1,119ms)  : 1092, 1145
     .   : milestone, 1119,

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

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (805ms)  : 787, 823
     .   : milestone, 805,
    master - mean (809ms)  : 792, 827
     .   : milestone, 809,

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

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (751ms)  : 728, 774
     .   : milestone, 751,
    master - mean (761ms)  : 739, 783
     .   : milestone, 761,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6048) - mean (191ms)  : 188, 194
     .   : milestone, 191,
    master - mean (192ms)  : 186, 198
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (1,197ms)  : 1166, 1229
     .   : milestone, 1197,
    master - mean (1,201ms)  : 1175, 1228
     .   : milestone, 1201,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6048) - mean (277ms)  : 273, 281
     .   : milestone, 277,
    master - mean (278ms)  : 273, 282
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (961ms)  : 939, 984
     .   : milestone, 961,
    master - mean (968ms)  : 947, 988
     .   : milestone, 968,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6048) - mean (265ms)  : 260, 269
     .   : milestone, 265,
    master - mean (266ms)  : 261, 271
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (6048) - mean (940ms)  : 916, 965
     .   : milestone, 940,
    master - mean (945ms)  : 917, 973
     .   : milestone, 945,

datadog-ddstaging · 2024-09-18T06:57:55Z

Datadog Report

Branch report: gleocadie/fix-dlsym-missing-symbol
Commit report: 3021e65
Test service: dd-trace-dotnet

✅ 0 Failed, 363663 Passed, 2331 Skipped, 16h 36m 43.13s Total Time

andrewlock · 2024-09-18T07:25:28Z

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 (6048) (11.187M)   : 0, 11186743
    master (10.970M)   : 0, 10969589
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6048) (7.317M)   : 0, 7317336
    master (7.289M)   : 0, 7289422
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.509M)   : 0, 7509112

    section Manual
    master (10.881M)   : 0, 10881297

    section Manual + Automatic
    This PR (6048) (6.889M)   : 0, 6889444
    master (6.861M)   : 0, 6860819

    section DD_TRACE_ENABLED=0
    master (10.102M)   : 0, 10102349

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6048) (9.548M)   : 0, 9547890
    master (9.401M)   : 0, 9401151
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6048) (6.643M)   : 0, 6642717
    master (6.258M)   : 0, 6258331

    section Trace stats
    master (6.916M)   : 0, 6916114

    section Manual
    master (9.602M)   : 0, 9602137

    section Manual + Automatic
    This PR (6048) (6.221M)   : 0, 6220787
    master (6.276M)   : 0, 6275721

    section DD_TRACE_ENABLED=0
    master (8.931M)   : 0, 8931200

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6048) (10.059M)   : 0, 10059027
    master (10.040M)   : 0, 10040451
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6048) (6.848M)   : 0, 6847718
    master (6.752M)   : 0, 6751864
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.262M)   : 0, 7262061

    section Manual
    master (10.100M)   : 0, 10100288

    section Manual + Automatic
    This PR (6048) (6.169M)   : 0, 6168579
    master (6.280M)   : 0, 6280374

    section DD_TRACE_ENABLED=0
    master (9.365M)   : 0, 9365451

andrewlock · 2024-09-18T07:56:01Z

Benchmarks Report for tracer 🐌

Benchmarks for #6048 compared to master:

1 benchmarks are faster, with geometric mean 1.180
1 benchmarks are slower, with geometric mean 1.179
1 benchmarks have fewer 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	7.78μs	44.7ns	334ns	0.015	0.00502	0	5.42 KB
master	`StartStopWithChild`	netcoreapp3.1	9.84μs	53.6ns	326ns	0.0244	0.00976	0	5.62 KB
master	`StartStopWithChild`	net472	16μs	54.9ns	213ns	1.01	0.285	0.0869	6.07 KB
#6048	`StartStopWithChild`	net6.0	7.72μs	44.5ns	339ns	0.0218	0.0109	0.00364	5.42 KB
#6048	`StartStopWithChild`	netcoreapp3.1	9.65μs	48.9ns	289ns	0.019	0.00948	0	5.62 KB
#6048	`StartStopWithChild`	net472	16μs	47.7ns	185ns	1.01	0.293	0.095	6.07 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	470μs	194ns	725ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	614μs	655ns	2.45μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	823μs	324ns	1.25μs	0.411	3.3 KB
#6048	`WriteAndFlushEnrichedTraces`	net6.0	479μs	458ns	1.77μs	0	2.7 KB
#6048	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	633μs	476ns	1.84μs	0	2.7 KB
#6048	`WriteAndFlushEnrichedTraces`	net472	838μs	656ns	2.54μs	0.417	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	196μs	1.12μs	8.41μs	0.187	18.45 KB
master	`SendRequest`	netcoreapp3.1	218μs	1.24μs	8.96μs	0.212	20.61 KB
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#6048	`SendRequest`	net6.0	200μs	1.14μs	8.91μs	0.188	18.45 KB
#6048	`SendRequest`	netcoreapp3.1	217μs	1.26μs	10.8μs	0.21	20.61 KB
#6048	`SendRequest`	net472	0.00206ns	0.000841ns	0.00326ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6048

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1	41.7 KB	41.49 KB	-211 B	-0.51%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	551μs	2.47μs	11.1μs	0.558	0	0	41.64 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	676μs	3.24μs	13μs	0.345	0	0	41.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	846μs	4μs	16μs	8.45	2.53	0.422	53.27 KB
#6048	`WriteAndFlushEnrichedTraces`	net6.0	569μs	2.94μs	14.7μs	0.568	0	0	41.66 KB
#6048	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	736μs	3.95μs	20.5μs	0.361	0	0	41.49 KB
#6048	`WriteAndFlushEnrichedTraces`	net472	853μs	4.09μs	17.3μs	8.62	2.59	0.431	53.3 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.26μs	1.89ns	7.31ns	0.0146	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.74μs	1.32ns	5.12ns	0.0138	1.02 KB
master	`ExecuteNonQuery`	net472	2.1μs	1.4ns	5.22ns	0.156	987 B
#6048	`ExecuteNonQuery`	net6.0	1.3μs	1.84ns	7.14ns	0.014	1.02 KB
#6048	`ExecuteNonQuery`	netcoreapp3.1	1.73μs	1.55ns	5.79ns	0.0139	1.02 KB
#6048	`ExecuteNonQuery`	net472	2.12μs	3.24ns	12.6ns	0.156	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.22μs	1.71ns	6.4ns	0.0134	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.46μs	0.395ns	1.48ns	0.0133	976 B
master	`CallElasticsearch`	net472	2.43μs	1.48ns	5.74ns	0.157	995 B
master	`CallElasticsearchAsync`	net6.0	1.24μs	0.403ns	1.51ns	0.0137	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.61μs	0.855ns	3.2ns	0.0137	1.02 KB
master	`CallElasticsearchAsync`	net472	2.6μs	2.22ns	8.6ns	0.167	1.05 KB
#6048	`CallElasticsearch`	net6.0	1.19μs	1.02ns	3.83ns	0.0134	976 B
#6048	`CallElasticsearch`	netcoreapp3.1	1.47μs	0.903ns	3.13ns	0.0135	976 B
#6048	`CallElasticsearch`	net472	2.48μs	0.811ns	2.93ns	0.157	995 B
#6048	`CallElasticsearchAsync`	net6.0	1.28μs	0.856ns	3.32ns	0.0135	952 B
#6048	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	2.89ns	10.8ns	0.0139	1.02 KB
#6048	`CallElasticsearchAsync`	net472	2.63μs	1.28ns	4.97ns	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.24μs	0.996ns	3.73ns	0.013	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.68μs	3.06ns	11ns	0.0127	952 B
master	`ExecuteAsync`	net472	1.77μs	2.13ns	8.25ns	0.145	915 B
#6048	`ExecuteAsync`	net6.0	1.21μs	0.691ns	2.59ns	0.0134	952 B
#6048	`ExecuteAsync`	netcoreapp3.1	1.59μs	0.611ns	2.37ns	0.0128	952 B
#6048	`ExecuteAsync`	net472	1.78μs	2.13ns	8.25ns	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.31μs	7.05ns	27.3ns	0.0302	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.05μs	6.45ns	25ns	0.0352	2.76 KB
master	`SendAsync`	net472	7.76μs	12.2ns	47.4ns	0.5	3.15 KB
#6048	`SendAsync`	net6.0	4.31μs	3.48ns	13.5ns	0.0301	2.22 KB
#6048	`SendAsync`	netcoreapp3.1	5.1μs	2.62ns	10.2ns	0.038	2.76 KB
#6048	`SendAsync`	net472	7.84μs	3.14ns	11.8ns	0.498	3.15 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.52μs	2.23ns	8.62ns	0.0229	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.1μs	0.854ns	3.2ns	0.022	1.64 KB
master	`EnrichedLog`	net472	2.44μs	0.393ns	1.42ns	0.249	1.57 KB
#6048	`EnrichedLog`	net6.0	1.48μs	0.983ns	3.68ns	0.023	1.64 KB
#6048	`EnrichedLog`	netcoreapp3.1	2.18μs	0.575ns	2.23ns	0.0225	1.64 KB
#6048	`EnrichedLog`	net472	2.54μs	1.4ns	5.05ns	0.249	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	114μs	173ns	647ns	0.0577	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	119μs	232ns	899ns	0.0597	0	4.28 KB
master	`EnrichedLog`	net472	146μs	85.5ns	320ns	0.655	0.218	4.46 KB
#6048	`EnrichedLog`	net6.0	114μs	163ns	631ns	0.0572	0	4.28 KB
#6048	`EnrichedLog`	netcoreapp3.1	118μs	214ns	828ns	0	0	4.28 KB
#6048	`EnrichedLog`	net472	145μs	74.4ns	278ns	0.653	0.218	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	3.12μs	3.59ns	13.9ns	0.031	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.1μs	3.68ns	14.3ns	0.0287	2.2 KB
master	`EnrichedLog`	net472	4.8μs	5.52ns	21.4ns	0.319	2.02 KB
#6048	`EnrichedLog`	net6.0	3.05μs	2.92ns	10.9ns	0.0306	2.2 KB
#6048	`EnrichedLog`	netcoreapp3.1	4.1μs	2.09ns	8.09ns	0.0288	2.2 KB
#6048	`EnrichedLog`	net472	4.81μs	0.869ns	3.25ns	0.319	2.02 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`SendReceive`	net6.0	1.29μs	0.777ns	3.01ns	0.016	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.75μs	1.04ns	4.03ns	0.0157	0	1.14 KB
master	`SendReceive`	net472	2.06μs	0.776ns	3ns	0.184	0	1.16 KB
#6048	`SendReceive`	net6.0	1.41μs	1.61ns	6.25ns	0.0161	0	1.14 KB
#6048	`SendReceive`	netcoreapp3.1	1.77μs	0.826ns	3.2ns	0.016	0	1.14 KB
#6048	`SendReceive`	net472	2.18μs	0.668ns	2.59ns	0.183	0.00108	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.78μs	3.5ns	13.1ns	0.0223	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.8μs	1.61ns	6.22ns	0.0209	1.65 KB
master	`EnrichedLog`	net472	4.22μs	1.31ns	4.72ns	0.323	2.04 KB
#6048	`EnrichedLog`	net6.0	2.86μs	1.01ns	3.79ns	0.0228	1.6 KB
#6048	`EnrichedLog`	netcoreapp3.1	3.99μs	1.58ns	6.1ns	0.022	1.65 KB
#6048	`EnrichedLog`	net472	4.51μs	1.86ns	6.96ns	0.322	2.04 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6048

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.179	395.66	466.58

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	396ns	0.274ns	1.02ns	0.00818	576 B
master	`StartFinishSpan`	netcoreapp3.1	562ns	0.364ns	1.36ns	0.00766	576 B
master	`StartFinishSpan`	net472	657ns	0.827ns	3.2ns	0.0916	578 B
master	`StartFinishScope`	net6.0	469ns	0.354ns	1.37ns	0.00965	696 B
master	`StartFinishScope`	netcoreapp3.1	754ns	0.455ns	1.7ns	0.00939	696 B
master	`StartFinishScope`	net472	813ns	0.738ns	2.76ns	0.105	658 B
#6048	`StartFinishSpan`	net6.0	467ns	0.655ns	2.36ns	0.00803	576 B
#6048	`StartFinishSpan`	netcoreapp3.1	619ns	0.726ns	2.81ns	0.00774	576 B
#6048	`StartFinishSpan`	net472	603ns	0.924ns	3.2ns	0.0916	578 B
#6048	`StartFinishScope`	net6.0	483ns	0.343ns	1.33ns	0.00967	696 B
#6048	`StartFinishScope`	netcoreapp3.1	727ns	0.661ns	2.56ns	0.00916	696 B
#6048	`StartFinishScope`	net472	864ns	0.784ns	2.93ns	0.105	658 B

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

Faster 🎉 in #6048

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑netcoreapp3.1	1.180	1,034.42	876.92

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	584ns	0.351ns	1.36ns	0.00958	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.03μs	0.748ns	2.9ns	0.00916	696 B
master	`RunOnMethodBegin`	net472	1.07μs	0.88ns	3.41ns	0.104	658 B
#6048	`RunOnMethodBegin`	net6.0	636ns	0.616ns	2.38ns	0.00961	696 B
#6048	`RunOnMethodBegin`	netcoreapp3.1	879ns	3.55ns	13.8ns	0.00949	696 B
#6048	`RunOnMethodBegin`	net472	1.16μs	1.14ns	4.43ns	0.104	658 B

andrewlock · 2024-09-18T08:43:37Z

tracer/build/_build/Build.Utilities.cs

-              // put the symbol at the beginning of each line to make diff clearer when whole blocks of text are missing
-              var lines = diff.text.TrimEnd(trimChar: '\n').Split(Environment.NewLine);
-              return string.Join(Environment.NewLine, lines.Select(l => symbol + l));
+              PrintDiff(diff);


This is just a direct method extraction, so it can be reused in other targets

andrewlock · 2024-09-18T08:44:38Z

tracer/test/snapshots/native-wrapper-symbols.verified.txt

+__errno_location
+__tls_get_addr
+access
+asprintf


Note: no dlysm in here. Prior to the fix in this PR, dlsym was here which caused the issues

bouwkast · 2024-09-18T10:25:52Z

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

@@ -137,6 +138,84 @@ partial class Build
                arguments: $"--build {NativeBuildDirectory} --parallel {Environment.ProcessorCount} --target wrapper");
        });

+    Target TestNativeWrapper => _ => _


gleocadie

LGTM (even I cannot approve 😓 )

## Summary of changes This PR addresses the issue #6045 ## Reason for change When using the `dlsym` function, the compiler adds in the import symbols table that we need the `dlsym` symbol. Before being a universal binary (same binary used for glibc-based linux and musl-libc-based linux) and the compiler added in a `DT_NEEDED` section the library `libdl.so` (the library containing `dlsym`). When the wrapper is loaded, it will look through all the `DT_NEEDED` sections to find a library that contains the `dlsym` symbol. Since being a universal binary, the `DT_NEEDED` sections are removed (part of being universal) and we have to resolve by hand needed symbols (`dlsym`, `pthread_once` ..). If we use `dlsym` (or other symbol), we will hit this issue. ## Implementation details - use `__dd_dlsym` instead ## Test coverage Added a snapshot test using `nm` that verifies that the undefined symbols in the universal binary haven't changed. It's equivalent to running ```bash nm -D Datadog.Linux.ApiWrapper.x64.so | grep ' U ' | awk '{print $2}' | sed 's/@.*//' | sort ``` but done using Nuke instead. It would probably make sense for this to be a "normal" test in the native tests, but given it has a dependency on `nm`, which is _definitely_ available in the universal build dockerfile it was quicker and easier to get this up and running directly. When it fails, it prints the diff and throws an exception, e.g. ```bash System.Exception: Found differences in undefined symbols (dlsym) in the Native Wrapper library. Verify that these changes are expected, and will not cause problems. Removing symbols is generally a safe operation, but adding them could cause crashes. If the new symbols are safe to add, update the snapshot file at C:\repos\dd-trace-dotnet\tracer\test\snapshots\native-wrapper-symbols-x64.verified.txt with the new values ``` ## Other details This will be hotfixed onto 3.3.1 and 2.59.1 --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>

This PR addresses the issue #6045 When using the `dlsym` function, the compiler adds in the import symbols table that we need the `dlsym` symbol. Before being a universal binary (same binary used for glibc-based linux and musl-libc-based linux) and the compiler added in a `DT_NEEDED` section the library `libdl.so` (the library containing `dlsym`). When the wrapper is loaded, it will look through all the `DT_NEEDED` sections to find a library that contains the `dlsym` symbol. Since being a universal binary, the `DT_NEEDED` sections are removed (part of being universal) and we have to resolve by hand needed symbols (`dlsym`, `pthread_once` ..). If we use `dlsym` (or other symbol), we will hit this issue. - use `__dd_dlsym` instead Added a snapshot test using `nm` that verifies that the undefined symbols in the universal binary haven't changed. It's equivalent to running ```bash nm -D Datadog.Linux.ApiWrapper.x64.so | grep ' U ' | awk '{print $2}' | sed 's/@.*//' | sort ``` but done using Nuke instead. It would probably make sense for this to be a "normal" test in the native tests, but given it has a dependency on `nm`, which is _definitely_ available in the universal build dockerfile it was quicker and easier to get this up and running directly. When it fails, it prints the diff and throws an exception, e.g. ```bash System.Exception: Found differences in undefined symbols (dlsym) in the Native Wrapper library. Verify that these changes are expected, and will not cause problems. Removing symbols is generally a safe operation, but adding them could cause crashes. If the new symbols are safe to add, update the snapshot file at C:\repos\dd-trace-dotnet\tracer\test\snapshots\native-wrapper-symbols-x64.verified.txt with the new values ``` This will be hotfixed onto 3.3.1 and 2.59.1 --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>

## Summary of changes This PR addresses the issue #6045 ## Reason for change When using the `dlsym` function, the compiler adds in the import symbols table that we need the `dlsym` symbol. Before being a universal binary (same binary used for glibc-based linux and musl-libc-based linux) and the compiler added in a `DT_NEEDED` section the library `libdl.so` (the library containing `dlsym`). When the wrapper is loaded, it will look through all the `DT_NEEDED` sections to find a library that contains the `dlsym` symbol. Since being a universal binary, the `DT_NEEDED` sections are removed (part of being universal) and we have to resolve by hand needed symbols (`dlsym`, `pthread_once` ..). If we use `dlsym` (or other symbol), we will hit this issue. ## Implementation details - use `__dd_dlsym` instead ## Test coverage Added a snapshot test using `nm` that verifies that the undefined symbols in the universal binary haven't changed. It's equivalent to running ```bash nm -D Datadog.Linux.ApiWrapper.x64.so | grep ' U ' | awk '{print $2}' | sed 's/@.*//' | sort ``` but done using Nuke instead. It would probably make sense for this to be a "normal" test in the native tests, but given it has a dependency on `nm`, which is _definitely_ available in the universal build dockerfile it was quicker and easier to get this up and running directly. When it fails, it prints the diff and throws an exception, e.g. ```bash System.Exception: Found differences in undefined symbols (dlsym) in the Native Wrapper library. Verify that these changes are expected, and will not cause problems. Removing symbols is generally a safe operation, but adding them could cause crashes. If the new symbols are safe to add, update the snapshot file at C:\repos\dd-trace-dotnet\tracer\test\snapshots\native-wrapper-symbols-x64.verified.txt with the new values ``` ## Other details This is a hotfix for - #6048 Co-authored-by: Gregory LEOCADIE <gregory.leocadie@datadoghq.com>

Fix dlsym issue

62fa7be

github-actions bot added the area:profiler Issues related to the continous-profiler label Sep 18, 2024

gleocadie marked this pull request as ready for review September 18, 2024 07:18

gleocadie requested a review from a team as a code owner September 18, 2024 07:18

andrewlock requested review from a team as code owners September 18, 2024 08:14

andrewlock force-pushed the gleocadie/fix-dlsym-missing-symbol branch from 5ff7c72 to 37b8995 Compare September 18, 2024 08:17

andrewlock added the type:bug label Sep 18, 2024

andrewlock force-pushed the gleocadie/fix-dlsym-missing-symbol branch from 37b8995 to 99fd16d Compare September 18, 2024 08:35

andrewlock reviewed Sep 18, 2024

View reviewed changes

Add a snapshot test for the native symbols in the universal binary

3021e65

andrewlock force-pushed the gleocadie/fix-dlsym-missing-symbol branch from 99fd16d to 3021e65 Compare September 18, 2024 08:49

andrewlock approved these changes Sep 18, 2024

View reviewed changes

bouwkast reviewed Sep 18, 2024

View reviewed changes

gleocadie commented Sep 18, 2024

View reviewed changes

andrewlock merged commit 9421a3b into master Sep 18, 2024
78 of 81 checks passed

andrewlock deleted the gleocadie/fix-dlsym-missing-symbol branch September 18, 2024 10:36

github-actions bot added this to the vNext-v3 milestone Sep 18, 2024

bouwkast mentioned this pull request Sep 18, 2024

Reverts [Crashtracking] Fix the handling of COMPlus_DbgMiniDumpName #6046

Closed

andrewlock mentioned this pull request Sep 18, 2024

Fix dlsym issue (#6048 => hotfix) #6049

Merged

andrewlock mentioned this pull request Sep 18, 2024

Fix dlsym issue (#6048 -> v2) #6051

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dlsym issue #6048

Fix dlsym issue #6048

gleocadie commented Sep 18, 2024 •

edited by lucaspimentel

Loading

andrewlock commented Sep 18, 2024 •

edited

Loading

datadog-ddstaging bot commented Sep 18, 2024 •

edited

Loading

andrewlock commented Sep 18, 2024 •

edited

Loading

andrewlock commented Sep 18, 2024

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6048

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6048

Raw results

Faster 🎉 in #6048

Raw results

andrewlock Sep 18, 2024 •

edited

Loading

andrewlock Sep 18, 2024

bouwkast Sep 18, 2024

gleocadie left a comment

Fix dlsym issue #6048

Fix dlsym issue #6048

Conversation

gleocadie commented Sep 18, 2024 • edited by lucaspimentel Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

andrewlock commented Sep 18, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

datadog-ddstaging bot commented Sep 18, 2024 • edited Loading

Datadog Report

andrewlock commented Sep 18, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Sep 18, 2024

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6048

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6048

Raw results

Faster 🎉 in #6048

Raw results

andrewlock Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

andrewlock Sep 18, 2024

Choose a reason for hiding this comment

bouwkast Sep 18, 2024

Choose a reason for hiding this comment

gleocadie left a comment

Choose a reason for hiding this comment

gleocadie commented Sep 18, 2024 •

edited by lucaspimentel

Loading

andrewlock commented Sep 18, 2024 •

edited

Loading

datadog-ddstaging bot commented Sep 18, 2024 •

edited

Loading

andrewlock commented Sep 18, 2024 •

edited

Loading

andrewlock Sep 18, 2024 •

edited

Loading