fix: Segmenter logic to correctly detect end of a player (#18153)

PostHog · Oct 24, 2023 · 7332c46 · 7332c46
1 parent 7ce0d34
commit 7332c46
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 29 deletions.
diff --git a/...c/scenes/session-recordings/player/__snapshots__/sessionRecordingPlayerLogic.test.ts.snap b/...c/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
@@ -784,7 +784,23 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
                     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<sessionRecordingPlayerLogicType>(
                         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
@@ -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
@@ -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
@@ -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<string, eventWithTime[]> => {
@@ -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