♻️ [RUMF-1015] use the url corresponding to the start of the event

packages/core/src/tools/utils.ts

@@ -527,6 +527,13 @@ type Combined<A, B> = A extends null ? B : B extends null ? A : Merged<A, B>
 export function combine<A, B>(a: A, b: B): Combined<A, B>
 export function combine<A, B, C>(a: A, b: B, c: C): Combined<Combined<A, B>, C>
 export function combine<A, B, C, D>(a: A, b: B, c: C, d: D): Combined<Combined<Combined<A, B>, C>, D>
+export function combine<A, B, C, D, E>(
+  a: A,
+  b: B,
+  c: C,
+  d: D,
+  e: E
+): Combined<Combined<Combined<Combined<A, B>, C>, D>, E>
 export function combine(...sources: any[]): unknown {
   let destination: any
 

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

@@ -1,15 +1,16 @@
-import { RelativeTime, Configuration, Observable } from '@datadog/browser-core'
+import { RelativeTime, Configuration, Observable, noop, relativeNow } from '@datadog/browser-core'
 import { RumSession } from '@datadog/browser-rum-core'
 import { createRumSessionMock, RumSessionMock } from '../../test/mockRumSession'
 import { isIE } from '../../../core/test/specHelper'
 import { noopRecorderApi, setup, TestSetupBuilder } from '../../test/specHelper'
-import { RumPerformanceNavigationTiming } from '../browser/performanceCollection'
+import { RumPerformanceNavigationTiming, RumPerformanceEntry } from '../browser/performanceCollection'
 
 import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle'
 import { SESSION_KEEP_ALIVE_INTERVAL, THROTTLE_VIEW_UPDATE_PERIOD } from '../domain/rumEventsCollection/view/trackViews'
 import { startViewCollection } from '../domain/rumEventsCollection/view/viewCollection'
 import { RumEvent } from '../rumEvent.types'
 import { LocationChange } from '../browser/locationChangeObservable'
+import { startLongTaskCollection } from '../domain/rumEventsCollection/longTask/longTaskCollection'
 import { startRumEventCollection } from './startRum'
 
 function collectServerEvents(lifeCycle: LifeCycle) {
@@ -33,7 +34,9 @@ function startRum(
     applicationId,
     lifeCycle,
     configuration,
+    location,
     session,
+    locationChangeObservable,
     () => ({
       context: {},
       user: {},
@@ -48,6 +51,8 @@ function startRum(
     foregroundContexts,
     noopRecorderApi
   )
+
+  startLongTaskCollection(lifeCycle)
   return {
     stop: () => {
       rumEventCollectionStop()
@@ -190,7 +195,7 @@ describe('rum session keep alive', () => {
   })
 })
 
-describe('rum view url', () => {
+describe('rum events url', () => {
   const FAKE_NAVIGATION_ENTRY: RumPerformanceNavigationTiming = {
     domComplete: 456 as RelativeTime,
     domContentLoadedEventEnd: 345 as RelativeTime,
@@ -232,6 +237,34 @@ describe('rum view url', () => {
     setupBuilder.cleanup()
   })
 
+  it('should attach the url corresponding to the start of the event', () => {
+    const { lifeCycle, clock, changeLocation } = setupBuilder
+      .withFakeClock()
+      .withFakeLocation('http://foo.com/')
+      .build()
+    clock.tick(10)
+    changeLocation('http://foo.com/?bar=bar')
+    clock.tick(10)
+    changeLocation('http://foo.com/?bar=qux')
+
+    lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, {
+      entryType: 'longtask',
+      startTime: relativeNow() - 5,
+      toJSON: noop,
+      duration: 5,
+    } as RumPerformanceEntry)
+
+    clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
+
+    expect(serverRumEvents.length).toBe(3)
+    const [firstViewUpdate, longTaskEvent, lastViewUpdate] = serverRumEvents
+
+    expect(firstViewUpdate.view.url).toBe('http://foo.com/')
+    expect(lastViewUpdate.view.url).toBe('http://foo.com/')
+
+    expect(longTaskEvent.view.url).toBe('http://foo.com/?bar=bar')
+  })
+
   it('should keep the same URL when updating a view ended by a URL change', () => {
     const { changeLocation } = setupBuilder.withFakeLocation('http://foo.com/').build()
 

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

@@ -1,4 +1,4 @@
-import { combine, Configuration, InternalMonitoring } from '@datadog/browser-core'
+import { combine, Configuration, InternalMonitoring, Observable } from '@datadog/browser-core'
 import { createDOMMutationObservable } from '../browser/domMutationObservable'
 import { startPerformanceCollection } from '../browser/performanceCollection'
 import { startRumAssembly } from '../domain/assembly'
@@ -15,7 +15,8 @@ import { startViewCollection } from '../domain/rumEventsCollection/view/viewColl
 import { RumSession, startRumSession } from '../domain/rumSession'
 import { CommonContext } from '../rawRumEvent.types'
 import { startRumBatch } from '../transport/batch'
-import { createLocationChangeObservable } from '../browser/locationChangeObservable'
+import { startUrlContexts } from '../domain/urlContexts'
+import { createLocationChangeObservable, LocationChange } from '../browser/locationChangeObservable'
 import { RecorderApi, RumInitConfiguration } from './rumPublicApi'
 
 export function startRum(
@@ -41,11 +42,13 @@ export function startRum(
     )
   )
 
-  const { parentContexts, foregroundContexts } = startRumEventCollection(
+  const { parentContexts, foregroundContexts, urlContexts } = startRumEventCollection(
     initConfiguration.applicationId,
     lifeCycle,
     configuration,
+    location,
     session,
+    locationChangeObservable,
     getCommonContext
   )
 
@@ -69,7 +72,7 @@ export function startRum(
   startRequestCollection(lifeCycle, configuration)
   startPerformanceCollection(lifeCycle, configuration)
 
-  const internalContext = startInternalContext(initConfiguration.applicationId, session, parentContexts)
+  const internalContext = startInternalContext(initConfiguration.applicationId, session, parentContexts, urlContexts)
 
   return {
     addAction,
@@ -87,18 +90,22 @@ export function startRumEventCollection(
   applicationId: string,
   lifeCycle: LifeCycle,
   configuration: Configuration,
+  location: Location,
   session: RumSession,
+  locationChangeObservable: Observable<LocationChange>,
   getCommonContext: () => CommonContext
 ) {
   const parentContexts = startParentContexts(lifeCycle, session)
+  const urlContexts = startUrlContexts(lifeCycle, locationChangeObservable, location)
   const foregroundContexts = startForegroundContexts()
   const batch = startRumBatch(configuration, lifeCycle)
 
-  startRumAssembly(applicationId, configuration, lifeCycle, session, parentContexts, getCommonContext)
+  startRumAssembly(applicationId, configuration, lifeCycle, session, parentContexts, urlContexts, getCommonContext)
 
   return {
     parentContexts,
     foregroundContexts,
+    urlContexts,
     stop: () => {
       // prevent batch from previous tests to keep running and send unwanted requests
       // could be replaced by stopping all the component when they will all have a stop method

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

@@ -35,17 +35,23 @@ describe('rum assembly', () => {
           },
           view: {
             id: 'abcde',
-            referrer: 'url',
-            url: 'url',
           },
         }),
       })
-      .beforeBuild(({ applicationId, configuration, lifeCycle, session, parentContexts }) => {
+      .beforeBuild(({ applicationId, configuration, lifeCycle, session, parentContexts, urlContexts }) => {
         serverRumEvents = []
         lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (serverRumEvent) =>
           serverRumEvents.push(serverRumEvent)
         )
-        startRumAssembly(applicationId, configuration, lifeCycle, session, parentContexts, () => commonContext)
+        startRumAssembly(
+          applicationId,
+          configuration,
+          lifeCycle,
+          session,
+          parentContexts,
+          urlContexts,
+          () => commonContext
+        )
       })
   })
 
@@ -431,15 +437,22 @@ describe('rum assembly', () => {
       notifyRawRumEvent(lifeCycle, {
         rawRumEvent: createRawRumEvent(RumEventType.ACTION),
       })
-      expect(serverRumEvents[0].view).toEqual({
-        id: 'abcde',
-        referrer: 'url',
-        url: 'url',
-      })
+      expect(serverRumEvents[0].view.id).toBe('abcde')
       expect(serverRumEvents[0].session.id).toBe('1234')
     })
   })
 
+  describe('url context', () => {
+    it('should be merged with event attributes', () => {
+      const { lifeCycle, fakeLocation } = setupBuilder.build()
+      notifyRawRumEvent(lifeCycle, {
+        rawRumEvent: createRawRumEvent(RumEventType.ACTION),
+      })
+      expect(serverRumEvents[0].view.url).toBe(fakeLocation.href!)
+      expect(serverRumEvents[0].view.referrer).toBe(document.referrer)
+    })
+  })
+
   describe('event generation condition', () => {
     it('when tracked, it should generate event', () => {
       const { lifeCycle } = setupBuilder.build()

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

@@ -29,6 +29,7 @@ import { RumEvent } from '../rumEvent.types'
 import { LifeCycle, LifeCycleEventType } from './lifeCycle'
 import { ParentContexts } from './parentContexts'
 import { RumSession, RumSessionPlan } from './rumSession'
+import { UrlContexts } from './urlContexts'
 
 export interface BrowserWindow extends Window {
   _DATADOG_SYNTHETICS_PUBLIC_ID?: string
@@ -65,6 +66,7 @@ export function startRumAssembly(
   lifeCycle: LifeCycle,
   session: RumSession,
   parentContexts: ParentContexts,
+  urlContexts: UrlContexts,
   getCommonContext: () => CommonContext
 ) {
   const reportError = (error: RawError) => {
@@ -80,7 +82,8 @@ export function startRumAssembly(
     LifeCycleEventType.RAW_RUM_EVENT_COLLECTED,
     ({ startTime, rawRumEvent, domainContext, savedCommonContext, customerContext }) => {
       const viewContext = parentContexts.findView(startTime)
-      if (session.isTracked() && viewContext && viewContext.session.id === session.getId()) {
+      const urlContext = urlContexts.findUrl(startTime)
+      if (session.isTracked() && viewContext && urlContext && viewContext.session.id === session.getId()) {
         const actionContext = parentContexts.findAction(startTime)
         const commonContext = savedCommonContext || getCommonContext()
         const rumContext: RumContext = {
@@ -103,8 +106,8 @@ export function startRumAssembly(
           synthetics: getSyntheticsContext(),
         }
         const serverRumEvent = (needToAssembleWithAction(rawRumEvent)
-          ? combine(rumContext, viewContext, actionContext, rawRumEvent)
-          : combine(rumContext, viewContext, rawRumEvent)) as RumEvent & Context
+          ? combine(rumContext, urlContext, viewContext, actionContext, rawRumEvent)
+          : combine(rumContext, urlContext, viewContext, rawRumEvent)) as RumEvent & Context
 
         serverRumEvent.context = combine(commonContext.context, customerContext)
 

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

@@ -5,12 +5,12 @@ import { CLEAR_OLD_CONTEXTS_INTERVAL, ContextHistory } from './contextHistory'
 const EXPIRE_DELAY = 10 * ONE_MINUTE
 
 describe('contextHistory', () => {
-  let contextHistory: ContextHistory<{ value: string }, { value: string }>
+  let contextHistory: ContextHistory<{ value: string }>
   let clock: Clock
 
   beforeEach(() => {
     clock = mockClock()
-    contextHistory = new ContextHistory((raw) => ({ value: raw.value }), EXPIRE_DELAY)
+    contextHistory = new ContextHistory(EXPIRE_DELAY)
   })
 
   afterEach(() => {

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

@@ -8,22 +8,22 @@ interface PreviousContext<T> {
 
 export const CLEAR_OLD_CONTEXTS_INTERVAL = ONE_MINUTE
 
-export class ContextHistory<Raw, Built> {
-  private current: Raw | undefined
+export class ContextHistory<Context> {
+  private current: Context | undefined
   private currentStart: RelativeTime | undefined
-  private previousContexts: Array<PreviousContext<Built>> = []
+  private previousContexts: Array<PreviousContext<Context>> = []
   private clearOldContextsInterval: number
 
-  constructor(private buildContext: (r: Raw) => Built, private expireDelay: number) {
+  constructor(private expireDelay: number) {
     this.clearOldContextsInterval = setInterval(() => this.clearOldContexts(), CLEAR_OLD_CONTEXTS_INTERVAL)
   }
 
   find(startTime?: RelativeTime) {
-    if (startTime === undefined) {
-      return this.current ? this.buildContext(this.current) : undefined
-    }
-    if (this.current !== undefined && this.currentStart !== undefined && startTime >= this.currentStart) {
-      return this.buildContext(this.current)
+    if (
+      startTime === undefined ||
+      (this.current !== undefined && this.currentStart !== undefined && startTime >= this.currentStart)
+    ) {
+      return this.current
     }
     for (const previousContext of this.previousContexts) {
       if (startTime > previousContext.endTime) {
@@ -36,7 +36,7 @@ export class ContextHistory<Raw, Built> {
     return undefined
   }
 
-  setCurrent(current: Raw, startTime: RelativeTime) {
+  setCurrent(current: Context, startTime: RelativeTime) {
     this.current = current
     this.currentStart = startTime
   }
@@ -54,7 +54,7 @@ export class ContextHistory<Raw, Built> {
     if (this.current !== undefined && this.currentStart !== undefined) {
       this.previousContexts.unshift({
         endTime,
-        context: this.buildContext(this.current),
+        context: this.current,
         startTime: this.currentStart,
       })
       this.clearCurrent()

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

@@ -1,11 +1,14 @@
 import { createRumSessionMock } from 'packages/rum-core/test/mockRumSession'
+import { RelativeTime } from '@datadog/browser-core'
 import { setup, TestSetupBuilder } from '../../test/specHelper'
 import { startInternalContext } from './internalContext'
 import { ParentContexts } from './parentContexts'
+import { UrlContexts } from './urlContexts'
 
 describe('internal context', () => {
   let setupBuilder: TestSetupBuilder
   let parentContextsStub: Partial<ParentContexts>
+  let findUrlSpy: jasmine.Spy<UrlContexts['findUrl']>
   let internalContext: ReturnType<typeof startInternalContext>
 
   beforeEach(() => {
@@ -21,15 +24,14 @@ describe('internal context', () => {
         },
         view: {
           id: 'abcde',
-          referrer: 'referrer',
-          url: 'url',
         },
       }),
     }
     setupBuilder = setup()
       .withParentContexts(parentContextsStub)
-      .beforeBuild(({ applicationId, session, parentContexts }) => {
-        internalContext = startInternalContext(applicationId, session, parentContexts)
+      .beforeBuild(({ applicationId, session, parentContexts, urlContexts }) => {
+        findUrlSpy = spyOn(urlContexts, 'findUrl').and.callThrough()
+        internalContext = startInternalContext(applicationId, session, parentContexts, urlContexts)
       })
   })
 
@@ -38,7 +40,7 @@ describe('internal context', () => {
   })
 
   it('should return current internal context', () => {
-    setupBuilder.build()
+    const { fakeLocation } = setupBuilder.build()
 
     expect(internalContext.get()).toEqual({
       application_id: 'appId',
@@ -48,8 +50,8 @@ describe('internal context', () => {
       },
       view: {
         id: 'abcde',
-        referrer: 'referrer',
-        url: 'url',
+        referrer: document.referrer,
+        url: fakeLocation.href!,
       },
     })
   })
@@ -66,5 +68,6 @@ describe('internal context', () => {
 
     expect(parentContextsStub.findView).toHaveBeenCalledWith(123)
     expect(parentContextsStub.findAction).toHaveBeenCalledWith(123)
+    expect(findUrlSpy).toHaveBeenCalledWith(123 as RelativeTime)
   })
 })