[RUMF-158] fix view id associated to different session id

packages/rum/src/rum.ts

@@ -22,7 +22,7 @@ import { matchRequestTiming } from './matchRequestTiming'
 import { computePerformanceResourceDetails, computeResourceKind, computeSize, isValidResource } from './resourceUtils'
 import { RumGlobal } from './rum.entry'
 import { RumSession } from './rumSession'
-import { trackView, viewId, viewLocation, ViewMeasures } from './viewTracker'
+import { trackView, viewContext, ViewMeasures } from './viewTracker'
 
 export interface PerformancePaintTiming extends PerformanceEntry {
   entryType: 'paint'
@@ -136,20 +136,20 @@ export function startRum(
       date: new Date().getTime(),
       screen: {
         // needed for retro compatibility
-        id: viewId,
-        url: viewLocation.href,
+        id: viewContext.id,
+        url: viewContext.location.href,
       },
-      sessionId: session.getId(),
+      sessionId: viewContext.sessionId,
       view: {
-        id: viewId,
+        id: viewContext.id,
         referrer: document.referrer,
-        url: viewLocation.href,
+        url: viewContext.location.href,
       },
     }),
     () => globalContext
   )
 
-  trackView(window.location, lifeCycle, batch.addRumEvent, batch.beforeFlushOnUnload)
+  trackView(window.location, lifeCycle, session, batch.addRumEvent, batch.beforeFlushOnUnload)
   trackErrors(lifeCycle, batch.addRumEvent)
   trackRequests(configuration, lifeCycle, session, batch.addRumEvent)
   trackPerformanceTiming(configuration, lifeCycle, batch.addRumEvent)
@@ -165,9 +165,9 @@ export function startRum(
     getInternalContext: monitor(() => {
       return {
         application_id: applicationId,
-        session_id: session.getId(),
+        session_id: viewContext.sessionId,
         view: {
-          id: viewId,
+          id: viewContext.id,
         },
       }
     }),

packages/rum/src/viewTracker.ts

@@ -2,6 +2,7 @@ import { Batch, generateUUID, monitor, msToNs, throttle } from '@datadog/browser
 
 import { LifeCycle, LifeCycleEventType } from './lifeCycle'
 import { PerformancePaintTiming, RumEvent, RumEventCategory } from './rum'
+import { RumSession } from './rumSession'
 
 export interface ViewMeasures {
   firstContentfulPaint?: number
@@ -15,8 +16,13 @@ export interface ViewMeasures {
   userActionCount: number
 }
 
-export let viewId: string
-export let viewLocation: Location
+interface ViewContext {
+  id: string
+  location: Location
+  sessionId: string | undefined
+}
+
+export let viewContext: ViewContext
 
 const THROTTLE_VIEW_UPDATE_PERIOD = 3000
 let startTimestamp: number
@@ -27,23 +33,28 @@ let viewMeasures: ViewMeasures
 export function trackView(
   location: Location,
   lifeCycle: LifeCycle,
+  session: RumSession,
   addRumEvent: (event: RumEvent) => void,
   beforeFlushOnUnload: (handler: () => void) => void
 ) {
   const scheduleViewUpdate = throttle(monitor(() => updateView(addRumEvent)), THROTTLE_VIEW_UPDATE_PERIOD, {
     leading: false,
   })
 
-  newView(location, addRumEvent)
-  trackHistory(location, addRumEvent)
+  newView(location, session, addRumEvent)
+  trackHistory(location, session, addRumEvent)
   trackMeasures(lifeCycle, scheduleViewUpdate)
-  trackRenewSession(location, lifeCycle, addRumEvent)
+  trackRenewSession(location, lifeCycle, session, addRumEvent)
 
   beforeFlushOnUnload(() => updateView(addRumEvent))
 }
 
-function newView(location: Location, addRumEvent: (event: RumEvent) => void) {
-  viewId = generateUUID()
+function newView(location: Location, session: RumSession, addRumEvent: (event: RumEvent) => void) {
+  viewContext = {
+    id: generateUUID(),
+    location: { ...location },
+    sessionId: session.getId(),
+  }
   startTimestamp = new Date().getTime()
   startOrigin = performance.now()
   documentVersion = 1
@@ -53,7 +64,6 @@ function newView(location: Location, addRumEvent: (event: RumEvent) => void) {
     resourceCount: 0,
     userActionCount: 0,
   }
-  viewLocation = { ...location }
   addViewEvent(addRumEvent)
 }
 
@@ -78,26 +88,26 @@ function addViewEvent(addRumEvent: (event: RumEvent) => void) {
   })
 }
 
-function trackHistory(location: Location, addRumEvent: (event: RumEvent) => void) {
+function trackHistory(location: Location, session: RumSession, addRumEvent: (event: RumEvent) => void) {
   const originalPushState = history.pushState
   history.pushState = monitor(function(this: History['pushState']) {
     originalPushState.apply(this, arguments as any)
-    onUrlChange(location, addRumEvent)
+    onUrlChange(location, session, addRumEvent)
   })
   const originalReplaceState = history.replaceState
   history.replaceState = monitor(function(this: History['replaceState']) {
     originalReplaceState.apply(this, arguments as any)
-    onUrlChange(location, addRumEvent)
+    onUrlChange(location, session, addRumEvent)
   })
   window.addEventListener('popstate', () => {
-    onUrlChange(location, addRumEvent)
+    onUrlChange(location, session, addRumEvent)
   })
 }
 
-function onUrlChange(location: Location, addRumEvent: (event: RumEvent) => void) {
-  if (areDifferentViews(viewLocation, location)) {
+function onUrlChange(location: Location, session: RumSession, addRumEvent: (event: RumEvent) => void) {
+  if (areDifferentViews(viewContext.location, location)) {
     updateView(addRumEvent)
-    newView(location, addRumEvent)
+    newView(location, session, addRumEvent)
   }
 }
 
@@ -146,9 +156,14 @@ function trackMeasures(lifeCycle: LifeCycle, scheduleViewUpdate: () => void) {
   })
 }
 
-function trackRenewSession(location: Location, lifeCycle: LifeCycle, addRumEvent: (event: RumEvent) => void) {
+function trackRenewSession(
+  location: Location,
+  lifeCycle: LifeCycle,
+  session: RumSession,
+  addRumEvent: (event: RumEvent) => void
+) {
   lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => {
     updateView(addRumEvent)
-    newView(location, addRumEvent)
+    newView(location, session, addRumEvent)
   })
 }

packages/rum/test/rum.spec.ts

@@ -17,6 +17,10 @@ function getEntry(addRumEvent: (event: RumEvent) => void, index: number) {
   return (addRumEvent as jasmine.Spy).calls.argsFor(index)[0] as RumEvent
 }
 
+function getServerRequestBodies<T>(server: sinon.SinonFakeServer) {
+  return server.requests.map((r) => JSON.parse(r.requestBody) as T)
+}
+
 const configuration = {
   ...DEFAULT_CONFIGURATION,
   internalMonitoringEndpoint: 'monitoring',
@@ -282,6 +286,50 @@ describe('rum session', () => {
     lifeCycle.notify(LifeCycleEventType.REQUEST_COLLECTED, FAKE_REQUEST as RequestDetails)
     expect(server.requests.length).toEqual(2)
   })
+
+  it('when the session is renewed, a final view event then a new view event should be sent', () => {
+    let sessionId = '42'
+    const session = {
+      getId: () => sessionId,
+      isTracked: () => true,
+      isTrackedWithResource: () => true,
+    }
+    const lifeCycle = new LifeCycle()
+    server.requests = []
+    startRum('appId', lifeCycle, configuration as Configuration, session)
+
+    interface ExpectedRequestBody {
+      evt: {
+        category: string
+      }
+      session_id: string
+      view: {
+        id: string
+      }
+    }
+
+    const initialRequests = getServerRequestBodies<ExpectedRequestBody>(server)
+    expect(initialRequests.length).toEqual(1)
+    expect(initialRequests[0].evt.category).toEqual('view')
+    expect(initialRequests[0].session_id).toEqual('42')
+
+    server.requests = []
+    sessionId = '43'
+    lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED)
+
+    const subsequentRequests = getServerRequestBodies<ExpectedRequestBody>(server)
+    expect(subsequentRequests.length).toEqual(2)
+
+    // Final view event
+    expect(subsequentRequests[0].evt.category).toEqual('view')
+    expect(subsequentRequests[0].session_id).toEqual('42')
+    expect(subsequentRequests[0].view.id).toEqual(initialRequests[0].view.id)
+
+    // New view event
+    expect(subsequentRequests[1].evt.category).toEqual('view')
+    expect(subsequentRequests[1].session_id).toEqual('43')
+    expect(subsequentRequests[1].view.id).not.toEqual(initialRequests[0].view.id)
+  })
 })
 
 describe('rum init', () => {

packages/rum/test/viewTracker.spec.ts

@@ -1,6 +1,7 @@
 import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
 import { PerformanceLongTaskTiming, PerformancePaintTiming, RumEvent, RumViewEvent, UserAction } from '../src/rum'
-import { trackView, viewId, viewLocation } from '../src/viewTracker'
+import { RumSession } from '../src/rumSession'
+import { trackView, viewContext } from '../src/viewTracker'
 
 function setup({
   addRumEvent,
@@ -16,7 +17,18 @@ function setup({
     fakeLocation.hash = url.hash
   })
   const fakeLocation: Partial<Location> = { pathname: '/foo' }
-  trackView(fakeLocation as Location, lifeCycle || new LifeCycle(), addRumEvent || (() => undefined), () => undefined)
+  const fakeSession = {
+    getId() {
+      return '42'
+    },
+  }
+  trackView(
+    fakeLocation as Location,
+    lifeCycle || new LifeCycle(),
+    fakeSession as RumSession,
+    addRumEvent || (() => undefined),
+    () => undefined
+  )
 }
 
 describe('rum track url change', () => {
@@ -25,29 +37,29 @@ describe('rum track url change', () => {
 
   beforeEach(() => {
     setup()
-    initialView = viewId
-    initialLocation = viewLocation
+    initialView = viewContext.id
+    initialLocation = viewContext.location
   })
 
   it('should update view id on path change', () => {
     history.pushState({}, '', '/bar')
 
-    expect(viewId).not.toEqual(initialView)
-    expect(viewLocation).not.toEqual(initialLocation)
+    expect(viewContext.id).not.toEqual(initialView)
+    expect(viewContext.location).not.toEqual(initialLocation)
   })
 
   it('should not update view id on search change', () => {
     history.pushState({}, '', '/foo?bar=qux')
 
-    expect(viewId).toEqual(initialView)
-    expect(viewLocation).toEqual(initialLocation)
+    expect(viewContext.id).toEqual(initialView)
+    expect(viewContext.location).toEqual(initialLocation)
   })
 
   it('should not update view id on hash change', () => {
     history.pushState({}, '', '/foo#bar')
 
-    expect(viewId).toEqual(initialView)
-    expect(viewLocation).toEqual(initialLocation)
+    expect(viewContext.id).toEqual(initialView)
+    expect(viewContext.location).toEqual(initialLocation)
   })
 })
 
@@ -57,10 +69,10 @@ describe('rum track renew session', () => {
     setup({
       lifeCycle,
     })
-    const initialView = viewId
+    const initialView = viewContext.id
     lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED)
 
-    expect(viewId).not.toEqual(initialView)
+    expect(viewContext.id).not.toEqual(initialView)
   })
 })