Repository navigation
Conversation
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 1 Pipeline job failed
ℹ️ InfoNo other issues found (see more)🧪 All tests passed 🎯 Code Coverage (details) Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 4c0f937 | Docs | View more details | Give us feedback! |
Bundles Sizes Evolution
|
Only RUM collects WebSockets, so the observable does not need to ship with Logs. The open context now also carries the extensions the server selected. Co-authored-by: Cursor <cursoragent@cursor.com>
Raw types for the websocket_connecting, websocket_open and websocket_closed vitals, and the domain context exposing the socket instance to beforeSend. The profiler vital history skips WebSocket vitals, which carry no duration. Co-authored-by: Cursor <cursoragent@cursor.com>
It holds the phase a connection reached (connecting, open, closed) with the facts that come with it, and aggregates messages per direction: count, total and max size, longest gap, and the deepest send queue. Co-authored-by: Cursor <cursoragent@cursor.com>
One vital per phase, sharing the connection id. Phase dates are placed from the connecting date on the monotonic clock, so a system clock change mid-connection shifts none of them. Co-authored-by: Cursor <cursoragent@cursor.com>
Each instrumented connection reports websocket_connecting, websocket_open and websocket_closed vitals, with the socket instance as domain context. Connections still tracked when the session expires or the collection stops are closed with the session_end reason. The collection gates itself on trackResources and betaTrackWebSockets / TRACK_WEBSOCKETS; it is not wired yet. Co-authored-by: Cursor <cursoragent@cursor.com>
…vitals startRum now starts the vital collection, which gates itself. The prototype resource event, its WEBSOCKET_COMPLETED life cycle event, its beforeSend field paths, its schema validation bypasses and its e2e scenario are removed. The resource domain context types it exposed are deprecated rather than removed, as customers who opted in early may reference them. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
e7ad414 to
997c69f
Compare
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. |
![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: 997c69f340
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
browser-sdk/packages/browser-rum-core/src/browser/webSocketObservable.ts
Lines 103 to 105 in 7c5911f
Report the client-initiated closing phase
issue: When application code calls WebSocket.close(), the observable instruments only send() and waits for the eventual close event, so no websocket_closing vital is produced. The generated event contract defines that phase with closing_date and close_initiator: 'client'; without instrumenting close(), client-initiated shutdowns are indistinguishable from server-initiated shutdowns in the resulting connection data.
ℹ️ 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".
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eb8b7f69e
ℹ️ 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".
| export const WebSocketVitalName = { | ||
| CONNECTING: 'websocket_connecting', | ||
| OPEN: 'websocket_open', | ||
| CLOSED: 'websocket_closed', | ||
| } as const |
There was a problem hiding this comment.
issue: When application code calls WebSocket.close(), this phase list has no closing state and the observable instruments only construction and send, so client- and server-initiated shutdowns both produce only the same closed vital. The generated RumVitalWebsocketClosingEvent contract is the sole place to report close_initiator: 'client'; without instrumenting close() and emitting this phase once, downstream connection data cannot distinguish who initiated the shutdown.
Useful? React with 👍 / 👎.
| /** Ends tracking without a close event, so with no close outcome to report. */ | ||
| recordTrackingEnd: ( | ||
| endClocks: ClocksState, | ||
| trackingEndReason: Exclude<WebSocketTrackingEndReason, typeof WebSocketTrackingEndReason.CLOSE_EVENT> |
There was a problem hiding this comment.
🥜 nitpick: UnobservedTrackingEndReason could be exported and reused here
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c0f937322
ℹ️ 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".
| // the peak is counted after the payload is enqueued, from the pre-send queue depth: | ||
| // `send()` grows the queue by exactly the payload size, whereas reading the socket again | ||
| // could catch a queue the browser has already partly flushed and understate the peak | ||
| snapshot.bufferedAmountMax = Math.max(snapshot.bufferedAmountMax, bufferedAmountPreSend + size) |
There was a problem hiding this comment.
issue: When application code calls send() after close() has moved the socket into CLOSING but before the close event arrives, the native method returns normally while silently discarding the payload, so the observable still reaches this path. Adding size to bufferedAmountPreSend then inflates buffered_amount_max with bytes that were never enqueued; check that the socket was OPEN before treating the call as an outbound enqueue.
Useful? React with 👍 / 👎.
This branch has not been deployed
3 participants
Motivation
The WebSocket prototype sent one
resourceevent per connection, and only once the connection ended: a long-lived socket showed nothing while open, and a socket that ended without acloseevent reported nothing. This PR replaces it with a stream of WebSocket vitals (vital.type: "websocket"), one per connection phase, all sharing a connection id. This first slice reports the three phases every connection goes through: connecting, open and closed.Part of the WebSocket vitals stack. Every PR targets the previous one; the first one targets the
boris.dibon/websocket-vitalsintegration branch, which is merged intomainin 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 currentmain.Changes
Commit by commit:
extensions.RumWebSocketVitalEventDomainContextexposes the live socket tobeforeSend. The profiler's vital history skips WebSocket vitals.trackedConnection.tsholds a connection's phase with the facts that come with it (a discriminated union, so no phase can be reached without its data), plus per-direction message aggregates.serializeWebSocketVital.tsis pure: state → raw vital, presence rules, ms → ns. Phase dates are placed from the connecting date on the monotonic clock, so a system clock change mid-connection shifts none of them.webSocketCollection.tsgates ontrackResourcesandbetaTrackWebSockets/track_websockets, tracks one connection per socket and notifiesRAW_RUM_EVENT_COLLECTEDdirectly. Connections still open when the session expires or the collection stops close withsession_end.startRum, deletesresource/webSocketCollection*,WEBSOCKET_COMPLETED, the prototype'sbeforeSendfield paths and the schema-validation bypasses.isWebSocketandRumWebSocketResourceEventDomainContextare deprecated rather than removed, since early adopters may reference them.Suggested reading order:
trackedConnection.ts→serializeWebSocketVital.ts→webSocketCollection.ts.Test instructions
yarn test:unit --spec "packages/browser-rum-core/src/domain/webSocket/*.spec.ts" --spec packages/browser-rum-core/src/browser/webSocketObservable.spec.tsyarn test:e2e -g "rum websockets"betaTrackWebSocketsin the sandbox, open and close a socket to an echo server, and check the threewebsocket_*vitals in the RUM intake requests.Note
check-staging-mergeis expected to fail:staging-40already 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