-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dynamic Instrumentation] DEBUG-3076 Code origin for exit spans #6216
Conversation
Datadog ReportBranch report: ❌ 48 Failed (3 Known Flaky), 314167 Passed, 2664 Skipped, 25h 16m 22.33s Total Time ❌ Failed Tests (48)
⌛ Performance Regressions vs Default Branch (1)
|
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:
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 (6216) - mean (69ms) : 68, 71
. : milestone, 69,
master - mean (69ms) : 68, 71
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (1,111ms) : 1091, 1131
. : milestone, 1111,
master - mean (1,107ms) : 1085, 1130
. : milestone, 1107,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6216) - mean (108ms) : 106, 110
. : milestone, 108,
master - mean (108ms) : 105, 111
. : milestone, 108,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (771ms) : 756, 785
. : milestone, 771,
master - mean (772ms) : 747, 798
. : milestone, 772,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6216) - mean (92ms) : 89, 94
. : milestone, 92,
master - mean (92ms) : 89, 94
. : milestone, 92,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (729ms) : 713, 744
. : milestone, 729,
master - mean (726ms) : 714, 737
. : milestone, 726,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6216) - mean (190ms) : 187, 193
. : milestone, 190,
master - mean (191ms) : 187, 194
. : milestone, 191,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (1,224ms) : 1202, 1245
. : milestone, 1224,
master - mean (1,223ms) : 1196, 1250
. : milestone, 1223,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6216) - mean (276ms) : 272, 281
. : milestone, 276,
master - mean (277ms) : 272, 281
. : milestone, 277,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (950ms) : 931, 970
. : milestone, 950,
master - mean (955ms) : 939, 970
. : milestone, 955,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6216) - mean (266ms) : 262, 270
. : milestone, 266,
master - mean (265ms) : 262, 269
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (6216) - mean (933ms) : 912, 955
. : milestone, 933,
master - mean (937ms) : 913, 961
. : milestone, 937,
|
Benchmarks Report for tracer 🐌Benchmarks for #6216 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SerilogBenchmark.EnrichedLog‑net6.0 | 1.133 | 2,626.49 | 2,975.71 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | EnrichedLog |
net6.0 | 2.63μs | 1.01ns | 3.91ns | 0.0224 | 0 | 0 | 1.6 KB |
master | EnrichedLog |
netcoreapp3.1 | 3.93μs | 2.17ns | 8.13ns | 0.0217 | 0 | 0 | 1.65 KB |
master | EnrichedLog |
net472 | 4.47μs | 3.48ns | 13ns | 0.323 | 0 | 0 | 2.04 KB |
#6216 | EnrichedLog |
net6.0 | 2.98μs | 1.39ns | 5.39ns | 0.0223 | 0 | 0 | 1.6 KB |
#6216 | EnrichedLog |
netcoreapp3.1 | 3.9μs | 1.04ns | 4.02ns | 0.0217 | 0 | 0 | 1.65 KB |
#6216 | EnrichedLog |
net472 | 4.38μs | 2.42ns | 9.38ns | 0.322 | 0 | 0 | 2.04 KB |
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartFinishSpan |
net6.0 | 401ns | 0.191ns | 0.713ns | 0.00807 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 572ns | 0.867ns | 3.36ns | 0.00773 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 696ns | 0.786ns | 3.05ns | 0.0917 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 488ns | 0.28ns | 1.08ns | 0.00976 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 798ns | 0.541ns | 2.1ns | 0.00926 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 917ns | 0.916ns | 3.55ns | 0.104 | 0 | 0 | 658 B |
#6216 | StartFinishSpan |
net6.0 | 390ns | 0.383ns | 1.48ns | 0.00803 | 0 | 0 | 576 B |
#6216 | StartFinishSpan |
netcoreapp3.1 | 622ns | 0.565ns | 2.19ns | 0.00778 | 0 | 0 | 576 B |
#6216 | StartFinishSpan |
net472 | 701ns | 0.231ns | 0.895ns | 0.0918 | 0 | 0 | 578 B |
#6216 | StartFinishScope |
net6.0 | 517ns | 0.298ns | 1.16ns | 0.00988 | 0 | 0 | 696 B |
#6216 | StartFinishScope |
netcoreapp3.1 | 802ns | 0.287ns | 1.07ns | 0.00966 | 0 | 0 | 696 B |
#6216 | StartFinishScope |
net472 | 846ns | 0.37ns | 1.43ns | 0.104 | 0 | 0 | 658 B |
Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️
Slower ⚠️ in #6216
Benchmark
diff/base
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0
1.186
649.81
770.55
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 | 1.186 | 649.81 | 770.55 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | RunOnMethodBegin |
net6.0 | 650ns | 0.76ns | 2.94ns | 0.00966 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 971ns | 0.724ns | 2.8ns | 0.00922 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.12μs | 0.911ns | 3.53ns | 0.104 | 0 | 0 | 658 B |
#6216 | RunOnMethodBegin |
net6.0 | 771ns | 0.606ns | 2.27ns | 0.00969 | 0 | 0 | 696 B |
#6216 | RunOnMethodBegin |
netcoreapp3.1 | 955ns | 0.785ns | 2.83ns | 0.00952 | 0 | 0 | 696 B |
#6216 | RunOnMethodBegin |
net472 | 1.17μs | 0.589ns | 2.2ns | 0.104 | 0 | 0 | 658 B |
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 (6216) (11.084M) : 0, 11084135
master (11.136M) : 0, 11135915
benchmarks/2.9.0 (11.033M) : 0, 11032866
section Automatic
This PR (6216) (7.240M) : 0, 7239506
master (7.187M) : 0, 7186704
benchmarks/2.9.0 (7.786M) : 0, 7785853
section Trace stats
master (7.427M) : 0, 7426907
section Manual
master (11.098M) : 0, 11097657
section Manual + Automatic
This PR (6216) (6.804M) : 0, 6803757
master (6.654M) : 0, 6654380
section DD_TRACE_ENABLED=0
master (10.119M) : 0, 10118579
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (6216) (9.607M) : 0, 9606780
master (9.394M) : 0, 9393776
benchmarks/2.9.0 (9.495M) : 0, 9494821
section Automatic
This PR (6216) (6.412M) : 0, 6411785
master (6.542M) : 0, 6541950
section Trace stats
master (6.783M) : 0, 6783288
section Manual
master (9.544M) : 0, 9543974
section Manual + Automatic
This PR (6216) (6.012M) : 0, 6011834
master (6.029M) : 0, 6028696
section DD_TRACE_ENABLED=0
master (8.862M) : 0, 8862284
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (6216) (10.075M) : 0, 10075328
master (10.125M) : 0, 10124778
benchmarks/2.9.0 (10.020M) : 0, 10019592
section Automatic
This PR (6216) (6.383M) : 0, 6383120
master (6.523M) : 0, 6523473
benchmarks/2.9.0 (7.255M) : 0, 7255257
section Trace stats
master (7.166M) : 0, 7165805
section Manual
master (9.735M) : 0, 9735078
section Manual + Automatic
This PR (6216) (6.048M) : 0, 6048205
master (5.934M) : 0, 5933603
section DD_TRACE_ENABLED=0
master (9.219M) : 0, 9219305
|
e094951
to
2aa42bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have given a review in person
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
665d7c7
to
e7391f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just some minor suggestions
On a different note, calling new StackTrace()
for every span seems very expensive... is this something that would only be enabled temporarily? It looks like it would be globally enabled as far as I can tell, which seems like it could have a huge impact on performance? 🤔
tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs
Show resolved
Hide resolved
tracer/test/Datadog.Trace.Tests/Debugger/SpanCodeOriginTests.cs
Outdated
Show resolved
Hide resolved
ccc2d56
to
a453128
Compare
Right now it's disabled by default but we plan to enabled it. The plan is also to improve this and not use |
a453128
to
ed456fb
Compare
9a1a33f
to
f363d68
Compare
…processor (#6242) ## Summary of changes This PR is changes the weay we acessing span in probe processor. See this [comment](#6216 (comment)) for more info.
Summary of changes
This PR introduce the Code origin for span feature. In this PR there is the implementation for exit span and the implementation for entry span will be in a following PR.
Reason for change
We want to let user the ability to see from where in the code each exit span was created, and in the future, let the user automatically put probe there.
Implementation details
We are adding code roigin information tags to the span.
Test coverage
SpanCodeOriginTests.cs