Repository navigation
Add Jetty 12.0 native WebSocket tracing and fix upgrade span completion - #12674
Conversation
ygree
added
inst: jetty
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
a44a0c1 to
3b33f1a
Compare
31a2644 to
5052a0c
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5052a0c550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5052a0c to
aac3649
Compare
ygree
changed the title
bb5bd89 to
71a43bb
Compare
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Correct the EE9 directive name so Muzzle validates its instrumentation. Add inverse assertions for EE8, EE9, and EE10 to verify rejection outside the supported [12,12.1) range.
a6e9a28 to
449cb65
Compare
Cover client receive spans, handshake links, and callback lifecycles. Require an active parent when starting client send spans. Leave 29 tests failing pending implementation.
Link client receive spans to the HTTP upgrade span. Require an active parent when starting client send spans, while preserving the span across subsequent fragments.
Let callbacks own pending messages and retain only the unfinished fragment sequence in connection state. Remove redundant error-handling locks.
Split mixed scenarios and remove redundant endpoint cases. Verify that rejected fragments preserve the unfinished message for close cleanup.
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Closing a session while a text handler is running can truncate its receive span and lose a later handler error. Fragmented client messages that start without tracing can also acquire an unrelated parent on a later fragment.
🤖 Bits Code Review · Commit a66ef77
Defer receive span completion until active text handlers exit, preserving handler duration and errors when the connection closes concurrently.
Create a root send span when no parent is active and reuse it across fragments, preserving the handshake link and complete message counts.
Move text receive state updates into synchronized methods so SpotBugs recognizes the locking, preserving span completion behavior.
def014f to
0b6a583
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #12674...
Detected 1 merge blocker(s) to address: 🔴 Blocking
|
|
/merge |
|
View all feedbacks in Devflow UI.
PR already in the queue with status waiting |
3 participants
What Does This Do
Adds tracing for Jetty 12.0.x's native WebSocket API:
websocket.receivespans for client and server sessions, plus server-sidewebsocket.closespans, covering listener and annotated endpoints, including handlers returning boxedVoid.websocket.sendspans for client and server sessions, using the active span as parent when present and creating root spans otherwise. Subsequent fragments preserve the first fragment's context.ReaderandInputStreamhandlers, preserving handshake links, application context, and existing sampling behavior. Client receives link to the HTTP handshake span rather than its application root.Also fixes Jetty HTTP client handshake spans left unfinished by WebSocket upgrades. Activates the handshake span during upgrade so client sessions capture its context, then finishes it with the HTTP 101 response. Records upgrade failures that Jetty catches and reports through the session future.
Expands WebSocket Muzzle compatibility checks and latest-dependency tests to cover the 12.0.x release line, replacing the 12.0.17 upper bound. Rejects incompatible Jetty 12.1+ WebSocket instrumentation to prevent partial installation and missing close cleanup.
Motivation
Jetty native WebSocket endpoints need dedicated instrumentation to trace incoming messages, outbound sends, and server close handlers. Streaming handlers, fragmentation, and asynchronous callbacks require lifecycle tracking to avoid premature completion, mixed contexts, and lost errors while keeping per-message overhead low.
WebSocket upgrades bypass the HTTP client's normal response completion listeners, including when Jetty swallows an upgrade failure, requiring explicit handshake span completion and error reporting.
Additional Notes
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: FRAPMS-6125