[Dynamic Instrumentation] DEBUG-3105 Add code origin for entry spans #6281

dudikeleti · 2024-11-14T01:56:24Z

Summary of changes

This PR extends our code origin capabilities by adding source code information to entry spans in ASP.NET Core applications.
In a previous PR, we implemented code origin for exit spans. This PR completes the feature by adding similar capabilities to entry spans, providing a comprehensive view of code execution paths.

Implementation details

The code origin information is injected when ASP.NET Core is about to execute the endpoint handler.

Performance Optimization

The implementation employs several techniques to minimize performance impact while maintaining robust functionality:

Assembly metadata is processed only upon first encounter, with subsequent operations utilizing cached data
Only essential endpoint information is retained
Utilizes System.Reflection.Metadata for efficient assembly inspection without loading full type information into memory
Employs pre-computed constant strings to reduce GC pressure and string allocations in hot paths

Test coverage

SpanCodeOriginTests

andrewlock · 2024-11-14T02:31:01Z

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 (6281) - mean (72ms)  : 61, 82
     .   : milestone, 72,
    master - mean (69ms)  : 67, 72
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (1,026ms)  : 1005, 1047
     .   : milestone, 1026,
    master - mean (1,026ms)  : 1003, 1049
     .   : milestone, 1026,

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

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (724ms)  : 708, 741
     .   : milestone, 724,
    master - mean (728ms)  : 704, 752
     .   : milestone, 728,

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

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (676ms)  : 660, 693
     .   : milestone, 676,
    master - mean (674ms)  : 656, 692
     .   : milestone, 674,

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

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (1,129ms)  : 1109, 1149
     .   : milestone, 1129,
    master - mean (1,126ms)  : 1097, 1155
     .   : milestone, 1126,

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

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (915ms)  : 884, 946
     .   : milestone, 915,
    master - mean (912ms)  : 884, 940
     .   : milestone, 912,

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

    section CallTarget+Inlining+NGEN
    This PR (6281) - mean (894ms)  : 865, 922
     .   : milestone, 894,
    master - mean (886ms)  : 857, 916
     .   : milestone, 886,

andrewlock · 2024-11-14T03:13:40Z

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 (6281) (11.210M)   : 0, 11209752
    master (10.964M)   : 0, 10964152
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6281) (7.195M)   : 0, 7195047
    master (7.166M)   : 0, 7165701
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.553M)   : 0, 7552596

    section Manual
    master (11.037M)   : 0, 11036554

    section Manual + Automatic
    This PR (6281) (6.580M)   : 0, 6580063
    master (6.706M)   : 0, 6705822

    section DD_TRACE_ENABLED=0
    master (10.151M)   : 0, 10150783

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6281) (9.510M)   : 0, 9509751
    master (9.585M)   : 0, 9584528
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6281) (6.455M)   : 0, 6455005
    master (6.497M)   : 0, 6496538

    section Trace stats
    master (6.703M)   : 0, 6702958

    section Manual
    master (9.509M)   : 0, 9509229

    section Manual + Automatic
    This PR (6281) (5.919M)   : 0, 5918574
    master (6.082M)   : 0, 6081784

    section DD_TRACE_ENABLED=0
    master (8.861M)   : 0, 8860554

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6281) (9.381M)   : 0, 9380526
    master (9.538M)   : 0, 9538182
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6281) (6.078M)   : 0, 6078342
    master (6.268M)   : 0, 6268404
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (6.907M)   : 0, 6906948

    section Manual
    master (9.724M)   : 0, 9724267

    section Manual + Automatic
    This PR (6281) (5.618M)   : crit ,0, 5617920
    master (5.919M)   : 0, 5918616

    section DD_TRACE_ENABLED=0
    master (8.995M)   : 0, 8994559

datadog-ddstaging · 2024-11-14T03:31:24Z

Datadog Report

Branch report: dudik/code-origin-entry-span
Commit report: 41be6c1
Test service: dd-trace-dotnet

✅ 0 Failed, 256055 Passed, 2880 Skipped, 32h 36m 6.67s Total Time

andrewlock · 2024-11-15T00:57:14Z

Benchmarks Report for tracer 🐌

Benchmarks for #6281 compared to master:

1 benchmarks are faster, with geometric mean 1.132
1 benchmarks are slower, with geometric mean 1.123
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	7.88μs	44ns	282ns	0.0153	0.00765	0	5.6 KB
master	`StartStopWithChild`	netcoreapp3.1	9.94μs	46.7ns	198ns	0.015	0.01	0	5.81 KB
master	`StartStopWithChild`	net472	15.9μs	45.9ns	178ns	1.04	0.311	0.104	6.21 KB
#6281	`StartStopWithChild`	net6.0	7.98μs	41.8ns	229ns	0.0154	0.0077	0	5.61 KB
#6281	`StartStopWithChild`	netcoreapp3.1	9.86μs	55.9ns	383ns	0.0202	0.0101	0	5.81 KB
#6281	`StartStopWithChild`	net472	16.1μs	48.8ns	189ns	1.04	0.307	0.0969	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	486μs	246ns	921ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	651μs	349ns	1.31μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	857μs	473ns	1.83μs	0.425	3.3 KB
#6281	`WriteAndFlushEnrichedTraces`	net6.0	487μs	410ns	1.59μs	0	2.7 KB
#6281	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	654μs	561ns	2.17μs	0	2.7 KB
#6281	`WriteAndFlushEnrichedTraces`	net472	838μs	552ns	2.14μ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	126μs	620ns	2.48μs	0.192	14.47 KB
master	`SendRequest`	netcoreapp3.1	145μs	283ns	1.09μs	0.213	17.28 KB
master	`SendRequest`	net472	0.00409ns	0.00151ns	0.00585ns	0	0 b
#6281	`SendRequest`	net6.0	128μs	346ns	1.34μs	0.19	14.47 KB
#6281	`SendRequest`	netcoreapp3.1	142μs	540ns	2.09μs	0.211	17.27 KB
#6281	`SendRequest`	net472	0.00128ns	0.000705ns	0.00273ns	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	588μs	3.5μs	34.5μs	0.568	0	0	41.55 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	690μs	3.92μs	28μs	0.353	0	0	41.79 KB
master	`WriteAndFlushEnrichedTraces`	net472	869μs	4.39μs	21μs	8.45	2.53	0.422	53.34 KB
#6281	`WriteAndFlushEnrichedTraces`	net6.0	577μs	3.05μs	15.6μs	0.556	0	0	41.66 KB
#6281	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	701μs	3.88μs	37μs	0.326	0	0	41.81 KB
#6281	`WriteAndFlushEnrichedTraces`	net472	842μs	3.94μs	15.3μs	8.58	2.45	0.408	53.3 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.38μs	1.25ns	4.68ns	0.0144	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.71μs	5.54ns	21.4ns	0.0135	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.11μs	3.24ns	12.5ns	0.156	0.00106	987 B
#6281	`ExecuteNonQuery`	net6.0	1.35μs	1.52ns	5.88ns	0.0143	0	1.02 KB
#6281	`ExecuteNonQuery`	netcoreapp3.1	1.77μs	2.01ns	7.51ns	0.0133	0	1.02 KB
#6281	`ExecuteNonQuery`	net472	2.06μs	3.62ns	14ns	0.157	0.00103	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	0.468ns	1.75ns	0.0136	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.55μs	1.94ns	7.53ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.56μs	1.75ns	6.76ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.3μs	0.569ns	2.13ns	0.0134	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.64μs	0.743ns	2.88ns	0.0139	1.02 KB
master	`CallElasticsearchAsync`	net472	2.65μs	1.91ns	7.39ns	0.166	1.05 KB
#6281	`CallElasticsearch`	net6.0	1.22μs	2.54ns	9.5ns	0.0132	976 B
#6281	`CallElasticsearch`	netcoreapp3.1	1.61μs	1.59ns	6.16ns	0.0128	976 B
#6281	`CallElasticsearch`	net472	2.63μs	1.8ns	6.96ns	0.158	995 B
#6281	`CallElasticsearchAsync`	net6.0	1.44μs	0.844ns	3.27ns	0.013	952 B
#6281	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	0.989ns	3.7ns	0.0134	1.02 KB
#6281	`CallElasticsearchAsync`	net472	2.63μs	0.775ns	2.79ns	0.167	1.05 KB

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

Faster 🎉 in #6281

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.132	1,400.60	1,236.88

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.4μs	0.507ns	1.9ns	0.0134	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.71μs	2.41ns	9.03ns	0.0131	952 B
master	`ExecuteAsync`	net472	1.84μs	0.885ns	3.43ns	0.145	915 B
#6281	`ExecuteAsync`	net6.0	1.24μs	0.831ns	3.22ns	0.0135	952 B
#6281	`ExecuteAsync`	netcoreapp3.1	1.64μs	0.873ns	3.15ns	0.0124	952 B
#6281	`ExecuteAsync`	net472	1.84μs	0.933ns	3.49ns	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.38μs	3.2ns	12.4ns	0.033	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.28μs	1.25ns	4.51ns	0.0368	2.85 KB
master	`SendAsync`	net472	7.42μs	3.12ns	12.1ns	0.493	3.12 KB
#6281	`SendAsync`	net6.0	4.45μs	2.35ns	9.1ns	0.0313	2.31 KB
#6281	`SendAsync`	netcoreapp3.1	5.39μs	2.38ns	8.9ns	0.038	2.85 KB
#6281	`SendAsync`	net472	7.5μs	1.74ns	6.73ns	0.493	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.58μs	0.738ns	2.56ns	0.0229	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.14μs	1.04ns	4.03ns	0.0224	1.64 KB
master	`EnrichedLog`	net472	2.59μs	0.949ns	3.68ns	0.249	1.57 KB
#6281	`EnrichedLog`	net6.0	1.48μs	0.737ns	2.66ns	0.023	1.64 KB
#6281	`EnrichedLog`	netcoreapp3.1	2.23μs	0.877ns	3.28ns	0.0225	1.64 KB
#6281	`EnrichedLog`	net472	2.59μs	0.895ns	3.35ns	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	113μs	94.8ns	367ns	0.0563	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	115μs	129ns	498ns	0.0572	0	4.28 KB
master	`EnrichedLog`	net472	151μs	111ns	429ns	0.678	0.226	4.46 KB
#6281	`EnrichedLog`	net6.0	114μs	76.7ns	297ns	0	0	4.28 KB
#6281	`EnrichedLog`	netcoreapp3.1	115μs	107ns	413ns	0	0	4.28 KB
#6281	`EnrichedLog`	net472	150μs	112ns	433ns	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μs	0.654ns	2.53ns	0.0312	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.32μs	1.25ns	4.86ns	0.0304	2.2 KB
master	`EnrichedLog`	net472	4.86μs	2.4ns	9.28ns	0.32	2.02 KB
#6281	`EnrichedLog`	net6.0	3.22μs	11.5ns	44.5ns	0.0303	2.2 KB
#6281	`EnrichedLog`	netcoreapp3.1	4.3μs	5.63ns	21.8ns	0.0279	2.2 KB
#6281	`EnrichedLog`	net472	4.92μs	1.29ns	4.98ns	0.319	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.43μs	2.65ns	10.3ns	0.0158	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.74μs	1.13ns	4.39ns	0.0157	1.14 KB
master	`SendReceive`	net472	1.98μs	1.55ns	5.99ns	0.183	1.16 KB
#6281	`SendReceive`	net6.0	1.33μs	0.732ns	2.84ns	0.0159	1.14 KB
#6281	`SendReceive`	netcoreapp3.1	1.77μs	1.06ns	4.12ns	0.0151	1.14 KB
#6281	`SendReceive`	net472	2.17μs	1.24ns	4.82ns	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.82μs	1.15ns	4.46ns	0.0226	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.96μs	1.32ns	5.11ns	0.0218	1.65 KB
master	`EnrichedLog`	net472	4.45μs	11.6ns	44.9ns	0.323	2.04 KB
#6281	`EnrichedLog`	net6.0	2.8μs	0.699ns	2.71ns	0.0225	1.6 KB
#6281	`EnrichedLog`	netcoreapp3.1	3.9μs	1.49ns	5.76ns	0.0214	1.65 KB
#6281	`EnrichedLog`	net472	4.44μs	2.91ns	11.3ns	0.324	2.04 KB

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

Slower ⚠️ in #6281

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.123	401.55	450.76

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	401ns	0.376ns	1.46ns	0.00817	576 B
master	`StartFinishSpan`	netcoreapp3.1	554ns	0.986ns	3.82ns	0.00783	576 B
master	`StartFinishSpan`	net472	630ns	1.05ns	4.07ns	0.0917	578 B
master	`StartFinishScope`	net6.0	558ns	0.645ns	2.41ns	0.0096	696 B
master	`StartFinishScope`	netcoreapp3.1	674ns	1.26ns	4.87ns	0.00927	696 B
master	`StartFinishScope`	net472	873ns	1.04ns	3.88ns	0.105	658 B
#6281	`StartFinishSpan`	net6.0	451ns	0.605ns	2.34ns	0.00795	576 B
#6281	`StartFinishSpan`	netcoreapp3.1	549ns	1.16ns	4.49ns	0.00771	576 B
#6281	`StartFinishSpan`	net472	573ns	1.11ns	4.29ns	0.0917	578 B
#6281	`StartFinishScope`	net6.0	537ns	0.67ns	2.6ns	0.00978	696 B
#6281	`StartFinishScope`	netcoreapp3.1	725ns	0.739ns	2.86ns	0.00939	696 B
#6281	`StartFinishScope`	net472	890ns	1.47ns	5.7ns	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	668ns	0.931ns	3.61ns	0.00974	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	928ns	1.55ns	6ns	0.00929	696 B
master	`RunOnMethodBegin`	net472	1.04μs	1.64ns	6.36ns	0.105	658 B
#6281	`RunOnMethodBegin`	net6.0	649ns	0.832ns	3.22ns	0.00979	696 B
#6281	`RunOnMethodBegin`	netcoreapp3.1	878ns	0.974ns	3.77ns	0.00933	696 B
#6281	`RunOnMethodBegin`	net472	1.06μs	1.61ns	6.22ns	0.104	658 B

GreenMatan

Left a few comments, all in all looking good

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs

GreenMatan · 2024-12-15T10:54:05Z

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs

+                    span.Tags.SetTag(_tags.Type, "exit");
+                    span.Tags.SetTag(_tags.Index[i], info.FrameIndex.ToString());
+                    span.Tags.SetTag(_tags.Method[i], method.Name);
+                    span.Tags.SetTag(_tags.TypeName[i], typeName);


It was also probably flawed before this PR, but - aren't we lying about the index? If we skip certain methods in PopulateUserFrames (skipping all 3rd parties), won't it break the UI since we'll provide Code Origin for frame indices of those 3rd parties? Shall we also count them and use the correct indices here?

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/EndpointDetector.cs

datadog-ddstaging · 2025-01-27T18:44:22Z

Datadog Report

Branch report: dudik/code-origin-entry-span
Commit report: 8ff8f60
Test service: dd-trace-dotnet

❌ 13 Failed (0 Known Flaky), 107928 Passed, 1353 Skipped, 16h 48m 52.88s Total Time

❌ Failed Tests (13)

This report shows up to 5 failed tests.

AttributesInstantiationsOnlyUseBuiltinTypes - Datadog.Trace.ClrProfiler.Managed.Tests.AttributeTests - Details

Expand for error

 Expected }}} to be empty, but found at least one item {"Datadog.Trace.DiagnosticListeners.Target.Handler => Datadog.Trace.DuckTyping.DuckAttribute(Kind : Datadog.Trace.DuckTyping.DuckKind)"}.

AttributesInstantiationsOnlyUseBuiltinTypes - Datadog.Trace.ClrProfiler.Managed.Tests.AttributeTests - Details

Expand for error

 Expected }}} to be empty, but found at least one item {"Datadog.Trace.DiagnosticListeners.Target.Handler => Datadog.Trace.DuckTyping.DuckAttribute(Kind : Datadog.Trace.DuckTyping.DuckKind)"}.

AttributesInstantiationsOnlyUseBuiltinTypes - Datadog.Trace.ClrProfiler.Managed.Tests.AttributeTests - Details

Expand for error

 Expected invalidAttributeUsages to be empty, but found at least one item {"Datadog.Trace.DiagnosticListeners.Target.Handler => Datadog.Trace.DuckTyping.DuckAttribute(Kind : Datadog.Trace.DuckTyping.DuckKind)"}.

AttributesInstantiationsOnlyUseBuiltinTypes - Datadog.Trace.ClrProfiler.Managed.Tests.AttributeTests - Details

Expand for error

 Expected invalidAttributeUsages to be empty, but found at least one item {"Datadog.Trace.DiagnosticListeners.Target.Handler => Datadog.Trace.DuckTyping.DuckAttribute(Kind : Datadog.Trace.DuckTyping.DuckKind)"}.

AttributesInstantiationsOnlyUseBuiltinTypes - Datadog.Trace.ClrProfiler.Managed.Tests.AttributeTests - Details

Expand for error

 Expected invalidAttributeUsages to be empty, but found at least one item {"Datadog.Trace.DiagnosticListeners.Target.Handler => Datadog.Trace.DuckTyping.DuckAttribute(Kind : Datadog.Trace.DuckTyping.DuckKind)"}.

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

tracer/src/Datadog.Trace/DiagnosticListeners/RequestDelegate.cs

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/EndpointDetector.cs

andrewlock · 2025-02-17T11:15:29Z

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/EndpointDetector.cs

+
+namespace Datadog.Trace.Debugger.SpanCodeOrigin;
+
+internal static class EndpointDetector


IMO, this EndpointDetector should have a bunch of unit tests. Right now I think we're relying on the SpanCondOriginTests, which is covering a lot more than just this.

I think it would be easier to make sure we've covered all the variations and corner cases if we did that.

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/EndpointDetector.cs

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

andrewlock · 2025-02-17T11:33:41Z

tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/EndpointDetector.cs

+            }
+
+            bool isPageModel = false, isSignalRHub = false, isCompilerGeneratedType = false;
+            var isController = IsInheritFromTypesOrHasAttribute(typeDef, metadataReader, ControllerAttributes, ControllerBaseNames);


So... here's where you run into trouble. Controllers don't have to derive from any particular type or have an attribute or even have methods with an attribute...

e.g. this is a perfectly good controller (and I just tested it to be sure!)

public class HomeController { public string Index() { return "This is the result"; } }

Of course, I'm not really sure where that leaves us... my inclination is that maybe we shouldn't be trying to second guess what ASP.NET Core does, otherwise we may end up stuck in a pit of complexity 🤔

Would it make sense to consider doing it the other way around? In the diagnostic observer, we know for a fact that an endpoint is a handler, because it's being invoked. We don't need to try to narrow it down.

Of course, that would probably mean a bigger hit on first request for an endpoint rather than trying to do it all in one go. But it would ensure correctness, plus it only gives the overhead for the endpoints that are actually used 🤔 WDYT? Is it worth testing to see the impact of that approach?

(FWIW, I don't think our benchmarks are going to be great for highlighting these various differences in perf...)

I totaly see your point here.
I'm afraid that reading the PDB for each span is something that we don't want to do, but maybe we need an hybrid approach here - reading once per endpoint, or trying to get from cache and fallback to pdb sacn etc. Currently this feature is disable by default and worth case scenario is that we won't have code origin info for various enpoints for beta testing customers, so maybe we should start with the current implementation just to get the feeling of what we are missing (using logs), and then move to the hybrid solution. What do you think? I can do the hybrid solution anyway, but I think that anyway it will required another round of improvements based on real testers.

github-actions bot added the area:debugger label Nov 14, 2024

dudikeleti force-pushed the dudik/code-origin-entry-span branch from 515f396 to 4974f19 Compare November 14, 2024 13:05

dudikeleti force-pushed the dudik/code-origin-entry-span branch 2 times, most recently from 6f51b1a to c8ef7f1 Compare November 15, 2024 15:49

dudikeleti marked this pull request as ready for review November 15, 2024 15:49

dudikeleti requested review from a team as code owners November 15, 2024 15:49

dudikeleti force-pushed the dudik/code-origin-entry-span branch 2 times, most recently from f55fdc9 to 11aa057 Compare December 10, 2024 10:41

dudikeleti force-pushed the dudik/code-origin-entry-span branch from 11aa057 to ee2953f Compare December 12, 2024 06:24

GreenMatan approved these changes Dec 15, 2024

View reviewed changes

dudikeleti force-pushed the dudik/code-origin-entry-span branch 3 times, most recently from 9c9b241 to adf87e9 Compare December 23, 2024 11:36

dudikeleti force-pushed the dudik/code-origin-entry-span branch 3 times, most recently from ef0eed8 to 41be6c1 Compare January 16, 2025 08:27

andrewlock reviewed Jan 16, 2025

View reviewed changes

dudikeleti force-pushed the dudik/code-origin-entry-span branch from 41be6c1 to e80b0cf Compare January 27, 2025 12:18

dudikeleti requested a review from andrewlock February 5, 2025 15:40

dudikeleti force-pushed the dudik/code-origin-entry-span branch 2 times, most recently from 6420564 to 8ff8f60 Compare February 10, 2025 16:31

lucaspimentel reviewed Feb 14, 2025

View reviewed changes

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs Outdated Show resolved Hide resolved

lucaspimentel reviewed Feb 14, 2025

View reviewed changes

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs Outdated Show resolved Hide resolved

lucaspimentel reviewed Feb 14, 2025

View reviewed changes

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs Outdated Show resolved Hide resolved

lucaspimentel reviewed Feb 14, 2025

View reviewed changes

tracer/src/Datadog.Trace/DiagnosticListeners/RequestDelegate.cs Outdated Show resolved Hide resolved

andrewlock reviewed Feb 17, 2025

View reviewed changes

dudikeleti added 10 commits February 26, 2025 18:37

Add code origin for entry spans

107d2a7

Some performance improvements and add EndpointDetector

e7393fe

Add tests

3754eb7

Addressing PR review

a3def3f

code origin is disable if LiveDebugger does not initialized

4dcf958

Addressing PR comments regarding DuckTyping and static access

280f2bd

Add minimal api support

3a755e6

enable nullability

42879ac

PR comments

b642791

change github1s to official github

e0ec2c0

dudikeleti force-pushed the dudik/code-origin-entry-span branch from adcb1b8 to e0ec2c0 Compare February 26, 2025 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dynamic Instrumentation] DEBUG-3105 Add code origin for entry spans #6281

[Dynamic Instrumentation] DEBUG-3105 Add code origin for entry spans #6281

dudikeleti commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 14, 2024 •

edited

Loading

datadog-ddstaging bot commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 15, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6281

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6281

Raw results

Raw results

GreenMatan left a comment

GreenMatan Dec 15, 2024

datadog-ddstaging bot commented Jan 27, 2025 •

edited

Loading

andrewlock Feb 17, 2025

andrewlock Feb 17, 2025

dudikeleti Feb 26, 2025


		namespace Datadog.Trace.Debugger.SpanCodeOrigin;

		internal static class EndpointDetector

[Dynamic Instrumentation] DEBUG-3105 Add code origin for entry spans #6281

Are you sure you want to change the base?

[Dynamic Instrumentation] DEBUG-3105 Add code origin for entry spans #6281

Conversation

dudikeleti commented Nov 14, 2024 • edited Loading

Summary of changes

Implementation details

Performance Optimization

Test coverage

andrewlock commented Nov 14, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Nov 14, 2024 • edited Loading

Throughput/Crank Report ⚡

datadog-ddstaging bot commented Nov 14, 2024 • edited Loading

Datadog Report

andrewlock commented Nov 15, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6281

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6281

Raw results

Raw results

GreenMatan left a comment

Choose a reason for hiding this comment

GreenMatan Dec 15, 2024

Choose a reason for hiding this comment

datadog-ddstaging bot commented Jan 27, 2025 • edited Loading

Datadog Report

❌ Failed Tests (13)

andrewlock Feb 17, 2025

Choose a reason for hiding this comment

andrewlock Feb 17, 2025

Choose a reason for hiding this comment

dudikeleti Feb 26, 2025

Choose a reason for hiding this comment

dudikeleti commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 14, 2024 •

edited

Loading

datadog-ddstaging bot commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 15, 2024 •

edited

Loading

datadog-ddstaging bot commented Jan 27, 2025 •

edited

Loading