Open
Conversation
sarahchen6
commented
Mar 13, 2026
sarahchen6
commented
Mar 13, 2026
sarahchen6
added
comp: testing
AlexeyKuznetsov-DD
approved these changes
Mar 13, 2026
Contributor
AlexeyKuznetsov-DD
left a comment
AlexeyKuznetsov-DD
left a comment
There was a problem hiding this comment.
LGTM, left minor style comments.
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 64 metrics, 7 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.073 s) : 0, 1072777
Total [baseline] (11.064 s) : 0, 11063726
Agent [candidate] (1.059 s) : 0, 1059273
Total [candidate] (11.043 s) : 0, 11042502
section appsec
Agent [baseline] (1.248 s) : 0, 1247813
Total [baseline] (11.07 s) : 0, 11069646
Agent [candidate] (1.241 s) : 0, 1240738
Total [candidate] (11.126 s) : 0, 11125685
section iast
Agent [baseline] (1.241 s) : 0, 1240841
Total [baseline] (11.291 s) : 0, 11290501
Agent [candidate] (1.232 s) : 0, 1232073
Total [candidate] (11.333 s) : 0, 11332719
section profiling
Agent [baseline] (1.184 s) : 0, 1183859
Total [baseline] (11.138 s) : 0, 11137546
Agent [candidate] (1.181 s) : 0, 1180731
Total [candidate] (11.03 s) : 0, 11029647
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.202 ms) : 0, 1202
crashtracking [candidate] (1.216 ms) : 0, 1216
BytebuddyAgent [baseline] (635.923 ms) : 0, 635923
BytebuddyAgent [candidate] (629.6 ms) : 0, 629600
AgentMeter [baseline] (29.483 ms) : 0, 29483
AgentMeter [candidate] (29.031 ms) : 0, 29031
GlobalTracer [baseline] (259.194 ms) : 0, 259194
GlobalTracer [candidate] (256.746 ms) : 0, 256746
AppSec [baseline] (32.159 ms) : 0, 32159
AppSec [candidate] (31.633 ms) : 0, 31633
Debugger [baseline] (61.003 ms) : 0, 61003
Debugger [candidate] (60.006 ms) : 0, 60006
Remote Config [baseline] (602.736 µs) : 0, 603
Remote Config [candidate] (586.855 µs) : 0, 587
Telemetry [baseline] (8.236 ms) : 0, 8236
Telemetry [candidate] (8.022 ms) : 0, 8022
Flare Poller [baseline] (8.78 ms) : 0, 8780
Flare Poller [candidate] (6.423 ms) : 0, 6423
section appsec
crashtracking [baseline] (1.188 ms) : 0, 1188
crashtracking [candidate] (1.181 ms) : 0, 1181
BytebuddyAgent [baseline] (657.33 ms) : 0, 657330
BytebuddyAgent [candidate] (654.684 ms) : 0, 654684
AgentMeter [baseline] (12.005 ms) : 0, 12005
AgentMeter [candidate] (12.0 ms) : 0, 12000
GlobalTracer [baseline] (258.904 ms) : 0, 258904
GlobalTracer [candidate] (257.385 ms) : 0, 257385
AppSec [baseline] (178.51 ms) : 0, 178510
AppSec [candidate] (176.898 ms) : 0, 176898
Debugger [baseline] (66.593 ms) : 0, 66593
Debugger [candidate] (65.862 ms) : 0, 65862
Remote Config [baseline] (626.851 µs) : 0, 627
Remote Config [candidate] (619.465 µs) : 0, 619
Telemetry [baseline] (8.335 ms) : 0, 8335
Telemetry [candidate] (8.249 ms) : 0, 8249
Flare Poller [baseline] (3.631 ms) : 0, 3631
Flare Poller [candidate] (3.596 ms) : 0, 3596
IAST [baseline] (24.4 ms) : 0, 24400
IAST [candidate] (24.078 ms) : 0, 24078
section iast
crashtracking [baseline] (1.217 ms) : 0, 1217
crashtracking [candidate] (1.2 ms) : 0, 1200
BytebuddyAgent [baseline] (805.401 ms) : 0, 805401
BytebuddyAgent [candidate] (799.185 ms) : 0, 799185
AgentMeter [baseline] (11.793 ms) : 0, 11793
AgentMeter [candidate] (11.563 ms) : 0, 11563
GlobalTracer [baseline] (248.901 ms) : 0, 248901
GlobalTracer [candidate] (247.983 ms) : 0, 247983
AppSec [baseline] (26.822 ms) : 0, 26822
AppSec [candidate] (26.638 ms) : 0, 26638
Debugger [baseline] (71.475 ms) : 0, 71475
Debugger [candidate] (70.952 ms) : 0, 70952
Remote Config [baseline] (537.023 µs) : 0, 537
Remote Config [candidate] (527.985 µs) : 0, 528
Telemetry [baseline] (9.431 ms) : 0, 9431
Telemetry [candidate] (9.164 ms) : 0, 9164
Flare Poller [baseline] (3.38 ms) : 0, 3380
Flare Poller [candidate] (3.35 ms) : 0, 3350
IAST [baseline] (25.635 ms) : 0, 25635
IAST [candidate] (25.428 ms) : 0, 25428
section profiling
crashtracking [baseline] (1.177 ms) : 0, 1177
crashtracking [candidate] (1.164 ms) : 0, 1164
BytebuddyAgent [baseline] (682.726 ms) : 0, 682726
BytebuddyAgent [candidate] (681.831 ms) : 0, 681831
AgentMeter [baseline] (8.663 ms) : 0, 8663
AgentMeter [candidate] (8.631 ms) : 0, 8631
GlobalTracer [baseline] (215.406 ms) : 0, 215406
GlobalTracer [candidate] (215.336 ms) : 0, 215336
AppSec [baseline] (32.397 ms) : 0, 32397
AppSec [candidate] (32.356 ms) : 0, 32356
Debugger [baseline] (64.515 ms) : 0, 64515
Debugger [candidate] (64.961 ms) : 0, 64961
Remote Config [baseline] (579.091 µs) : 0, 579
Remote Config [candidate] (574.426 µs) : 0, 574
Telemetry [baseline] (8.481 ms) : 0, 8481
Telemetry [candidate] (8.488 ms) : 0, 8488
Flare Poller [baseline] (4.302 ms) : 0, 4302
Flare Poller [candidate] (3.462 ms) : 0, 3462
ProfilingAgent [baseline] (94.662 ms) : 0, 94662
ProfilingAgent [candidate] (93.272 ms) : 0, 93272
Profiling [baseline] (95.24 ms) : 0, 95240
Profiling [candidate] (93.829 ms) : 0, 93829
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1056619
Total [baseline] (8.85 s) : 0, 8850080
Agent [candidate] (1.056 s) : 0, 1056046
Total [candidate] (8.815 s) : 0, 8815416
section iast
Agent [baseline] (1.23 s) : 0, 1230460
Total [baseline] (9.58 s) : 0, 9580320
Agent [candidate] (1.224 s) : 0, 1224498
Total [candidate] (9.564 s) : 0, 9564412
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.199 ms) : 0, 1199
crashtracking [candidate] (1.203 ms) : 0, 1203
BytebuddyAgent [baseline] (627.709 ms) : 0, 627709
BytebuddyAgent [candidate] (627.163 ms) : 0, 627163
AgentMeter [baseline] (28.968 ms) : 0, 28968
AgentMeter [candidate] (28.969 ms) : 0, 28969
GlobalTracer [baseline] (256.401 ms) : 0, 256401
GlobalTracer [candidate] (256.069 ms) : 0, 256069
AppSec [baseline] (31.719 ms) : 0, 31719
AppSec [candidate] (31.579 ms) : 0, 31579
Debugger [baseline] (59.421 ms) : 0, 59421
Debugger [candidate] (59.249 ms) : 0, 59249
Remote Config [baseline] (607.455 µs) : 0, 607
Remote Config [candidate] (578.39 µs) : 0, 578
Telemetry [baseline] (8.118 ms) : 0, 8118
Telemetry [candidate] (8.06 ms) : 0, 8060
Flare Poller [baseline] (6.531 ms) : 0, 6531
Flare Poller [candidate] (7.286 ms) : 0, 7286
section iast
crashtracking [baseline] (1.199 ms) : 0, 1199
crashtracking [candidate] (1.192 ms) : 0, 1192
BytebuddyAgent [baseline] (799.069 ms) : 0, 799069
BytebuddyAgent [candidate] (793.936 ms) : 0, 793936
AgentMeter [baseline] (11.514 ms) : 0, 11514
AgentMeter [candidate] (11.343 ms) : 0, 11343
GlobalTracer [baseline] (247.596 ms) : 0, 247596
GlobalTracer [candidate] (247.433 ms) : 0, 247433
AppSec [baseline] (26.657 ms) : 0, 26657
AppSec [candidate] (26.466 ms) : 0, 26466
Debugger [baseline] (68.585 ms) : 0, 68585
Debugger [candidate] (68.631 ms) : 0, 68631
Remote Config [baseline] (530.338 µs) : 0, 530
Remote Config [candidate] (525.251 µs) : 0, 525
Telemetry [baseline] (10.169 ms) : 0, 10169
Telemetry [candidate] (10.1 ms) : 0, 10100
Flare Poller [baseline] (3.604 ms) : 0, 3604
Flare Poller [candidate] (3.619 ms) : 0, 3619
IAST [baseline] (25.434 ms) : 0, 25434
IAST [candidate] (25.254 ms) : 0, 25254
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 17 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section baseline
no_agent (17.182 ms) : 17010, 17353
. : milestone, 17182,
appsec (18.501 ms) : 18313, 18689
. : milestone, 18501,
code_origins (18.047 ms) : 17865, 18229
. : milestone, 18047,
iast (18.58 ms) : 18393, 18767
. : milestone, 18580,
profiling (19.033 ms) : 18838, 19229
. : milestone, 19033,
tracing (17.706 ms) : 17528, 17883
. : milestone, 17706,
section candidate
no_agent (18.108 ms) : 17924, 18292
. : milestone, 18108,
appsec (18.387 ms) : 18204, 18570
. : milestone, 18387,
code_origins (17.65 ms) : 17474, 17825
. : milestone, 17650,
iast (17.809 ms) : 17630, 17988
. : milestone, 17809,
profiling (18.601 ms) : 18413, 18788
. : milestone, 18601,
tracing (17.752 ms) : 17576, 17928
. : milestone, 17752,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section baseline
no_agent (1.179 ms) : 1167, 1191
. : milestone, 1179,
iast (3.171 ms) : 3130, 3213
. : milestone, 3171,
iast_FULL (5.859 ms) : 5799, 5918
. : milestone, 5859,
iast_GLOBAL (3.456 ms) : 3402, 3510
. : milestone, 3456,
profiling (2.155 ms) : 2133, 2176
. : milestone, 2155,
tracing (1.778 ms) : 1764, 1793
. : milestone, 1778,
section candidate
no_agent (1.184 ms) : 1173, 1196
. : milestone, 1184,
iast (3.147 ms) : 3105, 3189
. : milestone, 3147,
iast_FULL (5.621 ms) : 5566, 5676
. : milestone, 5621,
iast_GLOBAL (3.544 ms) : 3482, 3605
. : milestone, 3544,
profiling (2.006 ms) : 1988, 2023
. : milestone, 2006,
tracing (1.838 ms) : 1822, 1853
. : milestone, 1838,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section baseline
no_agent (15.694 s) : 15694000, 15694000
. : milestone, 15694000,
appsec (14.737 s) : 14737000, 14737000
. : milestone, 14737000,
iast (17.984 s) : 17984000, 17984000
. : milestone, 17984000,
iast_GLOBAL (17.814 s) : 17814000, 17814000
. : milestone, 17814000,
profiling (15.637 s) : 15637000, 15637000
. : milestone, 15637000,
tracing (14.947 s) : 14947000, 14947000
. : milestone, 14947000,
section candidate
no_agent (15.205 s) : 15205000, 15205000
. : milestone, 15205000,
appsec (14.684 s) : 14684000, 14684000
. : milestone, 14684000,
iast (18.349 s) : 18349000, 18349000
. : milestone, 18349000,
iast_GLOBAL (17.965 s) : 17965000, 17965000
. : milestone, 17965000,
profiling (15.001 s) : 15001000, 15001000
. : milestone, 15001000,
tracing (14.96 s) : 14960000, 14960000
. : milestone, 14960000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~d4e8ad83a6, baseline=1.61.0-SNAPSHOT~79fbbef465
dateFormat X
axisFormat %s
section baseline
no_agent (1.471 ms) : 1459, 1483
. : milestone, 1471,
appsec (3.791 ms) : 3570, 4013
. : milestone, 3791,
iast (2.256 ms) : 2186, 2325
. : milestone, 2256,
iast_GLOBAL (2.291 ms) : 2221, 2360
. : milestone, 2291,
profiling (2.076 ms) : 2021, 2130
. : milestone, 2076,
tracing (2.063 ms) : 2009, 2116
. : milestone, 2063,
section candidate
no_agent (1.473 ms) : 1462, 1485
. : milestone, 1473,
appsec (2.572 ms) : 2514, 2630
. : milestone, 2572,
iast (2.259 ms) : 2189, 2328
. : milestone, 2259,
iast_GLOBAL (2.295 ms) : 2225, 2364
. : milestone, 2295,
profiling (2.147 ms) : 2088, 2206
. : milestone, 2147,
tracing (2.065 ms) : 2011, 2118
. : milestone, 2065,
|
Contributor
|
I noticed that some tables here and there missing couple of spaces, just review one more time and align as needed. |
Contributor
There was a problem hiding this comment.
👏 praise: Thanks for pushing the migration
I left a bunch of comments but here are the most important:
- I don't think we should inline constant. We're loosing both semantic and some test behavior (not testing a value but a special case for DDSpanId.MAX). We can either:
- Keep the
@MethodSourceto have complex value creation, - Create a
@TypeConverterto suportDouble.MIX/MAX_VALUEandDDSpanID.MAXthat sounds a better solution for this specific PR. Something like:
@TypeConverter
public static double toDouble(String s) {
if ("Double.MAX_VALUE".equals(s)) {
return Double.MAX_VALUE;
} else if () { // for the 2 other constants
}
return Double.parseDouble(s);
}And they could be re-usable. WDYT?
@ValueSourceseems a better fit when there is only one dimension (a list of value, in particular String ones) than@TableTest
| "large-size-32 | 6536977903480360123 | 3270264562721133536 | 32 ", | ||
| "large-size-40 | 6536977903480360123 | 3270264562721133536 | 40 " | ||
| }) | ||
| @ParameterizedTest(name = "test hexadecimal String representations high={0} low={1} size={2}") |
Contributor
There was a problem hiding this comment.
❔ question: Just checking (I haven't run them) but is {0} will the 1st parameter of the function, not the table 1st column? (ie highOrderBits, not scenario)
Contributor
Author
There was a problem hiding this comment.
Correct! {0} is the first method argument, so it's highOrderBits.
Comment on lines
+53
to
+56
| "max | 'ffffffffffffffff' | -1 ", | ||
| "long max | '7fffffffffffffff' | 9223372036854775807 ", | ||
| "long min | '8000000000000000' | -9223372036854775808", | ||
| "long min with leading zeros | '00008000000000000000' | -9223372036854775808", |
Contributor
There was a problem hiding this comment.
🔨 issue: Similarly, we're loosing DDSpan.MAX, Long.MAX_VALUE, Long.MIN_VALUE semantic in the following tests too
Contributor
There was a problem hiding this comment.
@PerfectSlayer @sarahchen6 what if we try to implement @TypeConverter that will be smart enough to do simple dynamic code execution? I opened issue, but library owner is not accepted my approach.
But since se are testing various approaches our selves, how about if we create some generic @TypeConverter in common module and will import it where needed. And that type converter would do all the stuff I described in that issue? basically we can write DDSpan.MAX as direct input in table test and it will be processed correctly by our dynamic type converter.
WDYT?
Contributor
There was a problem hiding this comment.
That could be something we can try yes. I would be curious how you would solve all the scope issue (I guess you will be no more able to access package-level information exposed for testing, and making them public is great either).
In the interim, we can start by @TypeConverter as recommended by the library author and keep collecting use case. That should unblock the migration and make us gain more experience about which solution would be best 👍
Contributor
Author
There was a problem hiding this comment.
Following up finally, but sounds good - I'll start with @TypeConverter!
Contributor
Author
| @MethodSource("convert64BitIdsFromToLongAndCheckStringsArguments") | ||
| @TableTest({ | ||
| "scenario | longId | expectedString | expectedHex ", | ||
| "zero | 0 | '0' | '00000000000000000000000000000000'", |
Contributor
There was a problem hiding this comment.
💭 thought: Dumb question, do you know if it supports all Java notation like 00000000_00000000_00000000_00000000? That might be easier to read 👀
Contributor
Author
There was a problem hiding this comment.
I just tried 00000000_00000000_00000000_00000000 out and looks like strings are treated literally, so the _s do not work
dd-trace-api/src/test/java/datadog/trace/api/IdGenerationStrategyTest.java
Outdated
Show resolved
Hide resolved
Comment on lines
+16
to
+26
| switch (token) { | ||
| case "Long.MAX_VALUE": | ||
| return Long.MAX_VALUE; | ||
| case "Long.MIN_VALUE": | ||
| return Long.MIN_VALUE; | ||
| case "DDSpanId.MAX": | ||
| return -1L; | ||
| case "DDSpanId.ZERO": | ||
| return 0L; | ||
| default: | ||
| return Long.decode(token); |
Contributor
There was a problem hiding this comment.
Just curious if we use real DDSpanId.MAX and DDSpanId.ZERO or it will be some sort of circular dependency? Probably my idea with reflection and some sort of dynamic code execution may be applied... Just thinking out loud, let's continue brainstorming here
Contributor
Author
There was a problem hiding this comment.
Yeah I'm not sure we can use the real DDSpanId.MAX and DDSpanId.ZERO because dd-trace-api depends on utils/test-utils for tests, so we can't have utils/test-utils reference datadog.trace.api.DDSpanId. Actually maybe I should move DDSpanID constants to be module-specific...
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
Reconfigure
dd-trace-apiJava tests to use TableTest dependencyMotivation
TableTest improves test readability significantly
Additional Notes
The migration was done using Codex and the
migrate-junit-source-to-tabletestskill. I reviewed the PR after. We can see the tests run here in the TestOpt dashboard.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.