Report scroll metrics when page is resized

packages/rum-core/src/browser/scrollHeightObservable.ts

@@ -0,0 +1,15 @@
+import { Observable, setInterval, clearInterval } from '@datadog/browser-core'
+
+export const SCROLL_HEIGHT_OBSERVABLE_INTERVAL_MS = 500
+
+export function createScrollHeightObservable() {
+  const observable = new Observable<number>(() => {
+    const setIntervalId = setInterval(() => {
+      observable.notify(Math.round((document.scrollingElement || document.documentElement).scrollHeight))
+    }, SCROLL_HEIGHT_OBSERVABLE_INTERVAL_MS)
+
+    return () => clearInterval(setIntervalId)
+  })
+
+  return observable
+}

packages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackCommonViewMetrics.ts

@@ -1,5 +1,6 @@
 import type { ClocksState, Duration, Observable } from '@datadog/browser-core'
 import { noop } from '@datadog/browser-core'
+import { createScrollHeightObservable } from '../../../../browser/scrollHeightObservable'
 import type { ViewLoadingType } from '../../../../rawRumEvent.types'
 import type { RumConfiguration } from '../../../configuration'
 import type { LifeCycle } from '../../../lifeCycle'
@@ -36,17 +37,6 @@ export function trackCommonViewMetrics(
     viewStart,
     (newLoadingTime) => {
       commonViewMetrics.loadingTime = newLoadingTime
-
-      // We compute scroll metrics at loading time to ensure we have scroll data when loading the view initially
-      // This is to ensure that we have the depth data even if the user didn't scroll or if the view is not scrollable.
-      const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues()
-
-      commonViewMetrics.scroll = {
-        maxDepth: scrollDepth,
-        maxDepthScrollHeight: scrollHeight,
-        maxDepthTime: newLoadingTime,
-        maxDepthScrollTop: scrollTop,
-      }
       scheduleViewUpdate()
     }
   )
@@ -57,6 +47,7 @@ export function trackCommonViewMetrics(
     (newScrollMetrics) => {
       commonViewMetrics.scroll = newScrollMetrics
     },
+    createScrollHeightObservable(),
     computeScrollValues
   )
 

packages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackScrollMetrics.spec.ts

@@ -1,20 +1,14 @@
 import type { RelativeTime, TimeStamp, Duration } from '@datadog/browser-core'
-import { DOM_EVENT } from '@datadog/browser-core'
+import { Observable, DOM_EVENT } from '@datadog/browser-core'
 import type { Clock } from '@datadog/browser-core/test'
 import { createNewEvent, mockClock } from '@datadog/browser-core/test'
 import type { TestSetupBuilder } from '../../../../../test'
 import { setup } from '../../../../../test'
-import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_VALIDATION_DELAY } from '../../../waitPageActivityEnd'
 import type { RumConfiguration } from '../../../configuration'
-import { THROTTLE_VIEW_UPDATE_PERIOD } from '../trackViews'
 import type { ViewTest } from '../setupViewTest.specHelper'
 import { setupViewTest } from '../setupViewTest.specHelper'
 import { THROTTLE_SCROLL_DURATION, type ScrollMetrics, trackScrollMetrics } from './trackScrollMetrics'
 
-const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = (PAGE_ACTIVITY_VALIDATION_DELAY * 0.8) as Duration
-
-const AFTER_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 1.1
-
 describe('trackScrollMetrics', () => {
   let setupBuilder: TestSetupBuilder
   let viewTest: ViewTest
@@ -37,14 +31,22 @@ describe('trackScrollMetrics', () => {
     let stopTrackScrollMetrics: () => void
     let clock: Clock
     let configuration: RumConfiguration
+    const scrollHeightObservable: Observable<number> = new Observable()
 
     const getMetrics = jasmine.createSpy('getMetrics')
 
-    const newScroll = (scrollParams: { scrollHeight: number; scrollDepth: number; scrollTop: number }) => {
+    const mockGetMetrics = (scrollParams: { scrollHeight: number; scrollDepth: number; scrollTop: number }) => {
       getMetrics.and.returnValue(scrollParams)
+    }
 
-      window.dispatchEvent(createNewEvent(DOM_EVENT.SCROLL))
+    const emitObservableValue = (scrollParams: { scrollHeight: number; scrollDepth: number; scrollTop: number }) => {
+      mockGetMetrics({ ...scrollParams, scrollHeight: 0 })
+      scrollHeightObservable.notify(scrollParams.scrollHeight)
+    }
 
+    const newScroll = (scrollParams: { scrollHeight: number; scrollDepth: number; scrollTop: number }) => {
+      mockGetMetrics(scrollParams)
+      window.dispatchEvent(createNewEvent(DOM_EVENT.SCROLL))
       clock.tick(THROTTLE_SCROLL_DURATION)
     }
 
@@ -55,6 +57,7 @@ describe('trackScrollMetrics', () => {
         configuration,
         { relative: 0 as RelativeTime, timeStamp: 0 as TimeStamp },
         (s) => (scrollMetrics = s),
+        scrollHeightObservable,
         getMetrics
       ).stop
     })
@@ -65,9 +68,31 @@ describe('trackScrollMetrics', () => {
       clock.cleanup()
     })
 
-    it('should update scroll metrics when scrolling the first time', () => {
-      newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })
+    it('should initially be undefined', () => {
+      expect(scrollMetrics).toBeUndefined()
+    })
+
+    it('should update scroll metrics before scrolling', () => {
+      emitObservableValue({
+        scrollHeight: 99,
+        scrollDepth: 99,
+        scrollTop: 99,
+      })
+      expect(scrollMetrics).toEqual({
+        maxDepthScrollHeight: 99,
+        maxDepth: 99,
+        maxDepthScrollTop: 99,
+        maxDepthTime: 0 as Duration,
+      })
+    })
 
+    it('should update scroll metrics when scrolling the first time, ignoring subsequent values emitted by the observable', () => {
+      newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })
+      emitObservableValue({
+        scrollHeight: 9999,
+        scrollDepth: 9999,
+        scrollTop: 9999,
+      })
       expect(scrollMetrics).toEqual({
         maxDepthScrollHeight: 1000,
         maxDepth: 500,
@@ -76,11 +101,16 @@ describe('trackScrollMetrics', () => {
       })
     })
 
-    it('should update scroll metrics when scroll depth has increased', () => {
+    it('should update scroll metrics when scroll depth has increased, still ignoring values emitted by the observable', () => {
       newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })
-
       newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })
 
+      emitObservableValue({
+        scrollHeight: 9999,
+        scrollDepth: 9999,
+        scrollTop: 9999,
+      })
+
       expect(scrollMetrics).toEqual({
         maxDepthScrollHeight: 1000,
         maxDepth: 600,
@@ -91,9 +121,14 @@ describe('trackScrollMetrics', () => {
 
     it('should NOT update scroll metrics when scroll depth has not increased', () => {
       newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })
-
       newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 })
 
+      emitObservableValue({
+        scrollHeight: 9999,
+        scrollDepth: 9999,
+        scrollTop: 9999,
+      })
+
       expect(scrollMetrics).toEqual({
         maxDepthScrollHeight: 1000,
         maxDepth: 600,
@@ -102,41 +137,4 @@ describe('trackScrollMetrics', () => {
       })
     })
   })
-
-  describe('on load', () => {
-    beforeEach(() => {
-      setupBuilder.withFakeClock()
-    })
-
-    it('should have an undefined loading time and empty scroll metrics if there is no activity on a route change', () => {
-      const { clock } = setupBuilder.build()
-      const { getViewUpdate, getViewUpdateCount, startView } = viewTest
-
-      startView()
-      clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
-
-      expect(getViewUpdateCount()).toEqual(3)
-      expect(getViewUpdate(2).commonViewMetrics.loadingTime).toBeUndefined()
-      expect(getViewUpdate(2).commonViewMetrics.scroll).toEqual(undefined)
-    })
-
-    it('should have a loading time equal to the activity time and scroll metrics if there is a unique activity on a route change', () => {
-      const { domMutationObservable, clock } = setupBuilder.build()
-      const { getViewUpdate, startView } = viewTest
-
-      startView()
-      clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
-      domMutationObservable.notify()
-      clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY)
-      clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
-
-      expect(getViewUpdate(3).commonViewMetrics.loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
-      expect(getViewUpdate(3).commonViewMetrics.scroll).toEqual({
-        maxDepthScrollHeight: jasmine.any(Number),
-        maxDepth: jasmine.any(Number),
-        maxDepthTime: jasmine.any(Number),
-        maxDepthScrollTop: jasmine.any(Number),
-      })
-    })
-  })
 })

packages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackScrollMetrics.ts

@@ -1,4 +1,4 @@
-import type { ClocksState, Duration } from '@datadog/browser-core'
+import type { ClocksState, Duration, Observable } from '@datadog/browser-core'
 import { ONE_SECOND, elapsed, relativeNow, throttle, addEventListener, DOM_EVENT } from '@datadog/browser-core'
 import type { RumConfiguration } from '../../../configuration'
 import { getScrollY } from '../../../../browser/scroll'
@@ -18,11 +18,28 @@ export function trackScrollMetrics(
   configuration: RumConfiguration,
   viewStart: ClocksState,
   callback: (scrollMetrics: ScrollMetrics) => void,
+  scrollHeightObservable: Observable<number>,
   getScrollValues = computeScrollValues
 ) {
   let maxDepth = 0
+
+  const subscription = scrollHeightObservable.subscribe((scrollHeight) => {
+    const { scrollDepth, scrollTop } = getScrollValues()
+
+    const now = relativeNow()
+    const maxDepthTime = elapsed(viewStart.relative, now)
+    maxDepth = scrollDepth
+    callback({
+      maxDepth,
+      maxDepthScrollHeight: scrollHeight,
+      maxDepthTime,
+      maxDepthScrollTop: scrollTop,
+    })
+  })
+
   const handleScrollEvent = throttle(
     () => {
+      subscription.unsubscribe()
       const { scrollHeight, scrollDepth, scrollTop } = getScrollValues()
 
       if (scrollDepth > maxDepth) {
@@ -48,6 +65,7 @@ export function trackScrollMetrics(
   return {
     stop: () => {
       handleScrollEvent.cancel()
+      subscription.unsubscribe()
       stop()
     },
   }