-
Notifications
You must be signed in to change notification settings - Fork 318
Fix semaphore permit leak in API Security span post-processor #10010
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
base: master
Are you sure you want to change the base?
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 59 metrics, 6 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.107 s) : 0, 1106711
Total [baseline] (10.832 s) : 0, 10831560
Agent [candidate] (1.103 s) : 0, 1102507
Total [candidate] (10.851 s) : 0, 10850723
section appsec
Agent [baseline] (1.283 s) : 0, 1282878
Total [baseline] (11.124 s) : 0, 11124351
Agent [candidate] (1.286 s) : 0, 1285641
Total [candidate] (11.242 s) : 0, 11242095
section iast
Agent [baseline] (1.242 s) : 0, 1242283
Total [baseline] (11.228 s) : 0, 11227977
Agent [candidate] (1.239 s) : 0, 1239075
Total [candidate] (11.25 s) : 0, 11249686
section profiling
Agent [baseline] (1.233 s) : 0, 1232954
Total [baseline] (11.099 s) : 0, 11099292
Agent [candidate] (1.245 s) : 0, 1244767
Total [candidate] (11.075 s) : 0, 11075073
gantt
title petclinic - break down per module: candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.497 ms) : 0, 1497
crashtracking [candidate] (1.479 ms) : 0, 1479
BytebuddyAgent [baseline] (711.556 ms) : 0, 711556
BytebuddyAgent [candidate] (708.6 ms) : 0, 708600
GlobalTracer [baseline] (249.537 ms) : 0, 249537
GlobalTracer [candidate] (249.109 ms) : 0, 249109
AppSec [baseline] (32.157 ms) : 0, 32157
AppSec [candidate] (31.871 ms) : 0, 31871
Debugger [baseline] (64.387 ms) : 0, 64387
Debugger [candidate] (64.166 ms) : 0, 64166
Remote Config [baseline] (629.811 µs) : 0, 630
Remote Config [candidate] (628.656 µs) : 0, 629
Telemetry [baseline] (8.3 ms) : 0, 8300
Telemetry [candidate] (8.202 ms) : 0, 8202
Flare Poller [baseline] (3.71 ms) : 0, 3710
Flare Poller [candidate] (3.654 ms) : 0, 3654
section appsec
crashtracking [baseline] (1.478 ms) : 0, 1478
crashtracking [candidate] (1.491 ms) : 0, 1491
BytebuddyAgent [baseline] (731.769 ms) : 0, 731769
BytebuddyAgent [candidate] (732.05 ms) : 0, 732050
GlobalTracer [baseline] (240.995 ms) : 0, 240995
GlobalTracer [candidate] (242.02 ms) : 0, 242020
AppSec [baseline] (173.581 ms) : 0, 173581
AppSec [candidate] (175.306 ms) : 0, 175306
Debugger [baseline] (62.572 ms) : 0, 62572
Debugger [candidate] (62.027 ms) : 0, 62027
Remote Config [baseline] (682.018 µs) : 0, 682
Remote Config [candidate] (667.912 µs) : 0, 668
Telemetry [baseline] (8.246 ms) : 0, 8246
Telemetry [candidate] (8.289 ms) : 0, 8289
Flare Poller [baseline] (3.924 ms) : 0, 3924
Flare Poller [candidate] (3.831 ms) : 0, 3831
IAST [baseline] (24.699 ms) : 0, 24699
IAST [candidate] (24.958 ms) : 0, 24958
section iast
crashtracking [baseline] (1.506 ms) : 0, 1506
crashtracking [candidate] (1.486 ms) : 0, 1486
BytebuddyAgent [baseline] (834.208 ms) : 0, 834208
BytebuddyAgent [candidate] (831.989 ms) : 0, 831989
GlobalTracer [baseline] (238.243 ms) : 0, 238243
GlobalTracer [candidate] (237.558 ms) : 0, 237558
AppSec [baseline] (31.999 ms) : 0, 31999
AppSec [candidate] (31.911 ms) : 0, 31911
Debugger [baseline] (60.776 ms) : 0, 60776
Debugger [candidate] (60.394 ms) : 0, 60394
Remote Config [baseline] (540.728 µs) : 0, 541
Remote Config [candidate] (541.461 µs) : 0, 541
Telemetry [baseline] (7.589 ms) : 0, 7589
Telemetry [candidate] (7.562 ms) : 0, 7562
Flare Poller [baseline] (3.514 ms) : 0, 3514
Flare Poller [candidate] (3.439 ms) : 0, 3439
IAST [baseline] (28.989 ms) : 0, 28989
IAST [candidate] (29.342 ms) : 0, 29342
section profiling
crashtracking [baseline] (1.459 ms) : 0, 1459
crashtracking [candidate] (1.457 ms) : 0, 1457
BytebuddyAgent [baseline] (735.069 ms) : 0, 735069
BytebuddyAgent [candidate] (743.813 ms) : 0, 743813
GlobalTracer [baseline] (222.729 ms) : 0, 222729
GlobalTracer [candidate] (224.662 ms) : 0, 224662
AppSec [baseline] (32.29 ms) : 0, 32290
AppSec [candidate] (32.474 ms) : 0, 32474
Debugger [baseline] (63.232 ms) : 0, 63232
Debugger [candidate] (63.103 ms) : 0, 63103
Remote Config [baseline] (631.665 µs) : 0, 632
Remote Config [candidate] (673.138 µs) : 0, 673
Telemetry [baseline] (7.98 ms) : 0, 7980
Telemetry [candidate] (7.942 ms) : 0, 7942
Flare Poller [baseline] (3.79 ms) : 0, 3790
Flare Poller [candidate] (3.844 ms) : 0, 3844
ProfilingAgent [baseline] (96.615 ms) : 0, 96615
ProfilingAgent [candidate] (97.005 ms) : 0, 97005
Profiling [baseline] (97.206 ms) : 0, 97206
Profiling [candidate] (97.586 ms) : 0, 97586
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.1 s) : 0, 1099547
Total [baseline] (8.834 s) : 0, 8834323
Agent [candidate] (1.102 s) : 0, 1101640
Total [candidate] (8.892 s) : 0, 8891995
section iast
Agent [baseline] (1.253 s) : 0, 1252611
Total [baseline] (9.592 s) : 0, 9591896
Agent [candidate] (1.249 s) : 0, 1248825
Total [candidate] (9.587 s) : 0, 9587064
gantt
title insecure-bank - break down per module: candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.487 ms) : 0, 1487
crashtracking [candidate] (1.484 ms) : 0, 1484
BytebuddyAgent [baseline] (706.532 ms) : 0, 706532
BytebuddyAgent [candidate] (708.29 ms) : 0, 708290
GlobalTracer [baseline] (248.926 ms) : 0, 248926
GlobalTracer [candidate] (249.094 ms) : 0, 249094
AppSec [baseline] (31.949 ms) : 0, 31949
AppSec [candidate] (31.798 ms) : 0, 31798
Debugger [baseline] (63.47 ms) : 0, 63470
Debugger [candidate] (63.587 ms) : 0, 63587
Remote Config [baseline] (623.557 µs) : 0, 624
Remote Config [candidate] (641.907 µs) : 0, 642
Telemetry [baseline] (8.123 ms) : 0, 8123
Telemetry [candidate] (8.222 ms) : 0, 8222
Flare Poller [baseline] (3.635 ms) : 0, 3635
Flare Poller [candidate] (3.707 ms) : 0, 3707
section iast
crashtracking [baseline] (1.519 ms) : 0, 1519
crashtracking [candidate] (1.489 ms) : 0, 1489
BytebuddyAgent [baseline] (842.838 ms) : 0, 842838
BytebuddyAgent [candidate] (838.831 ms) : 0, 838831
GlobalTracer [baseline] (238.887 ms) : 0, 238887
GlobalTracer [candidate] (238.842 ms) : 0, 238842
AppSec [baseline] (31.76 ms) : 0, 31760
AppSec [candidate] (33.216 ms) : 0, 33216
Debugger [baseline] (60.412 ms) : 0, 60412
Debugger [candidate] (60.722 ms) : 0, 60722
Remote Config [baseline] (552.932 µs) : 0, 553
Remote Config [candidate] (570.393 µs) : 0, 570
Telemetry [baseline] (7.605 ms) : 0, 7605
Telemetry [candidate] (7.739 ms) : 0, 7739
Flare Poller [baseline] (3.468 ms) : 0, 3468
Flare Poller [candidate] (3.545 ms) : 0, 3545
IAST [baseline] (30.372 ms) : 0, 30372
IAST [candidate] (28.827 ms) : 0, 28827
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 15 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section baseline
no_agent (17.319 ms) : 17147, 17491
. : milestone, 17319,
appsec (18.847 ms) : 18657, 19037
. : milestone, 18847,
code_origins (17.727 ms) : 17549, 17904
. : milestone, 17727,
iast (17.963 ms) : 17788, 18138
. : milestone, 17963,
profiling (18.77 ms) : 18579, 18962
. : milestone, 18770,
tracing (17.903 ms) : 17722, 18085
. : milestone, 17903,
section candidate
no_agent (18.094 ms) : 17910, 18278
. : milestone, 18094,
appsec (18.987 ms) : 18795, 19179
. : milestone, 18987,
code_origins (17.798 ms) : 17620, 17976
. : milestone, 17798,
iast (17.547 ms) : 17370, 17724
. : milestone, 17547,
profiling (18.781 ms) : 18589, 18974
. : milestone, 18781,
tracing (17.795 ms) : 17613, 17976
. : milestone, 17795,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section baseline
no_agent (1.195 ms) : 1183, 1206
. : milestone, 1195,
iast (3.16 ms) : 3113, 3207
. : milestone, 3160,
iast_FULL (5.86 ms) : 5802, 5919
. : milestone, 5860,
iast_GLOBAL (3.684 ms) : 3633, 3735
. : milestone, 3684,
profiling (2.161 ms) : 2141, 2181
. : milestone, 2161,
tracing (1.817 ms) : 1801, 1832
. : milestone, 1817,
section candidate
no_agent (1.283 ms) : 1270, 1295
. : milestone, 1283,
iast (3.375 ms) : 3330, 3419
. : milestone, 3375,
iast_FULL (5.604 ms) : 5549, 5658
. : milestone, 5604,
iast_GLOBAL (3.554 ms) : 3499, 3610
. : milestone, 3554,
profiling (2.06 ms) : 2042, 2078
. : milestone, 2060,
tracing (1.841 ms) : 1824, 1857
. : milestone, 1841,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section baseline
no_agent (1.476 ms) : 1465, 1488
. : milestone, 1476,
appsec (2.46 ms) : 2407, 2512
. : milestone, 2460,
iast (2.231 ms) : 2165, 2297
. : milestone, 2231,
iast_GLOBAL (2.269 ms) : 2203, 2335
. : milestone, 2269,
profiling (2.078 ms) : 2024, 2131
. : milestone, 2078,
tracing (2.053 ms) : 2001, 2105
. : milestone, 2053,
section candidate
no_agent (1.481 ms) : 1469, 1493
. : milestone, 1481,
appsec (2.48 ms) : 2427, 2533
. : milestone, 2480,
iast (2.225 ms) : 2160, 2291
. : milestone, 2225,
iast_GLOBAL (2.276 ms) : 2210, 2342
. : milestone, 2276,
profiling (2.092 ms) : 2037, 2146
. : milestone, 2092,
tracing (2.058 ms) : 2006, 2110
. : milestone, 2058,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.57.0-SNAPSHOT~69a96c9c6a, baseline=1.57.0-SNAPSHOT~c8bb44440b
dateFormat X
axisFormat %s
section baseline
no_agent (15.28 s) : 15280000, 15280000
. : milestone, 15280000,
appsec (14.994 s) : 14994000, 14994000
. : milestone, 14994000,
iast (18.443 s) : 18443000, 18443000
. : milestone, 18443000,
iast_GLOBAL (17.889 s) : 17889000, 17889000
. : milestone, 17889000,
profiling (14.904 s) : 14904000, 14904000
. : milestone, 14904000,
tracing (15.018 s) : 15018000, 15018000
. : milestone, 15018000,
section candidate
no_agent (15.311 s) : 15311000, 15311000
. : milestone, 15311000,
appsec (14.784 s) : 14784000, 14784000
. : milestone, 14784000,
iast (18.708 s) : 18708000, 18708000
. : milestone, 18708000,
iast_GLOBAL (17.962 s) : 17962000, 17962000
. : milestone, 17962000,
profiling (15.281 s) : 15281000, 15281000
. : milestone, 15281000,
tracing (14.842 s) : 14842000, 14842000
. : milestone, 14842000,
|
| * This method only serves as a final confirmation gate before schema extraction. | ||
| */ | ||
| @Override | ||
| public boolean sampleRequest(AppSecRequestContext ctx) { |
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.
This method seems useless with the new approach but, I decided to maintain it to keep the checks although updateApiAccessIfExpired is not necessary anymore
What Does This Do
Fixed three issues causing API Security sampling to fail intermittently in standalone mode:
1. Semaphore permit leak in AppSecSpanPostProcessor
preSampleRequest()acquired a permit but early returns occurred before the try-finally block,releaseOne()was never calledprocess()method in try-finally, ensuringreleaseOne()is always called when a permit was acquired2. Race condition in ApiSecuritySamplerImpl
isExpired=truebefore any updated theaccessMappreSampleRequest()now updates map immediately after acquiring semaphore, preventing concurrent requests from seeing stale expirationstate
Motivation
API Security standalone system tests were failing intermittently in CI with
_sampling_priority_v1not being set to 2, causing traces to not be retained as expected.Related with APPSEC-57815
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]