Conversation
|
All contributors have signed the CLA ✍️ ✅ |
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
|
✨ Fix all issues with BitsAI or with Cursor
|
mormubis
force-pushed
the
adlrb/partial-view
branch
from
1e1e4f8 to
b9f667f
Compare
|
Nice work on this — the diff engine design looks solid, and the REPLACE/MERGE/APPEND strategy categorization is clean. I've been prototyping a parallel implementation against our staging backend and wanted to share some observations. High severity1. No post-assembly strip (~500-650B wasted per view_update)The diff in
These fields have REPLACE semantics — they don't change between updates. In my prototype I added a second pass in 2. No periodic full VIEW refresh (no recovery from dropped events)If any Suggestion: force a full 3. No full VIEW on view endWhen Suggestion: always emit a full Medium severity4.
|
mormubis
force-pushed
the
adlrb/partial-view
branch
4 times, most recently
from
1950458 to
4ca0710
Compare
mormubis
force-pushed
the
adlrb/partial-view
branch
3 times, most recently
from
40aee0f to
c60c158
Compare
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit c60c158a5c will soon be integrated into staging-12.
Commit c60c158a5c has been merged into staging-12 in merge commit f3d007064e. Check out the triggered DDCI request. If you need to revert this integration, you can use the following command: |
Integrated commit sha: c60c158 Co-authored-by: mormubis <adrian.delarosa@datadoghq.com>
mormubis
force-pushed
the
adlrb/partial-view
branch
from
c60c158 to
52a76bb
Compare
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit 52a76bbd97 will soon be integrated into staging-14.
Commit 52a76bbd97 has been merged into staging-14 in merge commit 1cf29217c5. Check out the triggered DDCI request. If you need to revert this integration, you can use the following command: |
Integrated commit sha: 52a76bb Co-authored-by: mormubis <adrian.delarosa@datadoghq.com>
| const viewId = serverRumEvent.view.id | ||
|
|
||
| // New view started | ||
| if (viewId !== currentViewId) { | ||
| currentViewId = viewId | ||
| lastSentView = serverRumEvent | ||
| viewUpdatesSinceCheckpoint = 0 | ||
| batch.upsert(serverRumEvent, viewId) | ||
| return | ||
| } | ||
|
|
||
| // View ended (is_active: false) | ||
| if (!(serverRumEvent.view as any).is_active) { | ||
| currentViewId = undefined | ||
| lastSentView = undefined | ||
| viewUpdatesSinceCheckpoint = 0 | ||
| batch.upsert(serverRumEvent, viewId) | ||
| return | ||
| } | ||
|
|
||
| // Checkpoint: every N intermediate updates, send a full view (unless disabled by flag) | ||
| if (!isExperimentalFeatureEnabled(ExperimentalFeature.PARTIAL_VIEW_UPDATES_NO_CHECKPOINT)) { | ||
| viewUpdatesSinceCheckpoint += 1 | ||
| if (viewUpdatesSinceCheckpoint >= PARTIAL_VIEW_UPDATE_CHECKPOINT_INTERVAL) { | ||
| viewUpdatesSinceCheckpoint = 0 | ||
| lastSentView = serverRumEvent | ||
| batch.upsert(serverRumEvent, viewId) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // Intermediate update: compute diff and send view_update. | ||
| // Note: view_update events are created here, post-assembly, and go directly to batch.add(). | ||
| // They intentionally bypass RAW_RUM_EVENT_COLLECTED → assembly → RUM_EVENT_COLLECTED, which | ||
| // means they skip beforeSend entirely. view_update is an internal bandwidth optimization — | ||
| // not a customer-visible event type, and not modifiable via beforeSend. | ||
| if (!lastSentView) { | ||
| // Safety fallback (should not happen in practice) | ||
| lastSentView = serverRumEvent | ||
| batch.upsert(serverRumEvent, viewId) | ||
| return | ||
| } |
There was a problem hiding this comment.
suggestion: all this could be simplified with a single condition shouldSendViewUpdate:
const shouldSendViewUdpate = lastSentView && viewId === currentViewId && serverRumEvent.view.is_active && viewUpdatesSinceCheckpoint > PARTIAL_VIEW_UPDATE_CHECKPOINT_INTERVAL
if (shouldSendViewUpdate) {
viewUpdatesSinceCheckpoint += 1
const diff = ...
...
} else {
viewUpdatesSinceCheckpoint = 0
batch.upsert(serverRumEvent, viewId)
}There was a problem hiding this comment.
I prefer early returns. Each path (new view, view end, checkpoint, diff) is readable on its own. I'm not sure the single condition improves it. What do you think?
There was a problem hiding this comment.
Well, a bit of logic is repeated in you current code, which makes things unnecessarily lengthy. Also there this some case:
if (!lastSentView) {
// Safety fallback (should not happen in practice)
lastSentView = serverRumEvent
batch.upsert(serverRumEvent, viewId)
return
}which seem incomplete because it doesn't reset viewUpdatesSinceCheckpoint. It's probably fine as you stated that it shouldn't happen in practice, but then either it happens and we should make sure it behaves correctly, or it shouldn't and we remove it.
It feels like you are struggling because of imprecise typings. How can we improve the schema so types are better defined?
There was a problem hiding this comment.
Removed the fallback. Replaced as any with as RumViewEvent since is_active is already typed there. Not sure if there's a better way to improve the schema to avoid the cast entirely. What do you think?
- Add PARTIAL_VIEW_UPDATES to ExperimentalFeature enum - Add VIEW_UPDATE to RumEventType, RawRumViewUpdateEvent, and RawRumEvent union - Add viewDiff.ts: isEqual and diffMerge utilities implementing MERGE / REPLACE / APPEND strategies for computing minimal diffs between assembled view events
When partial_view_updates is enabled, startRumBatch intercepts assembled view events and sends view_update diffs instead of full views for intermediate updates. Key design: diff runs post-assembly so beforeSend always sees full view events (backward-compatible). view_update events intentionally bypass the assembly pipeline — they are a bandwidth optimization, not a customer-visible event type. - computeAssembledViewDiff: diffs two assembled view events, always including required routing fields (view.id, view.url, _dd.document_version, format_version) - Routing state machine: handles new view / view-end / checkpoint / diff cases - view-end events (is_active: false) always sent as full view - Full view checkpoint every 100 updates for backend recovery - Exclude view_update from trackEventCounts and assembly beforeSend guard - Add E2E tests covering all routing cases
…t or APPEND array
mormubis
force-pushed
the
adlrb/partial-view
branch
from
75e714c to
5e79702
Compare
3 participants
Motivation
Every periodic view update was sending the full view payload even when only one counter changed. Benchmarks showed 50–90% of the data was redundant. This implements the partial view updates RFC to reduce bandwidth by sending only changed fields in subsequent view events.
Aligned with rum-events-format #355 (now merged).
Changes
When
partial_view_updatesis enabled, the SDK sends the first event perview.idas a fullview, then sendsview_updatediffs with only changed fields. The diff runs post-assembly instartRumBatch.tssobeforeSendalways sees the full event (backward-compatible).view_updateevents bypass the assembly pipeline intentionally, they are a bandwidth optimization and not a customer-visible event type.A full
viewcheckpoint is sent every 100 updates for backend recovery. Checkpoints can be disabled withpartial_view_updates_no_checkpoint.view_updateevents usebatch.addinstead ofupsert, so a batch can contain a fullviewfollowed byview_updateevents. This is intentional: if we consolidated them, we wouldn't be able to tell if the backend missed an intermediate update or it was never sent.Test instructions
Or manually:
Checklist