From 3db6e521cd57cc6cea0da0376583cef7fc527221 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Tue, 31 Aug 2021 16:14:09 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9C=A8=20Compute=20CLS=20using=20the?= =?UTF-8?q?=20new=20definition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/browser/performanceCollection.ts | 1 + .../view/trackViewMetrics.spec.ts | 10 +++- .../view/trackViewMetrics.ts | 56 ++++++++++++++++--- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/packages/rum-core/src/browser/performanceCollection.ts b/packages/rum-core/src/browser/performanceCollection.ts index bd2112712b..f4f6d41c9e 100644 --- a/packages/rum-core/src/browser/performanceCollection.ts +++ b/packages/rum-core/src/browser/performanceCollection.ts @@ -74,6 +74,7 @@ export interface RumFirstInputTiming { export interface RumLayoutShiftTiming { entryType: 'layout-shift' + startTime: RelativeTime value: number hadRecentInput: boolean } diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index fe90c46625..f785683a05 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -1,4 +1,4 @@ -import { Context, RelativeTime, Duration } from '@datadog/browser-core' +import { Context, RelativeTime, Duration, relativeNow } from '@datadog/browser-core' import { LifeCycleEventType, RumEvent } from '@datadog/browser-rum-core' import { TestSetupBuilder, setup, setupViewTest, ViewTest } from '../../../../test/specHelper' import { RumPerformanceNavigationTiming } from '../../../browser/performanceCollection' @@ -311,15 +311,17 @@ describe('rum track view metrics', () => { it('should accumulate layout shift values', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - + const now = relativeNow() lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', + startTime: now, hadRecentInput: false, value: 0.1, }) lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', + startTime: (now + 100) as RelativeTime, hadRecentInput: false, value: 0.2, }) @@ -333,15 +335,18 @@ describe('rum track view metrics', () => { it('should round the cumulative layout shift value to 4 decimals', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest + const now = relativeNow() lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', + startTime: now, hadRecentInput: false, value: 1.23456789, }) lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', + startTime: (now + 100) as RelativeTime, hadRecentInput: false, value: 1.11111111111, }) @@ -358,6 +363,7 @@ describe('rum track view metrics', () => { lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', + startTime: relativeNow(), hadRecentInput: true, value: 0.1, }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index 51446c23e9..2037722980 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -1,5 +1,14 @@ -import { Duration, noop, elapsed, round, timeStampNow, Configuration } from '@datadog/browser-core' -import { supportPerformanceTimingEvent } from '../../../browser/performanceCollection' +import { + Duration, + noop, + elapsed, + round, + timeStampNow, + Configuration, + RelativeTime, + ONE_SECOND, +} from '@datadog/browser-core' +import { RumLayoutShiftTiming, supportPerformanceTimingEvent } from '../../../browser/performanceCollection' import { ViewLoadingType } from '../../../rawRumEvent.types' import { LifeCycle, LifeCycleEventType } from '../../lifeCycle' import { EventCounts, trackEventCounts } from '../../trackEventCounts' @@ -47,8 +56,8 @@ export function trackViewMetrics( let stopCLSTracking: () => void if (isLayoutShiftSupported()) { viewMetrics.cumulativeLayoutShift = 0 - ;({ stop: stopCLSTracking } = trackLayoutShift(lifeCycle, (layoutShift) => { - viewMetrics.cumulativeLayoutShift = round(viewMetrics.cumulativeLayoutShift! + layoutShift, 4) + ;({ stop: stopCLSTracking } = trackCumulativeLayoutShift(lifeCycle, (layoutShift) => { + viewMetrics.cumulativeLayoutShift = Math.max(viewMetrics.cumulativeLayoutShift!, round(layoutShift, 4)) scheduleViewUpdate() })) } else { @@ -120,18 +129,47 @@ function trackActivityLoadingTime( } /** - * Track layout shifts (LS) occurring during the Views. This yields multiple values that can be - * added up to compute the cumulated layout shift (CLS). + * Track the cumulative layout shifts (CLS). + * Layout shifts are grouped into session windows. + * The minimum gap between session windows is 1 second. + * The maximum duration of a session window is 5 second. + * The session window layout shift value is the sum of layout shifts inside it. + * The CLS value is the max of session windows values. + * + * This yields a new value whenever the CLS value is updated (a higher session window value is computed). * * See isLayoutShiftSupported to check for browser support. * - * Documentation: https://web.dev/cls/ + * Documentation: + * https://web.dev/cls/ + * https://web.dev/evolving-cls/ * Reference implementation: https://github.com/GoogleChrome/web-vitals/blob/master/src/getCLS.ts */ -function trackLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift: number) => void) { +function trackCumulativeLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift: number) => void) { + let clsValue = 0 + let windowValue = 0 + let windowStartedAt = 0 as RelativeTime + let windowLastEntryAt = 0 as RelativeTime + + const updateSessionWindow = (entry: RumLayoutShiftTiming) => { + const durationSinceStart = entry.startTime - windowStartedAt + const durationSinceLastEntry = entry.startTime - windowLastEntryAt + if (durationSinceLastEntry < 1 * ONE_SECOND && durationSinceStart < 5 * ONE_SECOND) { + windowValue += entry.value + } else { + windowValue = entry.value + windowStartedAt = entry.startTime + } + windowLastEntryAt = entry.startTime + } + const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => { if (entry.entryType === 'layout-shift' && !entry.hadRecentInput) { - callback(entry.value) + updateSessionWindow(entry) + if (windowValue > clsValue) { + clsValue = windowValue + callback(clsValue) + } } }) From cc9027b76bbc3921fada96ac0008ee91799bf1ce Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Wed, 1 Sep 2021 12:02:55 +0200 Subject: [PATCH 02/10] =?UTF-8?q?=F0=9F=8E=A8=20Refactor=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.spec.ts | 55 ++++++------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index f785683a05..182bb78538 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -3,6 +3,7 @@ import { LifeCycleEventType, RumEvent } from '@datadog/browser-rum-core' import { TestSetupBuilder, setup, setupViewTest, ViewTest } from '../../../../test/specHelper' import { RumPerformanceNavigationTiming } from '../../../browser/performanceCollection' import { RumEventType } from '../../../rawRumEvent.types' +import { LifeCycle } from '../../lifeCycle' import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, @@ -281,6 +282,14 @@ describe('rum track view metrics', () => { describe('cumulativeLayoutShift', () => { let isLayoutShiftSupported: boolean + const newLayoutShift = (lifeCycle: LifeCycle, value: number, hadRecentInput = false) => { + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { + entryType: 'layout-shift', + startTime: relativeNow(), + hadRecentInput, + value, + }) + } beforeEach(() => { if (!('PerformanceObserver' in window) || !('supportedEntryTypes' in PerformanceObserver)) { pending('No PerformanceObserver support') @@ -308,24 +317,12 @@ describe('rum track view metrics', () => { expect(getViewUpdate(0).cumulativeLayoutShift).toBe(undefined) }) - it('should accumulate layout shift values', () => { + it('should accumulate layout shift values for the first session window', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - const now = relativeNow() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { - entryType: 'layout-shift', - startTime: now, - hadRecentInput: false, - value: 0.1, - }) - - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { - entryType: 'layout-shift', - startTime: (now + 100) as RelativeTime, - hadRecentInput: false, - value: 0.2, - }) - + newLayoutShift(lifeCycle, 0.1) + clock.tick(100) + newLayoutShift(lifeCycle, 0.2) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(2) @@ -335,22 +332,9 @@ describe('rum track view metrics', () => { it('should round the cumulative layout shift value to 4 decimals', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - const now = relativeNow() - - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { - entryType: 'layout-shift', - startTime: now, - hadRecentInput: false, - value: 1.23456789, - }) - - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { - entryType: 'layout-shift', - startTime: (now + 100) as RelativeTime, - hadRecentInput: false, - value: 1.11111111111, - }) - + newLayoutShift(lifeCycle, 1.23456789) + clock.tick(100) + newLayoutShift(lifeCycle, 1.11111111111) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(2) @@ -361,12 +345,7 @@ describe('rum track view metrics', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { - entryType: 'layout-shift', - startTime: relativeNow(), - hadRecentInput: true, - value: 0.1, - }) + newLayoutShift(lifeCycle, 0.1, true) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) From 733f765bc23b7b0aacfbcb12387e86f611d92a04 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Wed, 1 Sep 2021 12:26:20 +0200 Subject: [PATCH 03/10] =?UTF-8?q?=E2=9C=85=20Add=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.spec.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 182bb78538..9cfc7ac39d 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -352,5 +352,58 @@ describe('rum track view metrics', () => { expect(getViewUpdateCount()).toEqual(1) expect(getViewUpdate(0).cumulativeLayoutShift).toBe(0) }) + + it('should create a new session window if the gap is more than 1 second', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest + // first session window + newLayoutShift(lifeCycle, 0.1) + clock.tick(100) + newLayoutShift(lifeCycle, 0.2) + // second session window + clock.tick(1001) + newLayoutShift(lifeCycle, 0.1) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + expect(getViewUpdateCount()).toEqual(2) + expect(getViewUpdate(1).cumulativeLayoutShift).toBe(0.3) + }) + + it('should create a new session if the current session is more than 5 second', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest + // first session window + newLayoutShift(lifeCycle, 0.1) + clock.tick(4500) + newLayoutShift(lifeCycle, 0.2) + // second session window + clock.tick(501) + newLayoutShift(lifeCycle, 0.1) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.3) + }) + + it('should get the max value sessions', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdate, getViewUpdateCount } = viewTest + // first session window + newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, 0.2) + // second session window + clock.tick(5001) + newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, 0.2) + // third session window + clock.tick(5001) + newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, 0.2) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.5) + }) }) }) From b3306ea2a474fed70fde9d5cdf2795dadf029695 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Wed, 1 Sep 2021 12:52:43 +0200 Subject: [PATCH 04/10] =?UTF-8?q?=F0=9F=8E=A8=20Avoid=20computing=20the=20?= =?UTF-8?q?max=20twice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/rumEventsCollection/view/trackViewMetrics.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index 2037722980..16d96f8a8d 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -57,7 +57,7 @@ export function trackViewMetrics( if (isLayoutShiftSupported()) { viewMetrics.cumulativeLayoutShift = 0 ;({ stop: stopCLSTracking } = trackCumulativeLayoutShift(lifeCycle, (layoutShift) => { - viewMetrics.cumulativeLayoutShift = Math.max(viewMetrics.cumulativeLayoutShift!, round(layoutShift, 4)) + viewMetrics.cumulativeLayoutShift = layoutShift scheduleViewUpdate() })) } else { @@ -168,7 +168,7 @@ function trackCumulativeLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift updateSessionWindow(entry) if (windowValue > clsValue) { clsValue = windowValue - callback(clsValue) + callback(round(clsValue, 4)) } } }) From 596901a8a78ed17e3d00baf0f126a2ca998655fa Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Fri, 3 Sep 2021 10:10:58 +0200 Subject: [PATCH 05/10] =?UTF-8?q?=F0=9F=91=8C=20Use=20function=20instead?= =?UTF-8?q?=20of=20arrow=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/rumEventsCollection/view/trackViewMetrics.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 9cfc7ac39d..a475db5672 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -282,7 +282,7 @@ describe('rum track view metrics', () => { describe('cumulativeLayoutShift', () => { let isLayoutShiftSupported: boolean - const newLayoutShift = (lifeCycle: LifeCycle, value: number, hadRecentInput = false) => { + function newLayoutShift(lifeCycle: LifeCycle, value: number, hadRecentInput = false) { lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', startTime: relativeNow(), @@ -290,6 +290,7 @@ describe('rum track view metrics', () => { value, }) } + beforeEach(() => { if (!('PerformanceObserver' in window) || !('supportedEntryTypes' in PerformanceObserver)) { pending('No PerformanceObserver support') From a53bc8208c4c395c8d1caa18f8020028d7f5b04d Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Fri, 3 Sep 2021 10:47:17 +0200 Subject: [PATCH 06/10] =?UTF-8?q?=F0=9F=91=8C=20Make=20arguments=20more=20?= =?UTF-8?q?readable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.spec.ts | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index a475db5672..084e70873f 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -282,7 +282,7 @@ describe('rum track view metrics', () => { describe('cumulativeLayoutShift', () => { let isLayoutShiftSupported: boolean - function newLayoutShift(lifeCycle: LifeCycle, value: number, hadRecentInput = false) { + function newLayoutShift(lifeCycle: LifeCycle, { value = 0.1, hadRecentInput = false }) { lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, { entryType: 'layout-shift', startTime: relativeNow(), @@ -321,9 +321,9 @@ describe('rum track view metrics', () => { it('should accumulate layout shift values for the first session window', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, { value: 0.1 }) clock.tick(100) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.2 }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(2) @@ -333,9 +333,9 @@ describe('rum track view metrics', () => { it('should round the cumulative layout shift value to 4 decimals', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - newLayoutShift(lifeCycle, 1.23456789) + newLayoutShift(lifeCycle, { value: 1.23456789 }) clock.tick(100) - newLayoutShift(lifeCycle, 1.11111111111) + newLayoutShift(lifeCycle, { value: 1.11111111111 }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(2) @@ -346,7 +346,7 @@ describe('rum track view metrics', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - newLayoutShift(lifeCycle, 0.1, true) + newLayoutShift(lifeCycle, { value: 0.1, hadRecentInput: true }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) @@ -358,12 +358,12 @@ describe('rum track view metrics', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest // first session window - newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, { value: 0.1 }) clock.tick(100) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.2 }) // second session window clock.tick(1001) - newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, { value: 0.1 }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(2) @@ -374,12 +374,12 @@ describe('rum track view metrics', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest // first session window - newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, { value: 0.1 }) clock.tick(4500) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.2 }) // second session window clock.tick(501) - newLayoutShift(lifeCycle, 0.1) + newLayoutShift(lifeCycle, { value: 0.1 }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(3) @@ -390,17 +390,17 @@ describe('rum track view metrics', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest // first session window - newLayoutShift(lifeCycle, 0.1) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.1 }) + newLayoutShift(lifeCycle, { value: 0.2 }) // second session window clock.tick(5001) - newLayoutShift(lifeCycle, 0.1) - newLayoutShift(lifeCycle, 0.2) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.1 }) + newLayoutShift(lifeCycle, { value: 0.2 }) + newLayoutShift(lifeCycle, { value: 0.2 }) // third session window clock.tick(5001) - newLayoutShift(lifeCycle, 0.2) - newLayoutShift(lifeCycle, 0.2) + newLayoutShift(lifeCycle, { value: 0.2 }) + newLayoutShift(lifeCycle, { value: 0.2 }) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(3) From 82252e35af5e47328916d07f397970ff938abe39 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Fri, 3 Sep 2021 10:47:45 +0200 Subject: [PATCH 07/10] =?UTF-8?q?=F0=9F=91=8C=20Update=20unit=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.spec.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 084e70873f..c61e5ff199 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -370,20 +370,17 @@ describe('rum track view metrics', () => { expect(getViewUpdate(1).cumulativeLayoutShift).toBe(0.3) }) - it('should create a new session if the current session is more than 5 second', () => { + it('should create a new session window if the current session window is more than 5 second', () => { const { lifeCycle, clock } = setupBuilder.withFakeClock().build() const { getViewUpdate, getViewUpdateCount } = viewTest - // first session window - newLayoutShift(lifeCycle, { value: 0.1 }) - clock.tick(4500) - newLayoutShift(lifeCycle, { value: 0.2 }) - // second session window - clock.tick(501) - newLayoutShift(lifeCycle, { value: 0.1 }) - + newLayoutShift(lifeCycle, { value: 0 }) + for (let i = 0; i < 6; i += 1) { + clock.tick(999) + newLayoutShift(lifeCycle, { value: 0.1 }) + } //=> window 1: 0.5 | window 2: 0.1 clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(3) - expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.3) + expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.5) }) it('should get the max value sessions', () => { From 3c0e37f481a24fc2c9ba7378ff1b482dea9e1436 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Fri, 3 Sep 2021 12:52:30 +0200 Subject: [PATCH 08/10] =?UTF-8?q?=F0=9F=91=8C=20Isolate=20the=20session=20?= =?UTF-8?q?window=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViewMetrics.ts | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index 16d96f8a8d..3a794f6113 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -56,8 +56,8 @@ export function trackViewMetrics( let stopCLSTracking: () => void if (isLayoutShiftSupported()) { viewMetrics.cumulativeLayoutShift = 0 - ;({ stop: stopCLSTracking } = trackCumulativeLayoutShift(lifeCycle, (layoutShift) => { - viewMetrics.cumulativeLayoutShift = layoutShift + ;({ stop: stopCLSTracking } = trackCumulativeLayoutShift(lifeCycle, (cumulativeLayoutShift) => { + viewMetrics.cumulativeLayoutShift = cumulativeLayoutShift scheduleViewUpdate() })) } else { @@ -147,27 +147,12 @@ function trackActivityLoadingTime( */ function trackCumulativeLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift: number) => void) { let clsValue = 0 - let windowValue = 0 - let windowStartedAt = 0 as RelativeTime - let windowLastEntryAt = 0 as RelativeTime - - const updateSessionWindow = (entry: RumLayoutShiftTiming) => { - const durationSinceStart = entry.startTime - windowStartedAt - const durationSinceLastEntry = entry.startTime - windowLastEntryAt - if (durationSinceLastEntry < 1 * ONE_SECOND && durationSinceStart < 5 * ONE_SECOND) { - windowValue += entry.value - } else { - windowValue = entry.value - windowStartedAt = entry.startTime - } - windowLastEntryAt = entry.startTime - } - + const window = slidingSessionWindow() const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => { if (entry.entryType === 'layout-shift' && !entry.hadRecentInput) { - updateSessionWindow(entry) - if (windowValue > clsValue) { - clsValue = windowValue + window.update(entry) + if (window.value() > clsValue) { + clsValue = window.value() callback(round(clsValue, 4)) } } @@ -178,6 +163,28 @@ function trackCumulativeLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift } } +function slidingSessionWindow() { + let value = 0 + let startTime: RelativeTime + let endTime: RelativeTime + return { + update: (entry: RumLayoutShiftTiming) => { + const shouldCreateNewWindow = + startTime === undefined || + entry.startTime - endTime >= ONE_SECOND || + entry.startTime - startTime >= 5 * ONE_SECOND + if (shouldCreateNewWindow) { + startTime = endTime = entry.startTime + value = entry.value + } else { + value += entry.value + endTime = entry.startTime + } + }, + value: () => value, + } +} + /** * Check whether `layout-shift` is supported by the browser. */ From 047fc5fd945d855cc41f73481f486bb8ee571fa9 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Fri, 3 Sep 2021 13:00:55 +0200 Subject: [PATCH 09/10] =?UTF-8?q?=F0=9F=8E=A8=20Fix=20linter=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/rumEventsCollection/view/trackViewMetrics.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index c61e5ff199..07e134cbe6 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -377,7 +377,7 @@ describe('rum track view metrics', () => { for (let i = 0; i < 6; i += 1) { clock.tick(999) newLayoutShift(lifeCycle, { value: 0.1 }) - } //=> window 1: 0.5 | window 2: 0.1 + } // window 1: 0.5 | window 2: 0.1 clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) expect(getViewUpdateCount()).toEqual(3) expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.5) From 6dd854da7cf1e5b446e46d282a56d94ee908507e Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Mon, 6 Sep 2021 10:14:26 +0200 Subject: [PATCH 10/10] =?UTF-8?q?=F0=9F=91=8C=20Clarify=20that=20the=20CLS?= =?UTF-8?q?=20is=20the=20max=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/rumEventsCollection/view/trackViewMetrics.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index 3a794f6113..eb0853e302 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -146,14 +146,14 @@ function trackActivityLoadingTime( * Reference implementation: https://github.com/GoogleChrome/web-vitals/blob/master/src/getCLS.ts */ function trackCumulativeLayoutShift(lifeCycle: LifeCycle, callback: (layoutShift: number) => void) { - let clsValue = 0 + let maxClsValue = 0 const window = slidingSessionWindow() const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => { if (entry.entryType === 'layout-shift' && !entry.hadRecentInput) { window.update(entry) - if (window.value() > clsValue) { - clsValue = window.value() - callback(round(clsValue, 4)) + if (window.value() > maxClsValue) { + maxClsValue = window.value() + callback(round(maxClsValue, 4)) } } })