-
Notifications
You must be signed in to change notification settings - Fork 323
Fix synchronization of ASM_DD Product listeners #10226
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
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 56 metrics, 9 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.089 s) : 0, 1088645
Total [baseline] (8.743 s) : 0, 8742591
Agent [candidate] (1.082 s) : 0, 1081993
Total [candidate] (8.715 s) : 0, 8715092
section iast
Agent [baseline] (1.238 s) : 0, 1237780
Total [baseline] (9.354 s) : 0, 9353501
Agent [candidate] (1.219 s) : 0, 1219344
Total [candidate] (9.33 s) : 0, 9329602
gantt
title insecure-bank - break down per module: candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.193 ms) : 0, 1193
crashtracking [candidate] (1.189 ms) : 0, 1189
BytebuddyAgent [baseline] (654.282 ms) : 0, 654282
BytebuddyAgent [candidate] (650.361 ms) : 0, 650361
GlobalTracer [baseline] (284.078 ms) : 0, 284078
GlobalTracer [candidate] (282.549 ms) : 0, 282549
AppSec [baseline] (32.58 ms) : 0, 32580
AppSec [candidate] (32.39 ms) : 0, 32390
Debugger [baseline] (67.665 ms) : 0, 67665
Debugger [candidate] (67.0 ms) : 0, 67000
Remote Config [baseline] (608.207 µs) : 0, 608
Remote Config [candidate] (632.411 µs) : 0, 632
Telemetry [baseline] (8.967 ms) : 0, 8967
Telemetry [candidate] (8.881 ms) : 0, 8881
Flare Poller [baseline] (3.725 ms) : 0, 3725
Flare Poller [candidate] (3.66 ms) : 0, 3660
section iast
crashtracking [baseline] (1.208 ms) : 0, 1208
crashtracking [candidate] (1.186 ms) : 0, 1186
BytebuddyAgent [baseline] (802.715 ms) : 0, 802715
BytebuddyAgent [candidate] (788.691 ms) : 0, 788691
GlobalTracer [baseline] (258.507 ms) : 0, 258507
GlobalTracer [candidate] (255.292 ms) : 0, 255292
IAST [baseline] (27.542 ms) : 0, 27542
IAST [candidate] (27.089 ms) : 0, 27089
AppSec [baseline] (34.669 ms) : 0, 34669
AppSec [candidate] (34.294 ms) : 0, 34294
Debugger [baseline] (65.058 ms) : 0, 65058
Debugger [candidate] (64.735 ms) : 0, 64735
Remote Config [baseline] (592.922 µs) : 0, 593
Remote Config [candidate] (609.018 µs) : 0, 609
Telemetry [baseline] (8.454 ms) : 0, 8454
Telemetry [candidate] (8.539 ms) : 0, 8539
Flare Poller [baseline] (3.552 ms) : 0, 3552
Flare Poller [candidate] (3.616 ms) : 0, 3616
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.083 s) : 0, 1082755
Total [baseline] (10.795 s) : 0, 10794771
Agent [candidate] (1.081 s) : 0, 1081136
Total [candidate] (10.844 s) : 0, 10843673
section appsec
Agent [baseline] (1.265 s) : 0, 1265011
Total [baseline] (11.063 s) : 0, 11063287
Agent [candidate] (1.279 s) : 0, 1279217
Total [candidate] (10.956 s) : 0, 10955965
section iast
Agent [baseline] (1.221 s) : 0, 1220702
Total [baseline] (11.183 s) : 0, 11183126
Agent [candidate] (1.223 s) : 0, 1222756
Total [candidate] (11.257 s) : 0, 11257040
section profiling
Agent [baseline] (1.207 s) : 0, 1207297
Total [baseline] (10.994 s) : 0, 10994497
Agent [candidate] (1.21 s) : 0, 1210160
Total [candidate] (10.872 s) : 0, 10872072
gantt
title petclinic - break down per module: candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.184 ms) : 0, 1184
crashtracking [candidate] (1.176 ms) : 0, 1176
BytebuddyAgent [baseline] (649.61 ms) : 0, 649610
BytebuddyAgent [candidate] (649.153 ms) : 0, 649153
GlobalTracer [baseline] (282.767 ms) : 0, 282767
GlobalTracer [candidate] (282.54 ms) : 0, 282540
AppSec [baseline] (32.401 ms) : 0, 32401
AppSec [candidate] (32.108 ms) : 0, 32108
Debugger [baseline] (67.961 ms) : 0, 67961
Debugger [candidate] (67.725 ms) : 0, 67725
Remote Config [baseline] (638.493 µs) : 0, 638
Remote Config [candidate] (589.476 µs) : 0, 589
Telemetry [baseline] (9.053 ms) : 0, 9053
Telemetry [candidate] (8.885 ms) : 0, 8885
Flare Poller [baseline] (3.782 ms) : 0, 3782
Flare Poller [candidate] (3.688 ms) : 0, 3688
section appsec
crashtracking [baseline] (1.178 ms) : 0, 1178
crashtracking [candidate] (1.205 ms) : 0, 1205
BytebuddyAgent [baseline] (690.561 ms) : 0, 690561
BytebuddyAgent [candidate] (700.191 ms) : 0, 700191
GlobalTracer [baseline] (258.123 ms) : 0, 258123
GlobalTracer [candidate] (261.426 ms) : 0, 261426
AppSec [baseline] (171.547 ms) : 0, 171547
AppSec [candidate] (173.733 ms) : 0, 173733
Debugger [baseline] (70.262 ms) : 0, 70262
Debugger [candidate] (68.41 ms) : 0, 68410
Remote Config [baseline] (720.026 µs) : 0, 720
Remote Config [candidate] (743.682 µs) : 0, 744
Telemetry [baseline] (8.929 ms) : 0, 8929
Telemetry [candidate] (9.051 ms) : 0, 9051
Flare Poller [baseline] (3.797 ms) : 0, 3797
Flare Poller [candidate] (3.849 ms) : 0, 3849
IAST [baseline] (24.429 ms) : 0, 24429
IAST [candidate] (24.925 ms) : 0, 24925
section iast
crashtracking [baseline] (1.18 ms) : 0, 1180
crashtracking [candidate] (1.181 ms) : 0, 1181
BytebuddyAgent [baseline] (789.421 ms) : 0, 789421
BytebuddyAgent [candidate] (789.49 ms) : 0, 789490
GlobalTracer [baseline] (254.948 ms) : 0, 254948
GlobalTracer [candidate] (255.697 ms) : 0, 255697
AppSec [baseline] (31.084 ms) : 0, 31084
AppSec [candidate] (35.488 ms) : 0, 35488
Debugger [baseline] (68.96 ms) : 0, 68960
Debugger [candidate] (65.811 ms) : 0, 65811
Remote Config [baseline] (584.8 µs) : 0, 585
Remote Config [candidate] (602.229 µs) : 0, 602
Telemetry [baseline] (8.511 ms) : 0, 8511
Telemetry [candidate] (8.556 ms) : 0, 8556
Flare Poller [baseline] (3.562 ms) : 0, 3562
Flare Poller [candidate] (3.555 ms) : 0, 3555
IAST [baseline] (27.194 ms) : 0, 27194
IAST [candidate] (27.087 ms) : 0, 27087
section profiling
ProfilingAgent [baseline] (97.676 ms) : 0, 97676
ProfilingAgent [candidate] (96.28 ms) : 0, 96280
crashtracking [baseline] (1.212 ms) : 0, 1212
crashtracking [candidate] (1.22 ms) : 0, 1220
BytebuddyAgent [baseline] (703.855 ms) : 0, 703855
BytebuddyAgent [candidate] (706.534 ms) : 0, 706534
GlobalTracer [baseline] (221.181 ms) : 0, 221181
GlobalTracer [candidate] (222.119 ms) : 0, 222119
AppSec [baseline] (31.957 ms) : 0, 31957
AppSec [candidate] (32.4 ms) : 0, 32400
Debugger [baseline] (68.262 ms) : 0, 68262
Debugger [candidate] (68.539 ms) : 0, 68539
Remote Config [baseline] (638.393 µs) : 0, 638
Remote Config [candidate] (659.5 µs) : 0, 660
Telemetry [baseline] (8.846 ms) : 0, 8846
Telemetry [candidate] (8.612 ms) : 0, 8612
Flare Poller [baseline] (3.71 ms) : 0, 3710
Flare Poller [candidate] (3.704 ms) : 0, 3704
Profiling [baseline] (98.287 ms) : 0, 98287
Profiling [candidate] (96.853 ms) : 0, 96853
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 17 metrics, 18 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section baseline
no_agent (1.183 ms) : 1172, 1195
. : milestone, 1183,
iast (3.217 ms) : 3174, 3260
. : milestone, 3217,
iast_FULL (5.759 ms) : 5700, 5819
. : milestone, 5759,
iast_GLOBAL (3.493 ms) : 3437, 3550
. : milestone, 3493,
profiling (1.913 ms) : 1898, 1928
. : milestone, 1913,
tracing (1.779 ms) : 1763, 1794
. : milestone, 1779,
section candidate
no_agent (1.165 ms) : 1154, 1176
. : milestone, 1165,
iast (3.18 ms) : 3141, 3219
. : milestone, 3180,
iast_FULL (5.697 ms) : 5641, 5753
. : milestone, 5697,
iast_GLOBAL (3.588 ms) : 3532, 3644
. : milestone, 3588,
profiling (2.125 ms) : 2106, 2144
. : milestone, 2125,
tracing (1.808 ms) : 1793, 1823
. : milestone, 1808,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section baseline
no_agent (19.459 ms) : 19260, 19659
. : milestone, 19459,
appsec (18.563 ms) : 18372, 18755
. : milestone, 18563,
code_origins (17.576 ms) : 17399, 17753
. : milestone, 17576,
iast (17.654 ms) : 17479, 17830
. : milestone, 17654,
profiling (18.657 ms) : 18472, 18841
. : milestone, 18657,
tracing (17.978 ms) : 17798, 18158
. : milestone, 17978,
section candidate
no_agent (18.659 ms) : 18469, 18850
. : milestone, 18659,
appsec (18.705 ms) : 18511, 18899
. : milestone, 18705,
code_origins (17.651 ms) : 17477, 17824
. : milestone, 17651,
iast (17.399 ms) : 17227, 17571
. : milestone, 17399,
profiling (18.945 ms) : 18755, 19136
. : milestone, 18945,
tracing (17.736 ms) : 17557, 17914
. : milestone, 17736,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section baseline
no_agent (15.529 s) : 15529000, 15529000
. : milestone, 15529000,
appsec (14.447 s) : 14447000, 14447000
. : milestone, 14447000,
iast (18.474 s) : 18474000, 18474000
. : milestone, 18474000,
iast_GLOBAL (18.048 s) : 18048000, 18048000
. : milestone, 18048000,
profiling (14.901 s) : 14901000, 14901000
. : milestone, 14901000,
tracing (15.052 s) : 15052000, 15052000
. : milestone, 15052000,
section candidate
no_agent (15.567 s) : 15567000, 15567000
. : milestone, 15567000,
appsec (14.662 s) : 14662000, 14662000
. : milestone, 14662000,
iast (18.133 s) : 18133000, 18133000
. : milestone, 18133000,
iast_GLOBAL (18.026 s) : 18026000, 18026000
. : milestone, 18026000,
profiling (14.941 s) : 14941000, 14941000
. : milestone, 14941000,
tracing (14.728 s) : 14728000, 14728000
. : milestone, 14728000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.57.0-SNAPSHOT~a9b2c43040, baseline=1.58.0-SNAPSHOT~fbdb1cffac
dateFormat X
axisFormat %s
section baseline
no_agent (1.479 ms) : 1468, 1491
. : milestone, 1479,
appsec (3.717 ms) : 3497, 3936
. : milestone, 3717,
iast (2.206 ms) : 2142, 2271
. : milestone, 2206,
iast_GLOBAL (2.255 ms) : 2190, 2319
. : milestone, 2255,
profiling (2.071 ms) : 2018, 2124
. : milestone, 2071,
tracing (2.044 ms) : 1992, 2095
. : milestone, 2044,
section candidate
no_agent (1.476 ms) : 1464, 1487
. : milestone, 1476,
appsec (3.661 ms) : 3446, 3875
. : milestone, 3661,
iast (2.208 ms) : 2144, 2273
. : milestone, 2208,
iast_GLOBAL (2.25 ms) : 2186, 2315
. : milestone, 2250,
profiling (2.051 ms) : 1999, 2103
. : milestone, 2051,
tracing (2.056 ms) : 2005, 2107
. : milestone, 2056,
|
|
I think we should discuss the implications of this with the guild. |
Yeah, this PR is a bit of an ugly fix, but it clearly addresses the problem we’re seeing. My main goal was to reliably reproduce the error, confirm that it’s caused by the ordering, and verify that changing the order actually fixes it. I fully agree this is something we should discuss and refine so we can find the best long term solution. I didn’t want to change the order for all Products yet, since I’m not sure what side effects that might have. |
| } | ||
| } | ||
|
|
||
| // Step 2: For products other than ASM_DD, apply changes immediately |
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'm wondering if this better handled in some ASM specific code.
I ask because I'd like to keep product specific concerns out the general remote config code.
That said, since this does sound like it is addressing a rather urgent issue, I'm fine with proceeding with the fix as is. I think I'd just like a few more comments explain why ASM is being handled differently.
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.
Thanks Doug. I’ll improve the comments and add a TODO so we can solve this properly after we discuss it in the guild. It would be great to get this change into the January 5 release. I’ll try to merge it today since I’ll be out for a couple of weeks.
What Does This Do
Change the order of apply/remove listeners to remove/apply only for ASM_DD product
Motivation
https://datadog.zendesk.com/agent/tickets/2388805
Additional Notes
ASM_DD Product listeners needs to remove configs before add/update due to WAF requirements
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:APPSEC-60250