From c4b300012d0fdefc31a5243e71ae260ed942e69a Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 30 Sep 2024 23:25:15 +0100 Subject: [PATCH] don't always compress --- .../replay/sessionrecording.test.ts | 132 +++++++----------- src/extensions/replay/sessionrecording.ts | 10 +- 2 files changed, 57 insertions(+), 85 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 0ec9d935a..fc110b397 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -105,9 +105,9 @@ const createIncrementalMouseEvent = () => { }) } -const createIncrementalMutationEvent = () => { +const createIncrementalMutationEvent = (mutations?: { texts: any[] }) => { const mutationData = { - texts: [], + texts: mutations?.texts || [], attributes: [], removes: [], adds: [], @@ -121,7 +121,7 @@ const createIncrementalMutationEvent = () => { }) } -const createIncrementalStyleSheetEvent = () => { +const createIncrementalStyleSheetEvent = (mutations?: { adds: any[] }) => { return createIncrementalSnapshot({ data: { // doesn't need to be a valid style sheet event @@ -129,7 +129,7 @@ const createIncrementalStyleSheetEvent = () => { id: 1, styleId: 1, removes: [], - adds: [], + adds: mutations.adds || [], replace: 'something', replaceSync: 'something', }, @@ -476,9 +476,9 @@ describe('SessionRecording', () => { const fullSnapshot = createFullSnapshot() _emit(fullSnapshot) expect(sessionRecording['buffer']).toEqual({ - data: [{ ...fullSnapshot, cv: '2024-10', data: expect.any(String) }], + data: [fullSnapshot], sessionId: sessionId, - size: 121, + size: 20, windowId: 'windowId', }) }) @@ -1493,22 +1493,9 @@ describe('SessionRecording', () => { // the second snapshot remains buffered in memory expect(sessionRecording['buffer']).toEqual({ - data: [ - firstSnapshotEvent, - { - ...secondSnapshot, - cv: '2024-10', - data: { - ...secondSnapshot.data, - adds: expect.any(String), - texts: expect.any(String), - attributes: expect.any(String), - removes: expect.any(String), - }, - }, - ], + data: [firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 549, + size: 186, windowId: expect.any(String), }) @@ -1530,22 +1517,9 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { - $snapshot_data: [ - firstSnapshotEvent, - { - ...secondSnapshot, - cv: '2024-10', - data: { - ...secondSnapshot.data, - adds: expect.any(String), - texts: expect.any(String), - attributes: expect.any(String), - removes: expect.any(String), - }, - }, - ], + $snapshot_data: [firstSnapshotEvent, secondSnapshot], $session_id: firstSessionId, - $snapshot_bytes: 549, + $snapshot_bytes: 186, $window_id: expect.any(String), }, { @@ -1605,22 +1579,9 @@ describe('SessionRecording', () => { // the second snapshot remains buffered in memory expect(sessionRecording['buffer']).toEqual({ - data: [ - firstSnapshotEvent, - { - ...secondSnapshot, - cv: '2024-10', - data: { - ...secondSnapshot.data, - adds: expect.any(String), - texts: expect.any(String), - attributes: expect.any(String), - removes: expect.any(String), - }, - }, - ], + data: [firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 549, + size: 186, windowId: expect.any(String), }) @@ -1646,22 +1607,9 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { - $snapshot_data: [ - firstSnapshotEvent, - { - ...secondSnapshot, - cv: '2024-10', - data: { - ...secondSnapshot.data, - adds: expect.any(String), - texts: expect.any(String), - attributes: expect.any(String), - removes: expect.any(String), - }, - }, - ], + $snapshot_data: [firstSnapshotEvent, secondSnapshot], $session_id: firstSessionId, - $snapshot_bytes: 549, + $snapshot_bytes: 186, $window_id: expect.any(String), }, { @@ -1685,22 +1633,9 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { - $snapshot_data: [ - firstSnapshotEvent, - { - ...secondSnapshot, - cv: '2024-10', - data: { - ...secondSnapshot.data, - adds: expect.any(String), - texts: expect.any(String), - attributes: expect.any(String), - removes: expect.any(String), - }, - }, - ], + $snapshot_data: [firstSnapshotEvent, secondSnapshot], $session_id: firstSessionId, - $snapshot_bytes: 549, + $snapshot_bytes: 186, $window_id: expect.any(String), }, { @@ -2039,7 +1974,13 @@ describe('SessionRecording', () => { }) it('compresses full snapshot data', () => { - _emit(createFullSnapshot()) + _emit( + createFullSnapshot({ + data: { + content: Array(30).fill(uuidv7()).join(''), + }, + }) + ) sessionRecording['_flushBuffer']() expect(posthog.capture).toHaveBeenCalledWith( @@ -2060,8 +2001,29 @@ describe('SessionRecording', () => { ) }) + it('does not compress small full snapshot data', () => { + _emit(createFullSnapshot({ data: { content: 'small' } })) + sessionRecording['_flushBuffer']() + + expect(posthog.capture).toHaveBeenCalledWith( + '$snapshot', + { + $snapshot_data: [ + { + data: { content: 'small' }, + type: 2, + }, + ], + $session_id: sessionId, + $snapshot_bytes: expect.any(Number), + $window_id: 'windowId', + }, + captureOptions + ) + }) + it('compresses incremental snapshot mutation data', () => { - _emit(createIncrementalMutationEvent()) + _emit(createIncrementalMutationEvent({ texts: [Array(30).fill(uuidv7()).join('')] })) sessionRecording['_flushBuffer']() expect(posthog.capture).toHaveBeenCalledWith( @@ -2090,7 +2052,7 @@ describe('SessionRecording', () => { }) it('compresses incremental snapshot style data', () => { - _emit(createIncrementalStyleSheetEvent()) + _emit(createIncrementalStyleSheetEvent({ adds: [Array(30).fill(uuidv7()).join('')] })) sessionRecording['_flushBuffer']() expect(posthog.capture).toHaveBeenCalledWith( @@ -2119,6 +2081,8 @@ describe('SessionRecording', () => { ) }) + it('does not compress small incremental snapshot data', () => {}) + it('does not compress incremental snapshot non full data', () => { const mouseEvent = createIncrementalMouseEvent() _emit(mouseEvent) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index be5e687a1..f7c6d5162 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -39,7 +39,9 @@ const BASE_ENDPOINT = '/s/' const FIVE_MINUTES = 1000 * 60 * 5 const TWO_SECONDS = 2000 export const RECORDING_IDLE_THRESHOLD_MS = FIVE_MINUTES -export const RECORDING_MAX_EVENT_SIZE = 1024 * 1024 * 0.9 // ~1mb (with some wiggle room) +const ONE_KB = 1024 +const PARTIAL_COMPRESSION_THRESHOLD = ONE_KB +export const RECORDING_MAX_EVENT_SIZE = ONE_KB * ONE_KB * 0.9 // ~1mb (with some wiggle room) export const RECORDING_BUFFER_TIMEOUT = 2000 // 2 seconds export const SESSION_RECORDING_BATCH_KEY = 'recordings' @@ -144,6 +146,11 @@ function gzipToString(data: unknown): string { // but we want to be able to inspect metadata during ingestion, and don't want to compress the entire event // so we have a custom packer that only compresses part of some events function compressEvent(event: eventWithTime, ph: PostHog): eventWithTime | compressedEventWithTime { + const originalSize = estimateSize(event) + if (originalSize < PARTIAL_COMPRESSION_THRESHOLD) { + return event + } + try { if (event.type === EventType.FullSnapshot) { return { @@ -940,6 +947,7 @@ export class SessionRecording { const eventToSend = this.instance.config.session_recording.compress_events ?? true ? compressEvent(event, this.instance) : event const size = estimateSize(eventToSend) + const properties = { $snapshot_bytes: size, $snapshot_data: eventToSend,