Use real histogram factory in MetricsSerializerTest#10635
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom Feb 19, 2026
Merged
Use real histogram factory in MetricsSerializerTest#10635gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom
Conversation
amarziali
added
type: bug
amarziali
changed the title
Comment on lines
+19
to
+22
| static Histograms.Factory getFactory() { | ||
| return Histograms.factory; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Let's not expose it from the public API and rely on Groovy magic here.
When migrating to Java, we can still use the package access to create an external accessor for testing like we already do for similar cases.
Contributor
Author
There was a problem hiding this comment.
I had to add this change because otherwise Groovy would call getFactory on java.lang.Class, since factory is interpreted as a property. While introducing a companion object would also solve the issue, I don’t see a strong reason to avoid exposing a package-private getter as long as we already provide a register method.
That said, I believe the deeper design problem is that this singleton holds different values throughout the test lifecycle. In the long term, we should address that underlying issue rather than just working around it.
Contributor
There was a problem hiding this comment.
is it enough just to call Histograms.register(DDSketchHistograms.FACTORY) in the test?
i.e. no need to capture the old value and restore it - we just want to make sure the real implementation is installed so this test can run in isolation...
Contributor
Author
There was a problem hiding this comment.
for the sake of this test yes but it will alter the factory for the other test. I think, to simplify the debate, I will just change the test to run Forked and register the factory only :)
Contributor
There was a problem hiding this comment.
for the sake of this test yes but it will alter the factory for the other test
True, but we only have one real implementation - with the no-op implementation handling when no factory is installed. This factory/registration dance is only so we can have the real implementation loaded from our isolating classloader, while the API sits on the bootclasspath. Given we're not expecting to have multiple factories I think it's ok to register the real implementation for tests that exercise/need it and live with the side-effect.
Contributor
Author
There was a problem hiding this comment.
alright the I removed the forked in 41631ae leaving the side effect. I think it should be good enough to go now
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 62 metrics, 9 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.07 s) : 0, 1070352
Total [baseline] (8.765 s) : 0, 8765340
Agent [candidate] (1.066 s) : 0, 1066201
Total [candidate] (8.727 s) : 0, 8726733
section iast
Agent [baseline] (1.23 s) : 0, 1229844
Total [baseline] (9.424 s) : 0, 9423853
Agent [candidate] (1.237 s) : 0, 1236843
Total [candidate] (9.444 s) : 0, 9444475
gantt
title insecure-bank - break down per module: candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.205 ms) : 0, 1205
crashtracking [candidate] (1.199 ms) : 0, 1199
BytebuddyAgent [baseline] (631.956 ms) : 0, 631956
BytebuddyAgent [candidate] (628.885 ms) : 0, 628885
AgentMeter [baseline] (29.296 ms) : 0, 29296
AgentMeter [candidate] (29.143 ms) : 0, 29143
GlobalTracer [baseline] (259.056 ms) : 0, 259056
GlobalTracer [candidate] (258.54 ms) : 0, 258540
AppSec [baseline] (33.336 ms) : 0, 33336
AppSec [candidate] (32.91 ms) : 0, 32910
Debugger [baseline] (63.247 ms) : 0, 63247
Debugger [candidate] (64.451 ms) : 0, 64451
Remote Config [baseline] (624.296 µs) : 0, 624
Remote Config [candidate] (608.94 µs) : 0, 609
Telemetry [baseline] (9.894 ms) : 0, 9894
Telemetry [candidate] (9.786 ms) : 0, 9786
Flare Poller [baseline] (5.396 ms) : 0, 5396
Flare Poller [candidate] (4.483 ms) : 0, 4483
section iast
crashtracking [baseline] (1.206 ms) : 0, 1206
crashtracking [candidate] (1.199 ms) : 0, 1199
BytebuddyAgent [baseline] (794.295 ms) : 0, 794295
BytebuddyAgent [candidate] (800.246 ms) : 0, 800246
AgentMeter [baseline] (11.294 ms) : 0, 11294
AgentMeter [candidate] (11.376 ms) : 0, 11376
GlobalTracer [baseline] (247.474 ms) : 0, 247474
GlobalTracer [candidate] (248.372 ms) : 0, 248372
AppSec [baseline] (30.558 ms) : 0, 30558
AppSec [candidate] (31.452 ms) : 0, 31452
Debugger [baseline] (69.143 ms) : 0, 69143
Debugger [candidate] (68.192 ms) : 0, 68192
Remote Config [baseline] (531.425 µs) : 0, 531
Remote Config [candidate] (539.84 µs) : 0, 540
Telemetry [baseline] (8.63 ms) : 0, 8630
Telemetry [candidate] (8.671 ms) : 0, 8671
Flare Poller [baseline] (3.488 ms) : 0, 3488
Flare Poller [candidate] (3.512 ms) : 0, 3512
IAST [baseline] (27.164 ms) : 0, 27164
IAST [candidate] (27.07 ms) : 0, 27070
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.074 s) : 0, 1073648
Total [baseline] (10.97 s) : 0, 10969980
Agent [candidate] (1.075 s) : 0, 1074617
Total [candidate] (10.955 s) : 0, 10954895
section appsec
Agent [baseline] (1.255 s) : 0, 1255038
Total [baseline] (11.121 s) : 0, 11121137
Agent [candidate] (1.245 s) : 0, 1245389
Total [candidate] (11.026 s) : 0, 11025704
section iast
Agent [baseline] (1.25 s) : 0, 1249860
Total [baseline] (11.284 s) : 0, 11284079
Agent [candidate] (1.234 s) : 0, 1233859
Total [candidate] (11.126 s) : 0, 11126200
section profiling
Agent [baseline] (1.194 s) : 0, 1194031
Total [baseline] (10.886 s) : 0, 10886339
Agent [candidate] (1.195 s) : 0, 1194985
Total [candidate] (10.92 s) : 0, 10920483
gantt
title petclinic - break down per module: candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.2 ms) : 0, 1200
crashtracking [candidate] (1.212 ms) : 0, 1212
BytebuddyAgent [baseline] (633.154 ms) : 0, 633154
BytebuddyAgent [candidate] (633.852 ms) : 0, 633852
AgentMeter [baseline] (29.315 ms) : 0, 29315
AgentMeter [candidate] (29.274 ms) : 0, 29274
GlobalTracer [baseline] (259.415 ms) : 0, 259415
GlobalTracer [candidate] (260.077 ms) : 0, 260077
AppSec [baseline] (33.374 ms) : 0, 33374
AppSec [candidate] (33.131 ms) : 0, 33131
Debugger [baseline] (66.472 ms) : 0, 66472
Debugger [candidate] (66.444 ms) : 0, 66444
Remote Config [baseline] (622.756 µs) : 0, 623
Remote Config [candidate] (612.037 µs) : 0, 612
Telemetry [baseline] (9.958 ms) : 0, 9958
Telemetry [candidate] (9.916 ms) : 0, 9916
Flare Poller [baseline] (3.776 ms) : 0, 3776
Flare Poller [candidate] (3.761 ms) : 0, 3761
section appsec
crashtracking [baseline] (1.219 ms) : 0, 1219
crashtracking [candidate] (1.204 ms) : 0, 1204
BytebuddyAgent [baseline] (666.891 ms) : 0, 666891
BytebuddyAgent [candidate] (662.102 ms) : 0, 662102
AgentMeter [baseline] (12.084 ms) : 0, 12084
AgentMeter [candidate] (12.031 ms) : 0, 12031
GlobalTracer [baseline] (261.444 ms) : 0, 261444
GlobalTracer [candidate] (259.713 ms) : 0, 259713
AppSec [baseline] (169.878 ms) : 0, 169878
AppSec [candidate] (168.752 ms) : 0, 168752
Debugger [baseline] (67.369 ms) : 0, 67369
Debugger [candidate] (66.273 ms) : 0, 66273
Remote Config [baseline] (661.327 µs) : 0, 661
Remote Config [candidate] (620.469 µs) : 0, 620
Telemetry [baseline] (9.531 ms) : 0, 9531
Telemetry [candidate] (9.349 ms) : 0, 9349
Flare Poller [baseline] (3.702 ms) : 0, 3702
Flare Poller [candidate] (3.651 ms) : 0, 3651
IAST [baseline] (25.955 ms) : 0, 25955
IAST [candidate] (25.542 ms) : 0, 25542
section iast
crashtracking [baseline] (1.21 ms) : 0, 1210
crashtracking [candidate] (1.199 ms) : 0, 1199
BytebuddyAgent [baseline] (809.088 ms) : 0, 809088
BytebuddyAgent [candidate] (796.604 ms) : 0, 796604
AgentMeter [baseline] (11.705 ms) : 0, 11705
AgentMeter [candidate] (11.381 ms) : 0, 11381
GlobalTracer [baseline] (250.823 ms) : 0, 250823
GlobalTracer [candidate] (248.578 ms) : 0, 248578
AppSec [baseline] (33.563 ms) : 0, 33563
AppSec [candidate] (31.431 ms) : 0, 31431
Debugger [baseline] (66.839 ms) : 0, 66839
Debugger [candidate] (68.994 ms) : 0, 68994
Remote Config [baseline] (542.965 µs) : 0, 543
Remote Config [candidate] (529.462 µs) : 0, 529
Telemetry [baseline] (8.754 ms) : 0, 8754
Telemetry [candidate] (8.681 ms) : 0, 8681
Flare Poller [baseline] (3.505 ms) : 0, 3505
Flare Poller [candidate] (3.426 ms) : 0, 3426
IAST [baseline] (27.514 ms) : 0, 27514
IAST [candidate] (27.005 ms) : 0, 27005
section profiling
crashtracking [baseline] (1.201 ms) : 0, 1201
crashtracking [candidate] (1.196 ms) : 0, 1196
BytebuddyAgent [baseline] (682.621 ms) : 0, 682621
BytebuddyAgent [candidate] (684.76 ms) : 0, 684760
AgentMeter [baseline] (8.533 ms) : 0, 8533
AgentMeter [candidate] (8.58 ms) : 0, 8580
GlobalTracer [baseline] (215.889 ms) : 0, 215889
GlobalTracer [candidate] (216.439 ms) : 0, 216439
AppSec [baseline] (32.857 ms) : 0, 32857
AppSec [candidate] (32.617 ms) : 0, 32617
Debugger [baseline] (67.325 ms) : 0, 67325
Debugger [candidate] (67.706 ms) : 0, 67706
Remote Config [baseline] (635.98 µs) : 0, 636
Remote Config [candidate] (636.477 µs) : 0, 636
Telemetry [baseline] (9.045 ms) : 0, 9045
Telemetry [candidate] (9.048 ms) : 0, 9048
Flare Poller [baseline] (3.8 ms) : 0, 3800
Flare Poller [candidate] (3.74 ms) : 0, 3740
ProfilingAgent [baseline] (101.035 ms) : 0, 101035
ProfilingAgent [candidate] (99.414 ms) : 0, 99414
Profiling [baseline] (101.637 ms) : 0, 101637
Profiling [candidate] (99.982 ms) : 0, 99982
LoadParameters
See matching parameters
SummaryFound 7 performance improvements and 2 performance regressions! Performance is the same for 11 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section baseline
no_agent (17.119 ms) : 16948, 17290
. : milestone, 17119,
appsec (19.814 ms) : 19606, 20021
. : milestone, 19814,
code_origins (18.622 ms) : 18431, 18812
. : milestone, 18622,
iast (18.753 ms) : 18561, 18945
. : milestone, 18753,
profiling (19.607 ms) : 19403, 19810
. : milestone, 19607,
tracing (17.716 ms) : 17539, 17893
. : milestone, 17716,
section candidate
no_agent (17.265 ms) : 17091, 17440
. : milestone, 17265,
appsec (18.552 ms) : 18363, 18741
. : milestone, 18552,
code_origins (18.011 ms) : 17832, 18191
. : milestone, 18011,
iast (17.776 ms) : 17598, 17953
. : milestone, 17776,
profiling (18.87 ms) : 18679, 19062
. : milestone, 18870,
tracing (18.796 ms) : 18602, 18990
. : milestone, 18796,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section baseline
no_agent (1.183 ms) : 1172, 1194
. : milestone, 1183,
iast (3.147 ms) : 3107, 3186
. : milestone, 3147,
iast_FULL (5.852 ms) : 5793, 5910
. : milestone, 5852,
iast_GLOBAL (3.679 ms) : 3611, 3748
. : milestone, 3679,
profiling (2.125 ms) : 2104, 2147
. : milestone, 2125,
tracing (1.836 ms) : 1819, 1853
. : milestone, 1836,
section candidate
no_agent (1.192 ms) : 1180, 1204
. : milestone, 1192,
iast (3.151 ms) : 3113, 3189
. : milestone, 3151,
iast_FULL (5.953 ms) : 5893, 6013
. : milestone, 5953,
iast_GLOBAL (3.413 ms) : 3363, 3463
. : milestone, 3413,
profiling (2.092 ms) : 2072, 2112
. : milestone, 2092,
tracing (1.825 ms) : 1810, 1840
. : milestone, 1825,
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.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section baseline
no_agent (1.484 ms) : 1473, 1496
. : milestone, 1484,
appsec (3.781 ms) : 3559, 4004
. : milestone, 3781,
iast (2.255 ms) : 2187, 2324
. : milestone, 2255,
iast_GLOBAL (2.3 ms) : 2231, 2369
. : milestone, 2300,
profiling (2.096 ms) : 2040, 2152
. : milestone, 2096,
tracing (2.066 ms) : 2013, 2120
. : milestone, 2066,
section candidate
no_agent (1.478 ms) : 1467, 1490
. : milestone, 1478,
appsec (2.509 ms) : 2454, 2564
. : milestone, 2509,
iast (2.264 ms) : 2194, 2333
. : milestone, 2264,
iast_GLOBAL (2.304 ms) : 2235, 2373
. : milestone, 2304,
profiling (2.498 ms) : 2336, 2661
. : milestone, 2498,
tracing (2.056 ms) : 2003, 2109
. : milestone, 2056,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.60.0-SNAPSHOT~41631ae4a7, baseline=1.60.0-SNAPSHOT~2fa3c0c243
dateFormat X
axisFormat %s
section baseline
no_agent (14.977 s) : 14977000, 14977000
. : milestone, 14977000,
appsec (15.063 s) : 15063000, 15063000
. : milestone, 15063000,
iast (18.569 s) : 18569000, 18569000
. : milestone, 18569000,
iast_GLOBAL (17.87 s) : 17870000, 17870000
. : milestone, 17870000,
profiling (14.729 s) : 14729000, 14729000
. : milestone, 14729000,
tracing (14.497 s) : 14497000, 14497000
. : milestone, 14497000,
section candidate
no_agent (15.667 s) : 15667000, 15667000
. : milestone, 15667000,
appsec (15.12 s) : 15120000, 15120000
. : milestone, 15120000,
iast (18.692 s) : 18692000, 18692000
. : milestone, 18692000,
iast_GLOBAL (17.779 s) : 17779000, 17779000
. : milestone, 17779000,
profiling (14.704 s) : 14704000, 14704000
. : milestone, 14704000,
tracing (14.749 s) : 14749000, 14749000
. : milestone, 14749000,
|
amarziali
force-pushed
the
andrea.marziali/fix-test
branch
from
a1bbe9b to
2a998e0
Compare
Contributor
Author
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
PerfectSlayer
approved these changes
Feb 19, 2026
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Thanks for the fix
mcculls
approved these changes
Feb 19, 2026
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
andrea.marziali/fix-test
branch
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.
What Does This Do
The test cannot work with a no-op implementation since the msgpack writer needs to serialize a non null binary.
In CI this test passes because
Histograms.registeris called before by another test providing a real implementation.However, locally, taken alone, it does fail.
This PR ensures that the right factory is used in the scope of this test.
Motivation
Additional Notes
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: [PROJ-IDENT]
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.