⚗️[RUM-2889] custom vitals improvements (#2606)

* 🐛 Fix multiple vital creation with multiple stop calls

* ✨ Add event limiter for vitals

* ✨ Allow to pass customer context to start/stop API

* ✨ Allow to pass startTime/stopTime

packages/core/src/tools/utils/timeUtils.ts

@@ -16,6 +16,10 @@ export function relativeToClocks(relative: RelativeTime) {
   return { relative, timeStamp: getCorrectedTimeStamp(relative) }
 }
 
+export function timeStampToClocks(timeStamp: TimeStamp) {
+  return { relative: getRelativeTime(timeStamp), timeStamp }
+}
+
 function getCorrectedTimeStamp(relativeTime: RelativeTime) {
   const correctedOrigin = (dateNow() - performance.now()) as TimeStamp
   // apply correction only for positive drift

packages/rum-core/src/boot/rumPublicApi.spec.ts

@@ -1,5 +1,7 @@
-import type { RelativeTime, Context, DeflateWorker, CustomerDataTrackerManager } from '@datadog/browser-core'
+import type { RelativeTime, Context, DeflateWorker, CustomerDataTrackerManager, TimeStamp } from '@datadog/browser-core'
 import {
+  timeStampToClocks,
+  clocksNow,
   addExperimentalFeatures,
   ExperimentalFeature,
   resetExperimentalFeatures,
@@ -722,6 +724,10 @@ describe('rum public api', () => {
   })
 
   describe('startDurationVital', () => {
+    beforeEach(() => {
+      setup().withFakeClock().build()
+    })
+
     afterEach(() => {
       resetExperimentalFeatures()
     })
@@ -744,12 +750,41 @@ describe('rum public api', () => {
       )
       rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-      ;(rumPublicApi as any).startDurationVital('foo')
-      expect(startDurationVitalSpy).toHaveBeenCalledWith({ name: 'foo', startClocks: jasmine.any(Object) })
+      ;(rumPublicApi as any).startDurationVital('foo', { context: { foo: 'bar' } })
+      expect(startDurationVitalSpy).toHaveBeenCalledWith({
+        name: 'foo',
+        startClocks: clocksNow(),
+        context: { foo: 'bar' },
+      })
+    })
+
+    it('should call startDurationVital with provided startTime when ff is enabled', () => {
+      addExperimentalFeatures([ExperimentalFeature.CUSTOM_VITALS])
+      const startDurationVitalSpy = jasmine.createSpy()
+      const rumPublicApi = makeRumPublicApi(
+        () => ({
+          ...noopStartRum(),
+          startDurationVital: startDurationVitalSpy,
+        }),
+        noopRecorderApi
+      )
+      const startTime = 1707755888000 as TimeStamp
+      rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+      ;(rumPublicApi as any).startDurationVital('foo', { startTime })
+      expect(startDurationVitalSpy).toHaveBeenCalledWith({
+        name: 'foo',
+        startClocks: timeStampToClocks(startTime),
+        context: undefined,
+      })
     })
   })
 
   describe('stopDurationVital', () => {
+    beforeEach(() => {
+      setup().withFakeClock().build()
+    })
+
     afterEach(() => {
       resetExperimentalFeatures()
     })
@@ -772,8 +807,33 @@ describe('rum public api', () => {
       )
       rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-      ;(rumPublicApi as any).stopDurationVital('foo')
-      expect(stopDurationVitalSpy).toHaveBeenCalledWith({ name: 'foo', stopClocks: jasmine.any(Object) })
+      ;(rumPublicApi as any).stopDurationVital('foo', { context: { foo: 'bar' } })
+      expect(stopDurationVitalSpy).toHaveBeenCalledWith({
+        name: 'foo',
+        stopClocks: clocksNow(),
+        context: { foo: 'bar' },
+      })
+    })
+
+    it('should call stopDurationVital with provided stopTime when ff is enabled', () => {
+      addExperimentalFeatures([ExperimentalFeature.CUSTOM_VITALS])
+      const stopDurationVitalSpy = jasmine.createSpy()
+      const rumPublicApi = makeRumPublicApi(
+        () => ({
+          ...noopStartRum(),
+          stopDurationVital: stopDurationVitalSpy,
+        }),
+        noopRecorderApi
+      )
+      const stopTime = 1707755888000 as TimeStamp
+      rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+      ;(rumPublicApi as any).stopDurationVital('foo', { stopTime })
+      expect(stopDurationVitalSpy).toHaveBeenCalledWith({
+        name: 'foo',
+        stopClocks: timeStampToClocks(stopTime),
+        context: undefined,
+      })
     })
   })
 

packages/rum-core/src/boot/rumPublicApi.ts

@@ -9,6 +9,7 @@ import type {
   TrackingConsent,
 } from '@datadog/browser-core'
 import {
+  timeStampToClocks,
   isExperimentalFeatureEnabled,
   ExperimentalFeature,
   CustomerDataType,
@@ -107,18 +108,41 @@ export function makeRumPublicApi(startRumImpl: StartRum, recorderApi: RecorderAp
 
     (initConfiguration, configuration, deflateWorker, initialViewOptions) => {
       if (isExperimentalFeatureEnabled(ExperimentalFeature.CUSTOM_VITALS)) {
-        ;(rumPublicApi as any).startDurationVital = monitor((name: string) => {
-          strategy.startDurationVital({
-            name: sanitize(name)!,
-            startClocks: clocksNow(),
-          })
-        })
-        ;(rumPublicApi as any).stopDurationVital = monitor((name: string) => {
-          strategy.stopDurationVital({
-            name: sanitize(name)!,
-            stopClocks: clocksNow(),
-          })
-        })
+        /**
+         * Start a custom duration vital
+         * stored in @vital.custom.<name>
+         *
+         * @param name name of the custom vital
+         * @param options.context custom context attached to the vital
+         * @param options.startTime epoch timestamp of the start of the custom vital (if not set, will use current time)
+         */
+        ;(rumPublicApi as any).startDurationVital = monitor(
+          (name: string, options?: { context?: object; startTime?: number }) => {
+            strategy.startDurationVital({
+              name: sanitize(name)!,
+              startClocks: options?.startTime ? timeStampToClocks(options.startTime as TimeStamp) : clocksNow(),
+              context: sanitize(options?.context) as Context,
+            })
+          }
+        )
+
+        /**
+         * Stop a custom duration vital
+         * stored in @vital.custom.<name>
+         *
+         * @param name name of the custom vital
+         * @param options.context custom context attached to the vital
+         * @param options.stopTime epoch timestamp of the stop of the custom vital (if not set, will use current time)
+         */
+        ;(rumPublicApi as any).stopDurationVital = monitor(
+          (name: string, options?: { context?: object; stopTime?: number }) => {
+            strategy.stopDurationVital({
+              name: sanitize(name)!,
+              stopClocks: options?.stopTime ? timeStampToClocks(options.stopTime as TimeStamp) : clocksNow(),
+              context: sanitize(options?.context) as Context,
+            })
+          }
+        )
       }
 
       if (initConfiguration.storeContextsAcrossPages) {

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

@@ -1,4 +1,4 @@
-import type { ClocksState, RelativeTime } from '@datadog/browser-core'
+import type { ClocksState, RelativeTime, TimeStamp } from '@datadog/browser-core'
 import { ErrorSource, ExperimentalFeature, ONE_MINUTE, display } from '@datadog/browser-core'
 import {
   initEventBridgeStub,
@@ -18,7 +18,7 @@ import {
   cleanupCiVisibilityWindowValues,
 } from '../../test'
 import type { RumEventDomainContext } from '../domainContext.types'
-import type { RawRumActionEvent, RawRumErrorEvent, RawRumEvent } from '../rawRumEvent.types'
+import type { RawRumActionEvent, RawRumEvent } from '../rawRumEvent.types'
 import { RumEventType } from '../rawRumEvent.types'
 import type { RumActionEvent, RumErrorEvent, RumEvent, RumResourceEvent } from '../rumEvent.types'
 import { startRumAssembly } from './assembly'
@@ -827,128 +827,90 @@ describe('rum assembly', () => {
       })
     })
   })
-
-  describe('error events limitation', () => {
-    it('stops sending error events when reaching the limit', () => {
-      const { lifeCycle } = setupBuilder.withConfiguration({ eventRateLimiterThreshold: 1 }).build()
-      notifyRawRumErrorEvent(lifeCycle, 'foo')
-      notifyRawRumErrorEvent(lifeCycle, 'bar')
-
-      expect(serverRumEvents.length).toBe(1)
-      expect((serverRumEvents[0] as RumErrorEvent).error.message).toBe('foo')
-      expect(reportErrorSpy).toHaveBeenCalledTimes(1)
-      expect(reportErrorSpy.calls.argsFor(0)[0]).toEqual(
-        jasmine.objectContaining({
-          message: 'Reached max number of errors by minute: 1',
-          source: ErrorSource.AGENT,
+  ;[
+    {
+      eventType: RumEventType.ERROR,
+      message: 'Reached max number of errors by minute: 1',
+    },
+    {
+      eventType: RumEventType.ACTION,
+      message: 'Reached max number of actions by minute: 1',
+    },
+    {
+      eventType: RumEventType.VITAL,
+      message: 'Reached max number of vitals by minute: 1',
+    },
+  ].forEach(({ eventType, message }) => {
+    describe(`${eventType} events limitation`, () => {
+      it(`stops sending ${eventType} events when reaching the limit`, () => {
+        const { lifeCycle } = setupBuilder.withConfiguration({ eventRateLimiterThreshold: 1 }).build()
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 100 as TimeStamp }),
         })
-      )
-    })
-
-    it('does not take discarded errors into account', () => {
-      const { lifeCycle } = setupBuilder
-        .withConfiguration({
-          eventRateLimiterThreshold: 1,
-          beforeSend: (event) => {
-            if (event.type === RumEventType.ERROR && (event as RumErrorEvent).error.message === 'discard me') {
-              return false
-            }
-          },
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 200 as TimeStamp }),
         })
-        .build()
-      notifyRawRumErrorEvent(lifeCycle, 'discard me')
-      notifyRawRumErrorEvent(lifeCycle, 'discard me')
-      notifyRawRumErrorEvent(lifeCycle, 'discard me')
-      notifyRawRumErrorEvent(lifeCycle, 'foo')
-      expect(serverRumEvents.length).toBe(1)
-      expect((serverRumEvents[0] as RumErrorEvent).error.message).toBe('foo')
-      expect(reportErrorSpy).not.toHaveBeenCalled()
-    })
-
-    it('allows to send new errors after a minute', () => {
-      const { lifeCycle, clock } = setupBuilder
-        .withFakeClock()
-        .withConfiguration({ eventRateLimiterThreshold: 1 })
-        .build()
-      notifyRawRumErrorEvent(lifeCycle, 'foo')
-      notifyRawRumErrorEvent(lifeCycle, 'bar')
-      clock.tick(ONE_MINUTE)
-      notifyRawRumErrorEvent(lifeCycle, 'baz')
-
-      expect(serverRumEvents.length).toBe(2)
-      expect((serverRumEvents[0] as RumErrorEvent).error.message).toBe('foo')
-      expect((serverRumEvents[1] as RumErrorEvent).error.message).toBe('baz')
-    })
 
-    function notifyRawRumErrorEvent(lifeCycle: LifeCycle, message: string) {
-      const rawRumEvent = createRawRumEvent(RumEventType.ERROR) as RawRumErrorEvent
-      rawRumEvent.error.message = message
-      notifyRawRumEvent(lifeCycle, {
-        rawRumEvent,
+        expect(serverRumEvents.length).toBe(1)
+        expect(serverRumEvents[0].date).toBe(100)
+        expect(reportErrorSpy).toHaveBeenCalledTimes(1)
+        expect(reportErrorSpy.calls.argsFor(0)[0]).toEqual(
+          jasmine.objectContaining({
+            message,
+            source: ErrorSource.AGENT,
+          })
+        )
       })
-    }
-  })
-
-  describe('action events limitation', () => {
-    it('stops sending action events when reaching the limit', () => {
-      const { lifeCycle } = setupBuilder.withConfiguration({ eventRateLimiterThreshold: 1 }).build()
 
-      notifyRumActionEvent(lifeCycle, 'foo')
-      notifyRumActionEvent(lifeCycle, 'bar')
-
-      expect(serverRumEvents.length).toBe(1)
-      expect((serverRumEvents[0] as RumActionEvent).action.target?.name).toBe('foo')
-      expect(reportErrorSpy).toHaveBeenCalledTimes(1)
-      expect(reportErrorSpy.calls.argsFor(0)[0]).toEqual(
-        jasmine.objectContaining({
-          message: 'Reached max number of actions by minute: 1',
-          source: ErrorSource.AGENT,
+      it(`does not take discarded ${eventType} events into account`, () => {
+        const { lifeCycle } = setupBuilder
+          .withConfiguration({
+            eventRateLimiterThreshold: 1,
+            beforeSend: (event) => {
+              if (event.type === eventType && event.date === 100) {
+                return false
+              }
+            },
+          })
+          .build()
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 100 as TimeStamp }),
         })
-      )
-    })
-
-    it('does not take discarded actions into account', () => {
-      const { lifeCycle } = setupBuilder
-        .withConfiguration({
-          eventRateLimiterThreshold: 1,
-          beforeSend: (event) => {
-            if (event.type === RumEventType.ACTION && (event as RumActionEvent).action.target?.name === 'discard me') {
-              return false
-            }
-          },
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 100 as TimeStamp }),
         })
-        .build()
-      notifyRumActionEvent(lifeCycle, 'discard me')
-      notifyRumActionEvent(lifeCycle, 'discard me')
-      notifyRumActionEvent(lifeCycle, 'discard me')
-      notifyRumActionEvent(lifeCycle, 'foo')
-      expect(serverRumEvents.length).toBe(1)
-      expect((serverRumEvents[0] as RumActionEvent).action.target?.name).toBe('foo')
-      expect(reportErrorSpy).not.toHaveBeenCalled()
-    })
-
-    it('allows to send new actions after a minute', () => {
-      const { lifeCycle, clock } = setupBuilder
-        .withFakeClock()
-        .withConfiguration({ eventRateLimiterThreshold: 1 })
-        .build()
-      notifyRumActionEvent(lifeCycle, 'foo')
-      notifyRumActionEvent(lifeCycle, 'bar')
-      clock.tick(ONE_MINUTE)
-      notifyRumActionEvent(lifeCycle, 'baz')
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 100 as TimeStamp }),
+        })
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 200 as TimeStamp }),
+        })
+        expect(serverRumEvents.length).toBe(1)
+        expect(serverRumEvents[0].date).toBe(200)
+        expect(reportErrorSpy).not.toHaveBeenCalled()
+      })
 
-      expect(serverRumEvents.length).toBe(2)
-      expect((serverRumEvents[0] as RumActionEvent).action.target?.name).toBe('foo')
-      expect((serverRumEvents[1] as RumActionEvent).action.target?.name).toBe('baz')
-    })
+      it(`allows to send new ${eventType} events after a minute`, () => {
+        const { lifeCycle, clock } = setupBuilder
+          .withFakeClock()
+          .withConfiguration({ eventRateLimiterThreshold: 1 })
+          .build()
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 100 as TimeStamp }),
+        })
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 200 as TimeStamp }),
+        })
+        clock.tick(ONE_MINUTE)
+        notifyRawRumEvent(lifeCycle, {
+          rawRumEvent: createRawRumEvent(eventType, { date: 300 as TimeStamp }),
+        })
 
-    function notifyRumActionEvent(lifeCycle: LifeCycle, name: string) {
-      const rawRumEvent = createRawRumEvent(RumEventType.ACTION) as RawRumActionEvent
-      rawRumEvent.action.target.name = name
-      notifyRawRumEvent(lifeCycle, {
-        rawRumEvent,
+        expect(serverRumEvents.length).toBe(2)
+        expect(serverRumEvents[0].date).toBe(100)
+        expect(serverRumEvents[1].date).toBe(300)
       })
-    }
+    })
   })
 })