[Tracing] Instrument static methods defined in non-generic value types#7920
Conversation
e7ef3d6 to
5d57d03
Compare
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (7920) and master.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 68.30 ± (68.33 - 68.58) ms | 78.56 ± (78.58 - 78.99) ms | +15.0% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 72.05 ± (71.98 - 72.20) ms | 82.23 ± (82.10 - 82.51) ms | +14.1% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1025.15 ± (1027.07 - 1033.25) ms | 1110.89 ± (1111.19 - 1119.24) ms | +8.4% | ❌⬆️ |
HttpMessageHandler
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 193.79 ± (193.64 - 194.38) ms | 207.19 ± (207.70 - 209.75) ms | +6.9% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 197.11 ± (197.02 - 197.59) ms | 212.35 ± (212.62 - 214.58) ms | +7.7% | ❌⬆️ |
Full Metrics Comparison
FakeDbCommand
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 68.30 ± (68.33 - 68.58) ms | 78.56 ± (78.58 - 78.99) ms | +15.0% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 72.05 ± (71.98 - 72.20) ms | 82.23 ± (82.10 - 82.51) ms | +14.1% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1025.15 ± (1027.07 - 1033.25) ms | 1110.89 ± (1111.19 - 1119.24) ms | +8.4% | ❌⬆️ |
| .NET Core 3.1 - Baseline | ||||
| process.internal_duration_ms | 22.46 ± (22.44 - 22.48) ms | 24.77 ± (24.70 - 24.83) ms | +10.3% | ✅⬆️ |
| process.time_to_main_ms | 86.82 ± (86.67 - 86.96) ms | 101.53 ± (101.29 - 101.77) ms | +16.9% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 15.51 ± (15.51 - 15.52) MB | 15.50 ± (15.50 - 15.50) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 12 ± (12 - 12) | 12 ± (12 - 12) | +0.0% | ✅ |
| .NET Core 3.1 - Bailout | ||||
| process.internal_duration_ms | 22.40 ± (22.37 - 22.43) ms | 24.81 ± (24.74 - 24.88) ms | +10.7% | ✅⬆️ |
| process.time_to_main_ms | 87.84 ± (87.73 - 87.95) ms | 103.63 ± (103.36 - 103.90) ms | +18.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 15.55 ± (15.55 - 15.56) MB | 15.54 ± (15.54 - 15.55) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 13 ± (13 - 13) | 13 ± (13 - 13) | +0.0% | ✅ |
| .NET Core 3.1 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 255.49 ± (251.86 - 259.12) ms | 291.56 ± (289.33 - 293.79) ms | +14.1% | ✅⬆️ |
| process.time_to_main_ms | 503.66 ± (503.12 - 504.20) ms | 560.59 ± (559.77 - 561.41) ms | +11.3% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 53.23 ± (53.20 - 53.25) MB | 53.22 ± (53.20 - 53.24) MB | -0.0% | ✅ |
| runtime.dotnet.threads.count | 28 ± (28 - 28) | 28 ± (28 - 28) | +0.9% | ✅⬆️ |
| .NET 6 - Baseline | ||||
| process.internal_duration_ms | 21.04 ± (21.01 - 21.06) ms | 23.07 ± (23.01 - 23.13) ms | +9.7% | ✅⬆️ |
| process.time_to_main_ms | 75.06 ± (74.92 - 75.21) ms | 87.47 ± (87.23 - 87.71) ms | +16.5% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 15.23 ± (15.23 - 15.23) MB | 15.22 ± (15.21 - 15.22) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 10 ± (10 - 10) | 10 ± (10 - 10) | +0.0% | ✅ |
| .NET 6 - Bailout | ||||
| process.internal_duration_ms | 20.87 ± (20.84 - 20.90) ms | 23.13 ± (23.08 - 23.19) ms | +10.8% | ✅⬆️ |
| process.time_to_main_ms | 75.77 ± (75.68 - 75.86) ms | 89.40 ± (89.15 - 89.65) ms | +18.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 15.36 ± (15.36 - 15.37) MB | 15.34 ± (15.34 - 15.35) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 11 ± (11 - 11) | 11 ± (11 - 11) | +0.0% | ✅ |
| .NET 6 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 253.80 ± (252.89 - 254.71) ms | 277.07 ± (276.10 - 278.03) ms | +9.2% | ✅⬆️ |
| process.time_to_main_ms | 481.12 ± (480.50 - 481.75) ms | 529.75 ± (528.95 - 530.55) ms | +10.1% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 53.98 ± (53.95 - 54.01) MB | 53.89 ± (53.87 - 53.91) MB | -0.2% | ✅ |
| runtime.dotnet.threads.count | 28 ± (28 - 28) | 28 ± (28 - 28) | +0.2% | ✅⬆️ |
| .NET 8 - Baseline | ||||
| process.internal_duration_ms | 19.24 ± (19.21 - 19.26) ms | 21.19 ± (21.13 - 21.25) ms | +10.2% | ✅⬆️ |
| process.time_to_main_ms | 73.87 ± (73.76 - 73.98) ms | 86.42 ± (86.23 - 86.61) ms | +17.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 12.27 ± (12.27 - 12.28) MB | 12.26 ± (12.25 - 12.26) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 10 ± (10 - 10) | 10 ± (10 - 10) | +0.0% | ✅ |
| .NET 8 - Bailout | ||||
| process.internal_duration_ms | 19.20 ± (19.17 - 19.23) ms | 20.93 ± (20.86 - 21.00) ms | +9.0% | ✅⬆️ |
| process.time_to_main_ms | 75.08 ± (75.00 - 75.16) ms | 86.74 ± (86.52 - 86.95) ms | +15.5% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 12.34 ± (12.33 - 12.35) MB | 12.33 ± (12.32 - 12.34) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 11 ± (11 - 11) | 11 ± (11 - 11) | +0.0% | ✅ |
| .NET 8 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 181.20 ± (180.26 - 182.14) ms | 202.81 ± (201.96 - 203.66) ms | +11.9% | ✅⬆️ |
| process.time_to_main_ms | 461.65 ± (461.02 - 462.29) ms | 507.63 ± (506.65 - 508.60) ms | +10.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 41.23 ± (41.20 - 41.25) MB | 41.69 ± (41.65 - 41.72) MB | +1.1% | ✅⬆️ |
| runtime.dotnet.threads.count | 27 ± (27 - 27) | 27 ± (27 - 27) | +0.2% | ✅⬆️ |
HttpMessageHandler
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 193.79 ± (193.64 - 194.38) ms | 207.19 ± (207.70 - 209.75) ms | +6.9% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 197.11 ± (197.02 - 197.59) ms | 212.35 ± (212.62 - 214.58) ms | +7.7% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1145.42 ± (1145.20 - 1150.97) ms | 1203.32 ± (1204.17 - 1211.83) ms | +5.1% | ✅⬆️ |
| .NET Core 3.1 - Baseline | ||||
| process.internal_duration_ms | 193.39 ± (192.93 - 193.85) ms | 205.03 ± (204.36 - 205.71) ms | +6.0% | ✅⬆️ |
| process.time_to_main_ms | 89.41 ± (89.12 - 89.70) ms | 94.39 ± (94.03 - 94.74) ms | +5.6% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 20.85 ± (20.83 - 20.88) MB | 20.66 ± (20.64 - 20.67) MB | -0.9% | ✅ |
| runtime.dotnet.threads.count | 20 ± (20 - 20) | 20 ± (20 - 20) | +1.5% | ✅⬆️ |
| .NET Core 3.1 - Bailout | ||||
| process.internal_duration_ms | 191.60 ± (191.32 - 191.88) ms | 202.47 ± (201.91 - 203.04) ms | +5.7% | ✅⬆️ |
| process.time_to_main_ms | 90.28 ± (90.12 - 90.45) ms | 95.36 ± (95.07 - 95.66) ms | +5.6% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 20.78 ± (20.76 - 20.81) MB | 20.62 ± (20.60 - 20.64) MB | -0.8% | ✅ |
| runtime.dotnet.threads.count | 21 ± (21 - 21) | 21 ± (21 - 21) | +0.9% | ✅⬆️ |
| .NET Core 3.1 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 443.88 ± (441.58 - 446.18) ms | 460.18 ± (457.71 - 462.65) ms | +3.7% | ✅⬆️ |
| process.time_to_main_ms | 509.20 ± (508.55 - 509.84) ms | 536.96 ± (536.04 - 537.89) ms | +5.5% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 63.62 ± (63.51 - 63.72) MB | 63.05 ± (62.93 - 63.18) MB | -0.9% | ✅ |
| runtime.dotnet.threads.count | 29 ± (29 - 29) | 29 ± (29 - 29) | +0.1% | ✅⬆️ |
| .NET 6 - Baseline | ||||
| process.internal_duration_ms | 196.37 ± (196.07 - 196.67) ms | 208.49 ± (207.81 - 209.17) ms | +6.2% | ✅⬆️ |
| process.time_to_main_ms | 77.04 ± (76.81 - 77.26) ms | 81.51 ± (81.22 - 81.81) ms | +5.8% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 20.96 ± (20.93 - 20.99) MB | 20.78 ± (20.76 - 20.79) MB | -0.9% | ✅ |
| runtime.dotnet.threads.count | 19 ± (19 - 19) | 20 ± (20 - 20) | +0.9% | ✅⬆️ |
| .NET 6 - Bailout | ||||
| process.internal_duration_ms | 195.41 ± (195.11 - 195.70) ms | 207.72 ± (207.04 - 208.39) ms | +6.3% | ✅⬆️ |
| process.time_to_main_ms | 78.00 ± (77.86 - 78.14) ms | 82.30 ± (81.97 - 82.63) ms | +5.5% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 21.02 ± (20.99 - 21.05) MB | 20.90 ± (20.88 - 20.92) MB | -0.6% | ✅ |
| runtime.dotnet.threads.count | 20 ± (20 - 21) | 21 ± (20 - 21) | +0.5% | ✅⬆️ |
| .NET 6 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 463.92 ± (462.32 - 465.51) ms | 481.41 ± (479.47 - 483.34) ms | +3.8% | ✅⬆️ |
| process.time_to_main_ms | 487.27 ± (486.58 - 487.96) ms | 513.97 ± (512.83 - 515.10) ms | +5.5% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 63.13 ± (63.02 - 63.24) MB | 62.59 ± (62.48 - 62.70) MB | -0.9% | ✅ |
| runtime.dotnet.threads.count | 30 ± (30 - 30) | 30 ± (30 - 30) | +0.6% | ✅⬆️ |
| .NET 8 - Baseline | ||||
| process.internal_duration_ms | 194.33 ± (194.00 - 194.66) ms | 206.59 ± (205.97 - 207.21) ms | +6.3% | ✅⬆️ |
| process.time_to_main_ms | 77.18 ± (76.98 - 77.38) ms | 81.38 ± (81.06 - 81.70) ms | +5.4% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.32 ± (16.30 - 16.34) MB | 16.22 ± (16.21 - 16.24) MB | -0.6% | ✅ |
| runtime.dotnet.threads.count | 19 ± (19 - 19) | 19 ± (19 - 19) | +1.0% | ✅⬆️ |
| .NET 8 - Bailout | ||||
| process.internal_duration_ms | 193.13 ± (192.82 - 193.45) ms | 207.27 ± (206.58 - 207.96) ms | +7.3% | ✅⬆️ |
| process.time_to_main_ms | 77.91 ± (77.76 - 78.06) ms | 82.72 ± (82.40 - 83.04) ms | +6.2% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.42 ± (16.39 - 16.45) MB | 16.29 ± (16.28 - 16.30) MB | -0.8% | ✅ |
| runtime.dotnet.threads.count | 20 ± (20 - 20) | 20 ± (20 - 20) | +1.2% | ✅⬆️ |
| .NET 8 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 371.48 ± (370.24 - 372.72) ms | 430.93 ± (423.69 - 438.18) ms | +16.0% | ✅⬆️ |
| process.time_to_main_ms | 468.84 ± (468.24 - 469.44) ms | 492.24 ± (491.22 - 493.26) ms | +5.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 52.98 ± (52.94 - 53.01) MB | 54.92 ± (54.80 - 55.04) MB | +3.7% | ✅⬆️ |
| runtime.dotnet.threads.count | 29 ± (29 - 29) | 29 ± (29 - 29) | +0.3% | ✅⬆️ |
Comparison explanation
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 highlighted 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).
Duration charts
FakeDbCommand (.NET Framework 4.8)
gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (79ms) : 76, 82
master - mean (68ms) : 67, 70
section Bailout
This PR (7920) - mean (82ms) : crit, 80, 85
master - mean (72ms) : 71, 73
section CallTarget+Inlining+NGEN
This PR (7920) - mean (1,115ms) : crit, 1055, 1176
master - mean (1,030ms) : 986, 1074
FakeDbCommand (.NET Core 3.1)
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (134ms) : 130, 138
master - mean (115ms) : 113, 118
section Bailout
This PR (7920) - mean (137ms) : crit, 132, 141
master - mean (116ms) : 114, 118
section CallTarget+Inlining+NGEN
This PR (7920) - mean (889ms) : crit, 839, 940
master - mean (797ms) : 742, 852
FakeDbCommand (.NET 6)
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (118ms) : 114, 122
master - mean (101ms) : 99, 104
section Bailout
This PR (7920) - mean (120ms) : crit, 115, 125
master - mean (102ms) : 101, 103
section CallTarget+Inlining+NGEN
This PR (7920) - mean (845ms) : crit, 818, 873
master - mean (776ms) : 755, 797
FakeDbCommand (.NET 8)
gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (116ms) : 113, 120
master - mean (100ms) : 97, 102
section Bailout
This PR (7920) - mean (116ms) : crit, 113, 119
master - mean (101ms) : 99, 102
section CallTarget+Inlining+NGEN
This PR (7920) - mean (753ms) : crit, 728, 778
master - mean (686ms) : 673, 698
HttpMessageHandler (.NET Framework 4.8)
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (209ms) : 193, 224
master - mean (194ms) : 190, 198
section Bailout
This PR (7920) - mean (214ms) : crit, 199, 228
master - mean (197ms) : 195, 200
section CallTarget+Inlining+NGEN
This PR (7920) - mean (1,208ms) : 1154, 1262
master - mean (1,148ms) : 1107, 1189
HttpMessageHandler (.NET Core 3.1)
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (310ms) : 294, 326
master - mean (292ms) : 284, 300
section Bailout
This PR (7920) - mean (309ms) : crit, 294, 323
master - mean (291ms) : 287, 295
section CallTarget+Inlining+NGEN
This PR (7920) - mean (1,038ms) : 996, 1080
master - mean (991ms) : 953, 1029
HttpMessageHandler (.NET 6)
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (301ms) : 287, 314
master - mean (282ms) : 278, 287
section Bailout
This PR (7920) - mean (300ms) : crit, 289, 311
master - mean (282ms) : 279, 286
section CallTarget+Inlining+NGEN
This PR (7920) - mean (1,033ms) : 989, 1076
master - mean (988ms) : 950, 1026
HttpMessageHandler (.NET 8)
gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7920) - mean (301ms) : 285, 316
master - mean (282ms) : 277, 287
section Bailout
This PR (7920) - mean (303ms) : crit, 286, 319
master - mean (281ms) : 277, 286
section CallTarget+Inlining+NGEN
This PR (7920) - mean (968ms) : crit, 852, 1083
master - mean (873ms) : 849, 896
BenchmarksBenchmark execution time: 2026-02-06 17:36:29 Comparing candidate commit b58ce81 in PR branch Found 11 performance improvements and 4 performance regressions! Performance is the same for 155 metrics, 22 unstable metrics. scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild netcoreapp3.1
scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs netcoreapp3.1
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.CharSliceBenchmark.OriginalCharSlice net6.0
scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net6.0
scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
scenario:Benchmarks.Trace.RedisBenchmark.SendReceive net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net6.0
|
…the tracer. This skips instrumentation for the live debugger product, and it needs to be tested against structs that are generic instantiations.
…t static methods in reference types and value types can be instrumented. Also add tests for static methods inside generic reference type and generic value types -- this only succeeds for generic reference types.
5d57d03 to
cbfdd00
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbfdd004c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (caller->type.valueType) | ||
| { | ||
| // Static methods in a ValueType can't be instrumented. | ||
| // In the future this can be supported by adding a local for the valuetype and initialize it to the default | ||
| // value. After the signature modification we need to emit the following IL to initialize and load into the | ||
| // stack. | ||
| // ldloca.s [localIndex] | ||
| // initobj [valueType] | ||
| // ldloc.s [localIndex] | ||
| Logger::Warn("*** CallTarget_RewriterCallback(): Static methods in a ValueType cannot be instrumented. "); | ||
| return S_FALSE; | ||
| reWriterWrapper.LoadLocalAddress(staticValueTypeIndex); | ||
| if (caller->type.type_spec != mdTypeSpecNil) | ||
| { |
There was a problem hiding this comment.
Avoid passing value-type instance when CallTarget falls back to object
For static methods on value types nested in generic parents, GetCurrentTypeRef falls back to mdTokenNil, so WriteBeginMethod/EndMethod specialize CallTarget with TTarget=object. This branch now always emits initobj + ldloc for any value type, so the stack holds an unboxed struct while the calltarget method spec expects object, which can produce invalid IL/JIT failures when instrumenting Outer<T>.Inner.StaticMethod. Consider checking the generic-parent case (same condition used in GetCurrentTypeRef) and using LoadNull or boxing instead of pushing the value type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was useful as I didn't cover this edge case. Although we don't instrument this case today, in our CallTargetNativeTest program we can enable this scenario. With the latest set of commits, we correctly handle this and load a null value as the CallTarget "instance" so we can in fact instrument this scenario.
…e the value type is enclosed in a generic parent type. We can easily support this because in such cases we cannot obtain the actual type token for struct, so we default to loading a null value on the stack as the "instance" for the CallTarget instrumentation, just as we would for instrumenting the static method of a reference type.
andrewlock
left a comment
There was a problem hiding this comment.
Nice, thanks! Just tiny nits/suggestions!
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs
Outdated
Show resolved
Hide resolved
tracer/test/test-applications/instrumentation/CallTargetNativeTest/With0Arguments.cs
Show resolved
Hide resolved
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
…early call out the instrumentation scenarios that don't work today
…e OpenTelemetry Baggage API (#7921) ## Summary of changes Enable updates to Baggage from either the Datadog Baggage API and the OpenTelemetry Baggage API to be visible to the other. Relies on #7920 for properly instrumenting the static methods in the `OpenTelemetry.Baggage` struct. ## Reason for change This allows users of either Baggage API to make changes to the automatically-propagated Baggage header. Before, only changes made through the Datadog Baggage API would be reflected. ## Implementation details ### APIs supported Instruments the following methods in the OpenTelemetry Baggage API: - `static OpenTelemetry.Baggage.ClearBaggage(Baggage)` - `static OpenTelemetry.Baggage.Current { get; set; }` - `static OpenTelemetry.Baggage.RemoveBaggage(string, Baggage)` - `static OpenTelemetry.Baggage.SetBaggage(string, string, Baggage)` - `static OpenTelemetry.Baggage.SetBaggage(IEnumerable<KeyValuePair<string, string?>>, Baggage)` The following public API's are also supported indirectly by our instrumentation of the `OpenTelemetry.Baggage.Current` getter: - `static OpenTelemetry.Baggage.GetBaggage(Baggage)` - `static OpenTelemetry.Baggage.GetBaggage(string, Baggage)` - `static OpenTelemetry.Baggage.GetEnumerator(Baggage)` ### OpenTelemetry Baggage Model In the OpenTelemetry model, baggage is immutable. This means every time the user wants to update `OpenTelemetry.Baggage.Current`, they must either call the setter directly with an new instance of `OpenTelemetry.Baggage`, or they must call one of the static APIs. The static APIs are largely intuitive, but it's worth calling two interesting behaviors of the APIs that modify baggage: 1. If a `OpenTelemetry.Baggage` argument is provided (optional), that object will be the source from which the modification will be enacted on. If not, then `OpenTelemetry.Baggage.Current` will be the source. 2. Before returning from the API call, the resulting (immutable) `OpenTelemetry.Baggage` is set as the new `OpenTelemetry.Baggage.Current` value. ### Interoperability Approach For this approach, `Datadog.Trace.Baggage.Current` will be the source of truth for the "current" baggage. For example, our HTTP server instrumentations will first populate `Datadog.Trace.Baggage.Current` with the contents of the incoming `baggage` HTTP header, and users of the `Datadog.Trace.Baggage` API can update the mutable `Datadog.Trace.Baggage.Current`. Then, if the user wants get or set `OpenTelemetry.Baggage.Current`, that is where our instrumentation must keep it in sync with `Datadog.Trace.Baggage.Current`. Each time `OpenTelemetry.Baggage.set_Current` is invoked, we must replace the contents of `Datadog.Trace.Baggage.Current` entirely by clearing out the existing values and add all of the new KeyValuePairs. Each time `OpenTelemetry.Baggage.get_Current` is invoked, we must create a new (immutable) `OpenTelemetry.Baggage` using the contents of `Datadog.Trace.Baggage.Current` and assign it to `OpenTelemetry.Baggage.Current` before the property is accessed and returned to the API caller. By design, no work is done if only the `Datadog.Trace.Baggage` APIs are used. Also, users are aware of the immutable nature of the `OpenTelemetry.Baggage` APIs, so they will be used to doing at least one allocation per baggage modification, but we can certainly improve the overhead of this interop with approaches noted in the 'Other details' section below. ## Test coverage Added a new `OtelBaggageController` to the `Samples.AspNetCoreMinimalApis` and `Samples.AspNetCoreMvc*` applications, and added integration test cases (with snapshots) to `tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AspNetCore/AspNetCoreMinimalApisTests.cs`. ## Other details We can follow up this PR with changes to optimize the sharing of Baggage state between the two API's. In order to do this, we would need to do the following: - Change the internal storage mechanism of items in `Datadog.Trace.Baggage` to be `Dictionary<string,string>` - Change `Datadog.Trace.Baggage` to disallow `null` values in baggage key-value pairs, to align with the OpenTelemetry storage model - When returning from the static `OpenTelemetry.Baggage` methods that modify state, take the reference to the internal `Dictionary<string,string>` object and assign it to the `Datadog.Trace.Baggage.Current` items dictionary More immediately, we could implement tracking of writes to `Datadog.Trace.Baggage.Current` so we don't have to create a fresh copy of it every time `OpenTelemetry.Baggage.get_Current` is called, but I'd rather separate that out from this PR.
Summary of changes
Adds the capability to instrument static methods defined in non-generic value types
Reason for change
We will need to instrument OpenTelemetry Baggage, which utilizes static methods in a Baggage struct
Implementation details
TracerMethodRewriter::Rewritesuch that when it is loading the object instance to put on the stack for a static method, for non-generic value types it does the following:ldloca.s [localIndex](Loads the variable addresses)initobj [valueType](Initializes, or re-initializes, an empty struct)ldloc.s [localIndex](Loads the struct onto the stack)Test coverage
The
CallTargetNativeTestis updated with the following cases to demonstrate the new functionality, especially as it compares to reference types:ArgumentsGenericStatictypes demonstrate the successful instrumentation of static methods in generic reference typesArgumentsStaticStructtypes demonstrate the successful instrumentation of static methods in value typesArgumentsGenericStaticStructtypes demonstrate the unsuccessful instrumentation of static methods in generic value typesOther details
Note: This does not implement the functionality for the live debugger IL rewriting, though it does update the method call so that everything compiles.