[AAP] Collect DataContract JSON response body schemas - #8706
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
e-n-0
changed the title
BenchmarksBenchmark execution time: 2026-07-28 21:31:46 Comparing candidate commit 97b46ca in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 62 known flaky benchmarks, 64 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8706) and master.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Bailout | ||||
| duration | 74.24 ± (74.04 - 74.41) ms | 79.90 ± (79.44 - 79.91) ms | +7.6% | ❌⬆️ |
Full Metrics Comparison
FakeDbCommand
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 70.69 ± (70.74 - 71.08) ms | 71.04 ± (71.17 - 71.54) ms | +0.5% | ✅⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 74.24 ± (74.04 - 74.41) ms | 79.90 ± (79.44 - 79.91) ms | +7.6% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1086.20 ± (1088.36 - 1096.51) ms | 1088.22 ± (1088.35 - 1094.71) ms | +0.2% | ✅⬆️ |
| .NET Core 3.1 - Baseline | ||||
| process.internal_duration_ms | 22.06 ± (22.03 - 22.09) ms | 22.24 ± (22.19 - 22.29) ms | +0.8% | ✅⬆️ |
| process.time_to_main_ms | 81.03 ± (80.85 - 81.21) ms | 83.30 ± (83.00 - 83.60) ms | +2.8% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 11.01 ± (11.00 - 11.01) MB | 11.00 ± (11.00 - 11.00) MB | -0.1% | ✅ |
| runtime.dotnet.threads.count | 12 ± (12 - 12) | 12 ± (12 - 12) | +0.0% | ✅ |
| .NET Core 3.1 - Bailout | ||||
| process.internal_duration_ms | 21.91 ± (21.88 - 21.94) ms | 22.24 ± (22.20 - 22.28) ms | +1.5% | ✅⬆️ |
| process.time_to_main_ms | 82.26 ± (82.07 - 82.44) ms | 85.42 ± (85.15 - 85.69) ms | +3.8% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 11.04 ± (11.04 - 11.04) MB | 11.03 ± (11.02 - 11.03) 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 | 210.37 ± (209.58 - 211.16) ms | 209.47 ± (208.46 - 210.48) ms | -0.4% | ✅ |
| process.time_to_main_ms | 538.41 ± (537.37 - 539.46) ms | 537.29 ± (536.02 - 538.56) ms | -0.2% | ✅ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 49.61 ± (49.59 - 49.64) MB | 49.47 ± (49.43 - 49.50) MB | -0.3% | ✅ |
| runtime.dotnet.threads.count | 28 ± (28 - 28) | 28 ± (28 - 28) | +0.0% | ✅⬆️ |
| .NET 6 - Baseline | ||||
| process.internal_duration_ms | 20.94 ± (20.91 - 20.98) ms | 21.09 ± (21.04 - 21.14) ms | +0.7% | ✅⬆️ |
| process.time_to_main_ms | 72.02 ± (71.75 - 72.30) ms | 73.78 ± (73.50 - 74.05) ms | +2.4% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 10.71 ± (10.70 - 10.71) MB | 10.72 ± (10.72 - 10.73) MB | +0.2% | ✅⬆️ |
| runtime.dotnet.threads.count | 10 ± (10 - 10) | 10 ± (10 - 10) | +0.0% | ✅ |
| .NET 6 - Bailout | ||||
| process.internal_duration_ms | 21.12 ± (21.07 - 21.16) ms | 20.82 ± (20.79 - 20.85) ms | -1.4% | ✅ |
| process.time_to_main_ms | 75.60 ± (75.37 - 75.84) ms | 72.53 ± (72.41 - 72.66) ms | -4.1% | ✅ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 10.83 ± (10.83 - 10.83) MB | 10.84 ± (10.83 - 10.84) MB | +0.1% | ✅⬆️ |
| runtime.dotnet.threads.count | 11 ± (11 - 11) | 11 ± (11 - 11) | +0.0% | ✅ |
| .NET 6 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 369.92 ± (367.51 - 372.34) ms | 374.90 ± (372.77 - 377.03) ms | +1.3% | ✅⬆️ |
| process.time_to_main_ms | 545.20 ± (544.11 - 546.29) ms | 547.26 ± (545.91 - 548.62) ms | +0.4% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 50.60 ± (50.58 - 50.62) MB | 50.61 ± (50.59 - 50.64) MB | +0.0% | ✅⬆️ |
| runtime.dotnet.threads.count | 28 ± (28 - 28) | 28 ± (28 - 28) | -0.0% | ✅ |
| .NET 8 - Baseline | ||||
| process.internal_duration_ms | 19.03 ± (19.01 - 19.06) ms | 19.34 ± (19.29 - 19.39) ms | +1.6% | ✅⬆️ |
| process.time_to_main_ms | 69.96 ± (69.83 - 70.09) ms | 72.74 ± (72.44 - 73.05) ms | +4.0% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 7.73 ± (7.72 - 7.73) MB | 7.77 ± (7.77 - 7.78) MB | +0.6% | ✅⬆️ |
| runtime.dotnet.threads.count | 10 ± (10 - 10) | 10 ± (10 - 10) | +0.0% | ✅ |
| .NET 8 - Bailout | ||||
| process.internal_duration_ms | 19.01 ± (18.98 - 19.04) ms | 18.96 ± (18.93 - 19.00) ms | -0.3% | ✅ |
| process.time_to_main_ms | 70.86 ± (70.74 - 70.98) ms | 71.30 ± (71.17 - 71.44) ms | +0.6% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 7.78 ± (7.78 - 7.79) MB | 7.82 ± (7.82 - 7.83) MB | +0.5% | ✅⬆️ |
| runtime.dotnet.threads.count | 11 ± (11 - 11) | 11 ± (11 - 11) | +0.0% | ✅ |
| .NET 8 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 296.05 ± (293.85 - 298.25) ms | 303.01 ± (300.44 - 305.59) ms | +2.4% | ✅⬆️ |
| process.time_to_main_ms | 494.31 ± (493.14 - 495.48) ms | 495.36 ± (494.13 - 496.58) ms | +0.2% | ✅⬆️ |
| runtime.dotnet.exceptions.count | 0 ± (0 - 0) | 0 ± (0 - 0) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 38.03 ± (38.01 - 38.06) MB | 38.05 ± (38.02 - 38.08) MB | +0.0% | ✅⬆️ |
| runtime.dotnet.threads.count | 27 ± (27 - 27) | 27 ± (27 - 27) | -0.9% | ✅ |
HttpMessageHandler
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 213.64 ± (213.36 - 214.30) ms | 211.04 ± (211.50 - 212.42) ms | -1.2% | ✅ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 218.31 ± (217.91 - 218.69) ms | 215.32 ± (214.80 - 215.73) ms | -1.4% | ✅ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1276.30 ± (1276.14 - 1283.47) ms | 1262.40 ± (1262.15 - 1268.27) ms | -1.1% | ✅ |
| .NET Core 3.1 - Baseline | ||||
| process.internal_duration_ms | 204.33 ± (203.92 - 204.75) ms | 202.35 ± (202.03 - 202.67) ms | -1.0% | ✅ |
| process.time_to_main_ms | 90.44 ± (90.18 - 90.71) ms | 89.23 ± (88.94 - 89.51) ms | -1.3% | ✅ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.08 ± (16.06 - 16.10) MB | 16.20 ± (16.18 - 16.22) MB | +0.7% | ✅⬆️ |
| runtime.dotnet.threads.count | 20 ± (20 - 20) | 20 ± (20 - 20) | +0.2% | ✅⬆️ |
| .NET Core 3.1 - Bailout | ||||
| process.internal_duration_ms | 203.60 ± (203.14 - 204.05) ms | 202.50 ± (202.11 - 202.88) ms | -0.5% | ✅ |
| process.time_to_main_ms | 91.22 ± (90.99 - 91.45) ms | 90.88 ± (90.67 - 91.10) ms | -0.4% | ✅ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.10 ± (16.08 - 16.12) MB | 16.21 ± (16.19 - 16.22) MB | +0.7% | ✅⬆️ |
| runtime.dotnet.threads.count | 21 ± (20 - 21) | 21 ± (21 - 21) | +1.6% | ✅⬆️ |
| .NET Core 3.1 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 400.82 ± (399.43 - 402.20) ms | 395.69 ± (394.25 - 397.13) ms | -1.3% | ✅ |
| process.time_to_main_ms | 573.44 ± (571.97 - 574.91) ms | 560.54 ± (559.27 - 561.81) ms | -2.3% | ✅ |
| runtime.dotnet.exceptions.count | 3 ± (3 - 3) | 3 ± (3 - 3) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 59.84 ± (59.75 - 59.93) MB | 59.65 ± (59.50 - 59.81) MB | -0.3% | ✅ |
| runtime.dotnet.threads.count | 30 ± (30 - 30) | 30 ± (30 - 30) | -0.3% | ✅ |
| .NET 6 - Baseline | ||||
| process.internal_duration_ms | 209.66 ± (209.16 - 210.16) ms | 206.99 ± (206.52 - 207.45) ms | -1.3% | ✅ |
| process.time_to_main_ms | 79.51 ± (79.27 - 79.75) ms | 78.27 ± (78.01 - 78.53) ms | -1.6% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.50 ± (16.48 - 16.52) MB | 16.37 ± (16.34 - 16.39) MB | -0.8% | ✅ |
| runtime.dotnet.threads.count | 20 ± (20 - 20) | 20 ± (19 - 20) | -0.3% | ✅ |
| .NET 6 - Bailout | ||||
| process.internal_duration_ms | 208.81 ± (208.33 - 209.28) ms | 206.42 ± (206.00 - 206.84) ms | -1.1% | ✅ |
| process.time_to_main_ms | 80.56 ± (80.34 - 80.79) ms | 79.38 ± (79.16 - 79.60) ms | -1.5% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 16.47 ± (16.45 - 16.49) MB | 16.46 ± (16.43 - 16.49) MB | -0.0% | ✅ |
| runtime.dotnet.threads.count | 21 ± (20 - 21) | 21 ± (20 - 21) | +0.1% | ✅⬆️ |
| .NET 6 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 577.15 ± (575.09 - 579.21) ms | 579.92 ± (577.39 - 582.44) ms | +0.5% | ✅⬆️ |
| process.time_to_main_ms | 589.68 ± (587.82 - 591.54) ms | 577.73 ± (576.40 - 579.05) ms | -2.0% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 61.44 ± (61.35 - 61.53) MB | 61.50 ± (61.42 - 61.58) MB | +0.1% | ✅⬆️ |
| runtime.dotnet.threads.count | 31 ± (31 - 31) | 31 ± (31 - 31) | +0.2% | ✅⬆️ |
| .NET 8 - Baseline | ||||
| process.internal_duration_ms | 209.25 ± (208.84 - 209.66) ms | 206.84 ± (206.44 - 207.24) ms | -1.2% | ✅ |
| process.time_to_main_ms | 78.64 ± (78.32 - 78.96) ms | 77.99 ± (77.73 - 78.24) ms | -0.8% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 11.70 ± (11.67 - 11.72) MB | 11.76 ± (11.74 - 11.78) MB | +0.5% | ✅⬆️ |
| runtime.dotnet.threads.count | 19 ± (19 - 19) | 19 ± (19 - 19) | -0.4% | ✅ |
| .NET 8 - Bailout | ||||
| process.internal_duration_ms | 208.70 ± (208.19 - 209.20) ms | 207.56 ± (207.10 - 208.03) ms | -0.5% | ✅ |
| process.time_to_main_ms | 80.55 ± (80.32 - 80.78) ms | 79.74 ± (79.48 - 80.00) ms | -1.0% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 11.75 ± (11.73 - 11.78) MB | 11.79 ± (11.77 - 11.81) MB | +0.3% | ✅⬆️ |
| runtime.dotnet.threads.count | 20 ± (20 - 20) | 20 ± (20 - 20) | +0.3% | ✅⬆️ |
| .NET 8 - CallTarget+Inlining+NGEN | ||||
| process.internal_duration_ms | 534.92 ± (527.53 - 542.31) ms | 511.93 ± (506.93 - 516.92) ms | -4.3% | ✅ |
| process.time_to_main_ms | 540.81 ± (539.68 - 541.94) ms | 529.20 ± (528.22 - 530.18) ms | -2.1% | ✅ |
| runtime.dotnet.exceptions.count | 4 ± (4 - 4) | 4 ± (4 - 4) | +0.0% | ✅ |
| runtime.dotnet.mem.committed | 51.55 ± (51.47 - 51.63) MB | 51.42 ± (51.36 - 51.48) MB | -0.3% | ✅ |
| runtime.dotnet.threads.count | 30 ± (30 - 30) | 30 ± (30 - 30) | -0.2% | ✅ |
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 (8706) - mean (71ms) : 69, 74
master - mean (71ms) : 68, 73
section Bailout
This PR (8706) - mean (80ms) : crit, 76, 83
master - mean (74ms) : 72, 76
section CallTarget+Inlining+NGEN
This PR (8706) - mean (1,092ms) : 1046, 1137
master - mean (1,092ms) : 1035, 1150
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 (8706) - mean (113ms) : 106, 120
master - mean (109ms) : 105, 113
section Bailout
This PR (8706) - mean (115ms) : 109, 121
master - mean (110ms) : 108, 112
section CallTarget+Inlining+NGEN
This PR (8706) - mean (783ms) : 763, 804
master - mean (786ms) : 764, 808
FakeDbCommand (.NET 6)
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8706) - mean (102ms) : 96, 107
master - mean (99ms) : 94, 103
section Bailout
This PR (8706) - mean (99ms) : 97, 101
master - mean (103ms) : 98, 108
section CallTarget+Inlining+NGEN
This PR (8706) - mean (955ms) : 912, 998
master - mean (945ms) : 902, 989
FakeDbCommand (.NET 8)
gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8706) - mean (100ms) : 92, 107
master - mean (96ms) : 93, 99
section Bailout
This PR (8706) - mean (97ms) : 95, 100
master - mean (97ms) : 95, 98
section CallTarget+Inlining+NGEN
This PR (8706) - mean (829ms) : 784, 875
master - mean (819ms) : 783, 855
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 (8706) - mean (212ms) : 205, 218
master - mean (214ms) : 209, 219
section Bailout
This PR (8706) - mean (215ms) : 210, 220
master - mean (218ms) : 215, 222
section CallTarget+Inlining+NGEN
This PR (8706) - mean (1,265ms) : 1222, 1309
master - mean (1,280ms) : 1227, 1333
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 (8706) - mean (302ms) : 295, 308
master - mean (304ms) : 296, 313
section Bailout
This PR (8706) - mean (304ms) : 298, 309
master - mean (304ms) : 299, 310
section CallTarget+Inlining+NGEN
This PR (8706) - mean (1,000ms) : 978, 1021
master - mean (1,016ms) : 995, 1037
HttpMessageHandler (.NET 6)
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8706) - mean (295ms) : 288, 302
master - mean (299ms) : 290, 307
section Bailout
This PR (8706) - mean (296ms) : 290, 302
master - mean (298ms) : 291, 306
section CallTarget+Inlining+NGEN
This PR (8706) - mean (1,193ms) : 1160, 1226
master - mean (1,208ms) : 1175, 1240
HttpMessageHandler (.NET 8)
gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8706) - mean (295ms) : 288, 302
master - mean (299ms) : 292, 306
section Bailout
This PR (8706) - mean (298ms) : 290, 307
master - mean (300ms) : 292, 309
section CallTarget+Inlining+NGEN
This PR (8706) - mean (1,086ms) : 985, 1187
master - mean (1,123ms) : 1000, 1245
e-n-0
force-pushed
the
flavien.darche/aap/response-body-outputstream
branch
from
May 28, 2026 19:26
81bc156 to
c3a6ff6
Compare
dromanol
force-pushed
the
flavien.darche/aap/response-body-outputstream
branch
from
June 5, 2026 11:48
59fdcb0 to
4676412
Compare
dromanol
marked this pull request as ready for review
June 8, 2026 13:52
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02fc0bb751
ℹ️ 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".
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 489c3e4b95
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 603961b556
ℹ️ 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".
dromanol
marked this pull request as draft
June 15, 2026 08:36
dromanol
force-pushed
the
flavien.darche/aap/response-body-outputstream
branch
2 times, most recently
from
July 21, 2026 09:27
6d83061 to
8f12fa9
Compare
dromanol
marked this pull request as ready for review
July 21, 2026 13:55
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f12fa936e
ℹ️ 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".
dromanol
approved these changes
Jul 21, 2026
dromanol
left a comment
dromanol
left a comment
Contributor
There was a problem hiding this comment.
Finally, it looks good.
Resolve open codex P2 review comments on ObjectExtractor:
- emit Uri as a scalar string
- omit public readonly fields on non-[DataContract] POCOs
- include get-only collection properties (populated via Add)
- use the field contract for [Serializable] types (raw field names, honor [NonSerialized])
- treat non-generic IDictionary (e.g. Hashtable) as a dictionary
- track visited collection instances to avoid cyclic stack overflow
- extract DateTimeOffset as the serializer's {DateTime, OffsetMinutes} object
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…extraction foreach (DictionaryEntry entry in source) over a non-generic IDictionary unboxed the enumerator's object? Current into DictionaryEntry, triggering CS8605 (unboxing a possibly null value) under #nullable. Iterate via IDictionaryEnumerator.Entry (a non-null DictionaryEntry) instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andrewlock
approved these changes
Jul 28, 2026
Member
There was a problem hiding this comment.
I have not fully reviewed this file, I just don't have the capacity to grok it unfortunately, so I'm going to defer to trusting you 😄
dromanol
force-pushed
the
flavien.darche/aap/response-body-outputstream
branch
from
July 28, 2026 14:11
7e8aa14 to
bb07ba9
Compare
…tion - Wrap DataContract-only extraction code and tests in #if NETFRAMEWORK so the .NET Core Datadog.Trace assembly no longer carries the unused paths - Invert OnMethodBegin guard for readability and annotate TryGetCaptureContext graph with [NotNullWhen(true)], removing the null-forgiving operator Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39f4ae1ad9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The System.Runtime.Serialization support entry was listed under AspNetMvc, but the InstrumentMethod attributes gate the calltargets under IntegrationId.AspNet. Align the entry to AspNet and move it to its sorted position. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 tasks
dromanol
added a commit
that referenced
this pull request
Jul 30, 2026
…8958) ## Summary An internal codex security review found that `ObjectExtractor`'s object-property extraction (`ExtractProperties`) enforces `WafConstants.MaxContainerDepth`, but the list/dictionary recursion paths (`ExtractListOrArray`, `ExtractDictionary`, `ExtractDictionaryAsKeyValuePairs`, `ExtractNonGenericDictionary`) increment depth on recursion without ever checking it against the limit. A deeply nested (acyclic) enumerable or dictionary graph can therefore recurse until `StackOverflowException`, which is process-fatal in .NET. This became more reachable after #8706 broadened the `IEnumerable` routing to any enumerable type (not just arrays/`List<T>`), widening what data can reach the unbounded recursion (e.g. `HashSet<T>`, custom `IEnumerable` wrappers, JSON-like collection types) via AAP request/response extraction. - Adds the same depth-cap check used by `ExtractProperties` to all four container-recursion paths. - Confirmed the vulnerability directly: reverting the fix and running a new regression test with 100k nested lists reliably crashed the test host process with a genuine stack overflow. ## Test plan - [x] Added `TestDeeplyNestedAcyclicListDoesNotStackOverflow` — 100k nested lists, asserts no crash (crashes the process without the fix). - [x] Added `TestNestedListRespectsMaxContainerDepth` — asserts the list is truncated to empty exactly at `MaxContainerDepth`, mirroring the existing `TestNestedObjectsAboveLimit` object-property test. - [x] Ran full `ObjectExtractorTests` suite across all target frameworks — all passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Adds API Security response body schema collection for ASP.NET MVC/Web API responses serialized with
DataContractJsonSerializer.WriteObject(response.OutputStream, data)(doc).This adds a .NET Framework AppSec instrumentation for
DataContractJsonSerializer.WriteObject(Stream, object). The hook only reports when the serializer writes to the current ASP.NET response stream, AppSec is enabled, response body parsing is enabled, and an ASP.NET MVC/Web API scope is active.It also adds a
DataContractObjectExtractorfor this path. For[DataContract]types, it reports only[DataMember]members, usesDataMember(Name = ...), and skips ignored or unmarked members.Reason for change
A case seen writes the response with
DataContractJsonSerializerdirectly toHttpResponse.OutputStream, which bypasses the existingJsonResult.Dataresponse body extraction.Without this instrumentation, those endpoints skipped AAP API Security traces.
Implementation details
System.Runtime.Serialization.Json.DataContractJsonSerializer.WriteObject(Stream, object)HttpContext.Response.OutputStream, so serializing toMemoryStream, files, or other streams are ignored.Test coverage
ActionResultand callsDataContractJsonSerializer.WriteObject(response.OutputStream, data).Other details
https://datadoghq.atlassian.net/browse/APPSEC-68123