Skip to content

Commit

Permalink
fix: web vitals delayed capture (#1474)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Oct 16, 2024
1 parent 4ece26b commit 7304175
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
32 changes: 26 additions & 6 deletions src/__tests__/extensions/web-vitals.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { createPosthogInstance } from '../helpers/posthog-instance'
import { uuidv7 } from '../../uuidv7'
import { PostHog } from '../../posthog-core'
import { DecideResponse, SupportedWebVitalsMetrics } from '../../types'
import { DecideResponse, PerformanceCaptureConfig, SupportedWebVitalsMetrics } from '../../types'
import { assignableWindow } from '../../utils/globals'
import { FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS, FIFTEEN_MINUTES_IN_MILLIS } from '../../extensions/web-vitals'
import { DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS, FIFTEEN_MINUTES_IN_MILLIS } from '../../extensions/web-vitals'

jest.mock('../../utils/logger')
jest.useFakeTimers()
Expand Down Expand Up @@ -134,12 +134,12 @@ describe('web vitals', () => {
])
})

it('should emit after 8 seconds even when only 1 to 3 metrics captured', async () => {
it('should emit after 5 seconds even when only 1 to 3 metrics captured', async () => {
onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' })

expect(onCapture).toBeCalledTimes(0)

jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)
jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)

// for some reason advancing the timer emits a $pageview event as well 🤷
// expect(onCapture).toBeCalledTimes(2)
Expand All @@ -155,12 +155,32 @@ describe('web vitals', () => {
])
})

it('should emit after configured timeout even when only 1 to 3 metrics captured', async () => {
;(posthog.config.capture_performance as PerformanceCaptureConfig).web_vitals_delayed_flush_ms = 1000
onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' })

expect(onCapture).toBeCalledTimes(0)

jest.advanceTimersByTime(1000 + 1)

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 () => {
onCLSCallback?.({ name: 'CLS', value: FIFTEEN_MINUTES_IN_MILLIS, extra: 'property' })

expect(onCapture).toBeCalledTimes(0)

jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)
jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)

expect(onCapture.mock.calls).toEqual([])
})
Expand All @@ -171,7 +191,7 @@ describe('web vitals', () => {

expect(onCapture).toBeCalledTimes(0)

jest.advanceTimersByTime(FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)
jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1)

expect(onCapture).toBeCalledTimes(1)
})
Expand Down
11 changes: 9 additions & 2 deletions src/extensions/web-vitals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { assignableWindow, window } from '../../utils/globals'

type WebVitalsMetricCallback = (metric: any) => void

export const FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS = 8000
export const DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS = 5000
const ONE_MINUTE_IN_MILLIS = 60 * 1000
export const FIFTEEN_MINUTES_IN_MILLIS = 15 * ONE_MINUTE_IN_MILLIS

Expand Down Expand Up @@ -38,6 +38,13 @@ export class WebVitalsAutocapture {
: this.instance.persistence?.props[WEB_VITALS_ALLOWED_METRICS] || ['CLS', 'FCP', 'INP', 'LCP']
}

public get flushToCaptureTimeoutMs(): number {
const clientConfig: number | undefined = isObject(this.instance.config.capture_performance)
? this.instance.config.capture_performance.web_vitals_delayed_flush_ms
: undefined
return clientConfig || DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS
}

public get _maxAllowedValue(): number {
const configured =
isObject(this.instance.config.capture_performance) &&
Expand Down Expand Up @@ -163,7 +170,7 @@ export class WebVitalsAutocapture {
// poor performance is >4s, we wait twice that time to send
// this is in case we haven't received all metrics
// we'll at least gather some
this._delayedFlushTimer = setTimeout(this._flushToCapture, FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS)
this._delayedFlushTimer = setTimeout(this._flushToCapture, this.flushToCaptureTimeoutMs)
}

if (isUndefined(this.buffer.url)) {
Expand Down
6 changes: 6 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ export interface PerformanceCaptureConfig {
* NB setting this does not override whether the capture is enabled
*/
web_vitals_allowed_metrics?: SupportedWebVitalsMetrics[]
/**
* we delay flushing web vitals metrics to reduce the number of events we send
* this is the maximum time we will wait before sending the metrics
* if not set it defaults to 5 seconds
*/
web_vitals_delayed_flush_ms?: number
}

export interface HeatmapConfig {
Expand Down

0 comments on commit 7304175

Please sign in to comment.