From 9c7a19cd2b43a6b25639fd1e57c47b7c7eb8b43b Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 13 Sep 2024 16:53:00 +0200 Subject: [PATCH] Use performanceObserver for lcp entries --- .../src/browser/performanceCollection.spec.ts | 7 ++- .../src/browser/performanceCollection.ts | 1 - .../src/domain/view/trackViews.spec.ts | 7 ++- .../viewMetrics/trackInitialViewMetrics.ts | 1 - .../trackLargestContentfulPaint.spec.ts | 33 +++++------ .../trackLargestContentfulPaint.ts | 59 +++++++++---------- 6 files changed, 51 insertions(+), 57 deletions(-) diff --git a/packages/rum-core/src/browser/performanceCollection.spec.ts b/packages/rum-core/src/browser/performanceCollection.spec.ts index fbacc017bb..33755c9893 100644 --- a/packages/rum-core/src/browser/performanceCollection.spec.ts +++ b/packages/rum-core/src/browser/performanceCollection.spec.ts @@ -22,7 +22,6 @@ describe('startPerformanceCollection', () => { ;[ RumPerformanceEntryType.LONG_TASK, RumPerformanceEntryType.PAINT, - RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, RumPerformanceEntryType.FIRST_INPUT, RumPerformanceEntryType.LAYOUT_SHIFT, RumPerformanceEntryType.EVENT, @@ -36,7 +35,11 @@ describe('startPerformanceCollection', () => { expect(entryCollectedCallback).toHaveBeenCalledWith([jasmine.objectContaining({ entryType })]) }) }) - ;[(RumPerformanceEntryType.NAVIGATION, RumPerformanceEntryType.RESOURCE)].forEach((entryType) => { + ;[ + (RumPerformanceEntryType.NAVIGATION, + RumPerformanceEntryType.RESOURCE, + RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT), + ].forEach((entryType) => { it(`should not notify ${entryType} timings`, () => { const { notifyPerformanceEntries } = mockPerformanceObserver() setupStartPerformanceCollection() diff --git a/packages/rum-core/src/browser/performanceCollection.ts b/packages/rum-core/src/browser/performanceCollection.ts index ecfe878c2c..169f655089 100644 --- a/packages/rum-core/src/browser/performanceCollection.ts +++ b/packages/rum-core/src/browser/performanceCollection.ts @@ -43,7 +43,6 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, configuration: ) const mainEntries = [RumPerformanceEntryType.LONG_TASK, RumPerformanceEntryType.PAINT] const experimentalEntries = [ - RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, RumPerformanceEntryType.FIRST_INPUT, RumPerformanceEntryType.LAYOUT_SHIFT, RumPerformanceEntryType.EVENT, diff --git a/packages/rum-core/src/domain/view/trackViews.spec.ts b/packages/rum-core/src/domain/view/trackViews.spec.ts index a10a0bde4d..de0c3763d9 100644 --- a/packages/rum-core/src/domain/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/view/trackViews.spec.ts @@ -461,9 +461,8 @@ describe('view metrics', () => { clock.tick(KEEP_TRACKING_AFTER_VIEW_DELAY - 1) lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ createPerformanceEntry(RumPerformanceEntryType.PAINT), - lcpEntry, ]) - notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.NAVIGATION)]) + notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.NAVIGATION), lcpEntry]) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) @@ -513,9 +512,11 @@ describe('view metrics', () => { lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ createPerformanceEntry(RumPerformanceEntryType.PAINT), + ]) + notifyPerformanceEntries([ + createPerformanceEntry(RumPerformanceEntryType.NAVIGATION), createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT), ]) - notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.NAVIGATION)]) clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts b/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts index 193ef0b76c..609b695ed2 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts @@ -38,7 +38,6 @@ export function trackInitialViewMetrics( }) const { stop: stopLCPTracking } = trackLargestContentfulPaint( - lifeCycle, configuration, firstHidden, window, diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.spec.ts b/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.spec.ts index eba6d15886..aa53ab0a26 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.spec.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.spec.ts @@ -6,22 +6,23 @@ import { restorePageVisibility, registerCleanupTask, } from '@datadog/browser-core/test' +import type { RumPerformanceEntry } from '../../../browser/performanceObservable' import { RumPerformanceEntryType } from '../../../browser/performanceObservable' -import { appendElement, createPerformanceEntry, mockRumConfiguration } from '../../../../test' -import { LifeCycle, LifeCycleEventType } from '../../lifeCycle' +import { appendElement, createPerformanceEntry, mockPerformanceObserver, mockRumConfiguration } from '../../../../test' import type { LargestContentfulPaint } from './trackLargestContentfulPaint' import { LCP_MAXIMUM_DELAY, trackLargestContentfulPaint } from './trackLargestContentfulPaint' import { trackFirstHidden } from './trackFirstHidden' describe('trackLargestContentfulPaint', () => { - const lifeCycle = new LifeCycle() let lcpCallback: jasmine.Spy<(lcp: LargestContentfulPaint) => void> let eventTarget: Window + let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void function startLCPTracking() { + ;({ notifyPerformanceEntries } = mockPerformanceObserver()) + const firstHidden = trackFirstHidden(mockRumConfiguration()) const largestContentfulPaint = trackLargestContentfulPaint( - lifeCycle, mockRumConfiguration(), firstHidden, eventTarget, @@ -42,16 +43,14 @@ describe('trackLargestContentfulPaint', () => { it('should provide the largest contentful paint timing', () => { startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ - createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT), - ]) + notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT)]) expect(lcpCallback).toHaveBeenCalledOnceWith({ value: 789 as RelativeTime, targetSelector: undefined }) }) it('should provide the largest contentful paint target selector', () => { startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { element: appendElement(''), }), @@ -64,9 +63,7 @@ describe('trackLargestContentfulPaint', () => { startLCPTracking() eventTarget.dispatchEvent(createNewEvent(DOM_EVENT.KEY_DOWN, { timeStamp: 1 })) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ - createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT), - ]) + notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT)]) expect(lcpCallback).not.toHaveBeenCalled() }) @@ -75,16 +72,14 @@ describe('trackLargestContentfulPaint', () => { setPageVisibility('hidden') startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ - createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT), - ]) + notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT)]) expect(lcpCallback).not.toHaveBeenCalled() }) it('should be discarded if it is reported after a long time', () => { startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { startTime: LCP_MAXIMUM_DELAY as RelativeTime, }), @@ -95,14 +90,14 @@ describe('trackLargestContentfulPaint', () => { it('should be discarded if it has a size inferior to the previous LCP entry', () => { startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { startTime: 1 as RelativeTime, size: 10, }), ]) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { startTime: 2 as RelativeTime, size: 5, @@ -114,14 +109,14 @@ describe('trackLargestContentfulPaint', () => { it('should notify multiple times when the size is bigger than the previous entry', () => { startLCPTracking() - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { startTime: 1 as RelativeTime, size: 5, }), ]) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, { startTime: 2 as RelativeTime, size: 10, diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.ts b/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.ts index d51b69c4ae..9aa9f420f1 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackLargestContentfulPaint.ts @@ -1,9 +1,7 @@ import type { RelativeTime } from '@datadog/browser-core' import { DOM_EVENT, ONE_MINUTE, addEventListeners, findLast } from '@datadog/browser-core' -import { LifeCycleEventType } from '../../lifeCycle' -import type { LifeCycle } from '../../lifeCycle' import type { RumConfiguration } from '../../configuration' -import { RumPerformanceEntryType } from '../../../browser/performanceObservable' +import { createPerformanceObservable, RumPerformanceEntryType } from '../../../browser/performanceObservable' import type { RumLargestContentfulPaintTiming } from '../../../browser/performanceObservable' import { getSelectorFromElement } from '../../getSelectorFromElement' import type { FirstHidden } from './trackFirstHidden' @@ -24,7 +22,6 @@ export interface LargestContentfulPaint { * Reference implementation: https://github.com/GoogleChrome/web-vitals/blob/master/src/onLCP.ts */ export function trackLargestContentfulPaint( - lifeCycle: LifeCycle, configuration: RumConfiguration, firstHidden: FirstHidden, eventTarget: Window, @@ -45,40 +42,40 @@ export function trackLargestContentfulPaint( ) let biggestLcpSize = 0 - const { unsubscribe: unsubscribeLifeCycle } = lifeCycle.subscribe( - LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, - (entries) => { - const lcpEntry = findLast( - entries, - (entry): entry is RumLargestContentfulPaintTiming => - entry.entryType === RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT && - entry.startTime < firstInteractionTimestamp && - entry.startTime < firstHidden.timeStamp && - entry.startTime < LCP_MAXIMUM_DELAY && - // Ensure to get the LCP entry with the biggest size, see - // https://bugs.chromium.org/p/chromium/issues/detail?id=1516655 - entry.size > biggestLcpSize - ) + const performanceLcpSubscription = createPerformanceObservable(configuration, { + type: RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT, + buffered: true, + }).subscribe((entries) => { + const lcpEntry = findLast( + entries, + (entry): entry is RumLargestContentfulPaintTiming => + entry.entryType === RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT && + entry.startTime < firstInteractionTimestamp && + entry.startTime < firstHidden.timeStamp && + entry.startTime < LCP_MAXIMUM_DELAY && + // Ensure to get the LCP entry with the biggest size, see + // https://bugs.chromium.org/p/chromium/issues/detail?id=1516655 + entry.size > biggestLcpSize + ) - if (lcpEntry) { - let lcpTargetSelector - if (lcpEntry.element) { - lcpTargetSelector = getSelectorFromElement(lcpEntry.element, configuration.actionNameAttribute) - } - - callback({ - value: lcpEntry.startTime, - targetSelector: lcpTargetSelector, - }) - biggestLcpSize = lcpEntry.size + if (lcpEntry) { + let lcpTargetSelector + if (lcpEntry.element) { + lcpTargetSelector = getSelectorFromElement(lcpEntry.element, configuration.actionNameAttribute) } + + callback({ + value: lcpEntry.startTime, + targetSelector: lcpTargetSelector, + }) + biggestLcpSize = lcpEntry.size } - ) + }) return { stop: () => { stopEventListener() - unsubscribeLifeCycle() + performanceLcpSubscription.unsubscribe() }, } }