Migrate dd-trace-core groovy files to java part 2#11062
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom Apr 10, 2026
Merged
Migrate dd-trace-core groovy files to java part 2#11062gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomasterfrom
Conversation
jpbempel
force-pushed
the
jpbempel/g2j-DDSpec
branch
2 times, most recently
from
35225f3 to
7e1ec4c
Compare
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Dropping few comments from a partial early review
Comment on lines
+53
to
+57
| static { | ||
| allowContextTesting(); | ||
| installConfigTransformer(); | ||
| makeConfigInstanceModifiable(); | ||
| } |
Contributor
There was a problem hiding this comment.
❔ question: Can this go to a @BeforeAll method?
Comment on lines
+43
to
+44
| private ListWriter writer = new ListWriter(); | ||
| private CoreTracer tracer = tracerBuilder().writer(writer).build(); |
Contributor
There was a problem hiding this comment.
🎯 suggestion: Moving initialization into a @BeforeEach method?
|
|
||
| private static final int SPAN_LINK_TAG_MAX_LENGTH = 25_000; | ||
| private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); | ||
| // W3C Trace Context standard header names (W3CHttpCodec is package-private) |
Contributor
There was a problem hiding this comment.
❔ question: Should we add an test helper to access them rather than duplicating?
Member
Author
There was a problem hiding this comment.
yep I have already a bridge for accessing the method factory
| "sampled | true | '01' | '1' ", | ||
| "not sampled | false | '00' | '-1' " | ||
| }) | ||
| @ParameterizedTest(name = "create span link from extracted context [{index}]") |
Contributor
There was a problem hiding this comment.
🎯 suggestion: Better use scenario than index for readability.
EDIT: That should be the default behavior if we drop the @ParameterizedTest as it does not add value compared to the test method name.
Suggested change
| @ParameterizedTest(name = "create span link from extracted context [{index}]") |
Comment on lines
+95
to
+99
| JSON_MAPPER.readValue( | ||
| spanLinksTag, | ||
| JSON_MAPPER | ||
| .getTypeFactory() | ||
| .constructCollectionType(List.class, TestSpanLinkJson.class)); |
Contributor
There was a problem hiding this comment.
🎯 suggestion: This feels it should go to the TestSpanlinkJson class. It's duplicated among test cases too.
Member
Author
There was a problem hiding this comment.
added for now just a static method to deserialize
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 59 metrics, 12 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.067 s) : 0, 1066717
Total [baseline] (11.144 s) : 0, 11144260
Agent [candidate] (1.057 s) : 0, 1057412
Total [candidate] (11.03 s) : 0, 11030290
section appsec
Agent [baseline] (1.252 s) : 0, 1252028
Total [baseline] (11.23 s) : 0, 11230095
Agent [candidate] (1.258 s) : 0, 1257543
Total [candidate] (11.21 s) : 0, 11210421
section iast
Agent [baseline] (1.224 s) : 0, 1223664
Total [baseline] (11.263 s) : 0, 11263463
Agent [candidate] (1.223 s) : 0, 1223332
Total [candidate] (11.177 s) : 0, 11177247
section profiling
Agent [baseline] (1.194 s) : 0, 1194431
Total [baseline] (11.131 s) : 0, 11131496
Agent [candidate] (1.186 s) : 0, 1186006
Total [candidate] (11.058 s) : 0, 11057836
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.241 ms) : 0, 1241
crashtracking [candidate] (1.222 ms) : 0, 1222
BytebuddyAgent [baseline] (639.267 ms) : 0, 639267
BytebuddyAgent [candidate] (633.538 ms) : 0, 633538
AgentMeter [baseline] (29.712 ms) : 0, 29712
AgentMeter [candidate] (29.323 ms) : 0, 29323
GlobalTracer [baseline] (250.805 ms) : 0, 250805
GlobalTracer [candidate] (249.207 ms) : 0, 249207
AppSec [baseline] (32.256 ms) : 0, 32256
AppSec [candidate] (31.975 ms) : 0, 31975
Debugger [baseline] (60.144 ms) : 0, 60144
Debugger [candidate] (59.828 ms) : 0, 59828
Remote Config [baseline] (595.824 µs) : 0, 596
Remote Config [candidate] (638.902 µs) : 0, 639
Telemetry [baseline] (8.08 ms) : 0, 8080
Telemetry [candidate] (8.125 ms) : 0, 8125
Flare Poller [baseline] (8.278 ms) : 0, 8278
Flare Poller [candidate] (7.376 ms) : 0, 7376
section appsec
crashtracking [baseline] (1.237 ms) : 0, 1237
crashtracking [candidate] (1.222 ms) : 0, 1222
BytebuddyAgent [baseline] (664.582 ms) : 0, 664582
BytebuddyAgent [candidate] (667.67 ms) : 0, 667670
AgentMeter [baseline] (12.053 ms) : 0, 12053
AgentMeter [candidate] (12.103 ms) : 0, 12103
GlobalTracer [baseline] (249.713 ms) : 0, 249713
GlobalTracer [candidate] (250.727 ms) : 0, 250727
AppSec [baseline] (184.49 ms) : 0, 184490
AppSec [candidate] (185.396 ms) : 0, 185396
Debugger [baseline] (66.167 ms) : 0, 66167
Debugger [candidate] (66.357 ms) : 0, 66357
Remote Config [baseline] (599.135 µs) : 0, 599
Remote Config [candidate] (609.945 µs) : 0, 610
Telemetry [baseline] (8.599 ms) : 0, 8599
Telemetry [candidate] (8.757 ms) : 0, 8757
Flare Poller [baseline] (3.579 ms) : 0, 3579
Flare Poller [candidate] (3.611 ms) : 0, 3611
IAST [baseline] (24.587 ms) : 0, 24587
IAST [candidate] (24.722 ms) : 0, 24722
section iast
crashtracking [baseline] (1.227 ms) : 0, 1227
crashtracking [candidate] (1.216 ms) : 0, 1216
BytebuddyAgent [baseline] (801.431 ms) : 0, 801431
BytebuddyAgent [candidate] (800.617 ms) : 0, 800617
AgentMeter [baseline] (11.323 ms) : 0, 11323
AgentMeter [candidate] (11.378 ms) : 0, 11378
GlobalTracer [baseline] (239.489 ms) : 0, 239489
GlobalTracer [candidate] (239.699 ms) : 0, 239699
AppSec [baseline] (32.308 ms) : 0, 32308
AppSec [candidate] (32.431 ms) : 0, 32431
Debugger [baseline] (59.179 ms) : 0, 59179
Debugger [candidate] (61.671 ms) : 0, 61671
Remote Config [baseline] (560.42 µs) : 0, 560
Remote Config [candidate] (1.119 ms) : 0, 1119
Telemetry [baseline] (12.267 ms) : 0, 12267
Telemetry [candidate] (9.983 ms) : 0, 9983
Flare Poller [baseline] (3.442 ms) : 0, 3442
Flare Poller [candidate] (3.447 ms) : 0, 3447
IAST [baseline] (26.41 ms) : 0, 26410
IAST [candidate] (25.698 ms) : 0, 25698
section profiling
crashtracking [baseline] (1.191 ms) : 0, 1191
crashtracking [candidate] (1.19 ms) : 0, 1190
BytebuddyAgent [baseline] (698.121 ms) : 0, 698121
BytebuddyAgent [candidate] (692.728 ms) : 0, 692728
AgentMeter [baseline] (9.154 ms) : 0, 9154
AgentMeter [candidate] (9.145 ms) : 0, 9145
GlobalTracer [baseline] (208.247 ms) : 0, 208247
GlobalTracer [candidate] (206.946 ms) : 0, 206946
AppSec [baseline] (32.726 ms) : 0, 32726
AppSec [candidate] (32.457 ms) : 0, 32457
Debugger [baseline] (66.403 ms) : 0, 66403
Debugger [candidate] (65.51 ms) : 0, 65510
Remote Config [baseline] (576.747 µs) : 0, 577
Remote Config [candidate] (567.502 µs) : 0, 568
Telemetry [baseline] (7.922 ms) : 0, 7922
Telemetry [candidate] (7.833 ms) : 0, 7833
Flare Poller [baseline] (3.59 ms) : 0, 3590
Flare Poller [candidate] (3.617 ms) : 0, 3617
ProfilingAgent [baseline] (94.807 ms) : 0, 94807
ProfilingAgent [candidate] (94.403 ms) : 0, 94403
Profiling [baseline] (95.389 ms) : 0, 95389
Profiling [candidate] (94.98 ms) : 0, 94980
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055992
Total [baseline] (8.843 s) : 0, 8843086
Agent [candidate] (1.065 s) : 0, 1064935
Total [candidate] (8.858 s) : 0, 8857693
section iast
Agent [baseline] (1.228 s) : 0, 1227821
Total [baseline] (9.576 s) : 0, 9576130
Agent [candidate] (1.224 s) : 0, 1224197
Total [candidate] (9.572 s) : 0, 9571700
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.224 ms) : 0, 1224
crashtracking [candidate] (1.246 ms) : 0, 1246
BytebuddyAgent [baseline] (631.734 ms) : 0, 631734
BytebuddyAgent [candidate] (638.438 ms) : 0, 638438
AgentMeter [baseline] (29.421 ms) : 0, 29421
AgentMeter [candidate] (29.614 ms) : 0, 29614
GlobalTracer [baseline] (248.035 ms) : 0, 248035
GlobalTracer [candidate] (250.7 ms) : 0, 250700
AppSec [baseline] (31.919 ms) : 0, 31919
AppSec [candidate] (32.211 ms) : 0, 32211
Debugger [baseline] (59.079 ms) : 0, 59079
Debugger [candidate] (59.558 ms) : 0, 59558
Remote Config [baseline] (602.151 µs) : 0, 602
Remote Config [candidate] (601.927 µs) : 0, 602
Telemetry [baseline] (8.05 ms) : 0, 8050
Telemetry [candidate] (8.207 ms) : 0, 8207
Flare Poller [baseline] (9.814 ms) : 0, 9814
Flare Poller [candidate] (8.049 ms) : 0, 8049
section iast
crashtracking [baseline] (1.226 ms) : 0, 1226
crashtracking [candidate] (1.228 ms) : 0, 1228
BytebuddyAgent [baseline] (805.244 ms) : 0, 805244
BytebuddyAgent [candidate] (802.002 ms) : 0, 802002
AgentMeter [baseline] (11.395 ms) : 0, 11395
AgentMeter [candidate] (11.345 ms) : 0, 11345
GlobalTracer [baseline] (239.308 ms) : 0, 239308
GlobalTracer [candidate] (239.115 ms) : 0, 239115
AppSec [baseline] (32.654 ms) : 0, 32654
AppSec [candidate] (30.317 ms) : 0, 30317
Debugger [baseline] (60.89 ms) : 0, 60890
Debugger [candidate] (62.535 ms) : 0, 62535
Remote Config [baseline] (537.925 µs) : 0, 538
Remote Config [candidate] (1.703 ms) : 0, 1703
Telemetry [baseline] (10.954 ms) : 0, 10954
Telemetry [candidate] (10.55 ms) : 0, 10550
Flare Poller [baseline] (3.495 ms) : 0, 3495
Flare Poller [candidate] (3.452 ms) : 0, 3452
IAST [baseline] (25.723 ms) : 0, 25723
IAST [candidate] (25.819 ms) : 0, 25819
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 petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section baseline
no_agent (19.631 ms) : 19424, 19838
. : milestone, 19631,
appsec (19.567 ms) : 19366, 19768
. : milestone, 19567,
code_origins (17.574 ms) : 17398, 17749
. : milestone, 17574,
iast (17.93 ms) : 17754, 18107
. : milestone, 17930,
profiling (18.007 ms) : 17831, 18183
. : milestone, 18007,
tracing (17.783 ms) : 17608, 17957
. : milestone, 17783,
section candidate
no_agent (18.306 ms) : 18119, 18493
. : milestone, 18306,
appsec (18.778 ms) : 18588, 18969
. : milestone, 18778,
code_origins (17.762 ms) : 17583, 17941
. : milestone, 17762,
iast (17.955 ms) : 17776, 18135
. : milestone, 17955,
profiling (18.385 ms) : 18205, 18565
. : milestone, 18385,
tracing (18.029 ms) : 17852, 18206
. : milestone, 18029,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section baseline
no_agent (1.226 ms) : 1214, 1239
. : milestone, 1226,
iast (3.185 ms) : 3144, 3226
. : milestone, 3185,
iast_FULL (6.003 ms) : 5944, 6063
. : milestone, 6003,
iast_GLOBAL (3.675 ms) : 3617, 3733
. : milestone, 3675,
profiling (2.288 ms) : 2265, 2312
. : milestone, 2288,
tracing (1.956 ms) : 1939, 1973
. : milestone, 1956,
section candidate
no_agent (1.283 ms) : 1270, 1295
. : milestone, 1283,
iast (3.383 ms) : 3337, 3429
. : milestone, 3383,
iast_FULL (6.175 ms) : 6110, 6240
. : milestone, 6175,
iast_GLOBAL (3.725 ms) : 3664, 3787
. : milestone, 3725,
profiling (2.365 ms) : 2339, 2390
. : milestone, 2365,
tracing (1.871 ms) : 1855, 1887
. : milestone, 1871,
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.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section baseline
no_agent (15.7 s) : 15700000, 15700000
. : milestone, 15700000,
appsec (14.899 s) : 14899000, 14899000
. : milestone, 14899000,
iast (18.467 s) : 18467000, 18467000
. : milestone, 18467000,
iast_GLOBAL (18.196 s) : 18196000, 18196000
. : milestone, 18196000,
profiling (14.997 s) : 14997000, 14997000
. : milestone, 14997000,
tracing (15.076 s) : 15076000, 15076000
. : milestone, 15076000,
section candidate
no_agent (15.265 s) : 15265000, 15265000
. : milestone, 15265000,
appsec (14.713 s) : 14713000, 14713000
. : milestone, 14713000,
iast (18.45 s) : 18450000, 18450000
. : milestone, 18450000,
iast_GLOBAL (18.183 s) : 18183000, 18183000
. : milestone, 18183000,
profiling (14.739 s) : 14739000, 14739000
. : milestone, 14739000,
tracing (14.677 s) : 14677000, 14677000
. : milestone, 14677000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~15c43a8621, baseline=1.62.0-SNAPSHOT~b266e2d0c2
dateFormat X
axisFormat %s
section baseline
no_agent (1.487 ms) : 1475, 1499
. : milestone, 1487,
appsec (3.829 ms) : 3605, 4052
. : milestone, 3829,
iast (2.282 ms) : 2212, 2351
. : milestone, 2282,
iast_GLOBAL (2.322 ms) : 2252, 2392
. : milestone, 2322,
profiling (2.104 ms) : 2048, 2159
. : milestone, 2104,
tracing (2.09 ms) : 2035, 2144
. : milestone, 2090,
section candidate
no_agent (1.49 ms) : 1478, 1501
. : milestone, 1490,
appsec (3.832 ms) : 3610, 4054
. : milestone, 3832,
iast (2.281 ms) : 2211, 2351
. : milestone, 2281,
iast_GLOBAL (2.326 ms) : 2256, 2397
. : milestone, 2326,
profiling (2.113 ms) : 2057, 2168
. : milestone, 2113,
tracing (2.083 ms) : 2029, 2137
. : milestone, 2083,
|
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Keep adding comments.
About the span link test case, I can help to rework it once you're ready to review as I have the context.
| "link after start only | false | true ", | ||
| "links before and after | true | true " | ||
| }) | ||
| @ParameterizedTest(name = "add span link at any time [{index}]") |
Contributor
There was a problem hiding this comment.
🎯 suggestion: Same suggestion about index vs scenario
Suggested change
| @ParameterizedTest(name = "add span link at any time [{index}]") |
| @ParameterizedTest(name = "add span link at any time [{index}]") | ||
| void addSpanLinkAtAnyTime(boolean beforeStart, boolean afterStart) throws Exception { | ||
| AgentTracer.SpanBuilder builder = tracer.buildSpan("test", "operation"); | ||
| List<SpanLink> links = new java.util.ArrayList<>(); |
Contributor
There was a problem hiding this comment.
🎯 suggestion: Use import
| .getTypeFactory() | ||
| .constructCollectionType(List.class, TestSpanLinkJson.class)); | ||
|
|
||
| assertEquals((beforeStart ? 1 : 0) + (afterStart ? 1 : 0), decodedSpanLinks.size()); |
Contributor
There was a problem hiding this comment.
🎯 suggestion:
Suggested change
| assertEquals((beforeStart ? 1 : 0) + (afterStart ? 1 : 0), decodedSpanLinks.size()); | |
| int expectedLinkCount = (beforeStart ? 1 : 0) + (afterStart ? 1 : 0); | |
| assertEquals(expectedLinkCount, decodedSpanLinks.size()); |
| Map<String, String> attributes = new HashMap<>(); | ||
| attributes.put("link-index", Integer.toString(index)); | ||
|
|
||
| return new DDSpanLink( |
Contributor
There was a problem hiding this comment.
❔ question: Should we have an implementation of SpanLink for tests that will open the constructor? Or hide it behind a static helper/constructor. But DDSpanLink don't seem right here.
Member
Author
There was a problem hiding this comment.
up to you here. you can do some refactoring afterward.
to me, does feel right. I don't see any issue.
| } | ||
| } | ||
|
|
||
| static class TestSpanLinkJson { |
Contributor
There was a problem hiding this comment.
🎯 suggestion: SpanLinkAsTag would better express the concern and the reason of the JSON encoding.
| protected boolean assertThreadsEachCleanup = true; | ||
| private boolean ignoreThreadCleanup; | ||
|
|
||
| static void allowContextTesting() { |
Contributor
There was a problem hiding this comment.
💭 thought: This will be a duplicate of the already merge feature here: https://github.com/DataDog/dd-trace-java/pull/11009/changes#diff-64b7ed73da5a4f2c4e2f4c285b90359f2f1be89b41dc4053377a0ac1eebe169f
We need to refactor it later.
migrate DDSpecification and DDCoreSpecification that are used by most of the test in this module. we are keeping the groovy version to be able to incrementally migrate tests. a first small test (DDSpanLinkTest )is migrated to prove the viability of the strategy.
jpbempel
force-pushed
the
jpbempel/g2j-DDSpec
branch
from
4e01627 to
9e0bebb
Compare
Contributor
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
jpbempel
added
comp: testing
remove PER_CLASS mode setupSpec -> beforeAll
PerfectSlayer
approved these changes
Apr 10, 2026
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
👏 praise: Great progress here! Thanks for keep tackling the migration challenge 🙏
Contributor
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
gh-worker-dd-mergequeue-cf854d
bot
merged commit 6564199
into
master
745 of 747 checks passed
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
2 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
migrate DDSpecification and DDCoreSpecification that are used by most of the test in this module.
we are keeping the groovy version to be able to incrementally migrate tests.
a first small test (DDSpanLinkTest )is migrated to prove the viability of the strategy.
Motivation
this is part of the effort to migrate groovy tests to Java/JUnit
part1: #11053
Additional Notes
Key design decisions for DDJavaSpecification:
@TestInstance(Lifecycle.PER_CLASS)— allows non-static@BeforeAll/@AfterAllmethods,mirrors Spock's per-class lifecycle where
setupSpec/cleanupSpecrun once per test classinstallConfigTransformer()in the static block — handles the ByteBuddy transformation(equivalent to
ConfigTransformSpockExtension) inline, makingConfig.INSTANCEpublic/static/volatile before
makeConfigInstanceModifiable()runsTestEnvironmentVariablesinner class — Java port ofControllableEnvironmentVariables(whichis Groovy and compiles after Java, so can't be referenced from Java source)
configModificationFailedis static package-private soConfigInstrumentationFailedListener(inner class) can access it
@BeforeAll setupSpec(),@AfterAll cleanupSpec(),@BeforeEach setup(),@AfterEach cleanup()— same semantics as SpockinjectSysConfig,injectEnvConfig,rebuildConfig, etc.) arepresent with Java overload pattern replacing Groovy's default parameters
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.