Migrate dd-trace-core groovy files to java part 8 by jpbempel · Pull Request #11437 · DataDog/dd-trace-java

jpbempel · 2026-05-21T14:15:48Z

What Does This Do

we migrate 8 tests:

CheckpointerTest
DataStreamsTransactionExtractorsTest
DataStreamsWritingTest
DefaultDataStreamsMonitoringTest
DefaultPathwayContextTest
SchemaBuilderTest
SchemaSamplerTest
TransactionContainerTest

Motivation

this is part of the effort to migrate groovy tests to Java/JUnit
part1: #11053
part2: #11062
part3: #11085
part4: #11146
part5: #11217
part6: #11362
part7: #11374

Additional Notes

I have create a Java version of TestHttpServer as this is used by one of the test.
We keep the groovy and the Java version until the migration is completed

Contributor Checklist

Format the title according to the contribution guidelines
Assign the type: and (comp: or inst:) labels in addition to any other useful labels
Avoid using close, fix, or any linking keywords when referencing an issue
Use solves instead, and assign the PR milestone to the issue
Update the CODEOWNERS file on source file addition, migration, or deletion
Update public documentation with any new configuration flags or behaviors

Jira ticket: [PROJ-IDENT]

Note: Once your PR is ready to merge, add it to the merge queue by commenting /merge. /merge -c cancels 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.

we migrate 8 tests: - CheckpointerTest - DataStreamsTransactionExtractorsTest - DataStreamsWritingTest - DefaultDataStreamsMonitoringTest - DefaultPathwayContextTest - SchemaBuilderTest - SchemaSamplerTest - TransactionContainerTest

dd-octo-sts · 2026-05-21T14:16:05Z

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

AlexeyKuznetsov-DD

LGTM, left several nit comments.

AlexeyKuznetsov-DD · 2026-05-21T14:49:51Z

+
+  @FunctionalInterface
+  public interface RequestHandler {
+    void handle(HandlerApi api) throws Exception;
+  }
+
+  public static JavaTestHttpServer httpServer(Consumer<JavaTestHttpServer> spec) {
+    JavaTestHttpServer server = new JavaTestHttpServer();
+    spec.accept(server);
+    server.start();
+    return server;
+  }
+
+  private final Server internalServer;
+  private HandlersSpec handlers;
+  private Consumer<Server> customizer = s -> {};
+
+  public String keystorePath;
+  private URI address;
+  private URI secureAddress;
+  private final AtomicReference<HandlerApi.RequestApi> last = new AtomicReference<>();
+
+  public final SSLContext sslContext;
+
+  private final X509TrustManager trustManager =


nit: a little bit unusual to see methods declared before variables... I think small re-ordering would not harm.

AlexeyKuznetsov-DD · 2026-05-21T14:54:18Z

+    for (Map.Entry<String, Object> entry : carrier.entries()) {
+      if ("dd-pathway-ctx-base64".equals(entry.getKey())) {
+        hasPathwayCtxBase64 = true;
+        break;
+      }
+    }


nit: Just curious if this can be refactored to java streams API?
something like:

boolean hasPathwayCtxBase64 = carrier.entries().stream() .anyMatch(entry -> "dd-pathway-ctx-base64".equals(entry.getKey()));

AlexeyKuznetsov-DD · 2026-05-21T14:55:47Z

+        DataStreamsTransactionExtractor.Type.HTTP_OUT_HEADERS, extractors.get(0).getType());
+    assertEquals("transaction_id", extractors.get(0).getValue());
+    assertEquals("second_extractor", extractors.get(1).getName());
+    assertEquals(DataStreamsTransactionExtractor.Type.HTTP_IN_HEADERS, extractors.get(1).getType());


nit: DataStreamsTransactionExtractor.Type.XXX can be static imports for readability.

AlexeyKuznetsov-DD · 2026-05-21T14:58:15Z

+      dataStreams.add(
+          new StatsPoint(
+              DataStreamsTags.create(null, null),
+              9,
+              0,
+              10,
+              timeSource.getCurrentTimeNanos(),
+              0,
+              0,
+              0,
+              null));
+      dataStreams.add(
+          new StatsPoint(
+              DataStreamsTags.create(
+                  "testType", DataStreamsTags.Direction.INBOUND, "testTopic", "testGroup", null),
+              1,
+              2,
+              5,


nit: probably with helpers functions can be more human-readable?
here and similar places.

AlexeyKuznetsov-DD · 2026-05-21T15:00:09Z

+    pointConsumer = new CapturingPointConsumer();
+  }
+
+  private static void verifyFirstPoint(StatsPoint point) {


nit: would assertFirstPoint be a better name ?

AlexeyKuznetsov-DD · 2026-05-21T15:02:10Z

+    boolean canSample1 = sampler.canSample(currentTimeMillis);
+    int weight1 = sampler.trySample(currentTimeMillis);
+    boolean canSample2 = sampler.canSample(currentTimeMillis + 1000);
+    boolean canSample3 = sampler.canSample(currentTimeMillis + 2000);
+    boolean canSample4 = sampler.canSample(currentTimeMillis + 30000);
+    int weight4 = sampler.trySample(currentTimeMillis + 30000);
+    boolean canSample5 = sampler.canSample(currentTimeMillis + 30001);
+
+    assertTrue(canSample1);
+    assertEquals(1, weight1);
+    assertFalse(canSample2);
+    assertFalse(canSample3);
+    assertTrue(canSample4);
+    assertEquals(3, weight4);
+    assertFalse(canSample5);


nit: why not just inline into asserts?

AlexeyKuznetsov-DD · 2026-05-21T15:04:11Z

+    StatsGroup group = bucket.getGroups().iterator().next();
+    assertEquals("type:testType", group.getTags().getType());
+    assertEquals("group:testGroup", group.getTags().getGroup());
+    assertEquals("topic:testTopic", group.getTags().getTopic());
+    assertEquals(3, group.getTags().nonNullSize());
+    assertEquals(1L, group.getHash());
+    assertEquals(2L, group.getParentHash());


nit: probably some assertGroup helper can make code more readable here and similar places.

jpbempel · 2026-05-21T15:43:20Z

LGTM, left several nit comments.

This phase is for migrating Groovy to Java, It is not for refactoring the tests. Most of the comments are for refactoring them, but the issue is already there in Groovy.
You can improve in following PRs if you think they require it.

fix forbidden apis

bric3 · 2026-05-21T15:30:27Z

+    boolean hasPathwayCtxBase64 = false;
+    for (Map.Entry<String, Object> entry : carrier.entries()) {
+      if ("dd-pathway-ctx-base64".equals(entry.getKey())) {
+        hasPathwayCtxBase64 = true;
+        break;
+      }
+    }


suggestion: My nitpick suggestion is not spotless compliant.

Suggested change

boolean hasPathwayCtxBase64 = false;

for (Map.Entry<String, Object> entry : carrier.entries()) {

if ("dd-pathway-ctx-base64".equals(entry.getKey())) {

hasPathwayCtxBase64 = true;

break;

}

}

boolean hasPathwayCtxBase64 = carrier.entries().stream()

.anyMatch(entry -> PROPAGATION_KEY_BASE64.equals(entry.getKey()));

already done

bric3 · 2026-05-21T15:40:23Z

+    // cleanup
+    dataStreams.close();


question: This was in the spock cleanup: block. If it makes sense to have it in an @After teardown. So we can differentiate test setup/teardown failures from the actual test.

the thing is it's not in all test of the class.
but maybe those tests needs more love and going to a refactor phase later

bric3 · 2026-05-21T15:42:29Z

+    long deadline = System.currentTimeMillis() + 1000;
+    while (System.currentTimeMillis() < deadline) {


nitpick: Might be better with a nano time.

considering the precision required, currentTimeMillis is fine here

bric3 · 2026-05-21T15:47:22Z

+    for (Entry<StatsBucket.SchemaKey, Long> entry : usages) {
+      StatsBucket.SchemaKey key = entry.getKey();
+      if (key.getSchemaId() == schemaId
+          && operation.equals(key.getOperation())
+          && key.isKey() == isKey) {
+        return entry;
+      }
+    }
+    return null;


nitpick: I was looking at using stream API, but it doesn't gain much lines

return usages.stream() .filter( entry -> { StatsBucket.SchemaKey key = entry.getKey(); return key.getSchemaId() == schemaId && operation.equals(key.getOperation()) && key.isKey() == isKey; }) .findFirst() .orElse(null);

bric3 · 2026-05-21T15:51:16Z

Looks good!

As follow up ideas : for the test server, I'm pondering if this is something that could be made a JUnit extension, so the server lifecyle is properly managed. Then the next question is where should it be remains in the :dd-java-agent:testing module, or in :utils:junit-utils.

sleep in synchronized block

dd-octo-sts · 2026-05-21T20:16:14Z

/merge

gh-worker-devflow-routing-ef8351 · 2026-05-21T20:16:18Z

View all feedbacks in Devflow UI.

2026-05-21 20:16:18 UTC ℹ️ Start processing command /merge

2026-05-21 20:16:23 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 1h (p90).

2026-05-21 21:23:12 UTC ℹ️ MergeQueue: This merge request was merged

Migrate dd-trace-core groovy files to java part 8

7c266f0

we migrate 8 tests: - CheckpointerTest - DataStreamsTransactionExtractorsTest - DataStreamsWritingTest - DefaultDataStreamsMonitoringTest - DefaultPathwayContextTest - SchemaBuilderTest - SchemaSamplerTest - TransactionContainerTest

jpbempel requested review from a team as code owners May 21, 2026 14:15

jpbempel requested a review from mcculls May 21, 2026 14:15

This comment has been minimized.

Sign in to view

AlexeyKuznetsov-DD approved these changes May 21, 2026

View reviewed changes

address comments

e174b6a

fix forbidden apis

bric3 reviewed May 21, 2026

View reviewed changes

jpbempel added comp: testing Testing tag: no release notes Changes to exclude from release notes labels May 21, 2026

fix spotbugs

b3470f5

sleep in synchronized block

jpbempel added this pull request to the merge queue May 21, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 21, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit d004413 into master May 21, 2026
570 checks passed

gh-worker-dd-mergequeue-cf854d Bot deleted the jpbempel/g2j-core-pt8 branch May 21, 2026 21:23

github-actions Bot added this to the 1.63.0 milestone May 21, 2026

		long deadline = System.currentTimeMillis() + 1000;
		while (System.currentTimeMillis() < deadline) {

Conversation

jpbempel commented May 21, 2026

What Does This Do

Motivation

Additional Notes

Contributor Checklist

Uh oh!

dd-octo-sts Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

AlexeyKuznetsov-DD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpbempel commented May 21, 2026

Uh oh!

bric3 May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpbempel May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bric3 commented May 21, 2026

Uh oh!

dd-octo-sts Bot commented May 21, 2026

Uh oh!

gh-worker-devflow-routing-ef8351 Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dd-octo-sts Bot commented May 21, 2026 •

edited

Loading

bric3 May 21, 2026 •

edited

Loading

jpbempel May 21, 2026 •

edited

Loading

gh-worker-devflow-routing-ef8351 Bot commented May 21, 2026 •

edited

Loading