Inject startup hook preferentially into static constructor when available #6154

andrewlock · 2024-10-15T08:48:08Z

Summary of changes

Before injecting the startup hook, check if the type has an explicit static constructor. If so, skip injecting.

Reason for change

This fixes an issue with manual instrumentation in v3 when the entrypoint contains an explicit static constructor. A simple reproduction is the following:

public static class Program
{
    private static readonly Datadog.Trace.Tracer _tracer = Datadog.Trace.Tracer.Instance;
    static Program()
    {
        _ = _tracer;
    }

    public static void Main(string[] args)
    {
        var builder = WebApplication.CreateBuilder(args);
        var app = builder.Build();

        app.MapGet("/", () => {
            using var scope = _tracer.StartActive("custom-operation-name");
            return "Hello World!";
        });

        app.Run();
    }
}

In this example, we inject into the entrypoint Program.Main(). We add the startup hook which initializes the instrumentation. However, the JIT sees that there is a static constructor and manually inserts a call to a static field to trigger its execution. This is inserted at the start of Program.Main i.e. before our startup hook.

Note that this only happens in .NET Core - in .NET Framework the .cctor is re-jitted before the entrypoint, so the problem doesn't exist. So we limit this implementation to .NET Core to reduce the blast radius.

In this situation, we don't see the initialization of the tracer, so we miss the ReJit calls, and the manual instrumentation library ends up in a half-instrumented state.

This is particularly noticeable for the manual instrumentation case, because we have a lot of instrumentation, but technically it applies to anything that is called from a static constructor in the entrypoint.

Implementation details

When choosing whether to inject the startup hook, check whether the entrypoint type contains a static constructor. If it does, and the constructor is not an implicit constructor, then skip instrumenting, on the basis that the JIT will subsequently inject a call to the static constructor, which we will then instrument, ensuring we are the first call in the app.

In some cases (i.e. IIS) we're specifically choosing where we want to instrument (System.Web.Compilation.BuildManager.InvokePreStartInitMethods()) so we don't try to change anything in that case.

The implicit static constructor is annoying. If there's a static field, but no explicit static constructor, then the profiling API will return a static constructor. However, we shouldn't skip on the basis of this implicit constructor, because JITCompilationStarted is never called for it, so we would end up initializing too late. To detect this, we check whether beforefieldinit is set to try to decide whether to rely on the static constructor actually being called or not.

There's obviously a risk with all this; causing things to initialize in the wrong order is a perennial problem. If for some (strange) reason the static constructor is not invoked next, then we may end up injection somewhere else, which would be... strange... but also Bad™😅 At least we're not doing this behavior in .NET FX which is often the source of weirdness!

Test coverage

Added a reproduction to Samples.ManualInstrumentation. Without the change to startup hook injection the tests fail at ThrowIf(string.IsNullOrEmpty(Tracer.Instance.DefaultServiceName)) (because it is empty) and also at Tracer.Instance.StartActive()

Other details

Discovered this behaviour while trying to solve #6124
#6184 actually resolved the issue, so this is just an edge case

datadog-ddstaging · 2024-10-15T09:12:04Z

Datadog Report

Branch report: andrew/static_constructor_issue
Commit report: 6fbd4da
Test service: dd-trace-dotnet

✅ 0 Failed, 372805 Passed, 2735 Skipped, 25h 29m 51.48s Total Time

andrewlock · 2024-10-15T09:19:20Z

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 (6154) - mean (71ms)  : 68, 74
     .   : milestone, 71,
    master - mean (70ms)  : 68, 73
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (1,113ms)  : 1091, 1134
     .   : milestone, 1113,
    master - mean (1,115ms)  : 1089, 1141
     .   : milestone, 1115,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6154) - mean (110ms)  : 106, 113
     .   : milestone, 110,
    master - mean (110ms)  : 107, 114
     .   : milestone, 110,

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (771ms)  : 754, 789
     .   : milestone, 771,
    master - mean (779ms)  : 761, 797
     .   : milestone, 779,

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

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (728ms)  : 713, 743
     .   : milestone, 728,
    master - mean (732ms)  : 717, 747
     .   : milestone, 732,

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

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (1,200ms)  : 1171, 1230
     .   : milestone, 1200,
    master - mean (1,203ms)  : 1179, 1227
     .   : milestone, 1203,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6154) - mean (277ms)  : 271, 282
     .   : milestone, 277,
    master - mean (275ms)  : 270, 280
     .   : milestone, 275,

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (942ms)  : 925, 958
     .   : milestone, 942,
    master - mean (947ms)  : 927, 967
     .   : milestone, 947,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6154) - mean (264ms)  : 260, 268
     .   : milestone, 264,
    master - mean (264ms)  : 261, 268
     .   : milestone, 264,

    section CallTarget+Inlining+NGEN
    This PR (6154) - mean (924ms)  : 906, 942
     .   : milestone, 924,
    master - mean (928ms)  : 909, 948
     .   : milestone, 928,

tracer/src/Datadog.Tracer.Native/cor_profiler.cpp

andrewlock · 2024-10-15T13:04:39Z

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 (6154) (11.092M)   : 0, 11092209
    master (11.152M)   : 0, 11151929
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6154) (7.227M)   : 0, 7227380
    master (7.329M)   : 0, 7328989
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.628M)   : 0, 7628456

    section Manual
    master (11.077M)   : 0, 11077108

    section Manual + Automatic
    This PR (6154) (6.813M)   : 0, 6813372
    master (6.707M)   : 0, 6706701

    section DD_TRACE_ENABLED=0
    master (10.259M)   : 0, 10259116

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6154) (9.613M)   : 0, 9613468
    master (9.523M)   : 0, 9522917
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6154) (6.663M)   : 0, 6662902
    master (6.629M)   : 0, 6629278

    section Trace stats
    master (6.928M)   : 0, 6927582

    section Manual
    master (9.502M)   : 0, 9502226

    section Manual + Automatic
    This PR (6154) (6.091M)   : 0, 6091447
    master (6.095M)   : 0, 6095300

    section DD_TRACE_ENABLED=0
    master (8.915M)   : 0, 8914995

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6154) (9.711M)   : 0, 9710918
    master (9.874M)   : 0, 9874264
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6154) (6.441M)   : 0, 6440870
    master (6.381M)   : 0, 6380708
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.160M)   : 0, 7159549

    section Manual
    master (9.891M)   : 0, 9891132

    section Manual + Automatic
    This PR (6154) (5.886M)   : 0, 5885597
    master (5.997M)   : 0, 5996831

    section DD_TRACE_ENABLED=0
    master (9.233M)   : 0, 9233418

andrewlock · 2024-10-15T13:09:29Z

Benchmarks Report for tracer 🐌

Benchmarks for #6154 compared to master:

1 benchmarks are faster, with geometric mean 1.178
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.5μs	42.6ns	307ns	0.0149	0.00745	0	5.42 KB
master	`StartStopWithChild`	netcoreapp3.1	9.83μs	51.5ns	258ns	0.0192	0.0096	0	5.61 KB
master	`StartStopWithChild`	net472	16.2μs	61.4ns	238ns	1.04	0.325	0.0974	6.06 KB
#6154	`StartStopWithChild`	net6.0	7.68μs	43.1ns	292ns	0.0152	0.00758	0	5.43 KB
#6154	`StartStopWithChild`	netcoreapp3.1	9.97μs	53.9ns	305ns	0.0188	0.00941	0	5.62 KB
#6154	`StartStopWithChild`	net472	16.1μs	56.4ns	219ns	1.03	0.326	0.0954	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	468μs	453ns	1.76μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	646μs	384ns	1.44μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	844μs	632ns	2.45μs	0.422	3.3 KB
#6154	`WriteAndFlushEnrichedTraces`	net6.0	475μs	157ns	566ns	0	2.7 KB
#6154	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	635μs	341ns	1.28μs	0	2.7 KB
#6154	`WriteAndFlushEnrichedTraces`	net472	839μs	513ns	1.92μs	0.419	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	202μs	1.18μs	10.8μs	0.194	18.45 KB
master	`SendRequest`	netcoreapp3.1	225μs	1.28μs	10.2μs	0.222	20.61 KB
master	`SendRequest`	net472	0.00142ns	0.000631ns	0.00244ns	0	0 b
#6154	`SendRequest`	net6.0	206μs	1.2μs	10.7μs	0.203	18.45 KB
#6154	`SendRequest`	netcoreapp3.1	226μs	1.32μs	11.3μs	0.219	20.61 KB
#6154	`SendRequest`	net472	0.000207ns	0.000156ns	0.000564ns	0	0 b

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

Fewer allocations 🎉 in #6154

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1	41.89 KB	41.68 KB	-214 B	-0.51%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	595μs	3.27μs	21.2μs	0.592	0	0	41.66 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	670μs	2.91μs	10.9μs	0.347	0	0	41.89 KB
master	`WriteAndFlushEnrichedTraces`	net472	899μs	4.15μs	16.1μs	8.48	2.23	0.446	53.33 KB
#6154	`WriteAndFlushEnrichedTraces`	net6.0	572μs	2.85μs	14.3μs	0.563	0	0	41.64 KB
#6154	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	712μs	4.07μs	32.6μs	0.355	0	0	41.68 KB
#6154	`WriteAndFlushEnrichedTraces`	net472	868μs	3.13μs	11.7μs	8.39	2.52	0.419	53.34 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.32μs	0.766ns	2.76ns	0.014	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.22ns	4.57ns	0.0133	1.02 KB
master	`ExecuteNonQuery`	net472	2.1μs	1.82ns	7.04ns	0.156	987 B
#6154	`ExecuteNonQuery`	net6.0	1.26μs	1.36ns	5.07ns	0.0145	1.02 KB
#6154	`ExecuteNonQuery`	netcoreapp3.1	1.68μs	1.37ns	5.3ns	0.0134	1.02 KB
#6154	`ExecuteNonQuery`	net472	2.17μs	2.1ns	8.13ns	0.157	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	0.907ns	3.39ns	0.0135	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.52μs	1.88ns	7.28ns	0.0128	976 B
master	`CallElasticsearch`	net472	2.61μs	2.47ns	9.24ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.34μs	2.6ns	9.36ns	0.0134	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	1.02ns	3.82ns	0.014	1.02 KB
master	`CallElasticsearchAsync`	net472	2.45μs	2.34ns	9.05ns	0.166	1.05 KB
#6154	`CallElasticsearch`	net6.0	1.24μs	0.719ns	2.69ns	0.0137	976 B
#6154	`CallElasticsearch`	netcoreapp3.1	1.55μs	0.406ns	1.41ns	0.0131	976 B
#6154	`CallElasticsearch`	net472	2.55μs	1.28ns	4.61ns	0.157	995 B
#6154	`CallElasticsearchAsync`	net6.0	1.23μs	0.55ns	2.06ns	0.013	952 B
#6154	`CallElasticsearchAsync`	netcoreapp3.1	1.6μs	1.5ns	5.61ns	0.0136	1.02 KB
#6154	`CallElasticsearchAsync`	net472	2.53μs	1.1ns	4.25ns	0.167	1.05 KB

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

Faster 🎉 in #6154

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.178	1,330.28	1,129.69

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.33μs	0.549ns	2.05ns	0.0133	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.68μs	0.628ns	2.43ns	0.0126	952 B
master	`ExecuteAsync`	net472	1.8μs	1.78ns	6.9ns	0.145	915 B
#6154	`ExecuteAsync`	net6.0	1.13μs	0.46ns	1.78ns	0.013	952 B
#6154	`ExecuteAsync`	netcoreapp3.1	1.61μs	0.636ns	2.38ns	0.0129	952 B
#6154	`ExecuteAsync`	net472	1.79μs	0.661ns	2.56ns	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.25μs	1.05ns	3.77ns	0.03	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.25μs	3.37ns	13.1ns	0.0356	2.76 KB
master	`SendAsync`	net472	7.8μs	4.87ns	18.2ns	0.496	3.15 KB
#6154	`SendAsync`	net6.0	4.09μs	1.23ns	4.6ns	0.0307	2.22 KB
#6154	`SendAsync`	netcoreapp3.1	4.96μs	2.35ns	8.81ns	0.0373	2.76 KB
#6154	`SendAsync`	net472	7.71μs	3.49ns	13.5ns	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.47μs	3.37ns	12.6ns	0.023	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.2μs	1.35ns	4.87ns	0.022	1.64 KB
master	`EnrichedLog`	net472	2.54μs	0.822ns	3.07ns	0.249	1.57 KB
#6154	`EnrichedLog`	net6.0	1.48μs	0.761ns	2.95ns	0.023	1.64 KB
#6154	`EnrichedLog`	netcoreapp3.1	2.24μs	1.09ns	4.22ns	0.0224	1.64 KB
#6154	`EnrichedLog`	net472	2.56μs	0.899ns	3.48ns	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	119μs	130ns	505ns	0.06	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	123μs	127ns	475ns	0.0618	0	4.28 KB
master	`EnrichedLog`	net472	153μs	186ns	697ns	0.692	0.231	4.46 KB
#6154	`EnrichedLog`	net6.0	117μs	103ns	399ns	0.0583	0	4.28 KB
#6154	`EnrichedLog`	netcoreapp3.1	120μs	161ns	625ns	0	0	4.28 KB
#6154	`EnrichedLog`	net472	151μs	260ns	1.01μs	0.679	0.226	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.19μs	1.03ns	3.87ns	0.0303	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.06μs	2.11ns	7.59ns	0.0286	2.2 KB
master	`EnrichedLog`	net472	4.75μs	2.42ns	9.37ns	0.319	2.02 KB
#6154	`EnrichedLog`	net6.0	3.08μs	0.986ns	3.82ns	0.0309	2.2 KB
#6154	`EnrichedLog`	netcoreapp3.1	4.21μs	1.85ns	7.18ns	0.0294	2.2 KB
#6154	`EnrichedLog`	net472	4.66μs	1.58ns	6.13ns	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.34μs	0.772ns	2.99ns	0.016	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.77μs	0.89ns	3.33ns	0.0159	0	1.14 KB
master	`SendReceive`	net472	2.13μs	0.993ns	3.85ns	0.183	0	1.16 KB
#6154	`SendReceive`	net6.0	1.43μs	0.884ns	3.42ns	0.0158	0	1.14 KB
#6154	`SendReceive`	netcoreapp3.1	1.77μs	1.07ns	4.13ns	0.015	0	1.14 KB
#6154	`SendReceive`	net472	2.08μs	0.835ns	3.24ns	0.183	0.00105	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.72μs	0.714ns	2.57ns	0.0219	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.9μs	3.06ns	11.8ns	0.0213	1.65 KB
master	`EnrichedLog`	net472	4.21μs	2.43ns	9.09ns	0.322	2.04 KB
#6154	`EnrichedLog`	net6.0	2.76μs	2.4ns	8.66ns	0.0222	1.6 KB
#6154	`EnrichedLog`	netcoreapp3.1	3.92μs	3.84ns	14.4ns	0.0218	1.65 KB
#6154	`EnrichedLog`	net472	4.58μs	6.39ns	24.7ns	0.323	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	403ns	0.26ns	0.974ns	0.0081	576 B
master	`StartFinishSpan`	netcoreapp3.1	608ns	1.55ns	6.02ns	0.00768	576 B
master	`StartFinishSpan`	net472	748ns	0.756ns	2.93ns	0.0916	578 B
master	`StartFinishScope`	net6.0	578ns	0.161ns	0.582ns	0.00969	696 B
master	`StartFinishScope`	netcoreapp3.1	748ns	0.634ns	2.46ns	0.0094	696 B
master	`StartFinishScope`	net472	942ns	0.829ns	3.1ns	0.104	658 B
#6154	`StartFinishSpan`	net6.0	405ns	0.174ns	0.675ns	0.00814	576 B
#6154	`StartFinishSpan`	netcoreapp3.1	653ns	0.221ns	0.795ns	0.00757	576 B
#6154	`StartFinishSpan`	net472	717ns	0.285ns	1.1ns	0.0916	578 B
#6154	`StartFinishScope`	net6.0	542ns	0.24ns	0.897ns	0.00962	696 B
#6154	`StartFinishScope`	netcoreapp3.1	738ns	0.446ns	1.73ns	0.00961	696 B
#6154	`StartFinishScope`	net472	872ns	0.851ns	3.19ns	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	611ns	0.366ns	1.42ns	0.00976	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	931ns	0.893ns	3.46ns	0.00939	696 B
master	`RunOnMethodBegin`	net472	1.19μs	0.685ns	2.47ns	0.104	658 B
#6154	`RunOnMethodBegin`	net6.0	634ns	0.328ns	1.27ns	0.00986	696 B
#6154	`RunOnMethodBegin`	netcoreapp3.1	913ns	0.445ns	1.67ns	0.0093	696 B
#6154	`RunOnMethodBegin`	net472	1.24μs	0.274ns	0.989ns	0.104	658 B

- Doesn't happen on .NET Framework - Make sure we _don't_ count implicit static constructor when deciding whether to skip

tonyredondo

just a nit/question

tonyredondo · 2024-10-29T09:29:35Z

tracer/src/Datadog.Tracer.Native/cor_profiler.cpp

+        {
+            mdMethodDef memberDef;
+            hr = metadataImport->FindMethod(caller.type.id, WStr(".cctor"), 0, 0, &memberDef);
+            if (FAILED(hr))


I remember Kevin saying that FAILED and SUCEEDED macros are unreliable, should we do hr != S_OK then?

All our existing usages of FindMethod() use Failed(hr), which is why I did here, but it's a good question, @kevingosse?

Discussed offline

In the code it doesn't look like it can return S_FALSE
And there are other places where we use it with if (FAILED(hr)), so probably fine

kevingosse approved these changes Oct 15, 2024

View reviewed changes

tracer/src/Datadog.Tracer.Native/cor_profiler.cpp Outdated Show resolved Hide resolved

andrewlock changed the title ~~Andrew/static constructor issue~~ Inject startup hook preferentially into static constructor when available Oct 15, 2024

andrewlock force-pushed the andrew/static_constructor_issue branch from a84d8e7 to ab116c7 Compare October 28, 2024 17:00

github-actions bot added the area:tests unit tests, integration tests label Oct 28, 2024

andrewlock added 2 commits October 29, 2024 09:02

Skip injecting entrypoint when have a static constructor

25f8eb4

- Doesn't happen on .NET Framework - Make sure we _don't_ count implicit static constructor when deciding whether to skip

Add static constructor to manual instrumentation program

6fbd4da

andrewlock force-pushed the andrew/static_constructor_issue branch from ab116c7 to 6fbd4da Compare October 29, 2024 09:02

andrewlock marked this pull request as ready for review October 29, 2024 09:23

andrewlock requested a review from a team as a code owner October 29, 2024 09:23

tonyredondo approved these changes Oct 29, 2024

View reviewed changes

andrewlock merged commit 42ad64d into master Oct 29, 2024
78 of 80 checks passed

andrewlock deleted the andrew/static_constructor_issue branch October 29, 2024 11:11

github-actions bot added this to the vNext-v3 milestone Oct 29, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inject startup hook preferentially into static constructor when available #6154

Inject startup hook preferentially into static constructor when available #6154

andrewlock commented Oct 15, 2024 •

edited

Loading

datadog-ddstaging bot commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6154

Raw results

Raw results

Raw results

Faster 🎉 in #6154

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

tonyredondo left a comment

tonyredondo Oct 29, 2024

andrewlock Oct 29, 2024

andrewlock Oct 29, 2024

Inject startup hook preferentially into static constructor when available #6154

Inject startup hook preferentially into static constructor when available #6154

Conversation

andrewlock commented Oct 15, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

datadog-ddstaging bot commented Oct 15, 2024 • edited Loading

Datadog Report

andrewlock commented Oct 15, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Oct 15, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Oct 15, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6154

Raw results

Raw results

Raw results

Faster 🎉 in #6154

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

tonyredondo left a comment

Choose a reason for hiding this comment

tonyredondo Oct 29, 2024

Choose a reason for hiding this comment

andrewlock Oct 29, 2024

Choose a reason for hiding this comment

andrewlock Oct 29, 2024

Choose a reason for hiding this comment

andrewlock commented Oct 15, 2024 •

edited

Loading

datadog-ddstaging bot commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading

andrewlock commented Oct 15, 2024 •

edited

Loading