From ccc8f24c25988bafde11ff0ae9ec77393c9f2dd9 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 24 Oct 2024 11:06:33 +0100 Subject: [PATCH 1/2] fix: allow canvas local config --- .../replay/sessionrecording.test.ts | 23 +++++++++++++++++++ src/extensions/replay/sessionrecording.ts | 20 +++++++++------- src/types.ts | 17 ++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index bbb709194..5ec402e08 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -320,6 +320,29 @@ describe('SessionRecording', () => { ) }) + describe('is canvas enabled', () => { + it.each([ + ['enabled when both enabled', true, true, true], + ['uses client side setting when set to false', true, false, false], + ['uses client side setting when set to true', false, true, true], + ['disabled when both disabled', false, false, false], + ['uses client side setting (disabled) if server side setting is not set', undefined, false, false], + ['uses client side setting (enabled) if server side setting is not set', undefined, true, true], + ['is disabled when nothing is set', undefined, undefined, false], + ['uses server side setting (disabled) if client side setting is not set', undefined, false, false], + ['uses server side setting (enabled) if client side setting is not set', undefined, true, true], + ])( + '%s', + (_name: string, serverSide: boolean | undefined, clientSide: boolean | undefined, expected: boolean) => { + posthog.persistence?.register({ + [SESSION_RECORDING_CANVAS_RECORDING]: { enabled: serverSide, fps: 4, quality: 0.1 }, + }) + posthog.config.session_recording.captureCanvas = { recordCanvas: clientSide } + expect(sessionRecording['canvasRecording']).toMatchObject({ enabled: expected }) + } + ) + }) + describe('network timing capture config', () => { it.each([ ['enabled when both enabled', true, true, true], diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 95420aa5f..2b97dac07 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -303,15 +303,19 @@ export class SessionRecording { return enabled_client_side ?? enabled_server_side } - private get canvasRecording(): { enabled: boolean; fps: number; quality: number } | undefined { + private get canvasRecording(): { enabled: boolean; fps: number; quality: number } { + const canvasRecording_client_side = this.instance.config.session_recording.captureCanvas const canvasRecording_server_side = this.instance.get_property(SESSION_RECORDING_CANVAS_RECORDING) - return canvasRecording_server_side && canvasRecording_server_side.fps && canvasRecording_server_side.quality - ? { - enabled: canvasRecording_server_side.enabled, - fps: canvasRecording_server_side.fps, - quality: canvasRecording_server_side.quality, - } - : undefined + + const enabled = canvasRecording_client_side?.recordCanvas ?? canvasRecording_server_side?.enabled ?? false + const fps = canvasRecording_client_side?.canvasFps ?? canvasRecording_server_side?.fps ?? 0 + const quality = canvasRecording_client_side?.canvasQuality ?? canvasRecording_server_side?.quality ?? 0 + + return { + enabled, + fps, + quality, + } } // network payload capture config has three parts diff --git a/src/types.ts b/src/types.ts index a652d729f..8c5058a5f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -274,6 +274,10 @@ export interface SessionRecordingOptions { collectFonts?: boolean inlineStylesheet?: boolean recordCrossOriginIframes?: boolean + /** + * Allows local config to override remote canvas recording settings from the decide response + */ + captureCanvas?: SessionRecordingCanvasOptions /** @deprecated - use maskCapturedNetworkRequestFn instead */ maskNetworkRequestFn?: ((data: NetworkRequest) => NetworkRequest | null | undefined) | null /** Modify the network request before it is captured. Returning null or undefined stops it being captured */ @@ -358,6 +362,13 @@ export interface CaptureOptions { export type FlagVariant = { flag: string; variant: string } +export type SessionRecordingCanvasOptions = { + recordCanvas?: boolean | null + canvasFps?: number | null + // the API returns a decimal between 0 and 1 as a string + canvasQuality?: string | null +} + export interface DecideResponse { supportedCompression: Compression[] featureFlags: Record @@ -384,16 +395,12 @@ export interface DecideResponse { | { endpoint?: string } - sessionRecording?: { + sessionRecording?: SessionRecordingCanvasOptions & { endpoint?: string consoleLogRecordingEnabled?: boolean // the API returns a decimal between 0 and 1 as a string sampleRate?: string | null minimumDurationMilliseconds?: number - recordCanvas?: boolean | null - canvasFps?: number | null - // the API returns a decimal between 0 and 1 as a string - canvasQuality?: string | null linkedFlag?: string | FlagVariant | null networkPayloadCapture?: Pick urlTriggers?: SessionRecordingUrlTrigger[] From 2bf62b4d10971b0d71ec06dcef916e4415cf755d Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 24 Oct 2024 11:46:17 +0100 Subject: [PATCH 2/2] clamp to range refactor --- src/__tests__/utils/number-utils.test.ts | 17 +++++++++++++++++ src/extensions/replay/sessionrecording.ts | 5 +++-- src/page-view.ts | 16 ++++++++-------- src/sessionid.ts | 21 ++++++++++----------- src/utils/index.ts | 4 ++-- src/utils/number-utils.ts | 17 +++++++++++++++++ 6 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 src/__tests__/utils/number-utils.test.ts create mode 100644 src/utils/number-utils.ts diff --git a/src/__tests__/utils/number-utils.test.ts b/src/__tests__/utils/number-utils.test.ts new file mode 100644 index 000000000..9647caf6d --- /dev/null +++ b/src/__tests__/utils/number-utils.test.ts @@ -0,0 +1,17 @@ +import { clampToRange } from '../../utils/number-utils' + +describe('number-utils', () => { + describe('clampToRange', () => { + it.each([ + // [value, result, min, max, expected result, test description] + ['returns max when value is not a number', null, 10, 100, 100], + ['returns max when value is not a number', 'not-a-number', 10, 100, 100], + ['returns max when value is greater than max', 150, 10, 100, 100], + ['returns min when value is less than min', 5, 10, 100, 10], + ['returns the value when it is within the range', 50, 10, 100, 50], + ])('%s', (_description, value, min, max, expected) => { + const result = clampToRange(value, min, max, 'Test Label') + expect(result).toBe(expected) + }) + }) +}) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 2b97dac07..771976460 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -42,6 +42,7 @@ import { buildNetworkRequestOptions } from './config' import { isLocalhost } from '../../utils/request-utils' import { MutationRateLimiter } from './mutation-rate-limiter' import { gzipSync, strFromU8, strToU8 } from 'fflate' +import { clampToRange } from '../../utils/number-utils' type SessionStartReason = | 'sampling_override' @@ -313,8 +314,8 @@ export class SessionRecording { return { enabled, - fps, - quality, + fps: clampToRange(fps, 0, 12, 'canvas recording fps'), + quality: clampToRange(quality, 0, 1, 'canvas recording quality'), } } diff --git a/src/page-view.ts b/src/page-view.ts index bf1952d6e..d34f52b06 100644 --- a/src/page-view.ts +++ b/src/page-view.ts @@ -1,6 +1,7 @@ import { window } from './utils/globals' import { PostHog } from './posthog-core' import { isUndefined } from './utils/type-utils' +import { clampToRange } from './utils/number-utils' interface PageViewEventProperties { $prev_pageview_pathname?: string @@ -71,10 +72,13 @@ export class PageViewManager { maxContentY = Math.ceil(maxContentY) // if the maximum scroll height is near 0, then the percentage is 1 - const lastScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(lastScrollY / maxScrollHeight, 0, 1) - const maxScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(maxScrollY / maxScrollHeight, 0, 1) - const lastContentPercentage = maxContentHeight <= 1 ? 1 : clamp(lastContentY / maxContentHeight, 0, 1) - const maxContentPercentage = maxContentHeight <= 1 ? 1 : clamp(maxContentY / maxContentHeight, 0, 1) + const lastScrollPercentage = + maxScrollHeight <= 1 ? 1 : clampToRange(lastScrollY / maxScrollHeight, 0, 1) + const maxScrollPercentage = maxScrollHeight <= 1 ? 1 : clampToRange(maxScrollY / maxScrollHeight, 0, 1) + const lastContentPercentage = + maxContentHeight <= 1 ? 1 : clampToRange(lastContentY / maxContentHeight, 0, 1) + const maxContentPercentage = + maxContentHeight <= 1 ? 1 : clampToRange(maxContentY / maxContentHeight, 0, 1) properties = { $prev_pageview_last_scroll: lastScrollY, @@ -100,7 +104,3 @@ export class PageViewManager { return properties } } - -function clamp(x: number, min: number, max: number) { - return Math.max(min, Math.min(x, max)) -} diff --git a/src/sessionid.ts b/src/sessionid.ts index 0be79c3e8..b2e81a413 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -8,6 +8,8 @@ import { window } from './utils/globals' import { isArray, isNumber, isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { clampToRange } from './utils/number-utils' + const MAX_SESSION_IDLE_TIMEOUT = 30 * 60 // 30 minutes const MIN_SESSION_IDLE_TIMEOUT = 60 // 1 minute const SESSION_LENGTH_LIMIT = 24 * 3600 * 1000 // 24 hours @@ -43,19 +45,16 @@ export class SessionIdManager { this._windowIdGenerator = windowIdGenerator || uuidv7 const persistenceName = config['persistence_name'] || config['token'] - let desiredTimeout = config['session_idle_timeout_seconds'] || MAX_SESSION_IDLE_TIMEOUT - - if (!isNumber(desiredTimeout)) { - logger.warn('session_idle_timeout_seconds must be a number. Defaulting to 30 minutes.') - desiredTimeout = MAX_SESSION_IDLE_TIMEOUT - } else if (desiredTimeout > MAX_SESSION_IDLE_TIMEOUT) { - logger.warn('session_idle_timeout_seconds cannot be greater than 30 minutes. Using 30 minutes instead.') - } else if (desiredTimeout < MIN_SESSION_IDLE_TIMEOUT) { - logger.warn('session_idle_timeout_seconds cannot be less than 60 seconds. Using 60 seconds instead.') - } + const desiredTimeout = config['session_idle_timeout_seconds'] || MAX_SESSION_IDLE_TIMEOUT this._sessionTimeoutMs = - Math.min(Math.max(desiredTimeout, MIN_SESSION_IDLE_TIMEOUT), MAX_SESSION_IDLE_TIMEOUT) * 1000 + clampToRange( + desiredTimeout, + MIN_SESSION_IDLE_TIMEOUT, + MAX_SESSION_IDLE_TIMEOUT, + 'session_idle_timeout_seconds' + ) * 1000 + this._window_id_storage_key = 'ph_' + persistenceName + '_window_id' this._primary_window_exists_storage_key = 'ph_' + persistenceName + '_primary_window_exists' diff --git a/src/utils/index.ts b/src/utils/index.ts index 435a107d8..39d4bec8e 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,7 +1,7 @@ import { Breaker, EventHandler, Properties } from '../types' -import { isArray, isFormData, isFunction, isNull, isNullish, isString, hasOwnProperty } from './type-utils' +import { hasOwnProperty, isArray, isFormData, isFunction, isNull, isNullish, isString } from './type-utils' import { logger } from './logger' -import { window, nativeForEach, nativeIndexOf } from './globals' +import { nativeForEach, nativeIndexOf, window } from './globals' const breaker: Breaker = {} diff --git a/src/utils/number-utils.ts b/src/utils/number-utils.ts new file mode 100644 index 000000000..e3ec455b8 --- /dev/null +++ b/src/utils/number-utils.ts @@ -0,0 +1,17 @@ +import { isNumber } from './type-utils' +import { logger } from './logger' + +export function clampToRange(value: unknown, min: number, max: number, label?: string): number { + if (!isNumber(value)) { + label && logger.warn(label + ' must be a number. Defaulting to max value:' + max) + return max + } else if (value > max) { + label && logger.warn(label + ' cannot be greater than max: ' + max + '. Using max value instead.') + return max + } else if (value < min) { + label && logger.warn(label + ' cannot be less than min: ' + min + '. Using min value instead.') + return min + } else { + return value + } +}