Skip to content

✨ Report the websocket_closing vital - #5094

Open
bdibon wants to merge 4 commits into
boris.dibon/websocket-vitals-2-basic-lifecyclefrom
boris.dibon/websocket-vitals-3-closing
Open

bdibon wants to merge 4 commits into
boris.dibon/websocket-vitals-2-basic-lifecyclefrom
boris.dibon/websocket-vitals-3-closing

Conversation

@bdibon

@bdibon bdibon commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

The browser fires no event when the closing handshake starts, so without instrumenting close() the backend cannot tell a connection the application closed from one the server or the network ended.

Part of the WebSocket vitals stack. Every PR targets the previous one; the first one targets the boris.dibon/websocket-vitals integration branch, which is merged into main in one go once the whole stack is approved. #5055 stays open as the reference implementation: the tip of this stack is content-identical to it, rebased on the current main.

  1. 🔧 Pin rum-events-format and add WebSocket test utilities #5092 🔧 Pin rum-events-format and add WebSocket test utilities
  2. ✨ Collect connecting, open and closed WebSocket vitals #5093 ✨ Collect connecting, open and closed WebSocket vitals
  3. ✨ Report the websocket_closing vital #5094 ✨ Report the websocket_closing vital 👈 this PR
  4. ✨ Pulse open WebSocket vitals on a shared heartbeat #5095 ✨ Pulse open WebSocket vitals on a shared heartbeat
  5. ✨ Report WebSocket silences and send queue depth, and protect WebSocket vitals in beforeSend #5096 ✨ Report WebSocket silences and send queue depth, and protect WebSocket vitals in beforeSend
  6. ✨ End WebSocket tracking when the page is discarded #5097 ✨ End WebSocket tracking when the page is discarded

Changes

Commit by commit:

  • ✨ Notify a closing context when the application calls close(): close() is instrumented; a call on a socket already closing or closed is a no-op and notifies nothing.
  • 🐛 Do not count a WebSocket payload the socket discarded: send() outside OPEN never reaches the wire (it throws before the handshake, and is silently dropped once closing or closed), so it is no longer counted as an outbound message.
  • ✨ Report the websocket_closing vital: carries closing_date and close_initiator: "client", takes no snapshot version, and the closed vital that follows keeps the closing date. A close() during the handshake reports the closing phase without an open date.
  • ✅ Cover the closing phase end to end: closing before open, and close() called twice.

Test instructions

  • Unit: yarn test:unit --spec "packages/browser-rum-core/src/domain/webSocket/*.spec.ts" --spec packages/browser-rum-core/src/browser/webSocketObservable.spec.ts
  • E2E: yarn test:e2e -g "rum websockets"

Note

check-staging-merge is expected to fail: staging-40 already contains the earlier merge of #5055, which touches the same files. It clears at the next staging bump; the other checks should be green.

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated documentation and/or relevant AGENTS.md file

@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Tests

✅ All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 85.71%
• Overall Coverage: 77.35% (+0.02%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 5ef2033 | Docs | View more details | Give us feedback!

@bdibon bdibon mentioned this pull request Oct 1, 2026
3 of 5 tasks
@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum Salesforce N/A 186.13 KiB N/A N/A N/A
Rum 185.57 KiB 186.10 KiB +546 B +0.29% ✅
Rum-salesforce Profiler N/A 8.35 KiB N/A N/A N/A
Rum Profiler 8.35 KiB 8.35 KiB 0 B 0.00% ✅
Rum-salesforce Recorder N/A 31.94 KiB N/A N/A N/A
Rum Recorder 31.94 KiB 31.94 KiB 0 B 0.00% ✅
Logs 59.34 KiB 59.34 KiB 0 B 0.00% ✅
Rum Slim 143.59 KiB 144.11 KiB +531 B +0.36% ✅
Worker 22.96 KiB 22.96 KiB 0 B 0.00% ✅
Rum Shopify N/A 216.52 KiB N/A N/A N/A
Rum-shopify Profiler N/A 8.35 KiB N/A N/A N/A
Rum-shopify Recorder N/A 4.77 KiB N/A N/A N/A

@bdibon
bdibon force-pushed the boris.dibon/websocket-vitals-3-closing branch from a488fea to 197a42b Compare October 1, 2026 11:59
@bdibon bdibon changed the title ✨ Report the websocket_closing vital on client close() ✨ Report the websocket_closing vital Oct 1, 2026
@bdibon
bdibon added this pull request to stack #5099 October 1, 2026 12:13
@bdibon
bdibon marked this pull request as ready for review October 1, 2026 12:18
@bdibon
bdibon requested a review from a team as a code owner October 1, 2026 12:18
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T12:54:17.428629Z 5ef2033 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread packages/browser-rum-core/src/browser/webSocketObservable.ts

@bcaudan bcaudan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM but could be impacted by #5093 (comment)

@bdibon
bdibon force-pushed the boris.dibon/websocket-vitals-3-closing branch from 197a42b to 2e6497f Compare October 7, 2026 14:27
@bdibon
bdibon removed this pull request from stack #5099 October 13, 2026 12:41
@bdibon
bdibon added this pull request to stack #5131 October 13, 2026 12:41
bdibon and others added 4 commits October 13, 2026 14:48
The browser fires no event when the closing handshake starts, so close() is instrumented. A call on a socket already closing or closed is a no-op and notifies nothing.

Co-authored-by: Cursor <cursoragent@cursor.com>
send() on a socket that is not OPEN never reaches the wire: it throws before the handshake completed, and is silently dropped once the socket is closing or closed.

Co-authored-by: Cursor <cursoragent@cursor.com>
…cket

The closing vital carries the closing date and the client as initiator. It takes no snapshot version, and the closed vital that follows keeps the closing date. A close() during the handshake reports the closing phase without an open date.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@bdibon
bdibon force-pushed the boris.dibon/websocket-vitals-3-closing branch from 2e6497f to 5ef2033 Compare October 13, 2026 12:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ef20335e3

ℹ️ 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".

observable.notify({
state: 'closing',
instance,
at: clocksNow(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Capture the closing clock before invoking close

issue: When WebSocket.prototype.close was already wrapped by customer code or another SDK that performs synchronous work, this samples the clock only after that wrapper returns. The resulting closing_date and view attribution can therefore be delayed—or assigned to a view started inside the wrapper—instead of representing when the application called close(). Capture the clock in the pre-call callback and use it from onPostCall, which still avoids reporting calls that throw.

Useful? React with 👍 / 👎.

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