🐛 [RUMF-1421] keep updating the view event counters after view end

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

@@ -0,0 +1,75 @@
+import type { Context } from '@datadog/browser-core'
+import { noop } from '@datadog/browser-core'
+import type { RumResourceEvent } from '../../../rumEvent.types'
+import { RumEventType } from '../../../rawRumEvent.types'
+import type { Clock } from '../../../../../core/test/specHelper'
+import { mockClock } from '../../../../../core/test/specHelper'
+import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
+import { KEEP_TRACKING_EVENT_COUNTS_AFTER_VIEW_DELAY, trackViewEventCounts } from './trackViewEventCounts'
+
+describe('trackViewEventCounts', () => {
+  const VIEW_ID = 'a'
+  const OTHER_VIEW_ID = 'b'
+  let lifeCycle: LifeCycle
+  let clock: Clock | undefined
+
+  beforeEach(() => {
+    lifeCycle = new LifeCycle()
+  })
+
+  afterEach(() => {
+    if (clock) clock.cleanup()
+  })
+
+  it('initializes eventCounts to 0', () => {
+    const { eventCounts } = trackViewEventCounts(lifeCycle, VIEW_ID, noop)
+
+    expect(eventCounts).toEqual({
+      actionCount: 0,
+      errorCount: 0,
+      longTaskCount: 0,
+      frustrationCount: 0,
+      resourceCount: 0,
+    })
+  })
+
+  it('increments counters', () => {
+    const { eventCounts } = trackViewEventCounts(lifeCycle, VIEW_ID, noop)
+
+    notifyResourceEvent()
+
+    expect(eventCounts.resourceCount).toBe(1)
+  })
+
+  it('does not increment counters related to other views', () => {
+    const { eventCounts } = trackViewEventCounts(lifeCycle, VIEW_ID, noop)
+
+    notifyResourceEvent(OTHER_VIEW_ID)
+
+    expect(eventCounts.resourceCount).toBe(0)
+  })
+
+  it('when calling scheduleStop, it keeps counting events for a bit of time', () => {
+    clock = mockClock()
+    const { scheduleStop, eventCounts } = trackViewEventCounts(lifeCycle, VIEW_ID, noop)
+
+    scheduleStop()
+
+    clock.tick(KEEP_TRACKING_EVENT_COUNTS_AFTER_VIEW_DELAY - 1)
+    notifyResourceEvent()
+
+    expect(eventCounts.resourceCount).toBe(1)
+
+    clock.tick(1)
+    notifyResourceEvent()
+
+    expect(eventCounts.resourceCount).toBe(1)
+  })
+
+  function notifyResourceEvent(viewId = VIEW_ID) {
+    lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, {
+      type: RumEventType.RESOURCE,
+      view: { id: viewId },
+    } as unknown as RumResourceEvent & Context)
+  }
+})

packages/rum-core/src/domain/rumEventsCollection/view/trackViewEventCounts.ts

@@ -0,0 +1,35 @@
+import { monitor, ONE_MINUTE } from '@datadog/browser-core'
+import type { LifeCycle } from '../../lifeCycle'
+import { trackEventCounts } from '../../trackEventCounts'
+
+// Some events are not being counted as they transcend views. To reduce the occurrence;
+// an arbitrary delay is added for stopping event counting after the view ends.
+//
+// Ideally, we would not stop and keep counting events until the end of the session.
+// But this might have a small performance impact if there are many many views:
+// we would need to go through each event to see if the related view matches.
+// So let's have a fairly short delay to avoid impacting performances too much.
+//
+// In the future, we could have views stored in a data structure similar to ContextHistory. Whenever
+// a child event is collected, we could look into this history to find the matching view and
+// increase the associated and increase its counter. Having a centralized data structure for it
+// would allow us to look for views more efficiently.
+//
+// For now, having a small cleanup delay will already improve the situation in most cases.
+
+export const KEEP_TRACKING_EVENT_COUNTS_AFTER_VIEW_DELAY = 5 * ONE_MINUTE
+
+export function trackViewEventCounts(lifeCycle: LifeCycle, viewId: string, onChange: () => void) {
+  const { stop, eventCounts } = trackEventCounts({
+    lifeCycle,
+    isChildEvent: (event) => event.view.id === viewId,
+    onChange,
+  })
+
+  return {
+    scheduleStop: () => {
+      setTimeout(monitor(stop), KEEP_TRACKING_EVENT_COUNTS_AFTER_VIEW_DELAY)
+    },
+    eventCounts,
+  }
+}

packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts

@@ -6,12 +6,9 @@ import { ViewLoadingType } from '../../../rawRumEvent.types'
 import type { RumConfiguration } from '../../configuration'
 import type { LifeCycle } from '../../lifeCycle'
 import { LifeCycleEventType } from '../../lifeCycle'
-import type { EventCounts } from '../../trackEventCounts'
-import { trackEventCounts } from '../../trackEventCounts'
 import { waitPageActivityEnd } from '../../waitPageActivityEnd'
 
 export interface ViewMetrics {
-  eventCounts: EventCounts
   loadingTime?: Duration
   cumulativeLayoutShift?: number
 }
@@ -21,27 +18,10 @@ export function trackViewMetrics(
   domMutationObservable: Observable<void>,
   configuration: RumConfiguration,
   scheduleViewUpdate: () => void,
-  viewId: string,
   loadingType: ViewLoadingType,
   viewStart: ClocksState
 ) {
-  const viewMetrics: ViewMetrics = {
-    eventCounts: {
-      errorCount: 0,
-      longTaskCount: 0,
-      resourceCount: 0,
-      actionCount: 0,
-      frustrationCount: 0,
-    },
-  }
-  const { stop: stopEventCountsTracking } = trackEventCounts({
-    lifeCycle,
-    isChildEvent: (event) => event.view.id === viewId,
-    callback: (newEventCounts) => {
-      viewMetrics.eventCounts = newEventCounts
-      scheduleViewUpdate()
-    },
-  })
+  const viewMetrics: ViewMetrics = {}
 
   const { stop: stopLoadingTimeTracking, setLoadEvent } = trackLoadingTime(
     lifeCycle,
@@ -67,7 +47,6 @@ export function trackViewMetrics(
   }
   return {
     stop: () => {
-      stopEventCountsTracking()
       stopLoadingTimeTracking()
       stopCLSTracking()
     },

packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts

@@ -26,6 +26,7 @@ import type { RumConfiguration } from '../../configuration'
 import type { Timings } from './trackInitialViewTimings'
 import { trackInitialViewTimings } from './trackInitialViewTimings'
 import { trackViewMetrics } from './trackViewMetrics'
+import { trackViewEventCounts } from './trackViewEventCounts'
 
 export interface ViewEvent {
   id: string
@@ -226,14 +227,12 @@ function newView(
     setLoadEvent,
     stop: stopViewMetricsTracking,
     viewMetrics,
-  } = trackViewMetrics(
+  } = trackViewMetrics(lifeCycle, domMutationObservable, configuration, scheduleViewUpdate, loadingType, startClocks)
+
+  const { scheduleStop: scheduleStopEventCountsTracking, eventCounts } = trackViewEventCounts(
     lifeCycle,
-    domMutationObservable,
-    configuration,
-    scheduleViewUpdate,
     id,
-    loadingType,
-    startClocks
+    scheduleViewUpdate
   )
 
   // Initial view update
@@ -258,6 +257,7 @@ function newView(
           timings,
           duration: elapsed(startClocks.timeStamp, currentEnd),
           isActive: endClocks === undefined,
+          eventCounts,
         },
         viewMetrics
       )
@@ -273,6 +273,7 @@ function newView(
       endClocks = clocks
       lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, { endClocks })
       stopViewMetricsTracking()
+      scheduleStopEventCountsTracking()
     },
     triggerUpdate() {
       // cancel any pending view updates execution

packages/rum-core/src/domain/trackEventCounts.spec.ts

@@ -3,7 +3,6 @@ import { objectValues } from '@datadog/browser-core'
 import type { RumEvent } from '../rumEvent.types'
 import { FrustrationType, RumEventType } from '../rawRumEvent.types'
 import { LifeCycle, LifeCycleEventType } from './lifeCycle'
-import type { EventCounts } from './trackEventCounts'
 import { trackEventCounts } from './trackEventCounts'
 
 describe('trackEventCounts', () => {
@@ -71,16 +70,14 @@ describe('trackEventCounts', () => {
   })
 
   it('invokes a potential callback when a count is increased', () => {
-    const spy = jasmine.createSpy<(eventCounts: EventCounts) => void>()
-    trackEventCounts({ lifeCycle, isChildEvent: () => true, callback: spy })
+    const spy = jasmine.createSpy<() => void>()
+    trackEventCounts({ lifeCycle, isChildEvent: () => true, onChange: spy })
 
     notifyCollectedRawRumEvent({ type: RumEventType.RESOURCE })
     expect(spy).toHaveBeenCalledTimes(1)
-    expect(spy.calls.mostRecent().args[0].resourceCount).toBe(1)
 
     notifyCollectedRawRumEvent({ type: RumEventType.RESOURCE })
     expect(spy).toHaveBeenCalledTimes(2)
-    expect(spy.calls.mostRecent().args[0].resourceCount).toBe(2)
   })
 
   it('does not take into account events that are not child events', () => {

packages/rum-core/src/domain/trackEventCounts.ts

@@ -15,11 +15,11 @@ export interface EventCounts {
 export function trackEventCounts({
   lifeCycle,
   isChildEvent,
-  callback = noop,
+  onChange: callback = noop,
 }: {
   lifeCycle: LifeCycle
   isChildEvent: (event: RumActionEvent | RumErrorEvent | RumLongTaskEvent | RumResourceEvent) => boolean
-  callback?: (eventCounts: EventCounts) => void
+  onChange?: () => void
 }) {
   const eventCounts: EventCounts = {
     errorCount: 0,
@@ -36,22 +36,22 @@ export function trackEventCounts({
     switch (event.type) {
       case RumEventType.ERROR:
         eventCounts.errorCount += 1
-        callback(eventCounts)
+        callback()
         break
       case RumEventType.ACTION:
         eventCounts.actionCount += 1
         if (event.action.frustration) {
           eventCounts.frustrationCount += event.action.frustration.type.length
         }
-        callback(eventCounts)
+        callback()
         break
       case RumEventType.LONG_TASK:
         eventCounts.longTaskCount += 1
-        callback(eventCounts)
+        callback()
         break
       case RumEventType.RESOURCE:
         eventCounts.resourceCount += 1
-        callback(eventCounts)
+        callback()
         break
     }
   })