From 7332c4681e7fb727f72df3da5814ad84d39035fd Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 24 Oct 2023 13:55:33 +0200 Subject: [PATCH] fix: Segmenter logic to correctly detect end of a player (#18153) --- .../sessionRecordingPlayerLogic.test.ts.snap | 16 +++--- .../player/sessionRecordingPlayerLogic.ts | 19 ++++++- .../__snapshots__/segmenter.test.ts.snap | 49 ++++++++++++------- .../player/utils/segmenter.test.ts | 16 ++++++ .../player/utils/segmenter.ts | 16 +++++- 5 files changed, 87 insertions(+), 29 deletions(-) diff --git a/frontend/src/scenes/session-recordings/player/__snapshots__/sessionRecordingPlayerLogic.test.ts.snap b/frontend/src/scenes/session-recordings/player/__snapshots__/sessionRecordingPlayerLogic.test.ts.snap index d3bca952828b2..db99fca1fdadc 100644 --- a/frontend/src/scenes/session-recordings/player/__snapshots__/sessionRecordingPlayerLogic.test.ts.snap +++ b/frontend/src/scenes/session-recordings/player/__snapshots__/sessionRecordingPlayerLogic.test.ts.snap @@ -52,27 +52,27 @@ exports[`sessionRecordingPlayerLogic loading session core loads metadata and sna }, "segments": [ { - "durationMs": 7255, - "endTimestamp": 1682952388132, + "durationMs": 5694, + "endTimestamp": 1682952386571, "isActive": true, "kind": "window", "startTimestamp": 1682952380877, "windowId": "187d7c761a0525d-05f175487d4b65-1d525634-384000-187d7c761a149d0", }, { - "durationMs": 527, - "endTimestamp": 1682952388659, + "durationMs": 1533, + "endTimestamp": 1682952388104, "isActive": false, "kind": "gap", - "startTimestamp": 1682952388132, - "windowId": "187d7c77dfe1d45-08bdcaf91135a2-1d525634-384000-187d7c77dff39a6", + "startTimestamp": 1682952386571, + "windowId": undefined, }, { - "durationMs": 4086, + "durationMs": 4641, "endTimestamp": 1682952392745, "isActive": true, "kind": "window", - "startTimestamp": 1682952388659, + "startTimestamp": 1682952388104, "windowId": "187d7c77dfe1d45-08bdcaf91135a2-1d525634-384000-187d7c77dff39a6", }, ], diff --git a/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts b/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts index d5df80dd1fda5..15b2b31a5187f 100644 --- a/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts +++ b/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts @@ -784,7 +784,23 @@ export const sessionRecordingPlayerLogic = kea( values.currentPlayerState === SessionPlayerState.SKIP) && values.timestampChangeTracking.timestampMatchesPrevious > 10 ) { - cache.debug?.('stuck session player detected', values.timestampChangeTracking) + // NOTE: We should investigate if this is still happening - logging to posthog recording so we can find this in the future + posthog.sessionRecording?.log( + 'stuck session player detected - this indicates an issue with the segmenter', + 'warn' + ) + cache.debug?.('stuck session player detected', { + timestampChangeTracking: values.timestampChangeTracking, + currentSegment: values.currentSegment, + snapshots: values.sessionPlayerData.snapshotsByWindowId[values.currentSegment?.windowId ?? ''], + player: values.player, + meta: values.player?.replayer.getMetaData(), + rrwebPlayerTime, + segments: values.sessionPlayerData.segments, + segmentIndex: + values.currentSegment && values.sessionPlayerData.segments.indexOf(values.currentSegment), + }) + actions.skipPlayerForward(rrwebPlayerTime, skip) newTimestamp = newTimestamp + skip } @@ -811,6 +827,7 @@ export const sessionRecordingPlayerLogic = kea( newTimestamp, segments: values.sessionPlayerData.segments, currentSegment: values.currentSegment, + nextSegment, segmentIndex: values.sessionPlayerData.segments.indexOf(values.currentSegment), }) // At the end of the recording. Pause the player and set fully to the end diff --git a/frontend/src/scenes/session-recordings/player/utils/__snapshots__/segmenter.test.ts.snap b/frontend/src/scenes/session-recordings/player/utils/__snapshots__/segmenter.test.ts.snap index 321fb07545db6..0e07868a1eb50 100644 --- a/frontend/src/scenes/session-recordings/player/utils/__snapshots__/segmenter.test.ts.snap +++ b/frontend/src/scenes/session-recordings/player/utils/__snapshots__/segmenter.test.ts.snap @@ -1,29 +1,42 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`segmenter includes inactive events in the active segment until a threshold 1`] = ` +exports[`segmenter ends a segment if it is the last window 1`] = ` [ { - "durationMs": 4000, - "endTimestamp": 1672531204000, + "durationMs": 100, + "endTimestamp": 1672531200100, "isActive": true, "kind": "window", "startTimestamp": 1672531200000, "windowId": "A", }, { - "durationMs": 2000, - "endTimestamp": 1672531206000, + "durationMs": 400, + "endTimestamp": 1672531200500, "isActive": false, "kind": "gap", - "startTimestamp": 1672531204000, - "windowId": "A", + "startTimestamp": 1672531200100, + "windowId": undefined, }, { - "durationMs": 594000, + "durationMs": 500, + "endTimestamp": "2023-01-01T00:00:01.000Z", + "isActive": false, + "kind": "window", + "startTimestamp": 1672531200500, + "windowId": "B", + }, +] +`; + +exports[`segmenter includes inactive events in the active segment until a threshold 1`] = ` +[ + { + "durationMs": 600000, "endTimestamp": 1672531800000, "isActive": false, "kind": "window", - "startTimestamp": 1672531206000, + "startTimestamp": 1672531200000, "windowId": "A", }, ] @@ -34,7 +47,7 @@ exports[`segmenter inserts gaps inclusively 1`] = ` { "durationMs": 100, "endTimestamp": 1672531200100, - "isActive": true, + "isActive": false, "kind": "window", "startTimestamp": 1672531200000, "windowId": "A", @@ -61,27 +74,27 @@ exports[`segmenter inserts gaps inclusively 1`] = ` exports[`segmenter matches snapshots 1`] = ` [ { - "durationMs": 7255, - "endTimestamp": 1682952388132, + "durationMs": 5694, + "endTimestamp": 1682952386571, "isActive": true, "kind": "window", "startTimestamp": 1682952380877, "windowId": "187d7c761a0525d-05f175487d4b65-1d525634-384000-187d7c761a149d0", }, { - "durationMs": 527, - "endTimestamp": 1682952388659, + "durationMs": 1533, + "endTimestamp": 1682952388104, "isActive": false, "kind": "gap", - "startTimestamp": 1682952388132, - "windowId": "187d7c77dfe1d45-08bdcaf91135a2-1d525634-384000-187d7c77dff39a6", + "startTimestamp": 1682952386571, + "windowId": undefined, }, { - "durationMs": 4086, + "durationMs": 4641, "endTimestamp": 1682952392745, "isActive": true, "kind": "window", - "startTimestamp": 1682952388659, + "startTimestamp": 1682952388104, "windowId": "187d7c77dfe1d45-08bdcaf91135a2-1d525634-384000-187d7c77dff39a6", }, ] diff --git a/frontend/src/scenes/session-recordings/player/utils/segmenter.test.ts b/frontend/src/scenes/session-recordings/player/utils/segmenter.test.ts index 49febaac84dbb..4272a67b256fd 100644 --- a/frontend/src/scenes/session-recordings/player/utils/segmenter.test.ts +++ b/frontend/src/scenes/session-recordings/player/utils/segmenter.test.ts @@ -65,4 +65,20 @@ describe('segmenter', () => { expect(segments).toMatchSnapshot() }) + + it('ends a segment if it is the last window', () => { + const start = dayjs('2023-01-01T00:00:00.000Z') + const end = start.add(1000, 'milliseconds') + + const snapshots: RecordingSnapshot[] = [ + { windowId: 'A', timestamp: start.valueOf(), type: 2, data: {} } as any, + { windowId: 'A', timestamp: start.valueOf() + 100, type: 3, data: {} } as any, + { windowId: 'B', timestamp: start.valueOf() + 500, type: 3, data: {} } as any, + { windowId: 'B', timestamp: end, type: 3, data: {} } as any, + ] + + const segments = createSegments(snapshots, start, end) + + expect(segments).toMatchSnapshot() + }) }) diff --git a/frontend/src/scenes/session-recordings/player/utils/segmenter.ts b/frontend/src/scenes/session-recordings/player/utils/segmenter.ts index 9e284fd6bdb45..f2e43b459f7a1 100644 --- a/frontend/src/scenes/session-recordings/player/utils/segmenter.ts +++ b/frontend/src/scenes/session-recordings/player/utils/segmenter.ts @@ -24,7 +24,11 @@ const activeSources = [ const ACTIVITY_THRESHOLD_MS = 5000 const isActiveEvent = (event: eventWithTime): boolean => { - return event.type === EventType.IncrementalSnapshot && activeSources.includes(event.data?.source) + return ( + event.type === EventType.FullSnapshot || + event.type === EventType.Meta || + (event.type === EventType.IncrementalSnapshot && activeSources.includes(event.data?.source)) + ) } export const mapSnapshotsToWindowId = (snapshots: RecordingSnapshot[]): Record => { @@ -47,7 +51,10 @@ export const createSegments = (snapshots: RecordingSnapshot[], start?: Dayjs, en const snapshotsByWindowId = mapSnapshotsToWindowId(snapshots) snapshots.forEach((snapshot, index) => { - const eventIsActive = isActiveEvent(snapshot) || index === 0 + const eventIsActive = isActiveEvent(snapshot) + const previousSnapshot = snapshots[index - 1] + const isPreviousSnapshotLastForWindow = + snapshotsByWindowId[previousSnapshot?.windowId]?.slice(-1)[0] === previousSnapshot // When do we create a new segment? // 1. If we don't have one yet @@ -68,6 +75,11 @@ export const createSegments = (snapshots: RecordingSnapshot[], start?: Dayjs, en isNewSegment = true } + // 5. If there are no more snapshots for this windowId + if (isPreviousSnapshotLastForWindow) { + isNewSegment = true + } + // NOTE: We have to make sure that we set this _after_ we use it lastActiveEventTimestamp = eventIsActive ? snapshot.timestamp : lastActiveEventTimestamp