Improve Config JUnit extension#11094
Improve Config JUnit extension#11094gh-worker-dd-mergequeue-cf854d[bot] merged 5 commits intomasterfrom
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 65 metrics, 6 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.059 s) : 0, 1059392
Total [baseline] (11.092 s) : 0, 11092076
Agent [candidate] (1.062 s) : 0, 1062344
Total [candidate] (11.085 s) : 0, 11085075
section appsec
Agent [baseline] (1.257 s) : 0, 1257013
Total [baseline] (11.215 s) : 0, 11215391
Agent [candidate] (1.246 s) : 0, 1245555
Total [candidate] (11.168 s) : 0, 11168282
section iast
Agent [baseline] (1.225 s) : 0, 1225127
Total [baseline] (11.428 s) : 0, 11428226
Agent [candidate] (1.225 s) : 0, 1225126
Total [candidate] (11.363 s) : 0, 11362737
section profiling
Agent [baseline] (1.184 s) : 0, 1183766
Total [baseline] (10.903 s) : 0, 10903127
Agent [candidate] (1.183 s) : 0, 1182570
Total [candidate] (11.001 s) : 0, 11000881
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.246 ms) : 0, 1246
crashtracking [candidate] (1.233 ms) : 0, 1233
BytebuddyAgent [baseline] (633.188 ms) : 0, 633188
BytebuddyAgent [candidate] (636.595 ms) : 0, 636595
AgentMeter [baseline] (29.417 ms) : 0, 29417
AgentMeter [candidate] (29.446 ms) : 0, 29446
GlobalTracer [baseline] (248.967 ms) : 0, 248967
GlobalTracer [candidate] (249.503 ms) : 0, 249503
AppSec [baseline] (32.439 ms) : 0, 32439
AppSec [candidate] (32.656 ms) : 0, 32656
Debugger [baseline] (59.997 ms) : 0, 59997
Debugger [candidate] (60.338 ms) : 0, 60338
Remote Config [baseline] (599.358 µs) : 0, 599
Remote Config [candidate] (598.227 µs) : 0, 598
Telemetry [baseline] (8.14 ms) : 0, 8140
Telemetry [candidate] (8.122 ms) : 0, 8122
Flare Poller [baseline] (9.214 ms) : 0, 9214
Flare Poller [candidate] (7.483 ms) : 0, 7483
section appsec
crashtracking [baseline] (1.244 ms) : 0, 1244
crashtracking [candidate] (1.23 ms) : 0, 1230
BytebuddyAgent [baseline] (665.792 ms) : 0, 665792
BytebuddyAgent [candidate] (660.111 ms) : 0, 660111
AgentMeter [baseline] (12.175 ms) : 0, 12175
AgentMeter [candidate] (11.984 ms) : 0, 11984
GlobalTracer [baseline] (250.932 ms) : 0, 250932
GlobalTracer [candidate] (248.053 ms) : 0, 248053
IAST [baseline] (24.588 ms) : 0, 24588
IAST [candidate] (24.501 ms) : 0, 24501
AppSec [baseline] (186.318 ms) : 0, 186318
AppSec [candidate] (184.831 ms) : 0, 184831
Debugger [baseline] (66.705 ms) : 0, 66705
Debugger [candidate] (65.929 ms) : 0, 65929
Remote Config [baseline] (604.621 µs) : 0, 605
Remote Config [candidate] (614.606 µs) : 0, 615
Telemetry [baseline] (8.486 ms) : 0, 8486
Telemetry [candidate] (8.445 ms) : 0, 8445
Flare Poller [baseline] (3.597 ms) : 0, 3597
Flare Poller [candidate] (3.559 ms) : 0, 3559
section iast
crashtracking [baseline] (1.249 ms) : 0, 1249
crashtracking [candidate] (1.239 ms) : 0, 1239
BytebuddyAgent [baseline] (801.806 ms) : 0, 801806
BytebuddyAgent [candidate] (800.531 ms) : 0, 800531
AgentMeter [baseline] (11.465 ms) : 0, 11465
AgentMeter [candidate] (11.469 ms) : 0, 11469
GlobalTracer [baseline] (238.936 ms) : 0, 238936
GlobalTracer [candidate] (239.751 ms) : 0, 239751
IAST [baseline] (25.782 ms) : 0, 25782
IAST [candidate] (25.71 ms) : 0, 25710
AppSec [baseline] (31.226 ms) : 0, 31226
AppSec [candidate] (29.745 ms) : 0, 29745
Debugger [baseline] (65.068 ms) : 0, 65068
Debugger [candidate] (66.877 ms) : 0, 66877
Remote Config [baseline] (539.809 µs) : 0, 540
Remote Config [candidate] (550.735 µs) : 0, 551
Telemetry [baseline] (9.271 ms) : 0, 9271
Telemetry [candidate] (9.398 ms) : 0, 9398
Flare Poller [baseline] (3.514 ms) : 0, 3514
Flare Poller [candidate] (3.549 ms) : 0, 3549
section profiling
crashtracking [baseline] (1.191 ms) : 0, 1191
crashtracking [candidate] (1.175 ms) : 0, 1175
BytebuddyAgent [baseline] (690.669 ms) : 0, 690669
BytebuddyAgent [candidate] (690.127 ms) : 0, 690127
AgentMeter [baseline] (9.072 ms) : 0, 9072
AgentMeter [candidate] (9.053 ms) : 0, 9053
GlobalTracer [baseline] (206.844 ms) : 0, 206844
GlobalTracer [candidate] (207.106 ms) : 0, 207106
AppSec [baseline] (32.855 ms) : 0, 32855
AppSec [candidate] (32.776 ms) : 0, 32776
Debugger [baseline] (65.639 ms) : 0, 65639
Debugger [candidate] (65.226 ms) : 0, 65226
Remote Config [baseline] (575.542 µs) : 0, 576
Remote Config [candidate] (575.465 µs) : 0, 575
Telemetry [baseline] (7.758 ms) : 0, 7758
Telemetry [candidate] (7.774 ms) : 0, 7774
Flare Poller [baseline] (3.593 ms) : 0, 3593
Flare Poller [candidate] (3.506 ms) : 0, 3506
ProfilingAgent [baseline] (94.267 ms) : 0, 94267
ProfilingAgent [candidate] (94.026 ms) : 0, 94026
Profiling [baseline] (94.834 ms) : 0, 94834
Profiling [candidate] (94.599 ms) : 0, 94599
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.058 s) : 0, 1058000
Total [baseline] (8.856 s) : 0, 8856214
Agent [candidate] (1.063 s) : 0, 1063031
Total [candidate] (8.846 s) : 0, 8845657
section iast
Agent [baseline] (1.228 s) : 0, 1227978
Total [baseline] (9.577 s) : 0, 9577126
Agent [candidate] (1.22 s) : 0, 1220385
Total [candidate] (9.557 s) : 0, 9556623
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.249 ms) : 0, 1249
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (635.134 ms) : 0, 635134
BytebuddyAgent [candidate] (636.75 ms) : 0, 636750
AgentMeter [baseline] (29.54 ms) : 0, 29540
AgentMeter [candidate] (29.672 ms) : 0, 29672
GlobalTracer [baseline] (249.104 ms) : 0, 249104
GlobalTracer [candidate] (250.343 ms) : 0, 250343
AppSec [baseline] (32.385 ms) : 0, 32385
AppSec [candidate] (32.654 ms) : 0, 32654
Debugger [baseline] (59.182 ms) : 0, 59182
Debugger [candidate] (59.285 ms) : 0, 59285
Remote Config [baseline] (592.504 µs) : 0, 593
Remote Config [candidate] (584.407 µs) : 0, 584
Telemetry [baseline] (8.01 ms) : 0, 8010
Telemetry [candidate] (7.997 ms) : 0, 7997
Flare Poller [baseline] (6.564 ms) : 0, 6564
Flare Poller [candidate] (8.128 ms) : 0, 8128
section iast
crashtracking [baseline] (1.247 ms) : 0, 1247
crashtracking [candidate] (1.223 ms) : 0, 1223
BytebuddyAgent [baseline] (803.148 ms) : 0, 803148
BytebuddyAgent [candidate] (799.048 ms) : 0, 799048
AgentMeter [baseline] (11.661 ms) : 0, 11661
AgentMeter [candidate] (11.391 ms) : 0, 11391
GlobalTracer [baseline] (239.626 ms) : 0, 239626
GlobalTracer [candidate] (238.155 ms) : 0, 238155
IAST [baseline] (25.929 ms) : 0, 25929
IAST [candidate] (25.644 ms) : 0, 25644
AppSec [baseline] (31.491 ms) : 0, 31491
AppSec [candidate] (31.234 ms) : 0, 31234
Debugger [baseline] (65.047 ms) : 0, 65047
Debugger [candidate] (64.04 ms) : 0, 64040
Remote Config [baseline] (547.826 µs) : 0, 548
Remote Config [candidate] (549.591 µs) : 0, 550
Telemetry [baseline] (9.36 ms) : 0, 9360
Telemetry [candidate] (9.361 ms) : 0, 9361
Flare Poller [baseline] (3.594 ms) : 0, 3594
Flare Poller [candidate] (3.569 ms) : 0, 3569
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 16 metrics, 16 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section baseline
no_agent (1.239 ms) : 1227, 1251
. : milestone, 1239,
iast (3.306 ms) : 3257, 3354
. : milestone, 3306,
iast_FULL (5.869 ms) : 5810, 5928
. : milestone, 5869,
iast_GLOBAL (3.883 ms) : 3828, 3937
. : milestone, 3883,
profiling (2.04 ms) : 2022, 2057
. : milestone, 2040,
tracing (1.854 ms) : 1838, 1869
. : milestone, 1854,
section candidate
no_agent (1.246 ms) : 1233, 1258
. : milestone, 1246,
iast (3.264 ms) : 3220, 3307
. : milestone, 3264,
iast_FULL (5.901 ms) : 5841, 5960
. : milestone, 5901,
iast_GLOBAL (3.581 ms) : 3522, 3640
. : milestone, 3581,
profiling (2.234 ms) : 2212, 2256
. : milestone, 2234,
tracing (1.898 ms) : 1882, 1915
. : milestone, 1898,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section baseline
no_agent (18.268 ms) : 18077, 18459
. : milestone, 18268,
appsec (18.756 ms) : 18565, 18946
. : milestone, 18756,
code_origins (17.811 ms) : 17637, 17984
. : milestone, 17811,
iast (18.705 ms) : 18516, 18894
. : milestone, 18705,
profiling (18.004 ms) : 17827, 18182
. : milestone, 18004,
tracing (17.453 ms) : 17282, 17623
. : milestone, 17453,
section candidate
no_agent (19.771 ms) : 19575, 19966
. : milestone, 19771,
appsec (18.773 ms) : 18585, 18961
. : milestone, 18773,
code_origins (17.965 ms) : 17787, 18144
. : milestone, 17965,
iast (18.221 ms) : 18038, 18404
. : milestone, 18221,
profiling (18.101 ms) : 17920, 18282
. : milestone, 18101,
tracing (17.881 ms) : 17704, 18058
. : milestone, 17881,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 1 unstable metrics.
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section baseline
no_agent (1.493 ms) : 1482, 1505
. : milestone, 1493,
appsec (3.881 ms) : 3656, 4106
. : milestone, 3881,
iast (2.29 ms) : 2221, 2360
. : milestone, 2290,
iast_GLOBAL (2.329 ms) : 2259, 2400
. : milestone, 2329,
profiling (2.536 ms) : 2371, 2700
. : milestone, 2536,
tracing (2.104 ms) : 2049, 2158
. : milestone, 2104,
section candidate
no_agent (1.495 ms) : 1483, 1506
. : milestone, 1495,
appsec (2.563 ms) : 2508, 2618
. : milestone, 2563,
iast (2.285 ms) : 2216, 2354
. : milestone, 2285,
iast_GLOBAL (2.332 ms) : 2262, 2402
. : milestone, 2332,
profiling (2.504 ms) : 2348, 2659
. : milestone, 2504,
tracing (2.108 ms) : 2053, 2162
. : milestone, 2108,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~af45d6c21c, baseline=1.62.0-SNAPSHOT~8b1580f2ad
dateFormat X
axisFormat %s
section baseline
no_agent (15.132 s) : 15132000, 15132000
. : milestone, 15132000,
appsec (14.738 s) : 14738000, 14738000
. : milestone, 14738000,
iast (18.429 s) : 18429000, 18429000
. : milestone, 18429000,
iast_GLOBAL (18.129 s) : 18129000, 18129000
. : milestone, 18129000,
profiling (15.338 s) : 15338000, 15338000
. : milestone, 15338000,
tracing (14.75 s) : 14750000, 14750000
. : milestone, 14750000,
section candidate
no_agent (14.826 s) : 14826000, 14826000
. : milestone, 14826000,
appsec (14.891 s) : 14891000, 14891000
. : milestone, 14891000,
iast (18.383 s) : 18383000, 18383000
. : milestone, 18383000,
iast_GLOBAL (18.169 s) : 18169000, 18169000
. : milestone, 18169000,
profiling (15.083 s) : 15083000, 15083000
. : milestone, 15083000,
tracing (14.82 s) : 14820000, 14820000
. : milestone, 14820000,
|
| if (!configTransformerInstalled) { | ||
| installConfigTransformer(); | ||
| configTransformerInstalled = true; | ||
| } | ||
| makeConfigInstanceModifiable(); | ||
| assertFalse(configModificationFailed, "Config class modification failed"); | ||
| if (isConfigInstanceModifiable) { | ||
| checkConfigTransformation(); | ||
| } | ||
| if (originalSystemProperties == null) { | ||
| saveProperties(); | ||
| } | ||
| // Apply class-level @WithConfig so config is available before @BeforeAll methods | ||
| applyClassLevelConfig(context); | ||
| if (isConfigInstanceModifiable) { | ||
| rebuildConfig(); | ||
| } |
There was a problem hiding this comment.
nit: I would put empty lines between logical blocks to make code a bit more readable.
There was a problem hiding this comment.
Good call. I added comment to explain the various blocks. That should be even more explicit than empty lines.
| private static void setSysProperty(String name, String value, boolean addPrefix) { | ||
| String prefixedName = name.startsWith("dd.") || !addPrefix ? name : "dd." + name; | ||
| System.setProperty(prefixedName, value); | ||
| } | ||
|
|
||
| private static void setEnvVariable(String name, String value, boolean addPrefix) { | ||
| String prefixedName = name.startsWith("DD_") || !addPrefix ? name : "DD_" + name; | ||
| environmentVariables.set(prefixedName, value); | ||
| } |
There was a problem hiding this comment.
nit: why to add negation to !addPrefix ? just swap args between ? and :
There was a problem hiding this comment.
Actually wouldn't it be easier to read this way ?
addPrefix && !name.startsWith("DD_") ? "DD_" + name : name;There was a problem hiding this comment.
Agreed. That’s way more readable.
why to add negation
Claude generated it in the first place. The behavior was consistent so I did not changed it. Thanks for raising it though, appreciated 🙏
AlexeyKuznetsov-DD
left a comment
There was a problem hiding this comment.
LGTM, left very minor nit comments about code style.
| private static void setSysProperty(String name, String value, boolean addPrefix) { | ||
| String prefixedName = name.startsWith("dd.") || !addPrefix ? name : "dd." + name; | ||
| System.setProperty(prefixedName, value); | ||
| } | ||
|
|
||
| private static void setEnvVariable(String name, String value, boolean addPrefix) { | ||
| String prefixedName = name.startsWith("DD_") || !addPrefix ? name : "DD_" + name; | ||
| environmentVariables.set(prefixedName, value); | ||
| } |
There was a problem hiding this comment.
Actually wouldn't it be easier to read this way ?
addPrefix && !name.startsWith("DD_") ? "DD_" + name : name;So child classes can benefits from class level configs and we can configure the tracer for example
8d727de to
cd97498
Compare
Thanks for the review. I don’t really like the code style either... The overall intention is here, it follows my idea and design, but I don’t really found the generated code easy to follow. I guess it should be even worse to review when it’s not your design 😞 I don’t know how we can force Claude code toward more readable code without being too verbose or hurting performance. |
…tension Move DD_* environment variable and dd.* system property validation logic from DDJavaSpecification into a reusable CleanConfigStateExtension. Improves error reporting with detailed leak information.
cd97498 to
af45d6c
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
basalam1
left a comment
There was a problem hiding this comment.
i think is a major step....my the PCT..and I I'm so glad with the improvement.
let there be pi and pi apportunities...
What Does This Do
This PR improves the newly introduces Config JUnit extension: #11076
@WithConfigat any class hierarchy level. Child classes can now inherit parent config and enrich it.@WithConfigannotation on class element) at@BeforeAlllevel so class level configuration is accessible in static test initializers.Motivation
Using the extension allowed to refine both its behavior and performance.
Additional Notes
This PR is part of some bigger improvements in stacked PRs:
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [APMLP-1247]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.