From d72284d4b3a16fe10a4353af361af3afcd752b9f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 18 Dec 2023 16:10:58 -0500 Subject: [PATCH 1/3] ref(replay): Change to use preset quality values Instead of using canvas recording quality values direct to rrweb, use presets to control image quality and fps. Closes https://github.com/getsentry/team-replay/issues/325 --- packages/replay/src/constants.ts | 30 +++++++++++++++++++ packages/replay/src/replay.ts | 9 ++---- packages/replay/src/types/replay.ts | 4 +-- .../replay/test/integration/rrweb.test.ts | 28 +++++++++++++++++ 4 files changed, 62 insertions(+), 9 deletions(-) diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 13ccea43df22..d50fad9cdd7e 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -53,3 +53,33 @@ export const MAX_REPLAY_DURATION = 3_600_000; // 60 minutes in ms; /** Default attributes to be ignored when `maskAllText` is enabled */ export const DEFAULT_IGNORED_ATTRIBUTES = ['title', 'placeholder']; + +export const CANVAS_QUALITY = { + low: { + sampling: { + canvas: 1, + }, + dataURLOptions: { + type: 'image/webp', + quality: 0.25, + }, + }, + normal: { + sampling: { + canvas: 2, + }, + dataURLOptions: { + type: 'image/webp', + quality: 0.4, + }, + }, + high: { + sampling: { + canvas: 4, + }, + dataURLOptions: { + type: 'image/webp', + quality: 0.5, + }, + }, +} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index d0dc3097bcf6..554ef2a630f1 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -6,6 +6,7 @@ import { logger } from '@sentry/utils'; import { BUFFER_CHECKOUT_TIME, + CANVAS_QUALITY, SESSION_IDLE_EXPIRE_DURATION, SESSION_IDLE_PAUSE_DURATION, SLOW_CLICK_SCROLL_TIMEOUT, @@ -340,14 +341,10 @@ export class ReplayContainer implements ReplayContainerInterface { ...(this.recordingMode === 'buffer' && { checkoutEveryNms: BUFFER_CHECKOUT_TIME }), emit: getHandleRecordingEmit(this), onMutation: this._onMutationHandler, - ...(canvas && { + ...(canvas && canvas.manager && { recordCanvas: true, - sampling: { canvas: canvas.fps || 4 }, - dataURLOptions: { - type: canvas.type || 'image/webp', - quality: canvas.quality || 0.6, - }, getCanvasManager: canvas.manager, + ...(CANVAS_QUALITY[canvas.quality || 'normal'] || CANVAS_QUALITY.normal) }), }); } catch (err) { diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index fb1f91c0e1a9..bd2c6adafca2 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -233,9 +233,7 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { captureExceptions: boolean; traceInternals: boolean; canvas: { - fps?: number; - quality?: number; - type?: string; + quality?: 'low' | 'normal' | 'high'; manager: (options: GetCanvasManagerOptions) => CanvasManagerInterface; }; }>; diff --git a/packages/replay/test/integration/rrweb.test.ts b/packages/replay/test/integration/rrweb.test.ts index 82dd18f2d6ec..dd6813bec7f1 100644 --- a/packages/replay/test/integration/rrweb.test.ts +++ b/packages/replay/test/integration/rrweb.test.ts @@ -1,3 +1,4 @@ +import type { CanvasManagerInterface, } from '../../src/types'; import { resetSdkMock } from '../mocks/resetSdkMock'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -40,4 +41,31 @@ describe('Integration | rrweb', () => { } `); }); + + it('calls rrweb.record with default canvas options', async () => { + const { mockRecord } = await resetSdkMock({ + replayOptions: { + _experiments: { + canvas: { + // @ts-expect-error This should return + // CanvasManagerInterface, but we don't care about it + // for this test + manager: () => null, + } + }, + } + }); + + expect(mockRecord).toHaveBeenLastCalledWith(expect.objectContaining({ + recordCanvas: true, + getCanvasManager: expect.any(Function), + dataURLOptions: { + quality: 0.4, + type: 'image/webp', + }, + sampling: { + canvas: 2, + } + })); + }); }); From 7acd14ac80821dc521a29d96d20a8865a9a7ff27 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 19 Dec 2023 10:12:53 -0500 Subject: [PATCH 2/3] normal->medium --- packages/replay/src/constants.ts | 2 +- packages/replay/src/replay.ts | 2 +- packages/replay/src/types/replay.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index d50fad9cdd7e..7909c354475f 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -64,7 +64,7 @@ export const CANVAS_QUALITY = { quality: 0.25, }, }, - normal: { + medium: { sampling: { canvas: 2, }, diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 554ef2a630f1..8f0fa2f6440c 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -344,7 +344,7 @@ export class ReplayContainer implements ReplayContainerInterface { ...(canvas && canvas.manager && { recordCanvas: true, getCanvasManager: canvas.manager, - ...(CANVAS_QUALITY[canvas.quality || 'normal'] || CANVAS_QUALITY.normal) + ...(CANVAS_QUALITY[canvas.quality || 'medium'] || CANVAS_QUALITY.medium) }), }); } catch (err) { diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index bd2c6adafca2..1b2b19d4d4cd 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -233,7 +233,7 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { captureExceptions: boolean; traceInternals: boolean; canvas: { - quality?: 'low' | 'normal' | 'high'; + quality?: 'low' | 'medium' | 'high'; manager: (options: GetCanvasManagerOptions) => CanvasManagerInterface; }; }>; From 3a44ba053173e2fd20f27e53eb581e1639df42b9 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 19 Dec 2023 14:55:19 -0500 Subject: [PATCH 3/3] lint --- packages/replay/src/constants.ts | 2 +- packages/replay/src/replay.ts | 11 ++--- .../replay/test/integration/rrweb.test.ts | 40 ++++++++++--------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 7909c354475f..ec3325a4253a 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -82,4 +82,4 @@ export const CANVAS_QUALITY = { quality: 0.5, }, }, -} +}; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 8f0fa2f6440c..87c71dddec3e 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -341,11 +341,12 @@ export class ReplayContainer implements ReplayContainerInterface { ...(this.recordingMode === 'buffer' && { checkoutEveryNms: BUFFER_CHECKOUT_TIME }), emit: getHandleRecordingEmit(this), onMutation: this._onMutationHandler, - ...(canvas && canvas.manager && { - recordCanvas: true, - getCanvasManager: canvas.manager, - ...(CANVAS_QUALITY[canvas.quality || 'medium'] || CANVAS_QUALITY.medium) - }), + ...(canvas && + canvas.manager && { + recordCanvas: true, + getCanvasManager: canvas.manager, + ...(CANVAS_QUALITY[canvas.quality || 'medium'] || CANVAS_QUALITY.medium), + }), }); } catch (err) { this._handleException(err); diff --git a/packages/replay/test/integration/rrweb.test.ts b/packages/replay/test/integration/rrweb.test.ts index dd6813bec7f1..3543a6771ce4 100644 --- a/packages/replay/test/integration/rrweb.test.ts +++ b/packages/replay/test/integration/rrweb.test.ts @@ -1,4 +1,4 @@ -import type { CanvasManagerInterface, } from '../../src/types'; +import type { CanvasManagerInterface } from '../../src/types'; import { resetSdkMock } from '../mocks/resetSdkMock'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -46,26 +46,28 @@ describe('Integration | rrweb', () => { const { mockRecord } = await resetSdkMock({ replayOptions: { _experiments: { - canvas: { - // @ts-expect-error This should return - // CanvasManagerInterface, but we don't care about it - // for this test - manager: () => null, - } + canvas: { + // @ts-expect-error This should return + // CanvasManagerInterface, but we don't care about it + // for this test + manager: () => null, + }, + }, }, - } }); - expect(mockRecord).toHaveBeenLastCalledWith(expect.objectContaining({ - recordCanvas: true, - getCanvasManager: expect.any(Function), - dataURLOptions: { - quality: 0.4, - type: 'image/webp', - }, - sampling: { - canvas: 2, - } - })); + expect(mockRecord).toHaveBeenLastCalledWith( + expect.objectContaining({ + recordCanvas: true, + getCanvasManager: expect.any(Function), + dataURLOptions: { + quality: 0.4, + type: 'image/webp', + }, + sampling: { + canvas: 2, + }, + }), + ); }); });