From 46c2b56c1b50cf98fe61d4827a76c3f75f6619ed Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Wed, 29 Jan 2025 09:05:55 +0100 Subject: [PATCH 1/2] Use assembly hook for action --- packages/rum-core/src/boot/startRum.ts | 2 +- .../domain/action/actionCollection.spec.ts | 29 +++++++++++++++++ .../src/domain/action/actionCollection.ts | 23 +++++++++++++ packages/rum-core/src/domain/assembly.spec.ts | 32 ++----------------- packages/rum-core/src/domain/assembly.ts | 18 +---------- 5 files changed, 56 insertions(+), 48 deletions(-) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index c0d3c42a31..882c99e2ce 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -255,6 +255,7 @@ export function startRumEventCollection( ) { const actionCollection = startActionCollection( lifeCycle, + hooks, domMutationObservable, windowOpenObservable, configuration, @@ -270,7 +271,6 @@ export function startRumEventCollection( hooks, sessionManager, viewHistory, - actionCollection.actionContexts, displayContext, ciVisibilityContext, featureFlagContexts, diff --git a/packages/rum-core/src/domain/action/actionCollection.spec.ts b/packages/rum-core/src/domain/action/actionCollection.spec.ts index 0d54f41c5c..239a9857a6 100644 --- a/packages/rum-core/src/domain/action/actionCollection.spec.ts +++ b/packages/rum-core/src/domain/action/actionCollection.spec.ts @@ -6,21 +6,28 @@ import { collectAndValidateRawRumEvents, mockPageStateHistory, mockRumConfigurat import type { RawRumEvent } from '../../rawRumEvent.types' import { RumEventType, ActionType } from '../../rawRumEvent.types' import { LifeCycle, LifeCycleEventType } from '../lifeCycle' +import type { Hooks } from '../../hooks' +import { createHooks, HookNames } from '../../hooks' +import type { ActionContexts } from './actionCollection' import { startActionCollection } from './actionCollection' const basePageStateHistory = mockPageStateHistory({ wasInPageStateAt: () => true }) describe('actionCollection', () => { const lifeCycle = new LifeCycle() + let hooks: Hooks let addAction: ReturnType['addAction'] let rawRumEvents: Array> + let actionContexts: ActionContexts beforeEach(() => { const domMutationObservable = new Observable() const windowOpenObservable = new Observable() + hooks = createHooks() const actionCollection = startActionCollection( lifeCycle, + hooks, domMutationObservable, windowOpenObservable, mockRumConfiguration(), @@ -28,6 +35,7 @@ describe('actionCollection', () => { ) registerCleanupTask(actionCollection.stop) addAction = actionCollection.addAction + actionContexts = actionCollection.actionContexts rawRumEvents = collectAndValidateRawRumEvents(lifeCycle) }) @@ -162,4 +170,25 @@ describe('actionCollection', () => { handlingStack: 'Error\n at foo\n at bar', }) }) + + describe('assembly hook', () => { + ;[RumEventType.RESOURCE, RumEventType.LONG_TASK, RumEventType.ERROR].forEach((eventType) => { + it(`should add action properties on ${eventType} from the context`, () => { + const actionId = '1' + spyOn(actionContexts, 'findActionId').and.returnValue(actionId) + const event = hooks.triggerHook(HookNames.Assemble, { eventType, startTime: 0 as RelativeTime }) + + expect(event).toEqual({ type: eventType, action: { id: actionId } }) + }) + }) + ;[RumEventType.VIEW, RumEventType.VITAL].forEach((eventType) => { + it(`should not add action properties on ${eventType} from the context`, () => { + const actionId = '1' + spyOn(actionContexts, 'findActionId').and.returnValue(actionId) + const event = hooks.triggerHook(HookNames.Assemble, { eventType, startTime: 0 as RelativeTime }) + + expect(event).toEqual(undefined) + }) + }) + }) }) diff --git a/packages/rum-core/src/domain/action/actionCollection.ts b/packages/rum-core/src/domain/action/actionCollection.ts index eebd10b20e..2e9691731b 100644 --- a/packages/rum-core/src/domain/action/actionCollection.ts +++ b/packages/rum-core/src/domain/action/actionCollection.ts @@ -17,6 +17,8 @@ import type { CommonContext } from '../contexts/commonContext' import type { PageStateHistory } from '../contexts/pageStateHistory' import { PageState } from '../contexts/pageStateHistory' import type { RumActionEventDomainContext } from '../../domainContext.types' +import type { PartialRumEvent, Hooks } from '../../hooks' +import { HookNames } from '../../hooks' import type { ActionContexts, ClickAction } from './trackClickActions' import { trackClickActions } from './trackClickActions' @@ -34,6 +36,7 @@ export type AutoAction = ClickAction export function startActionCollection( lifeCycle: LifeCycle, + hooks: Hooks, domMutationObservable: Observable, windowOpenObservable: Observable, configuration: RumConfiguration, @@ -43,6 +46,26 @@ export function startActionCollection( lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, processAction(action, pageStateHistory)) ) + hooks.register(HookNames.Assemble, ({ startTime, eventType }): PartialRumEvent | undefined => { + if ( + eventType !== RumEventType.ERROR && + eventType !== RumEventType.RESOURCE && + eventType !== RumEventType.LONG_TASK + ) { + return + } + + const actionId = actionContexts.findActionId(startTime) + if (!actionId) { + return + } + + return { + type: eventType, + action: { id: actionId }, + } + }) + let actionContexts: ActionContexts = { findActionId: noop as () => undefined } let stop: () => void = noop diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index 19c4dd61eb..804c94c678 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -15,15 +15,14 @@ import { createRumSessionManagerMock, createRawRumEvent, mockRumConfiguration, - mockActionContexts, mockDisplayContext, mockViewHistory, mockFeatureFlagContexts, } from '../../test' import type { RumEventDomainContext } from '../domainContext.types' -import type { RawRumActionEvent, RawRumEvent } from '../rawRumEvent.types' +import type { RawRumEvent } from '../rawRumEvent.types' import { RumEventType } from '../rawRumEvent.types' -import type { RumActionEvent, RumErrorEvent, RumEvent, RumResourceEvent } from '../rumEvent.types' +import type { RumErrorEvent, RumEvent, RumResourceEvent } from '../rumEvent.types' import { HookNames, createHooks } from '../hooks' import { startRumAssembly } from './assembly' import type { RawRumEventCollectedData } from './lifeCycle' @@ -507,32 +506,6 @@ describe('rum assembly', () => { }) }) - describe('action context', () => { - it('should be added on some event categories', () => { - const { lifeCycle, serverRumEvents } = setupAssemblyTestWithDefaults() - ;[RumEventType.RESOURCE, RumEventType.LONG_TASK, RumEventType.ERROR].forEach((category) => { - notifyRawRumEvent(lifeCycle, { - rawRumEvent: createRawRumEvent(category), - }) - expect(serverRumEvents[0].action).toEqual({ id: '7890' }) - serverRumEvents.length = 0 - }) - - notifyRawRumEvent(lifeCycle, { - rawRumEvent: createRawRumEvent(RumEventType.VIEW), - }) - expect(serverRumEvents[0].action).not.toBeDefined() - serverRumEvents.length = 0 - - const generatedRawRumActionEvent = createRawRumEvent(RumEventType.ACTION) as RawRumActionEvent - notifyRawRumEvent(lifeCycle, { - rawRumEvent: generatedRawRumActionEvent, - }) - expect((serverRumEvents[0] as RumActionEvent).action.id).toEqual(generatedRawRumActionEvent.action.id) - serverRumEvents.length = 0 - }) - }) - describe('service and version', () => { const extraConfigurationOptions = { service: 'default service', version: 'default version' } @@ -1000,7 +973,6 @@ function setupAssemblyTestWithDefaults({ hooks, rumSessionManager, { ...mockViewHistory(), findView: () => findView() }, - mockActionContexts(), mockDisplayContext(), { get: () => ciVisibilityContext } as CiVisibilityContext, featureFlagContexts, diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index fa57a4db8c..b430a16bd2 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -13,13 +13,7 @@ import { getConnectivity, } from '@datadog/browser-core' import type { RumEventDomainContext } from '../domainContext.types' -import type { - RawRumErrorEvent, - RawRumEvent, - RawRumLongTaskEvent, - RawRumResourceEvent, - RumContext, -} from '../rawRumEvent.types' +import type { RawRumEvent, RumContext } from '../rawRumEvent.types' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' import type { Hooks } from '../hooks' @@ -31,7 +25,6 @@ import { LifeCycleEventType } from './lifeCycle' import type { ViewHistory } from './contexts/viewHistory' import { SessionReplayState, type RumSessionManager } from './rumSessionManager' import type { RumConfiguration, FeatureFlagsForEvents } from './configuration' -import type { ActionContexts } from './action/actionCollection' import type { DisplayContext } from './contexts/displayContext' import type { CommonContext } from './contexts/commonContext' import type { ModifiableFieldPaths } from './limitModification' @@ -72,7 +65,6 @@ export function startRumAssembly( hooks: Hooks, sessionManager: RumSessionManager, viewHistory: ViewHistory, - actionContexts: ActionContexts, displayContext: DisplayContext, ciVisibilityContext: CiVisibilityContext, featureFlagContexts: FeatureFlagContexts, @@ -141,7 +133,6 @@ export function startRumAssembly( if (session && viewHistoryEntry) { const commonContext = savedCommonContext || getCommonContext() - const actionId = actionContexts.findActionId(startTime) const rumContext: RumContext = { _dd: { @@ -172,7 +163,6 @@ export function startRumAssembly( configuration.trackFeatureFlagsForEvents, featureFlagContexts ), - action: needToAssembleWithAction(rawRumEvent) && actionId ? { id: actionId } : undefined, synthetics: syntheticsContext, ci_test: ciVisibilityContext.get(), display: displayContext.get(), @@ -236,12 +226,6 @@ function shouldSend( return !rateLimitReached } -function needToAssembleWithAction( - event: RawRumEvent -): event is RawRumErrorEvent | RawRumResourceEvent | RawRumLongTaskEvent { - return [RumEventType.ERROR, RumEventType.RESOURCE, RumEventType.LONG_TASK].indexOf(event.type) !== -1 -} - function findFeatureFlagsContext( rawRumEvent: RawRumEvent, eventStartTime: RelativeTime, From 83e2e5fd519230fe7c172cf057052e2567c42982 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 13 Feb 2025 10:41:08 +0100 Subject: [PATCH 2/2] Fix assembly --- packages/rum-core/src/domain/assembly.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index 818341179c..dfe4e0186c 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -14,11 +14,7 @@ import { addTelemetryDebug, } from '@datadog/browser-core' import type { RumEventDomainContext } from '../domainContext.types' -<<<<<<< HEAD -import type { RawRumEvent, RumContext } from '../rawRumEvent.types' -======= -import type { RawRumErrorEvent, RawRumEvent, RawRumLongTaskEvent, RawRumResourceEvent } from '../rawRumEvent.types' ->>>>>>> main +import type { RawRumEvent } from '../rawRumEvent.types' import { RumEventType } from '../rawRumEvent.types' import type { CommonProperties, RumEvent } from '../rumEvent.types' import type { Hooks } from '../hooks'