From 2e8e3a5af925e821fc4485dd7e3fb13c16252d4d Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 6 Sep 2024 16:09:20 +0100 Subject: [PATCH 1/8] take a step --- src/__tests__/extensions/web-vitals.test.ts | 51 +++++++++++++++++++++ src/constants.ts | 1 + src/extensions/web-vitals/index.ts | 24 +++++++++- src/types.ts | 4 +- 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 74402533f..9e7655914 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -202,4 +202,55 @@ describe('web vitals', () => { } ) }) + + describe('sampling', () => { + it('does not capture when sample rate is 0', async () => { + posthog = await createPosthogInstance(uuidv7(), { + _onCapture: onCapture, + capture_performance: { web_vitals: true, web_vitals_sample_rate: 0 }, + }) + + posthog.webVitalsAutocapture!.afterDecideResponse({ + capturePerformance: { web_vitals: true }, + } as DecideResponse) + + emitAllMetrics() + + expect(onCapture).toBeCalledTimes(0) + }) + + it('captures roughly half the time when sample rate is 0.5', async () => { + posthog = await createPosthogInstance(uuidv7(), { + _onCapture: onCapture, + capture_performance: { web_vitals: true, web_vitals_sample_rate: 0.5 }, + }) + + posthog.webVitalsAutocapture!.afterDecideResponse({ + capturePerformance: { web_vitals: true }, + } as DecideResponse) + + // run this 100 times + for (let i = 0; i < 100; i++) { + emitAllMetrics() + } + + expect(onCapture.mock.calls.length).toBeGreaterThanOrEqual(46) + expect(onCapture.mock.calls.length).toBeLessThanOrEqual(54) + }) + + it('always captures when sample rate is 1', async () => { + posthog = await createPosthogInstance(uuidv7(), { + _onCapture: onCapture, + capture_performance: { web_vitals: true, web_vitals_sample_rate: 1 }, + }) + + posthog.webVitalsAutocapture!.afterDecideResponse({ + capturePerformance: { web_vitals: true }, + } as DecideResponse) + + emitAllMetrics() + + expect(onCapture).toBeCalledTimes(1) + }) + }) }) diff --git a/src/constants.ts b/src/constants.ts index 8913847fa..b9570370a 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -15,6 +15,7 @@ export const HEATMAPS_ENABLED_SERVER_SIDE = '$heatmaps_enabled_server_side' export const EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE = '$exception_capture_enabled_server_side' export const EXCEPTION_CAPTURE_ENDPOINT_SUFFIX = '$exception_capture_endpoint_suffix' export const WEB_VITALS_ENABLED_SERVER_SIDE = '$web_vitals_enabled_server_side' +export const WEB_VITALS_SAMPLE_RATE = '$web_vitals_sample_rate' export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side' export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side' export const SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE = '$session_recording_network_payload_capture' diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index 4376923cd..aecdf22aa 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -2,7 +2,7 @@ import { PostHog } from '../../posthog-core' import { DecideResponse } from '../../types' import { logger } from '../../utils/logger' import { isBoolean, isNullish, isNumber, isObject, isUndefined } from '../../utils/type-utils' -import { WEB_VITALS_ENABLED_SERVER_SIDE } from '../../constants' +import { WEB_VITALS_ENABLED_SERVER_SIDE, WEB_VITALS_SAMPLE_RATE } from '../../constants' import { assignableWindow, window } from '../../utils/globals' import Config from '../../config' @@ -19,9 +19,17 @@ export class WebVitalsAutocapture { private buffer: WebVitalsEventBuffer = { url: undefined, metrics: [], firstMetricTimestamp: undefined } private _delayedFlushTimer: ReturnType | undefined + private _sampleRate: number | undefined constructor(private readonly instance: PostHog) { this._enabledServerSide = !!this.instance.persistence?.props[WEB_VITALS_ENABLED_SERVER_SIDE] + const clientConfigSampleRate = isObject(this.instance.config.capture_performance) + ? this.instance.config.capture_performance?.web_vitals_sample_rate + : undefined + this._sampleRate = isUndefined(clientConfigSampleRate) + ? this.instance.persistence?.props[WEB_VITALS_SAMPLE_RATE] + : clientConfigSampleRate + this.startIfEnabled() } @@ -52,14 +60,22 @@ export class WebVitalsAutocapture { public afterDecideResponse(response: DecideResponse) { const webVitalsOptIn = isObject(response.capturePerformance) && !!response.capturePerformance.web_vitals + const webVitalsSampleRate = isObject(response.capturePerformance) + ? response.capturePerformance.web_vitals_sample_rate + : undefined if (this.instance.persistence) { this.instance.persistence.register({ [WEB_VITALS_ENABLED_SERVER_SIDE]: webVitalsOptIn, }) + this.instance.persistence.register({ + [WEB_VITALS_SAMPLE_RATE]: webVitalsSampleRate, + }) } // store this in-memory in case persistence is disabled this._enabledServerSide = webVitalsOptIn + // locally configured sample rate takes precedence over server config + this._sampleRate = isUndefined(this._sampleRate) ? webVitalsSampleRate : this._sampleRate this.startIfEnabled() } @@ -116,6 +132,12 @@ export class WebVitalsAutocapture { return } + const isSampled = isUndefined(this._sampleRate) ? true : Math.random() < this._sampleRate + if (!isSampled) { + logger.info(LOGGER_PREFIX + 'Dropping metric due to sample rate', metric) + return + } + this.buffer = this.buffer || { url: undefined, metrics: [], firstMetricTimestamp: undefined } const $currentUrl = this._currentURL() diff --git a/src/types.ts b/src/types.ts index adf912856..6158478d2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -96,7 +96,7 @@ export interface BootstrapConfig { export interface PerformanceCaptureConfig { /** works with session replay to use the browser's native performance observer to capture performance metrics */ network_timing?: boolean - /** works as a passenger event to use chrome's web vitals library to wrap fetch and capture web vitals */ + /** use chrome's web vitals library to wrap fetch and capture web vitals */ web_vitals?: boolean /** * We observe very large values reported by the Chrome web vitals library @@ -105,6 +105,8 @@ export interface PerformanceCaptureConfig { * if not set this defaults to 15 minutes */ __web_vitals_max_value?: number + web_vitals_sample_rate?: number + web_vitals_metrics?: true | ('LCP' | 'CLS' | 'FCP' | 'TBT' | 'INP')[] } export interface HeatmapConfig { From a8e5490c39f7e73f7df65901117a99b0d433cc62 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 6 Sep 2024 17:07:40 +0100 Subject: [PATCH 2/8] fiddling --- src/__tests__/extensions/web-vitals.test.ts | 69 ++++++++------------- src/constants.ts | 1 + src/extensions/web-vitals/index.ts | 49 +++++++++++---- src/types.ts | 10 ++- 4 files changed, 72 insertions(+), 57 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 9e7655914..8de05a691 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -203,54 +203,35 @@ describe('web vitals', () => { ) }) - describe('sampling', () => { - it('does not capture when sample rate is 0', async () => { - posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, - capture_performance: { web_vitals: true, web_vitals_sample_rate: 0 }, - }) - - posthog.webVitalsAutocapture!.afterDecideResponse({ - capturePerformance: { web_vitals: true }, - } as DecideResponse) - - emitAllMetrics() - - expect(onCapture).toBeCalledTimes(0) - }) - - it('captures roughly half the time when sample rate is 0.5', async () => { - posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, - capture_performance: { web_vitals: true, web_vitals_sample_rate: 0.5 }, - }) - - posthog.webVitalsAutocapture!.afterDecideResponse({ - capturePerformance: { web_vitals: true }, - } as DecideResponse) + describe.each(['client', 'server'])('sampling', (configSource) => { + describe.each([ + [0, 0, 0], + [1, 100, 100], + [0.5, 40, 60], + ])(`with sample rate of %f`, (sampleRate: number, min: number, max: number) => { + it(`with config from ${configSource} captures roughly half the time when sample rate is 0.5`, async () => { + posthog = await createPosthogInstance(uuidv7(), { + _onCapture: onCapture, + capture_performance: { + web_vitals: true, + web_vitals_sample_rate: configSource === 'client' ? sampleRate : undefined, + }, + }) - // run this 100 times - for (let i = 0; i < 100; i++) { - emitAllMetrics() - } + posthog.webVitalsAutocapture!.afterDecideResponse({ + capturePerformance: { + web_vitals: true, + web_vitals_sample_rate: configSource === 'server' ? sampleRate : undefined, + }, + } as DecideResponse) - expect(onCapture.mock.calls.length).toBeGreaterThanOrEqual(46) - expect(onCapture.mock.calls.length).toBeLessThanOrEqual(54) - }) + for (let i = 0; i < 100; i++) { + emitAllMetrics() + } - it('always captures when sample rate is 1', async () => { - posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, - capture_performance: { web_vitals: true, web_vitals_sample_rate: 1 }, + expect(onCapture.mock.calls.length).toBeGreaterThanOrEqual(min) + expect(onCapture.mock.calls.length).toBeLessThanOrEqual(max) }) - - posthog.webVitalsAutocapture!.afterDecideResponse({ - capturePerformance: { web_vitals: true }, - } as DecideResponse) - - emitAllMetrics() - - expect(onCapture).toBeCalledTimes(1) }) }) }) diff --git a/src/constants.ts b/src/constants.ts index b9570370a..95fc99142 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -16,6 +16,7 @@ export const EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE = '$exception_capture_enabled export const EXCEPTION_CAPTURE_ENDPOINT_SUFFIX = '$exception_capture_endpoint_suffix' export const WEB_VITALS_ENABLED_SERVER_SIDE = '$web_vitals_enabled_server_side' export const WEB_VITALS_SAMPLE_RATE = '$web_vitals_sample_rate' +export const WEB_VITALS_ALLOWED_METRICS = '$web_vitals_allowed_metrics' export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side' export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side' export const SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE = '$session_recording_network_payload_capture' diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index aecdf22aa..3005646dc 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -1,8 +1,8 @@ import { PostHog } from '../../posthog-core' -import { DecideResponse } from '../../types' +import { DecideResponse, SupportedWebVitalsMetrics } from '../../types' import { logger } from '../../utils/logger' import { isBoolean, isNullish, isNumber, isObject, isUndefined } from '../../utils/type-utils' -import { WEB_VITALS_ENABLED_SERVER_SIDE, WEB_VITALS_SAMPLE_RATE } from '../../constants' +import { WEB_VITALS_ALLOWED_METRICS, WEB_VITALS_ENABLED_SERVER_SIDE, WEB_VITALS_SAMPLE_RATE } from '../../constants' import { assignableWindow, window } from '../../utils/globals' import Config from '../../config' @@ -19,18 +19,31 @@ export class WebVitalsAutocapture { private buffer: WebVitalsEventBuffer = { url: undefined, metrics: [], firstMetricTimestamp: undefined } private _delayedFlushTimer: ReturnType | undefined - private _sampleRate: number | undefined constructor(private readonly instance: PostHog) { this._enabledServerSide = !!this.instance.persistence?.props[WEB_VITALS_ENABLED_SERVER_SIDE] + + this.startIfEnabled() + } + + public get sampleRate(): number | undefined { const clientConfigSampleRate = isObject(this.instance.config.capture_performance) ? this.instance.config.capture_performance?.web_vitals_sample_rate : undefined - this._sampleRate = isUndefined(clientConfigSampleRate) + return isUndefined(clientConfigSampleRate) ? this.instance.persistence?.props[WEB_VITALS_SAMPLE_RATE] : clientConfigSampleRate + } - this.startIfEnabled() + public get allowedMetrics(): SupportedWebVitalsMetrics[] { + const clientConfigMetricAllowList: SupportedWebVitalsMetrics[] | undefined = isObject( + this.instance.config.capture_performance + ) + ? this.instance.config.capture_performance?.web_vitals_metrics + : undefined + return !isUndefined(clientConfigMetricAllowList) + ? clientConfigMetricAllowList + : this.instance.persistence?.props[WEB_VITALS_ALLOWED_METRICS] || ['CLS', 'FCP', 'INP', 'LCP'] } public get _maxAllowedValue(): number { @@ -63,6 +76,9 @@ export class WebVitalsAutocapture { const webVitalsSampleRate = isObject(response.capturePerformance) ? response.capturePerformance.web_vitals_sample_rate : undefined + const allowedMetrics = isObject(response.capturePerformance) + ? response.capturePerformance.web_vitals_metrics + : undefined if (this.instance.persistence) { this.instance.persistence.register({ @@ -71,11 +87,12 @@ export class WebVitalsAutocapture { this.instance.persistence.register({ [WEB_VITALS_SAMPLE_RATE]: webVitalsSampleRate, }) + this.instance.persistence.register({ + [WEB_VITALS_ALLOWED_METRICS]: allowedMetrics, + }) } // store this in-memory in case persistence is disabled this._enabledServerSide = webVitalsOptIn - // locally configured sample rate takes precedence over server config - this._sampleRate = isUndefined(this._sampleRate) ? webVitalsSampleRate : this._sampleRate this.startIfEnabled() } @@ -132,7 +149,7 @@ export class WebVitalsAutocapture { return } - const isSampled = isUndefined(this._sampleRate) ? true : Math.random() < this._sampleRate + const isSampled = isUndefined(this.sampleRate) ? true : Math.random() < this.sampleRate if (!isSampled) { logger.info(LOGGER_PREFIX + 'Dropping metric due to sample rate', metric) return @@ -199,10 +216,18 @@ export class WebVitalsAutocapture { } // register performance observers - onLCP(this._addToBuffer) - onCLS(this._addToBuffer) - onFCP(this._addToBuffer) - onINP(this._addToBuffer) + if (this.allowedMetrics.indexOf('LCP') > -1) { + onLCP(this._addToBuffer) + } + if (this.allowedMetrics.indexOf('CLS') > -1) { + onCLS(this._addToBuffer) + } + if (this.allowedMetrics.indexOf('FCP') > -1) { + onFCP(this._addToBuffer) + } + if (this.allowedMetrics.indexOf('INP') > -1) { + onINP(this._addToBuffer) + } this._initialized = true } diff --git a/src/types.ts b/src/types.ts index 6158478d2..e56b011e7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -93,6 +93,8 @@ export interface BootstrapConfig { sessionID?: string } +export type SupportedWebVitalsMetrics = 'LCP' | 'CLS' | 'FCP' | 'INP' + export interface PerformanceCaptureConfig { /** works with session replay to use the browser's native performance observer to capture performance metrics */ network_timing?: boolean @@ -106,7 +108,13 @@ export interface PerformanceCaptureConfig { */ __web_vitals_max_value?: number web_vitals_sample_rate?: number - web_vitals_metrics?: true | ('LCP' | 'CLS' | 'FCP' | 'TBT' | 'INP')[] + /** + * By default all 4 metrics are captured + * You can set this config to restrict which metrics are captured + * e.g. ['CLS', 'FCP'] to only capture those two metrics + * NB setting this does not override whether the capture is enabled + */ + web_vitals_metrics?: SupportedWebVitalsMetrics[] } export interface HeatmapConfig { From 3c1325be79f938b4b393853bf3eac6b2fa7bd0ab Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 6 Sep 2024 17:19:01 +0100 Subject: [PATCH 3/8] more --- src/__tests__/extensions/web-vitals.test.ts | 205 ++++++++++++-------- src/extensions/web-vitals/index.ts | 4 +- 2 files changed, 124 insertions(+), 85 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 8de05a691..ad1c0b51e 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -1,7 +1,7 @@ import { createPosthogInstance } from '../helpers/posthog-instance' import { uuidv7 } from '../../uuidv7' import { PostHog } from '../../posthog-core' -import { DecideResponse } from '../../types' +import { DecideResponse, SupportedWebVitalsMetrics } from '../../types' import { assignableWindow } from '../../utils/globals' import { FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS, FIFTEEN_MINUTES_IN_MILLIS } from '../../extensions/web-vitals' @@ -44,107 +44,146 @@ describe('web vitals', () => { extra: 'property', }) - describe('the behaviour', () => { - beforeEach(async () => { - posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, - capture_performance: { web_vitals: true }, - // sometimes pageviews sneak in and make asserting on mock capture tricky - capture_pageview: false, - }) + describe.each([ + [ + undefined, + ['CLS', 'FCP', 'INP', 'LCP'] as SupportedWebVitalsMetrics[], + { + $web_vitals_LCP_event: expectedEmittedWebVitals('LCP'), + $web_vitals_LCP_value: 123.45, + $web_vitals_CLS_event: expectedEmittedWebVitals('CLS'), + $web_vitals_CLS_value: 123.45, + $web_vitals_FCP_event: expectedEmittedWebVitals('FCP'), + $web_vitals_FCP_value: 123.45, + $web_vitals_INP_event: expectedEmittedWebVitals('INP'), + $web_vitals_INP_value: 123.45, + }, + ], + [ + ['CLS', 'FCP', 'INP', 'LCP'] as SupportedWebVitalsMetrics[], + ['CLS', 'FCP', 'INP', 'LCP'] as SupportedWebVitalsMetrics[], + { + $web_vitals_LCP_event: expectedEmittedWebVitals('LCP'), + $web_vitals_LCP_value: 123.45, + $web_vitals_CLS_event: expectedEmittedWebVitals('CLS'), + $web_vitals_CLS_value: 123.45, + $web_vitals_FCP_event: expectedEmittedWebVitals('FCP'), + $web_vitals_FCP_value: 123.45, + $web_vitals_INP_event: expectedEmittedWebVitals('INP'), + $web_vitals_INP_value: 123.45, + }, + ], + [ + ['CLS', 'FCP'] as SupportedWebVitalsMetrics[], + ['CLS', 'FCP'] as SupportedWebVitalsMetrics[], + { + $web_vitals_CLS_event: expectedEmittedWebVitals('CLS'), + $web_vitals_CLS_value: 123.45, + $web_vitals_FCP_event: expectedEmittedWebVitals('FCP'), + $web_vitals_FCP_value: 123.45, + }, + ], + ])( + 'the behaviour when client config is %s', + ( + clientConfig: SupportedWebVitalsMetrics[] | undefined, + expectedAllowedMetrics: SupportedWebVitalsMetrics[], + expectedProperties: Record + ) => { + beforeEach(async () => { + posthog = await createPosthogInstance(uuidv7(), { + _onCapture: onCapture, + capture_performance: { web_vitals: true, web_vitals_metrics: clientConfig }, + // sometimes pageviews sneak in and make asserting on mock capture tricky + capture_pageview: false, + }) - loadScriptMock.mockImplementation((_path, callback) => { - // we need a set of fake web vitals handlers, so we can manually trigger the events - assignableWindow.postHogWebVitalsCallbacks = { - onLCP: (cb: any) => { - onLCPCallback = cb - }, - onCLS: (cb: any) => { - onCLSCallback = cb - }, - onFCP: (cb: any) => { - onFCPCallback = cb - }, - onINP: (cb: any) => { - onINPCallback = cb - }, - } - callback() + loadScriptMock.mockImplementation((_path, callback) => { + // we need a set of fake web vitals handlers, so we can manually trigger the events + assignableWindow.postHogWebVitalsCallbacks = { + onLCP: (cb: any) => { + onLCPCallback = cb + }, + onCLS: (cb: any) => { + onCLSCallback = cb + }, + onFCP: (cb: any) => { + onFCPCallback = cb + }, + onINP: (cb: any) => { + onINPCallback = cb + }, + } + callback() + }) + + posthog.requestRouter.loadScript = loadScriptMock + + // need to force this to get the web vitals script loaded + posthog.webVitalsAutocapture!.afterDecideResponse({ + capturePerformance: { web_vitals: true }, + } as unknown as DecideResponse) + + expect(posthog.webVitalsAutocapture.allowedMetrics).toEqual(expectedAllowedMetrics) }) - posthog.requestRouter.loadScript = loadScriptMock + it('should emit when all allowed metrics are captured', async () => { + emitAllMetrics() - // need to force this to get the web vitals script loaded - posthog.webVitalsAutocapture!.afterDecideResponse({ - capturePerformance: { web_vitals: true }, - } as unknown as DecideResponse) - }) + expect(onCapture).toBeCalledTimes(1) - it('should emit when all 4 metrics are captured', async () => { - emitAllMetrics() - - expect(onCapture).toBeCalledTimes(1) - - expect(onCapture.mock.lastCall).toMatchObject([ - '$web_vitals', - { - event: '$web_vitals', - properties: { - $web_vitals_LCP_event: expectedEmittedWebVitals('LCP'), - $web_vitals_LCP_value: 123.45, - $web_vitals_CLS_event: expectedEmittedWebVitals('CLS'), - $web_vitals_CLS_value: 123.45, - $web_vitals_FCP_event: expectedEmittedWebVitals('FCP'), - $web_vitals_FCP_value: 123.45, - $web_vitals_INP_event: expectedEmittedWebVitals('INP'), - $web_vitals_INP_value: 123.45, + expect(onCapture.mock.lastCall).toMatchObject([ + '$web_vitals', + { + event: '$web_vitals', + properties: expectedProperties, }, - }, - ]) - }) + ]) + }) - it('should emit after 8 seconds even when only 1 to 3 metrics captured', async () => { - randomlyAddAMetric('LCP', 123.45, { extra: 'property' }) + it('should emit after 8 seconds even when only 1 to 3 metrics captured', async () => { + randomlyAddAMetric('CLS', 123.45, { extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(onCapture).toBeCalledTimes(0) - jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) + jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - // for some reason advancing the timer emits a $pageview event as well 🤷 - // expect(onCapture).toBeCalledTimes(2) - expect(onCapture.mock.lastCall).toMatchObject([ - '$web_vitals', - { - event: '$web_vitals', - properties: { - $web_vitals_LCP_event: expectedEmittedWebVitals('LCP'), - $web_vitals_LCP_value: 123.45, + // for some reason advancing the timer emits a $pageview event as well 🤷 + // expect(onCapture).toBeCalledTimes(2) + expect(onCapture.mock.lastCall).toMatchObject([ + '$web_vitals', + { + event: '$web_vitals', + properties: { + $web_vitals_CLS_event: expectedEmittedWebVitals('CLS'), + $web_vitals_CLS_value: 123.45, + }, }, - }, - ]) - }) + ]) + }) - it('should ignore a ridiculous value', async () => { - randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) + it('should ignore a ridiculous value', async () => { + randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(onCapture).toBeCalledTimes(0) - jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) + jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - expect(onCapture).toBeCalledTimes(0) - }) + expect(onCapture).toBeCalledTimes(0) + }) - it('can be configured not to ignore a ridiculous value', async () => { - posthog.config.capture_performance = { __web_vitals_max_value: 0 } - randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) + it('can be configured not to ignore a ridiculous value', async () => { + posthog.config.capture_performance = { __web_vitals_max_value: 0 } + randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(onCapture).toBeCalledTimes(0) - jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) + jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - expect(onCapture).toBeCalledTimes(1) - }) - }) + expect(onCapture).toBeCalledTimes(1) + }) + } + ) describe('afterDecideResponse()', () => { beforeEach(async () => { diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index 3005646dc..12bebf74a 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -201,8 +201,8 @@ export class WebVitalsAutocapture { timestamp: Date.now(), }) - if (this.buffer.metrics.length === 4) { - // we have all 4 metrics + if (this.buffer.metrics.length === this.allowedMetrics.length) { + // we have all allowed metrics this._flushToCapture() } } From 8c590c312cf2e998c934d3ddc9a8ad500822a7c7 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 6 Sep 2024 17:25:42 +0100 Subject: [PATCH 4/8] add INP attribution --- src/entrypoints/web-vitals.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/entrypoints/web-vitals.ts b/src/entrypoints/web-vitals.ts index 1bef37d27..4f51b2a04 100644 --- a/src/entrypoints/web-vitals.ts +++ b/src/entrypoints/web-vitals.ts @@ -1,4 +1,5 @@ -import { onLCP, onINP, onCLS, onFCP } from 'web-vitals' +import { onLCP, onCLS, onFCP } from 'web-vitals' +import { onINP } from 'web-vitals/attribution' import { assignableWindow } from '../utils/globals' // TODO export types here as well? From 3bca8b265f54c8f1735c2a9dd25683b208eed88b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 6 Sep 2024 18:42:31 +0100 Subject: [PATCH 5/8] fix --- src/__tests__/extensions/web-vitals.test.ts | 28 ++++++++++++++++++--- src/entrypoints/web-vitals.ts | 5 ++-- src/extensions/web-vitals/index.ts | 22 +++++++++++----- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index ad1c0b51e..3e6b6d815 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -91,6 +91,7 @@ describe('web vitals', () => { expectedProperties: Record ) => { beforeEach(async () => { + onCapture.mockClear() posthog = await createPosthogInstance(uuidv7(), { _onCapture: onCapture, capture_performance: { web_vitals: true, web_vitals_metrics: clientConfig }, @@ -100,7 +101,8 @@ describe('web vitals', () => { loadScriptMock.mockImplementation((_path, callback) => { // we need a set of fake web vitals handlers, so we can manually trigger the events - assignableWindow.postHogWebVitalsCallbacks = { + assignableWindow.__PosthogExtensions__ = {} + assignableWindow.__PosthogExtensions__.postHogWebVitalsCallbacks = { onLCP: (cb: any) => { onLCPCallback = cb }, @@ -187,8 +189,9 @@ describe('web vitals', () => { describe('afterDecideResponse()', () => { beforeEach(async () => { - // we need a set of fake web vitals handlers so we can manually trigger the events - assignableWindow.postHogWebVitalsCallbacks = { + // we need a set of fake web vitals handlers, so we can manually trigger the events + assignableWindow.__PosthogExtensions__ = {} + assignableWindow.__PosthogExtensions__.postHogWebVitalsCallbacks = { onLCP: (cb: any) => { onLCPCallback = cb }, @@ -248,6 +251,25 @@ describe('web vitals', () => { [1, 100, 100], [0.5, 40, 60], ])(`with sample rate of %f`, (sampleRate: number, min: number, max: number) => { + beforeEach(async () => { + // we need a set of fake web vitals handlers, so we can manually trigger the events + assignableWindow.__PosthogExtensions__ = {} + assignableWindow.__PosthogExtensions__.postHogWebVitalsCallbacks = { + onLCP: (cb: any) => { + onLCPCallback = cb + }, + onCLS: (cb: any) => { + onCLSCallback = cb + }, + onFCP: (cb: any) => { + onFCPCallback = cb + }, + onINP: (cb: any) => { + onINPCallback = cb + }, + } + }) + it(`with config from ${configSource} captures roughly half the time when sample rate is 0.5`, async () => { posthog = await createPosthogInstance(uuidv7(), { _onCapture: onCapture, diff --git a/src/entrypoints/web-vitals.ts b/src/entrypoints/web-vitals.ts index 4f51b2a04..acfc77192 100644 --- a/src/entrypoints/web-vitals.ts +++ b/src/entrypoints/web-vitals.ts @@ -2,8 +2,6 @@ import { onLCP, onCLS, onFCP } from 'web-vitals' import { onINP } from 'web-vitals/attribution' import { assignableWindow } from '../utils/globals' -// TODO export types here as well? - const postHogWebVitalsCallbacks = { onLCP, onCLS, @@ -11,6 +9,7 @@ const postHogWebVitalsCallbacks = { onINP, } -assignableWindow.postHogWebVitalsCallbacks = postHogWebVitalsCallbacks +assignableWindow.__PosthogExtensions__ = assignableWindow.__PosthogExtensions__ || {} +assignableWindow.__POSTHOG_EXTENSIONS__postHogWebVitalsCallbacks = postHogWebVitalsCallbacks export default postHogWebVitalsCallbacks diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index 12bebf74a..747bdc6d3 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -6,6 +6,8 @@ import { WEB_VITALS_ALLOWED_METRICS, WEB_VITALS_ENABLED_SERVER_SIDE, WEB_VITALS_ import { assignableWindow, window } from '../../utils/globals' import Config from '../../config' +type WebVitalsMetricCallback = (metric: any) => void + export const FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS = 8000 const ONE_MINUTE_IN_MILLIS = 60 * 1000 export const FIFTEEN_MINUTES_IN_MILLIS = 15 * ONE_MINUTE_IN_MILLIS @@ -98,7 +100,7 @@ export class WebVitalsAutocapture { } private loadScript(cb: () => void): void { - if ((window as any).postHogWebVitalsCallbacks) { + if (assignableWindow.__PosthogExtensions__?.postHogWebVitalsCallbacks) { // already loaded cb() } @@ -208,7 +210,15 @@ export class WebVitalsAutocapture { } private _startCapturing = () => { - const { onLCP, onCLS, onFCP, onINP } = assignableWindow.postHogWebVitalsCallbacks + let onLCP: WebVitalsMetricCallback | undefined + let onCLS: WebVitalsMetricCallback | undefined + let onFCP: WebVitalsMetricCallback | undefined + let onINP: WebVitalsMetricCallback | undefined + + const posthogExtensions = assignableWindow.__PosthogExtensions__ + if (!isUndefined(posthogExtensions)) { + ;({ onLCP, onCLS, onFCP, onINP } = posthogExtensions.postHogWebVitalsCallbacks) + } if (!onLCP || !onCLS || !onFCP || !onINP) { logger.error(LOGGER_PREFIX + 'web vitals callbacks not loaded - not starting') @@ -217,16 +227,16 @@ export class WebVitalsAutocapture { // register performance observers if (this.allowedMetrics.indexOf('LCP') > -1) { - onLCP(this._addToBuffer) + onLCP(this._addToBuffer.bind(this)) } if (this.allowedMetrics.indexOf('CLS') > -1) { - onCLS(this._addToBuffer) + onCLS(this._addToBuffer.bind(this)) } if (this.allowedMetrics.indexOf('FCP') > -1) { - onFCP(this._addToBuffer) + onFCP(this._addToBuffer.bind(this)) } if (this.allowedMetrics.indexOf('INP') > -1) { - onINP(this._addToBuffer) + onINP(this._addToBuffer.bind(this)) } this._initialized = true From 8043fa89c1423694ed1d3f33478d30c7cee35422 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 7 Sep 2024 11:45:05 +0100 Subject: [PATCH 6/8] fix --- src/__tests__/extensions/web-vitals.test.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 3e6b6d815..8cb2e95c0 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -17,16 +17,6 @@ describe('web vitals', () => { let onINPCallback: ((metric: Record) => void) | undefined = undefined const loadScriptMock = jest.fn() - const randomlyAddAMetric = ( - metricName: string = 'metric', - metricValue: number = 600.1, - metricProperties: Record = {} - ) => { - const callbacks = [onLCPCallback, onCLSCallback, onFCPCallback, onINPCallback] - const randomIndex = Math.floor(Math.random() * callbacks.length) - callbacks[randomIndex]?.({ name: metricName, value: metricValue, ...metricProperties }) - } - const emitAllMetrics = () => { onLCPCallback?.({ name: 'LCP', value: 123.45, extra: 'property' }) onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' }) @@ -144,7 +134,7 @@ describe('web vitals', () => { }) it('should emit after 8 seconds even when only 1 to 3 metrics captured', async () => { - randomlyAddAMetric('CLS', 123.45, { extra: 'property' }) + onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' }) expect(onCapture).toBeCalledTimes(0) @@ -165,18 +155,18 @@ describe('web vitals', () => { }) it('should ignore a ridiculous value', async () => { - randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) + onCLSCallback?.({ name: 'CLS', value: FIFTEEN_MINUTES_IN_MILLIS, extra: 'property' }) expect(onCapture).toBeCalledTimes(0) jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - expect(onCapture).toBeCalledTimes(0) + expect(onCapture.mock.calls).toEqual([]) }) it('can be configured not to ignore a ridiculous value', async () => { posthog.config.capture_performance = { __web_vitals_max_value: 0 } - randomlyAddAMetric('LCP', FIFTEEN_MINUTES_IN_MILLIS, { extra: 'property' }) + onCLSCallback?.({ name: 'CLS', value: FIFTEEN_MINUTES_IN_MILLIS, extra: 'property' }) expect(onCapture).toBeCalledTimes(0) From 6dced38909453f38865a926648acc6cc26adffa2 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 7 Sep 2024 12:08:40 +0100 Subject: [PATCH 7/8] fix --- src/entrypoints/web-vitals.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/entrypoints/web-vitals.ts b/src/entrypoints/web-vitals.ts index acfc77192..0335e86d9 100644 --- a/src/entrypoints/web-vitals.ts +++ b/src/entrypoints/web-vitals.ts @@ -10,6 +10,6 @@ const postHogWebVitalsCallbacks = { } assignableWindow.__PosthogExtensions__ = assignableWindow.__PosthogExtensions__ || {} -assignableWindow.__POSTHOG_EXTENSIONS__postHogWebVitalsCallbacks = postHogWebVitalsCallbacks +assignableWindow.__PosthogExtensions__.postHogWebVitalsCallbacks = postHogWebVitalsCallbacks export default postHogWebVitalsCallbacks From c4d50160f6a2f34da1365f28a8fc1df326b671c0 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 7 Sep 2024 12:13:30 +0100 Subject: [PATCH 8/8] remove sampling --- src/__tests__/extensions/web-vitals.test.ts | 53 +-------------------- src/constants.ts | 1 - src/extensions/web-vitals/index.ts | 29 ++--------- src/types.ts | 3 +- 4 files changed, 7 insertions(+), 79 deletions(-) diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 8cb2e95c0..37233f0c5 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -84,7 +84,7 @@ describe('web vitals', () => { onCapture.mockClear() posthog = await createPosthogInstance(uuidv7(), { _onCapture: onCapture, - capture_performance: { web_vitals: true, web_vitals_metrics: clientConfig }, + capture_performance: { web_vitals: true, web_vitals_allowed_metrics: clientConfig }, // sometimes pageviews sneak in and make asserting on mock capture tricky capture_pageview: false, }) @@ -234,55 +234,4 @@ describe('web vitals', () => { } ) }) - - describe.each(['client', 'server'])('sampling', (configSource) => { - describe.each([ - [0, 0, 0], - [1, 100, 100], - [0.5, 40, 60], - ])(`with sample rate of %f`, (sampleRate: number, min: number, max: number) => { - beforeEach(async () => { - // we need a set of fake web vitals handlers, so we can manually trigger the events - assignableWindow.__PosthogExtensions__ = {} - assignableWindow.__PosthogExtensions__.postHogWebVitalsCallbacks = { - onLCP: (cb: any) => { - onLCPCallback = cb - }, - onCLS: (cb: any) => { - onCLSCallback = cb - }, - onFCP: (cb: any) => { - onFCPCallback = cb - }, - onINP: (cb: any) => { - onINPCallback = cb - }, - } - }) - - it(`with config from ${configSource} captures roughly half the time when sample rate is 0.5`, async () => { - posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, - capture_performance: { - web_vitals: true, - web_vitals_sample_rate: configSource === 'client' ? sampleRate : undefined, - }, - }) - - posthog.webVitalsAutocapture!.afterDecideResponse({ - capturePerformance: { - web_vitals: true, - web_vitals_sample_rate: configSource === 'server' ? sampleRate : undefined, - }, - } as DecideResponse) - - for (let i = 0; i < 100; i++) { - emitAllMetrics() - } - - expect(onCapture.mock.calls.length).toBeGreaterThanOrEqual(min) - expect(onCapture.mock.calls.length).toBeLessThanOrEqual(max) - }) - }) - }) }) diff --git a/src/constants.ts b/src/constants.ts index 95fc99142..f9e608e17 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -15,7 +15,6 @@ export const HEATMAPS_ENABLED_SERVER_SIDE = '$heatmaps_enabled_server_side' export const EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE = '$exception_capture_enabled_server_side' export const EXCEPTION_CAPTURE_ENDPOINT_SUFFIX = '$exception_capture_endpoint_suffix' export const WEB_VITALS_ENABLED_SERVER_SIDE = '$web_vitals_enabled_server_side' -export const WEB_VITALS_SAMPLE_RATE = '$web_vitals_sample_rate' export const WEB_VITALS_ALLOWED_METRICS = '$web_vitals_allowed_metrics' export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side' export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side' diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index 747bdc6d3..622fde5e0 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -2,7 +2,7 @@ import { PostHog } from '../../posthog-core' import { DecideResponse, SupportedWebVitalsMetrics } from '../../types' import { logger } from '../../utils/logger' import { isBoolean, isNullish, isNumber, isObject, isUndefined } from '../../utils/type-utils' -import { WEB_VITALS_ALLOWED_METRICS, WEB_VITALS_ENABLED_SERVER_SIDE, WEB_VITALS_SAMPLE_RATE } from '../../constants' +import { WEB_VITALS_ALLOWED_METRICS, WEB_VITALS_ENABLED_SERVER_SIDE } from '../../constants' import { assignableWindow, window } from '../../utils/globals' import Config from '../../config' @@ -28,20 +28,11 @@ export class WebVitalsAutocapture { this.startIfEnabled() } - public get sampleRate(): number | undefined { - const clientConfigSampleRate = isObject(this.instance.config.capture_performance) - ? this.instance.config.capture_performance?.web_vitals_sample_rate - : undefined - return isUndefined(clientConfigSampleRate) - ? this.instance.persistence?.props[WEB_VITALS_SAMPLE_RATE] - : clientConfigSampleRate - } - public get allowedMetrics(): SupportedWebVitalsMetrics[] { const clientConfigMetricAllowList: SupportedWebVitalsMetrics[] | undefined = isObject( this.instance.config.capture_performance ) - ? this.instance.config.capture_performance?.web_vitals_metrics + ? this.instance.config.capture_performance?.web_vitals_allowed_metrics : undefined return !isUndefined(clientConfigMetricAllowList) ? clientConfigMetricAllowList @@ -75,20 +66,16 @@ export class WebVitalsAutocapture { public afterDecideResponse(response: DecideResponse) { const webVitalsOptIn = isObject(response.capturePerformance) && !!response.capturePerformance.web_vitals - const webVitalsSampleRate = isObject(response.capturePerformance) - ? response.capturePerformance.web_vitals_sample_rate - : undefined + const allowedMetrics = isObject(response.capturePerformance) - ? response.capturePerformance.web_vitals_metrics + ? response.capturePerformance.web_vitals_allowed_metrics : undefined if (this.instance.persistence) { this.instance.persistence.register({ [WEB_VITALS_ENABLED_SERVER_SIDE]: webVitalsOptIn, }) - this.instance.persistence.register({ - [WEB_VITALS_SAMPLE_RATE]: webVitalsSampleRate, - }) + this.instance.persistence.register({ [WEB_VITALS_ALLOWED_METRICS]: allowedMetrics, }) @@ -151,12 +138,6 @@ export class WebVitalsAutocapture { return } - const isSampled = isUndefined(this.sampleRate) ? true : Math.random() < this.sampleRate - if (!isSampled) { - logger.info(LOGGER_PREFIX + 'Dropping metric due to sample rate', metric) - return - } - this.buffer = this.buffer || { url: undefined, metrics: [], firstMetricTimestamp: undefined } const $currentUrl = this._currentURL() diff --git a/src/types.ts b/src/types.ts index e56b011e7..bedd03c98 100644 --- a/src/types.ts +++ b/src/types.ts @@ -107,14 +107,13 @@ export interface PerformanceCaptureConfig { * if not set this defaults to 15 minutes */ __web_vitals_max_value?: number - web_vitals_sample_rate?: number /** * By default all 4 metrics are captured * You can set this config to restrict which metrics are captured * e.g. ['CLS', 'FCP'] to only capture those two metrics * NB setting this does not override whether the capture is enabled */ - web_vitals_metrics?: SupportedWebVitalsMetrics[] + web_vitals_allowed_metrics?: SupportedWebVitalsMetrics[] } export interface HeatmapConfig {