Capture events post replay triggering and send them as spans#1339
Conversation
There was a problem hiding this comment.
Pull Request Overview
Introduces a ring buffer (two-slot) with stable buffer cursor ({slot, offset}) to improve leading replay capture robustness across single buffer rotations, replacing prior event count boundary logic. Core changes refactor Recorder internals (buffers, cursor, event collection) and update ReplayManager to use the cursor while expanding test coverage for various rotation and timing scenarios.
- Implemented two-slot ring buffer and cursor-based leading boundary.
- Refactored export logic into _collectAll / _collectEventsFromCursor with replay.end marker helper.
- Updated ReplayManager to schedule and export leading spans using buffer cursor (removed isRecording gate) and added extensive new tests.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/browser/replay/recorder.js | Replaced event arrays with two-slot ring buffer, added bufferCursor, refactored collection/export logic, added helpers & static logging. |
| src/browser/replay/replayManager.js | Switched from event count to buffer cursor for leading capture; updated scheduling and export; added typedef annotations. |
| test/replay/util/recorder.js | Added helper accessors/mutators for internal buffers for test control. |
| test/replay/unit/recorder.collectEvents.test.js | Added unit tests for new collection methods and cursor behavior. |
| test/replay/integration/replayManager.bufferIndex.test.js | Integration tests validating cursor-based leading capture and state transitions. |
| test/replay/integration/replayManager.bufferIndex.checkoutResilience.test.js | Resilience tests across rotations, multiple checkouts, and error handling. |
| test/replay/integration/e2e.test.js | Added end-to-end test covering combined trailing + leading flow with cursor assertions. |
| test/browser.replay.recorder.test.js | Adjusted expectations for new options (postDuration) and removed obsolete skipped test. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (isCheckout && event.type === EventType.Meta) { | ||
| this._events.previous = this._events.current; | ||
| this._events.current = []; | ||
| this._buffers[(this._currentSlot = this._previousSlot)] = []; |
There was a problem hiding this comment.
[nitpick] The in-line assignment with an expression side-effect obscures intent (rotating the active slot and clearing the new current buffer). Refactor for clarity, e.g.
const nextSlot = this._previousSlot;
this._currentSlot = nextSlot;
this._buffers[nextSlot] = [];
to make rotation semantics easier to read and maintain.
| this._buffers[(this._currentSlot = this._previousSlot)] = []; | |
| const nextSlot = this._previousSlot; | |
| this._currentSlot = nextSlot; | |
| this._buffers[nextSlot] = []; |
There was a problem hiding this comment.
Lemme be clever! I can change on request. 🫥
matux
force-pushed
the
matux/post-replay
branch
from
32df702 to
b770d71
Compare
| _options; | ||
| _rrwebOptions; | ||
|
|
||
| _isReady = false; |
There was a problem hiding this comment.
Moved _isReady initialization here from the constructor to follow the pattern we use with the rest of the initialized-at-instancing properties.
| _currentSlot = 0; | ||
| /** Index of the finalized inactive slot (0|1). Frozen until next checkout. */ | ||
| get _previousSlot() { | ||
| return this._currentSlot ^ 1; |
There was a problem hiding this comment.
Since the ring has two slots we can just XOR which effectively flips the bit, so we avoid the modulo from (currentIndex + 1) % 2.
| } | ||
|
|
||
| this._events.current.push(event); | ||
| this._buffers[this._currentSlot].push(event); |
There was a problem hiding this comment.
👍 This push after resetting the current slot ensures the -1 cursor can never happen while isReady is set. Might be worth a code comment.
There was a problem hiding this comment.
Added @remarks to bufferCursor() jsdoc: d9b25ae
Note
PR Description written down with the help of Claude Code by human guided diff analyzing.
Summary
Introduces a ring buffer (two-slot) with stable buffer cursor (
{slot, offset}) to improve leading replay capture robustness across single buffer rotations, replacing prior event count boundary logic. Core changes refactorRecorderinternals (buffers, cursor, event collection) and updateReplayManagerto use the cursor while expanding test coverage for various rotation and timing scenarios._collectAll/_collectEventsFromCursorwithreplay.endmarker helper.ReplayManagerto schedule and export leading spans using buffer cursor and added extensive new tests.Key Changes
Recorder
_currentSlotand_previousSlot(XOR-based).bufferCursor()returning{slot: 0|1, offset: number}for stable position capture.exportRecordingSpan()to accept cursor or export all events._collectAll()and_collectEventsFromCursor(cursor)._replayEndEvent(),_logEvent().ReplayManager
exportRecordingSpan()for leading boundary.Benefits
_eventsfacade, added_previousSlotgetter.Test Coverage