Skip to content

⚗️ Partial view updates — batch optimizations#4756

Open
mormubis wants to merge 1 commit into
mainfrom
adlrb/partial-view
Open

⚗️ Partial view updates — batch optimizations#4756
mormubis wants to merge 1 commit into
mainfrom
adlrb/partial-view

Conversation

@mormubis

@mormubis mormubis commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Motivation

The initial partial view updates implementation (already on main) sends diffs via batch.add(). Two inefficiencies: when the full VIEW is still in the current batch, intermediate updates add separate view_update events on top of it instead of replacing the VIEW in place. And when multiple updates land in the same batch, each one becomes a separate view_update instead of a single aggregate.

Changes

startRumBatch.ts — refactored into createViewBatchRouter to make the routing logic unit-testable without the full batch setup. Two batch-level optimizations:

If the current batch already has a full VIEW, intermediate updates replace it in the upsert buffer (batchHasFullView path). No view_update emitted. Same as the non-experimental behavior.

If the batch was already flushed, updates compute an aggregate diff from batchBase (the last state the backend received) and upsert it under the same key. Multiple updates collapse to one view_update per batch.

Test instructions

  1. yarn dev, open http://localhost:8080
  2. Add enableExperimentalFeatures: ['partial_view_updates'] to the DD_RUM.init() call in sandbox/index.html
  3. Open DevTools → Network tab, filter by /proxy

Optimization 1 (no view_update emitted):

  1. In the console, call DD_RUM.setViewName('test') — update arrives while the VIEW is still in the batch
  2. Flush: window.dispatchEvent(new Event('beforeunload'))
  3. The proxy request should contain only a full VIEW, no view_update

Optimization 2 (aggregate view_update sent):

  1. Flush first: window.dispatchEvent(new Event('beforeunload'))
  2. In the console, call DD_RUM.setViewName('test2') — update arrives in a fresh batch
  3. Flush again: window.dispatchEvent(new Event('beforeunload'))
  4. The second proxy request should be noticeably smaller — it contains a view_update diff, not a full VIEW

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change
  • Added e2e/integration tests for this change
  • Updated documentation

// On flush: advance batchBase to lastSentView (what the backend now has) and clear the flag.
batch.flushController.flushObservable.subscribe(() => {
batchHasFullView = false
batchBase = lastSentView

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

batchBase tracks what the backend actually received, not just the latest event we processed. It advances to lastSentView on flush, so aggregate diffs in the next batch are always computed against the right base. If we used lastSentView directly as the diff base without this reset, we'd start each new batch from the wrong state.

Comment thread packages/browser-rum-core/src/transport/startRumBatch.ts Outdated
Comment thread packages/browser-rum-core/src/transport/startRumBatch.ts
@datadog-prod-us1-4

datadog-prod-us1-4 Bot commented Jun 10, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 88.10%
Overall Coverage: 77.01% (+0.17%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 3a03a5f | Docs | Datadog PR Page | Give us feedback!

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Jun 10, 2026

Copy link
Copy Markdown

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 172.40 KiB 172.57 KiB +166 B +0.09%
Rum Profiler 7.88 KiB 7.88 KiB 0 B 0.00%
Rum Recorder 21.22 KiB 21.22 KiB 0 B 0.00%
Logs 54.82 KiB 54.82 KiB 0 B 0.00%
Rum Slim 130.14 KiB 130.30 KiB +166 B +0.12%
Worker 22.96 KiB 22.96 KiB 0 B 0.00%

@mormubis mormubis force-pushed the adlrb/partial-view branch from 14ceaa8 to 6d8d24e Compare June 10, 2026 10:19
@mormubis mormubis changed the title ⚗️ Partial view updates ⚗️ Partial view updates — batch optimizations Jun 10, 2026
@mormubis mormubis force-pushed the adlrb/partial-view branch from ebd0be3 to 3a03a5f Compare June 11, 2026 14:27
@mormubis mormubis marked this pull request as ready for review June 12, 2026 09:53
@mormubis mormubis requested a review from a team as a code owner June 12, 2026 09:53

@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: 3a03a5f081

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

Comment on lines +127 to +131
lastSentView = serverRumEvent
batchBase = serverRumEvent
batchHasFullView = true
viewUpdatesSinceCheckpoint = 0
batch.upsert(viewEvent as unknown as Context, viewId)
batch.upsert(serverRumEvent, viewId)

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 Keep router state aligned when upsert flushes first

issue: When starting a new view while the current batch is near the 16 KiB bytes limit, batch.upsert() can synchronously flush the existing batch in notifyBeforeAddMessage before the new view is stored. Because lastSentView/batchHasFullView are updated before that call, this router's flush subscriber resets batchHasFullView to false, then the full VIEW is added to the fresh batch; the next update for that same view takes the opt-2 path and upserts a view_update under the same key, replacing the unsent full VIEW. In that scenario the intake receives only a view_update for a view it has never seen, so the initial view can be lost.

Useful? React with 👍 / 👎.

sessionExpireObservable,
}),
// On flush: advance batchBase to lastSentView (what the backend now has) and clear the flag.
batch.flushController.flushObservable.subscribe(() => {

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 Unsubscribe the router flush listener on stop

issue: This new flushObservable subscription is never tied into batch.stop(). In the non-event-bridge path, startRum() only calls batch.stop() during cleanup, and createBatch.stop() removes only the batch's own flush observer; this extra observer keeps the flush controller subscribed to page-exit/session-expire sources after RUM is stopped, so repeated stop/re-init cycles accumulate stale listeners and retain this router state.

Useful? React with 👍 / 👎.

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.

1 participant