Use a native logger for critical failures in the loader by kevingosse · Pull Request #5929 · DataDog/dd-trace-dotnet

kevingosse · 2024-08-22T10:59:19Z

Summary of changes

Wrap the call to the managed loader in a try/catch, and use a native logger to log the exception if any.

Reason for change

There's already a try/catch in the managed loader, but in some situations we can fail before even getting there (if jitting the method failed, for instance because of a trimmed runtime). Also, the managed loader logger can fail.

Implementation details

This PR adds a try/catch in the code emitted to load the managed loader. In the catch clause, we call a native logging function, with a direct unmanaged calli call to its address.

Because we have now this outer catch block, I removed the IsInSsi logic which isn't needed anymore.

andrewlock · 2024-08-22T11:24:05Z

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).

datadog-ddstaging · 2024-08-22T11:41:24Z

Datadog Report

Branch report: kevin/error_loader
Commit report: 4b29f0a
Test service: dd-trace-dotnet

✅ 0 Failed, 306654 Passed, 2349 Skipped, 22h 2m 18.67s Total Time

andrewlock · 2024-08-22T13:00:44Z

Benchmarks Report for tracer 🐌

Benchmarks for #5929 compared to master:

2 benchmarks are faster, with geometric mean 1.125
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.01μs	45.4ns	301ns	0.0157	0.00785	0	5.43 KB
master	`StartStopWithChild`	netcoreapp3.1	9.9μs	49.7ns	217ns	0.0146	0.00485	0	5.62 KB
master	`StartStopWithChild`	net472	16.1μs	50.3ns	195ns	1.04	0.326	0.103	6.06 KB
#5929	`StartStopWithChild`	net6.0	7.81μs	43.8ns	277ns	0.0115	0.00767	0	5.42 KB
#5929	`StartStopWithChild`	netcoreapp3.1	9.77μs	54.6ns	337ns	0.0236	0.00944	0	5.61 KB
#5929	`StartStopWithChild`	net472	16μs	49.4ns	185ns	1.01	0.298	0.0966	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	509μs	344ns	1.24μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	642μs	237ns	885ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	861μs	558ns	2.09μs	0.428	3.3 KB
#5929	`WriteAndFlushEnrichedTraces`	net6.0	466μs	168ns	630ns	0	2.7 KB
#5929	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	624μs	214ns	799ns	0	2.7 KB
#5929	`WriteAndFlushEnrichedTraces`	net472	843μs	589ns	2.28μs	0.422	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	185μs	1.06μs	7.39μs	0.183	18.45 KB
master	`SendRequest`	netcoreapp3.1	214μs	1.1μs	6.7μs	0.204	20.61 KB
master	`SendRequest`	net472	0.000732ns	0.000431ns	0.00161ns	0	0 b
#5929	`SendRequest`	net6.0	189μs	1.05μs	6.88μs	0.179	18.45 KB
#5929	`SendRequest`	netcoreapp3.1	212μs	1.12μs	7.71μs	0.209	20.61 KB
#5929	`SendRequest`	net472	0.000939ns	0.000452ns	0.00175ns	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	564μs	3.25μs	25.8μs	0.551	0	0	41.6 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	694μs	3.58μs	16.4μs	0.355	0	0	41.83 KB
master	`WriteAndFlushEnrichedTraces`	net472	865μs	3.58μs	13.9μs	8.13	2.57	0.428	53.3 KB
#5929	`WriteAndFlushEnrichedTraces`	net6.0	543μs	2.31μs	8.65μs	0.561	0	0	41.64 KB
#5929	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	692μs	2.74μs	10.3μs	0.353	0	0	41.75 KB
#5929	`WriteAndFlushEnrichedTraces`	net472	884μs	2.26μs	8.74μs	8.13	2.57	0.428	53.31 KB

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

Faster 🎉 in #5929

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.121	1,315.94	1,173.53

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.32μs	1.49ns	5.75ns	0.0144	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.67μs	1.14ns	4.42ns	0.0135	1.02 KB
master	`ExecuteNonQuery`	net472	2.03μs	1.88ns	7.05ns	0.157	987 B
#5929	`ExecuteNonQuery`	net6.0	1.17μs	1.31ns	5.06ns	0.0141	1.02 KB
#5929	`ExecuteNonQuery`	netcoreapp3.1	1.65μs	1.21ns	4.67ns	0.0132	1.02 KB
#5929	`ExecuteNonQuery`	net472	1.94μs	2.67ns	10.3ns	0.156	987 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net6.0	1.21μs	1.88ns	7.02ns	0.0135	0	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.49μs	0.988ns	3.83ns	0.0135	0	976 B
master	`CallElasticsearch`	net472	2.49μs	1.67ns	6.45ns	0.158	0	995 B
master	`CallElasticsearchAsync`	net6.0	1.35μs	0.614ns	2.3ns	0.0135	0	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.63μs	0.668ns	2.5ns	0.0138	0	1.02 KB
master	`CallElasticsearchAsync`	net472	2.58μs	3.29ns	12.7ns	0.166	0	1.05 KB
#5929	`CallElasticsearch`	net6.0	1.27μs	0.818ns	3.17ns	0.0135	0	976 B
#5929	`CallElasticsearch`	netcoreapp3.1	1.57μs	0.758ns	2.84ns	0.0127	0	976 B
#5929	`CallElasticsearch`	net472	2.43μs	1.79ns	6.69ns	0.157	0.00122	995 B
#5929	`CallElasticsearchAsync`	net6.0	1.32μs	0.504ns	1.95ns	0.0132	0	952 B
#5929	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	0.724ns	2.71ns	0.0144	0	1.02 KB
#5929	`CallElasticsearchAsync`	net472	2.65μs	2.08ns	7.5ns	0.166	0.00133	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.33μs	1.13ns	4.06ns	0.0134	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.51μs	0.745ns	2.89ns	0.0127	952 B
master	`ExecuteAsync`	net472	1.81μs	1.35ns	5.24ns	0.145	915 B
#5929	`ExecuteAsync`	net6.0	1.28μs	0.781ns	3.02ns	0.0129	952 B
#5929	`ExecuteAsync`	netcoreapp3.1	1.56μs	0.653ns	2.35ns	0.0125	952 B
#5929	`ExecuteAsync`	net472	1.72μs	1.36ns	5.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.19μs	1.91ns	7.41ns	0.0313	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.06μs	1.56ns	5.83ns	0.0377	2.76 KB
master	`SendAsync`	net472	7.79μs	2.13ns	8.25ns	0.498	3.15 KB
#5929	`SendAsync`	net6.0	4.13μs	1.6ns	5.77ns	0.0309	2.22 KB
#5929	`SendAsync`	netcoreapp3.1	5.24μs	3.76ns	14.6ns	0.0366	2.76 KB
#5929	`SendAsync`	net472	7.75μs	2.28ns	8.85ns	0.497	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.64μs	0.963ns	3.6ns	0.023	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.01μs	0.896ns	3.35ns	0.0222	1.64 KB
master	`EnrichedLog`	net472	2.72μs	2.69ns	10.4ns	0.249	1.57 KB
#5929	`EnrichedLog`	net6.0	1.53μs	1.09ns	4.21ns	0.023	1.64 KB
#5929	`EnrichedLog`	netcoreapp3.1	2.2μs	1.54ns	5.95ns	0.0218	1.64 KB
#5929	`EnrichedLog`	net472	2.72μs	1.68ns	6.52ns	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	115μs	285ns	1.11μs	0.0567	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	119μs	223ns	833ns	0.0597	0	4.28 KB
master	`EnrichedLog`	net472	149μs	155ns	601ns	0.667	0.222	4.46 KB
#5929	`EnrichedLog`	net6.0	114μs	282ns	1.09μs	0.0573	0	4.28 KB
#5929	`EnrichedLog`	netcoreapp3.1	120μs	354ns	1.37μs	0	0	4.28 KB
#5929	`EnrichedLog`	net472	150μs	191ns	738ns	0.673	0.224	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.08μs	0.724ns	2.71ns	0.0309	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.25μs	1.81ns	7.03ns	0.0297	2.2 KB
master	`EnrichedLog`	net472	4.83μs	0.936ns	3.38ns	0.319	2.02 KB
#5929	`EnrichedLog`	net6.0	3.11μs	1.15ns	4.47ns	0.0311	2.2 KB
#5929	`EnrichedLog`	netcoreapp3.1	4.19μs	2.11ns	7.88ns	0.0292	2.2 KB
#5929	`EnrichedLog`	net472	4.94μs	0.844ns	3.16ns	0.32	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.42μs	0.767ns	2.97ns	0.0162	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.77μs	1.32ns	4.95ns	0.015	0	1.14 KB
master	`SendReceive`	net472	2.2μs	1.69ns	6.53ns	0.184	0	1.16 KB
#5929	`SendReceive`	net6.0	1.31μs	1.09ns	4.22ns	0.0157	0	1.14 KB
#5929	`SendReceive`	netcoreapp3.1	1.76μs	1.93ns	7.48ns	0.015	0	1.14 KB
#5929	`SendReceive`	net472	2.22μs	1.6ns	5.97ns	0.184	0.00111	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.76μs	1.38ns	4.78ns	0.0221	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.95μs	1.17ns	4.53ns	0.0219	1.65 KB
master	`EnrichedLog`	net472	4.6μs	1.63ns	6.33ns	0.322	2.04 KB
#5929	`EnrichedLog`	net6.0	2.84μs	1.27ns	4.94ns	0.0224	1.6 KB
#5929	`EnrichedLog`	netcoreapp3.1	3.96μs	0.756ns	2.72ns	0.0216	1.65 KB
#5929	`EnrichedLog`	net472	4.46μs	2.76ns	10.7ns	0.324	2.04 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	400ns	0.675ns	2.61ns	0.00802	576 B
master	`StartFinishSpan`	netcoreapp3.1	547ns	1.85ns	6.69ns	0.00772	576 B
master	`StartFinishSpan`	net472	660ns	0.227ns	0.878ns	0.0916	578 B
master	`StartFinishScope`	net6.0	513ns	0.596ns	2.31ns	0.00981	696 B
master	`StartFinishScope`	netcoreapp3.1	741ns	0.291ns	1.13ns	0.00933	696 B
master	`StartFinishScope`	net472	859ns	0.632ns	2.36ns	0.104	658 B
#5929	`StartFinishSpan`	net6.0	395ns	0.255ns	0.988ns	0.0081	576 B
#5929	`StartFinishSpan`	netcoreapp3.1	559ns	0.238ns	0.89ns	0.00783	576 B
#5929	`StartFinishSpan`	net472	596ns	0.41ns	1.59ns	0.0917	578 B
#5929	`StartFinishScope`	net6.0	498ns	0.17ns	0.658ns	0.00976	696 B
#5929	`StartFinishScope`	netcoreapp3.1	813ns	0.53ns	2.05ns	0.00939	696 B
#5929	`StartFinishScope`	net472	833ns	0.969ns	3.75ns	0.104	658 B

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

Faster 🎉 in #5929

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0	1.128	673.62	597.22

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	673ns	0.659ns	2.38ns	0.00958	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	942ns	0.427ns	1.65ns	0.00901	696 B
master	`RunOnMethodBegin`	net472	1.12μs	1.54ns	5.55ns	0.104	658 B
#5929	`RunOnMethodBegin`	net6.0	597ns	0.373ns	1.44ns	0.00959	696 B
#5929	`RunOnMethodBegin`	netcoreapp3.1	922ns	0.541ns	2.09ns	0.00928	696 B
#5929	`RunOnMethodBegin`	net472	1.08μs	0.198ns	0.766ns	0.104	658 B

andrewlock

LGTM AFAICT but you know, it's C++ so.... yeah

andrewlock · 2024-08-23T10:41:50Z

+    auto catchBegin = rewriterWrapper_void.CallMember(object_to_string_member_ref, true);
+    rewriterWrapper_void.StLocal(6);
+    rewriterWrapper_void.LoadStr(error_token);
+    rewriterWrapper_void.LoadLocal(6);
+    rewriterWrapper_void.CallMember(string_concat_member_ref, false);
+    rewriterWrapper_void.StLocal(6);
+    rewriterWrapper_void.LoadLocal(6);
+    rewriterWrapper_void.CallMember(string_to_char_array_member_ref, true);
+    rewriterWrapper_void.StLocal(8);
+    rewriterWrapper_void.LoadLocal(8);
+    rewriterWrapper_void.CreateInstr(CEE_LDC_I4_0);
+    rewriterWrapper_void.CreateInstr(CEE_LDELEMA, char_type_ref);
+    rewriterWrapper_void.CreateInstr(CEE_CONV_U);
+    rewriterWrapper_void.StLocal(7);
+    rewriterWrapper_void.CreateInstr(CEE_LDC_I4_3);
+    rewriterWrapper_void.LoadLocal(7);
+    rewriterWrapper_void.LoadLocal(6);
+    rewriterWrapper_void.CallMember(string_get_length_member_ref, true);
+    rewriterWrapper_void.LoadInt64((INT64)&NativeLog);
+    rewriterWrapper_void.CreateInstr(CEE_CONV_I);
+    rewriterWrapper_void.CreateInstr(CEE_CALLI, log_signature_token);
+    rewriterWrapper_void.CreateInstr(CEE_LDNULL);
+    rewriterWrapper_void.StLocal(8);
+    auto catchEnd = rewriterWrapper_void.CreateInstr(CEE_LEAVE_S, returnInstr);


Would love if we could get a https://sharplab.io/ kind of view of this 😄

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AbEBLDAfAASQAIBBAWACgBvKgXyqoIHZiBhK4r4ggDitqVuxAHQAzbDAwATYgAco2AG4BDAC4xiAZzXrsYYtgB2agMwAmYgH1sW0lq0wAtsAwBPADIQV0mNMZC3CJOMGoAFhCyCsrqmmHYvsBuWtgA5tq6avrEnMLcShAJxADiofaOLu6kRtIAym4uEBhaAEJuGlrEABS5eXkA2hAArmoAusRGekqaxmoAZMQqDs6ubgAKalBovX3cgyPjsxYLSxWrtdgAXjDbgbt7w2MTUzMmC1oNwE1aG1s79/snkdzO9Pt8Ltd/twAJQ8Uw5O7EQTCOjEAD0aOIMBqxAgYmIIXCkXYIBApTU5RWVRq9UazTaHQCwgIAE4MnowFCuAUigBZbqwgimHbI3YEACsxH4lC5ogwEDAKmahiM2DU3Vlwn6AAZxpMstMVerTlT1ptbvctQBGQ4mCyLZaVNwQm6avbmPUvI3aMHNX4Wy1cfqmW1mSwfOlaF0BwP9FDjIazXj9cYmp0MmBaGOW/rihNJlM+yMZrNuoNIcY6RRGdIhBwqVKuxEA5jjMBhFRQABU8mzAN4bY7UELcmMRj8suhstlIk2bllgjL6MxbDCMDAAGtDPilbAfG5iPKfBPm7sZFoxDIVEb7WxSTY7I7Vl5j/5T30ZGARNgUCIrUvP2/X9tSXRUMAwG9LH6eodGcEQABUwj3aRjFSUYYI0JwEKQmAfFQkQAEkTBgKB5U3PxSTYCAnDkTsYAAUQQdsVBrGAumBOY0EgrjgSnd88kAn8/1AmAAEcl2AKAxCVRwRE6IwIDUKwj18N9Ay4DBcMNWA1CGKAjCZdSFKUlSKMMwMMWIWoNDkYgrVhVd1y3bB8XCTQ01WQxOlM2RjEWYgxCGcCD02IZYNkaRqJUYwRARdTiDAiCwAwU5iGg5JMJEAAlIYTGwEJ0IyuDSDkOQABEor89LYKwnK8oKjDitKiqnGioxSUbJS2D02ATBatquj4+LEqUbAoHVYwdBYsBNC+Jo0sa2rcqyBqiqwkrysq9qQE6qwCK0AAJaiIEbBSwsG8zLUk0KYDkryrAiJwTuxYYtAArTNB0vSDJlfjhFsB7jtO16QFApUkpShwFrW7LlvymBCpqkQNv6qrFth+qEfRlGto60IrG6qBerUVGjAuv7uBGsaJqMKajBm4g5og6rMrqlasZhnHWuMPGlP2gAxIL3HgqAwo0aRyYk8bRduzoAcC4KrFC8L3pUbTQm+y77nloW3CV0XwtBimuEs6yYFs8wHPB4geVkclKSdao6l9Vp2kzLpHmIIi1F+B0zncf1cRGb0PPcaMg/Vb3fYjL4/XNCPvRj8ErhgIb1JkMiVDukDjcPaRM7u/9c4zhUs86cwAPz0u7uFXPEptwpZDvMkyifalneLN2tC6fVlFeeYeLtEEuN7w1Zk4yC5j4pdTZs4hTCt8CG75TstA7DARCouQ3C6KPNm0YYoBmrikg0QtfB0Yw9AgIxB/VKbxqI3wEDvw9sVScJYTUCB5AgOQgtiMQNyM9MStUmI2WQocDyn0zJXMiwlc7jgAO6digNDJGbN4aI0yhmJcOh4GdBQHAhUIgc7xRLl+QhlcvxCTIenKuX4i7DWtrbdBrM4YhEIsRKAf9agkVGjNLQ2C4KYM4d7EivD+H6EzCIXkq916UT/jvUe/cuKJhMMmUYd8LDaMtlrXYs9zbEBQIvCCrC5FQDXkqTeSjd4mGjofY+jM3bn0zFkXuN9X4PzUE/GAL9vSaRrJ/IBP85B/wARoIBa4QFFljp0GBb1i4MJELXeKyDUFsJERwjmSNcG53wSQzo4piFfgrkkghxBilJJocBEpKSwZL1YSzLJmMuEaB4XIPhUABGZmEUtVp4iOldJ6VoWR8ilSKO3j3L0sw1EFi0dxSC09c6GNsuKUxVkYZZRgJedcWQb7IzbgedG2zdlgH2UYQ5/s3AiBfBLGBKYT4uNGNCNOgYKF3SIeUwplSGkQ1Ss0/p7M+nZR2Zpc52ADkO08oCjGwKTlgr2ZCy50L3CkjuV0dRahNFzI0SmZZ8VVnECQBsyauh6aaEJBEWQqKblsD3BoIidMZpdAAERlXUD4E6CEoAqBmpvDAUA1g8IkJpKAYzwF+Fud4Xw4rrKdl0nIVlbzLTnn3sQdlnLIqpB5Xy26bBBXCrxLgEiEqGxSruaa+V40hhKr+aNcaKpmWaAgMAAAVnszJQKsEIrORcq5ppKIMpgEy8lLKqyoRVfcMJchVbTDul9fSsoGD8UVGodsXq4U+phoxGacgLkLhiWbWyzArbprCIzMiG57XUydWGzQEaayZtETkzKAB5d1ezSTwQgNZasqRybxQKZQ4llcqwauqFiIm0BcRgDAHpPwKoomUpYua2QpkoAgA1XUzoJB1J/IyP25t2SQV9tQoo+m6guiNtSFxG9UbdjDrukgHdo667gwdTTZ1CUhyFlhS209mxz0gB7auTspAiYqB3g+voT7OjSnIck+D1CgKkMrlIZw15/0nvRmBqAoEb5KBEEMPBagKnMBQ0JVJ9DyOvpfe+8Cn663TX7vabDmNAP9t5lYDw79wiDvoTQ3gxBtQIH5uJiTkmpP8wI0YIj2A/nYGILlMBa6MjSHrrbdiQ8R4zJMLomDAlpBGCFqRipCH04fQTRrfS8Vk33GnPxRN7VZQ6R2CmugQA=

dromanol · 2024-08-23T13:06:00Z

+    hr = metadata_emit->DefineTypeRefByName(corlib_ref, WStr("System.Char"), &char_type_ref);
+    if (FAILED(hr))
+    {
+        Logger::Warn("GenerateVoidILStartupMethod: DefineTypeRefByName::System.Char failed");


Shouldn't all these Warns be Errs? If this fails something nasty has happened

True, but the existing logs are all warnings. I can we can change that in another PR

dromanol

Nice 👍

## Summary of changes Wrap the call to the managed loader in a try/catch, and use a native logger to log the exception if any. ## Reason for change There's already a try/catch in the managed loader, but in some situations we can fail before even getting there (if jitting the method failed, for instance because of a trimmed runtime). Also, the managed loader logger can fail. ## Implementation details This PR adds a try/catch in the code emitted to load the managed loader. In the catch clause, we call a native logging function, with a direct unmanaged `calli` call to its address. Because we have now this outer catch block, I removed the `IsInSsi` logic which isn't needed anymore.

…#5939) ## Summary of changes Wrap the call to the managed loader in a try/catch, and use a native logger to log the exception if any. ## Reason for change There's already a try/catch in the managed loader, but in some situations we can fail before even getting there (if jitting the method failed, for instance because of a trimmed runtime). Also, the managed loader logger can fail. ## Implementation details This PR adds a try/catch in the code emitted to load the managed loader. In the catch clause, we call a native logging function, with a direct unmanaged `calli` call to its address. Because we have now this outer catch block, I removed the `IsInSsi` logic which isn't needed anymore. ## Other details Backport of #5929 Co-authored-by: Kevin Gosse <[email protected]>

Kevin Gosse added 4 commits August 21, 2024 18:59

Implement the native logging method

dddb702

Fix ToString implementation

7a0fef5

Better error message

4e600bd

Propagate the exception in the loader

8dc6bb1

Cleanup + comment

ecc25e2

kevingosse marked this pull request as ready for review August 22, 2024 11:38

kevingosse requested review from a team as code owners August 22, 2024 11:38

Kevin Gosse added 2 commits August 22, 2024 17:18

Revert changes on string.cpp (moved to a separate PR)

34eda27

*sigh*

4b29f0a

andrewlock approved these changes Aug 23, 2024

View reviewed changes

dromanol reviewed Aug 23, 2024

View reviewed changes

dromanol approved these changes Aug 23, 2024

View reviewed changes

kevingosse merged commit 4531f1b into master Aug 23, 2024

kevingosse deleted the kevin/error_loader branch August 23, 2024 15:02

github-actions Bot added this to the vNext-v3 milestone Aug 23, 2024

andrewlock added the area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) label Aug 23, 2024

andrewlock mentioned this pull request Aug 23, 2024

Use a native logger for critical failures in the loader (#5929 -> v2) #5939

Merged

Uh oh!

Conversation

kevingosse commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Uh oh!

andrewlock commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution-Time Benchmarks Report ⏱️

Uh oh!

datadog-ddstaging Bot commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

andrewlock commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5929

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5929

Raw results

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

kevingosse Aug 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dromanol Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

kevingosse Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

dromanol left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kevingosse commented Aug 22, 2024 •

edited

Loading

andrewlock commented Aug 22, 2024 •

edited

Loading

datadog-ddstaging Bot commented Aug 22, 2024 •

edited

Loading

andrewlock commented Aug 22, 2024 •

edited

Loading

kevingosse Aug 23, 2024 •

edited

Loading