Skip to content

Fix duplicated Vert.x HTTP server routes in handler chains - #20925

Open
trask with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-duplicated-http-server-routes
Open

trask with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-duplicated-http-server-routes

Conversation

Copilot AI commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Misc found while diagnosing another issue.

Multiple handlers on one Vert.x route could produce http.route=/virtual/virtual and span name GET /virtual/virtual instead of retaining /virtual.

  • Route composition: Distinguish an empty saved parent route from unsaved state so next() restores it correctly. Preserve mounted prefixes, legitimate repeated segments, route precedence, and custom span names.
  • Regression coverage: Exercise top-level and nested handler chains across supported Vert.x versions, including 4.5.3 standard and virtual-thread execution. Assert exported span names and the complete expected HTTP attributes.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-10-24 17:14 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicated Vert.x HTTP server routes for exported spans Fix duplicated Vert.x HTTP server routes in handler chains Oct 23, 2026
Copilot AI requested a review from trask October 23, 2026 23:47
@trask
trask requested a balanced review from Copilot October 23, 2026 23:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The latest-dependency flag is not forwarded to the Vert.x 3 test JVM, silently skipping the new regression cases for that version family.

1 open issue
What changed in this PR

Fixes duplicated HTTP routes in Vert.x handler chains by distinguishing an empty saved parent route from unsaved state.

Changes:

  • Restores route composition without losing mounted prefixes or legitimate repeated segments.
  • Adds regression cases for handler chains, custom span names, attributes, and span identity.
  • Adds Vert.x 4.5.3 standard and virtual-thread coverage and waits for server shutdown.
File Description
instrumentation/​vertx/​vertx-web-3.0/​testing/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​AbstractVertxWebServer.java Adds shared multi-handler route fixtures.
instrumentation/​vertx/​vertx-web-3.0/​testing/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​AbstractVertxHttpServerTest.java Adds route and span regression assertions and deployment hooks.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version5Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​Vertx5WebServer.java Registers top-level regression routes for Vert.x 5.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version45Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​VertxVirtualThreadHttpServerTest.java Runs inherited tests using virtual threads.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version41Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​Vertx41WebServer.java Registers top-level regression routes for Vert.x 4.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version41Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​Vertx41HttpServerTest.java Waits for asynchronous server shutdown.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version3Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​VertxWebServer.java Gates handler-chain fixtures on latest-dependency mode.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​version3Test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​VertxHttpServerTest.java Gates regression cases on Vert.x 3 handler support.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​RoutingContextUtil.java Stores an empty-route sentinel for restoration.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​RoutingContextInstrumentation.java Clarifies the comment describing next() behavior.
instrumentation/​vertx/​vertx-web-3.0/​javaagent/​build.gradle.kts Adds the Java 21-gated Vert.x 4.5.3 suite and client instrumentation dependency.

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 00:08
@trask
trask requested a balanced review from Copilot October 24, 2026 00:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused routing fix and regression coverage have no identified blocking issues; remaining feedback concerns nonblocking assertion strictness.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Use exact attribute assertions including standard HTTP attributes

instrumentation/​vertx/​vertx-web-3.0/​testing/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​AbstractVertxHttpServerTest.java:108

[Testing] Non-blocking: this partial attribute assertion accepts any unexpected attributes, contrary to the exact-assertion convention in .github/instructions/java-tests.instructions.md:145–147. Use hasAttributesSatisfyingExactly(...) and include the normal HTTP attributes alongside the four regression-specific assertions so the expected exported attribute set is checked.

1 resolved since last review

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 03:54
@trask
trask requested a balanced review from Copilot October 24, 2026 03:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The localized route-state correction has targeted cross-version regression coverage, with only non-blocking assertion-strengthening feedback remaining.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Require exact span attribute assertions in telemetry test

instrumentation/​vertx/​vertx-web-3.0/​testing/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​AbstractVertxHttpServerTest.java:108

[Testing] Non-blocking: hasAttributesSatisfying(...) accepts arbitrary additional attributes, leaving unexpected telemetry changes undetected by this otherwise complete span assertion. Use hasAttributesSatisfyingExactly(...) and include the expected HTTP server attributes alongside these regression attributes, following .github/instructions/java-tests.instructions.md:145.

Review effort: Balanced

@trask
trask marked this pull request as ready for review October 24, 2026 04:08
@trask
trask requested a review from a team as a code owner October 24, 2026 04:08
@trask
trask marked this pull request as draft October 24, 2026 04:08
Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 04:21
@trask
trask requested a balanced review from Copilot October 24, 2026 04:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused route-state correction has versioned regression coverage, with no blocking issues identified.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Use exact attribute assertions in new tests

instrumentation/​vertx/​vertx-web-3.0/​testing/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​vertx/​web/​v3_0/​AbstractVertxHttpServerTest.java:108

[Testing] Non-blocking: hasAttributesSatisfying accepts unexpected attributes, so these new assertions do not verify the complete exported attribute set. Prefer hasAttributesSatisfyingExactly with the expected HTTP and handler attributes, following the span-attribute assertion guidance in .github/instructions/java-tests.instructions.md.

Review effort: Balanced

@trask
trask marked this pull request as ready for review October 24, 2026 04:30
Co-authored-by: trask <218610+trask@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The production fix is narrowly scoped, relevant regression coverage is wired, and no unresolved blocking issues were identified.

No open issues 🎉

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 16:52
@trask
trask requested a balanced review from Copilot October 24, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The localized route-state correction has targeted regression coverage and no unresolved blocking findings.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants