[DO NOT MERGE] feat(sparkjava): Add sparkjava-gen-2.3 module (toolkit-generated)#11337
Draft
jordan-wong wants to merge 2 commits into
Draft
[DO NOT MERGE] feat(sparkjava): Add sparkjava-gen-2.3 module (toolkit-generated)#11337jordan-wong wants to merge 2 commits into
jordan-wong wants to merge 2 commits into
Conversation
Contributor
|
❌ New Groovy Files Detected Please avoid introducing new
Instead, rewrite the new file(s) in Java / JUnit. See the How to Test With JUnit Guide for more details. If this PR needs an exception, add the |
jordan-wong
changed the title
…allel to sparkjava-2.3)
This PR adds a new module `dd-java-agent/instrumentation/spark/sparkjava-gen-2.3`
containing an alternative SparkJava 2.3 instrumentation generated by the
APM instrumentation toolkit. Placed alongside existing `sparkjava-2.3` (not
replacing it) so reviewers can compare side by side.
Module name follows dd-trace-java's instrumentation-naming convention:
{framework}-gen-{version} so the name ends with the required version suffix
(initial attempt used sparkjava-2.3-gen which would have failed the
checkInstrumentationNaming Gradle task).
## What the toolkit did
One agent-driven workflow run produced the module end-to-end. Cost: $74.72,
duration ~2.5 hours, 10 reviewer iterations.
Key choices the agent made:
- Instruments `spark.route.Routes.find()` — matches the existing `sparkjava-2.3`
module's `RoutesInstrumentation` on `spark.route.Routes`
- Enriches the existing Jetty server span with `http.route` rather than creating
a new span (existing module follows the same approach; Jetty is always the
embedded server under SparkJava)
- Open-ended muzzle version range `[2.3,)` with `assertInverse = true`
- Spock tests extending `HttpServerTest`
## Verification
```
./gradlew :dd-java-agent:instrumentation:spark:sparkjava-gen-2.3:check \\
:dd-java-agent:instrumentation:spark:sparkjava-gen-2.3:muzzle \\
:dd-java-agent:instrumentation:spark:sparkjava-gen-2.3:latestDepTest
BUILD SUCCESSFUL in 46s
```
Multi-JVM CI matrix not run locally; standard CI will cover that.
## Reviewer notes
The toolkit's reviewer step flagged 7 TODOs across 10 iterations about
spec-compliance (suggesting `MatcherFilter.doFilter` as a target + creating
an own server span). The **existing** `sparkjava-2.3` module on master also
instruments `Routes` (not `MatcherFilter`) and relies on Jetty for span
creation — see `dd-java-agent/instrumentation/spark/sparkjava-2.3/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java`.
The agent's implementation matches the canonical pattern. The reviewer's
suggestions appear to apply generic HTTP-server guidance that doesn't fit
this library's convention.
## Provenance
Generated by apm-instrumentation-toolkit (DataDog/apm-instrumentation-toolkit
branch eval/java). Research artifacts in the toolkit repo:
- docs/eval-research/runs/sparkjava/attempt3-final/
- docs/eval-research/hypotheses/sparkjava.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jordan-wong
force-pushed
the
eval/sparkjava-2.3-gen-toolkit-attempt
branch
from
7f8bfb4 to
11a3039
Compare
jordan-wong
changed the title
Contributor
@jordan-wong we disallow new groovy files. As per our policy, new tests have to be written in java |
amarziali
reviewed
May 13, 2026
Contributor
amarziali
left a comment
amarziali
left a comment
There was a problem hiding this comment.
@jordan-wong I'm interested in knowing the reason why this instrumentation has been added. Originally, this has been added to enrich the route but generally speaking a LLM can take different directions on what should be supported in terms of observability. I'm interested in knowing the actual prompt. Is it on the PR description?
| implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { | ||
|
|
||
| public SparkJavaInstrumentation() { | ||
| super("sparkjava", "sparkjava-2.4"); |
Contributor
There was a problem hiding this comment.
why 2.4 if the module is called 2.3? Interestingly there is the same typo in the current sparkjava instrumentation that let me think again that the implementation is not completely blind
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.
Summary
Adds a new module
dd-java-agent/instrumentation/spark/sparkjava-2.3-gencontaining an alternative SparkJava 2.3 instrumentation generated end-to-end by the APM instrumentation toolkit'snew_integrationworkflow.The new module is placed alongside the existing
sparkjava-2.3module (not replacing it) so reviewers can compare the toolkit-generated implementation against the canonical one side by side. This is a research artifact — the goal is to evaluate how well the toolkit can produce a Java instrumentation that passes CI without human intervention.What's in this PR
dd-java-agent/instrumentation/spark/sparkjava-2.3-gen/build.gradledd-java-agent/instrumentation/spark/sparkjava-2.3-gen/src/main/java/.../SparkJavaInstrumentation.javadd-java-agent/instrumentation/spark/sparkjava-2.3-gen/src/test/groovy/.../SparkJavaServer.groovydd-java-agent/instrumentation/spark/sparkjava-2.3-gen/src/test/groovy/.../SparkJavaTest.groovysettings.gradle.ktsNo changes to any production files or other modules.
How the toolkit produced this
One agent-driven workflow run from a blind baseline (the existing
sparkjava-2.3module was hidden from the agent). The workflow ran for ~2.5 hours, cost ~$74, and went through 10 reviewer iterations before the workflow exited.Key choices the agent made:
spark.route.Routes.find()— same instrumented type as the existingsparkjava-2.3module (which is namedRoutesInstrumentation)http.routerather than creating a new span. Matches existing pattern — Jetty is always the embedded server under SparkJava, so a server span always exists[2.3,)withassertInverse = trueHttpServerTestfor full server-span validationVerification
Local CI-equivalent verification on the new module:
./gradlew :dd-java-agent:instrumentation:spark:sparkjava-2.3-gen:check \ :dd-java-agent:instrumentation:spark:sparkjava-2.3-gen:muzzle \ :dd-java-agent:instrumentation:spark:sparkjava-2.3-gen:latestDepTest BUILD SUCCESSFUL in 46sMulti-JVM matrix (JVM 8/17/21/25) not run locally; the standard CI pipeline will exercise that.
Reviewer notes
The toolkit's internal reviewer step did NOT approve this module — it ran 10 iterations and exited with
todos_remaining=7. The recurring spec-compliance complaint was that the agent should have instrumentedspark.http.matching.MatcherFilter.doFilter(and created its own server span) instead of usingRoutes.find()+ Jetty span enrichment.I cross-referenced this against the existing
sparkjava-2.3module on master:dd-java-agent/instrumentation/spark/sparkjava-2.3/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.javainstrumentedType()returns"spark.route.Routes"startSpan— relies on Jetty's existing instrumentationSo the agent's choice matches the existing canonical pattern. The reviewer's spec-compliance critique appears to be applying generic HTTP-server guidance ("each framework should own its server span") that doesn't fit this library's established convention.
Reviewers familiar with SparkJava should evaluate whether the existing pattern is itself correct. If it is, this PR is functionally equivalent to the existing module and can be considered a successful toolkit reproduction. If the existing module should be refactored to create its own span, this PR has the same issue.
Test plan
:sparkjava-2.3-gen:testand:sparkjava-2.3-gen:forkedTest:sparkjava-2.3-gen:muzzle:sparkjava-2.3-gen:latestDepTestsparkjava-2.3module (the two should coexist; we may want to disable one or pick one before merging)Provenance
Research artifacts (agent transcript, hypothesis file, all skill changes the toolkit applied) are preserved in DataDog/apm-instrumentation-toolkit branch
eval/java:docs/eval-research/runs/sparkjava/attempt3-final/— lean snapshotdocs/eval-research/hypotheses/sparkjava.md— hypothesis file documenting the research findingThis PR is a draft for review only — not intended for merge as-is. The duplicate module would need to be reconciled (likely by replacing the existing
sparkjava-2.3with this one, or by withdrawing) before merging.🤖 Generated with Claude Code