♻️ [RUM-6184] Use performanceObserver for paint entries (#2991)

DataDog · Sep 19, 2024 · 5e00489 · 5e00489
1 parent fbd15c7
commit 5e00489
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 68 deletions.
diff --git a/packages/rum-core/src/browser/performanceCollection.spec.ts b/packages/rum-core/src/browser/performanceCollection.spec.ts
@@ -19,26 +19,24 @@ describe('startPerformanceCollection', () => {
     })
   }
 
-  ;[
-    RumPerformanceEntryType.PAINT,
-    RumPerformanceEntryType.FIRST_INPUT,
-    RumPerformanceEntryType.LAYOUT_SHIFT,
-    RumPerformanceEntryType.EVENT,
-  ].forEach((entryType) => {
-    it(`should notify ${entryType}`, () => {
-      const { notifyPerformanceEntries } = mockPerformanceObserver()
-      setupStartPerformanceCollection()
-
-      notifyPerformanceEntries([createPerformanceEntry(entryType)])
-
-      expect(entryCollectedCallback).toHaveBeenCalledWith([jasmine.objectContaining({ entryType })])
-    })
-  })
+  ;[RumPerformanceEntryType.FIRST_INPUT, RumPerformanceEntryType.LAYOUT_SHIFT, RumPerformanceEntryType.EVENT].forEach(
+    (entryType) => {
+      it(`should notify ${entryType}`, () => {
+        const { notifyPerformanceEntries } = mockPerformanceObserver()
+        setupStartPerformanceCollection()
+
+        notifyPerformanceEntries([createPerformanceEntry(entryType)])
+
+        expect(entryCollectedCallback).toHaveBeenCalledWith([jasmine.objectContaining({ entryType })])
+      })
+    }
+  )
   ;[
     RumPerformanceEntryType.NAVIGATION,
     RumPerformanceEntryType.RESOURCE,
     RumPerformanceEntryType.LONG_TASK,
     RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT,
+    RumPerformanceEntryType.PAINT,
   ].forEach((entryType) => {
     it(`should not notify ${entryType} timings`, () => {
       const { notifyPerformanceEntries } = mockPerformanceObserver()

diff --git a/packages/rum-core/src/browser/performanceCollection.ts b/packages/rum-core/src/browser/performanceCollection.ts
@@ -41,7 +41,6 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, configuration:
     const handlePerformanceEntryList = monitor((entries: PerformanceObserverEntryList) =>
       handleRumPerformanceEntries(lifeCycle, entries.getEntries())
     )
-    const mainEntries = [RumPerformanceEntryType.PAINT]
     const experimentalEntries = [
       RumPerformanceEntryType.FIRST_INPUT,
       RumPerformanceEntryType.LAYOUT_SHIFT,
@@ -66,21 +65,19 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, configuration:
     } catch (e) {
       // Some old browser versions (ex: chrome 67) don't support the PerformanceObserver type and buffered options
       // In these cases, fallback to PerformanceObserver with entryTypes
-      mainEntries.push(...experimentalEntries)
-    }
-
-    const mainObserver = new PerformanceObserver(handlePerformanceEntryList)
-    try {
-      mainObserver.observe({ entryTypes: mainEntries })
-      cleanupTasks.push(() => mainObserver.disconnect())
-    } catch {
-      // Old versions of Safari are throwing "entryTypes contained only unsupported types"
-      // errors when observing only unsupported entry types.
-      //
-      // We could use `supportPerformanceTimingEvent` to make sure we don't invoke
-      // `observer.observe` with an unsupported entry type, but Safari 11 and 12 don't support
-      // `Performance.supportedEntryTypes`, so doing so would lose support for these versions
-      // even if they do support the entry type.
+      const mainObserver = new PerformanceObserver(handlePerformanceEntryList)
+      try {
+        mainObserver.observe({ entryTypes: experimentalEntries })
+        cleanupTasks.push(() => mainObserver.disconnect())
+      } catch {
+        // Old versions of Safari are throwing "entryTypes contained only unsupported types"
+        // errors when observing only unsupported entry types.
+        //
+        // We could use `supportPerformanceTimingEvent` to make sure we don't invoke
+        // `observer.observe` with an unsupported entry type, but Safari 11 and 12 don't support
+        // `Performance.supportedEntryTypes`, so doing so would lose support for these versions
+        // even if they do support the entry type.
+      }
     }
 
     if (supportPerformanceObject() && 'addEventListener' in performance) {

diff --git a/packages/rum-core/src/domain/view/trackViews.spec.ts b/packages/rum-core/src/domain/view/trackViews.spec.ts
@@ -504,10 +504,11 @@ describe('view metrics', () => {
 
         expect(getViewUpdateCount()).toEqual(3)
 
-        lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
+        notifyPerformanceEntries([
           createPerformanceEntry(RumPerformanceEntryType.PAINT),
+          createPerformanceEntry(RumPerformanceEntryType.NAVIGATION),
+          createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT),
         ])
-        notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.LARGEST_CONTENTFUL_PAINT)])
 
         clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
 

diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackFirstContentfulPaint.spec.ts b/packages/rum-core/src/domain/view/viewMetrics/trackFirstContentfulPaint.spec.ts
@@ -1,19 +1,21 @@
 import type { RelativeTime } from '@datadog/browser-core'
 import { registerCleanupTask, restorePageVisibility, setPageVisibility } from '@datadog/browser-core/test'
+import type { RumPerformanceEntry } from '../../../browser/performanceObservable'
 import { RumPerformanceEntryType } from '../../../browser/performanceObservable'
-import { createPerformanceEntry, mockRumConfiguration } from '../../../../test'
-import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
+import { createPerformanceEntry, mockPerformanceObserver, mockRumConfiguration } from '../../../../test'
 import { FCP_MAXIMUM_DELAY, trackFirstContentfulPaint } from './trackFirstContentfulPaint'
 import { trackFirstHidden } from './trackFirstHidden'
 
 describe('trackFirstContentfulPaint', () => {
-  const lifeCycle = new LifeCycle()
   let fcpCallback: jasmine.Spy<(value: RelativeTime) => void>
+  let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void
 
   function startTrackingFCP() {
+    ;({ notifyPerformanceEntries } = mockPerformanceObserver())
+
     fcpCallback = jasmine.createSpy()
     const firstHidden = trackFirstHidden(mockRumConfiguration())
-    const firstContentfulPaint = trackFirstContentfulPaint(lifeCycle, firstHidden, fcpCallback)
+    const firstContentfulPaint = trackFirstContentfulPaint(mockRumConfiguration(), firstHidden, fcpCallback)
 
     registerCleanupTask(() => {
       firstHidden.stop()
@@ -24,9 +26,7 @@ describe('trackFirstContentfulPaint', () => {
 
   it('should provide the first contentful paint timing', () => {
     startTrackingFCP()
-    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
-      createPerformanceEntry(RumPerformanceEntryType.PAINT),
-    ])
+    notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.PAINT)])
 
     expect(fcpCallback).toHaveBeenCalledTimes(1 as RelativeTime)
     expect(fcpCallback).toHaveBeenCalledWith(123 as RelativeTime)
@@ -36,15 +36,13 @@ describe('trackFirstContentfulPaint', () => {
     setPageVisibility('hidden')
     startTrackingFCP()
 
-    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
-      createPerformanceEntry(RumPerformanceEntryType.PAINT),
-    ])
+    notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.PAINT)])
     expect(fcpCallback).not.toHaveBeenCalled()
   })
 
   it('should be discarded if it is reported after a long time', () => {
     startTrackingFCP()
-    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
+    notifyPerformanceEntries([
       createPerformanceEntry(RumPerformanceEntryType.PAINT, { startTime: FCP_MAXIMUM_DELAY as RelativeTime }),
     ])
     expect(fcpCallback).not.toHaveBeenCalled()

diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackFirstContentfulPaint.ts b/packages/rum-core/src/domain/view/viewMetrics/trackFirstContentfulPaint.ts
@@ -1,37 +1,35 @@
 import type { RelativeTime } from '@datadog/browser-core'
 import { ONE_MINUTE, find } from '@datadog/browser-core'
-import type { LifeCycle } from '../../lifeCycle'
-import { LifeCycleEventType } from '../../lifeCycle'
 import type { RumPerformancePaintTiming } from '../../../browser/performanceObservable'
-import { RumPerformanceEntryType } from '../../../browser/performanceObservable'
+import { createPerformanceObservable, RumPerformanceEntryType } from '../../../browser/performanceObservable'
+import type { RumConfiguration } from '../../configuration'
 import type { FirstHidden } from './trackFirstHidden'
 
 // Discard FCP timings above a certain delay to avoid incorrect data
 // It happens in some cases like sleep mode or some browser implementations
 export const FCP_MAXIMUM_DELAY = 10 * ONE_MINUTE
 
 export function trackFirstContentfulPaint(
-  lifeCycle: LifeCycle,
+  configuration: RumConfiguration,
   firstHidden: FirstHidden,
   callback: (fcpTiming: RelativeTime) => void
 ) {
-  const { unsubscribe: unsubscribeLifeCycle } = lifeCycle.subscribe(
-    LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED,
-    (entries) => {
-      const fcpEntry = find(
-        entries,
-        (entry): entry is RumPerformancePaintTiming =>
-          entry.entryType === RumPerformanceEntryType.PAINT &&
-          entry.name === 'first-contentful-paint' &&
-          entry.startTime < firstHidden.timeStamp &&
-          entry.startTime < FCP_MAXIMUM_DELAY
-      )
-      if (fcpEntry) {
-        callback(fcpEntry.startTime)
-      }
+  const performanceSubscription = createPerformanceObservable(configuration, {
+    type: RumPerformanceEntryType.PAINT,
+    buffered: true,
+  }).subscribe((entries) => {
+    const fcpEntry = find(
+      entries,
+      (entry): entry is RumPerformancePaintTiming =>
+        entry.name === 'first-contentful-paint' &&
+        entry.startTime < firstHidden.timeStamp &&
+        entry.startTime < FCP_MAXIMUM_DELAY
+    )
+    if (fcpEntry) {
+      callback(fcpEntry.startTime)
     }
-  )
+  })
   return {
-    stop: unsubscribeLifeCycle,
+    stop: performanceSubscription.unsubscribe,
   }
 }
diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.spec.ts b/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.spec.ts
@@ -1,8 +1,9 @@
 import type { Duration, RelativeTime } from '@datadog/browser-core'
 import type { Clock } from '@datadog/browser-core/test'
 import { mockClock, registerCleanupTask } from '@datadog/browser-core/test'
+import type { RumPerformanceEntry } from '../../../browser/performanceObservable'
 import { RumPerformanceEntryType } from '../../../browser/performanceObservable'
-import { createPerformanceEntry, mockRumConfiguration } from '../../../../test'
+import { createPerformanceEntry, mockPerformanceObserver, mockRumConfiguration } from '../../../../test'
 import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
 import { trackInitialViewMetrics } from './trackInitialViewMetrics'
 
@@ -12,8 +13,11 @@ describe('trackInitialViewMetrics', () => {
   let scheduleViewUpdateSpy: jasmine.Spy<() => void>
   let trackInitialViewMetricsResult: ReturnType<typeof trackInitialViewMetrics>
   let setLoadEventSpy: jasmine.Spy<(loadEvent: Duration) => void>
+  let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void
 
   beforeEach(() => {
+    ;({ notifyPerformanceEntries } = mockPerformanceObserver())
+
     lifeCycle = new LifeCycle()
     const configuration = mockRumConfiguration()
     scheduleViewUpdateSpy = jasmine.createSpy()
@@ -32,8 +36,11 @@ describe('trackInitialViewMetrics', () => {
   })
 
   it('should merge metrics from various sources', () => {
-    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
+    notifyPerformanceEntries([
+      createPerformanceEntry(RumPerformanceEntryType.NAVIGATION),
       createPerformanceEntry(RumPerformanceEntryType.PAINT),
+    ])
+    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
       createPerformanceEntry(RumPerformanceEntryType.FIRST_INPUT),
     ])
     clock.tick(0)
@@ -51,8 +58,8 @@ describe('trackInitialViewMetrics', () => {
   })
 
   it('calls the `setLoadEvent` callback when the loadEvent timing is known', () => {
+    notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.PAINT)])
     lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
-      createPerformanceEntry(RumPerformanceEntryType.PAINT),
       createPerformanceEntry(RumPerformanceEntryType.FIRST_INPUT),
     ])
     clock.tick(0)

diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts b/packages/rum-core/src/domain/view/viewMetrics/trackInitialViewMetrics.ts
@@ -32,7 +32,7 @@ export function trackInitialViewMetrics(
   })
 
   const firstHidden = trackFirstHidden(configuration)
-  const { stop: stopFCPTracking } = trackFirstContentfulPaint(lifeCycle, firstHidden, (firstContentfulPaint) => {
+  const { stop: stopFCPTracking } = trackFirstContentfulPaint(configuration, firstHidden, (firstContentfulPaint) => {
     initialViewMetrics.firstContentfulPaint = firstContentfulPaint
     scheduleViewUpdate()
   })