Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RUMF-625] make sure view url doesn't change #469

Merged
merged 10 commits into from
Jul 31, 2020
Merged
2 changes: 1 addition & 1 deletion packages/core/src/urlPolyfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function getHash(url: string) {
return buildUrl(url).hash
}

function buildUrl(url: string, base?: string) {
export function buildUrl(url: string, base?: string) {
if (checkURLSupported()) {
return base !== undefined ? new URL(url, base) : new URL(url)
}
Expand Down
14 changes: 8 additions & 6 deletions packages/rum/src/lifeCycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ export class LifeCycle {
notify(eventType: LifeCycleEventType.REQUEST_COMPLETED, data: RequestCompleteEvent): void
notify(eventType: LifeCycleEventType.AUTO_ACTION_COMPLETED, data: AutoUserAction): void
notify(eventType: LifeCycleEventType.CUSTOM_ACTION_COLLECTED, data: CustomUserAction): void
notify(
eventType: LifeCycleEventType.AUTO_ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
{ id, startTime }: { id: string; startTime: number }
): void
notify(eventType: LifeCycleEventType.AUTO_ACTION_CREATED, data: { id: string; startTime: number }): void
notify(eventType: LifeCycleEventType.VIEW_CREATED, data: { id: string; startTime: number; location: Location }): void
notify(eventType: LifeCycleEventType.VIEW_UPDATED, data: View): void
notify(
eventType:
Expand Down Expand Up @@ -64,13 +62,17 @@ export class LifeCycle {
): Subscription
subscribe(eventType: LifeCycleEventType.AUTO_ACTION_COMPLETED, callback: (data: AutoUserAction) => void): Subscription
subscribe(
eventType: LifeCycleEventType.AUTO_ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
callback: ({ id, startTime }: { id: string; startTime: number }) => void
eventType: LifeCycleEventType.AUTO_ACTION_CREATED,
callback: (data: { id: string; startTime: number }) => void
): Subscription
subscribe(
eventType: LifeCycleEventType.CUSTOM_ACTION_COLLECTED,
callback: (data: CustomUserAction) => void
): Subscription
subscribe(
eventType: LifeCycleEventType.VIEW_CREATED,
callback: (data: { id: string; startTime: number; location: Location }) => void
): Subscription
subscribe(eventType: LifeCycleEventType.VIEW_UPDATED, callback: (data: View) => void): Subscription
subscribe(
eventType:
Expand Down
28 changes: 22 additions & 6 deletions packages/rum/src/parentContexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ export interface ActionContext extends Context {
}
}

interface CurrentContext {
interface CurrentViewContext {
id: string
startTime: number
location: Location
}

interface CurrentActionContext {
id: string
startTime: number
}
Expand All @@ -38,9 +44,9 @@ export interface ParentContexts {
stop: () => void
}

export function startParentContexts(location: Location, lifeCycle: LifeCycle, session: RumSession): ParentContexts {
let currentView: CurrentContext | undefined
let currentAction: CurrentContext | undefined
export function startParentContexts(lifeCycle: LifeCycle, session: RumSession): ParentContexts {
let currentView: CurrentViewContext | undefined
let currentAction: CurrentActionContext | undefined
let currentSessionId: string | undefined

let previousViews: Array<PreviousContext<ViewContext>> = []
Expand All @@ -58,6 +64,10 @@ export function startParentContexts(location: Location, lifeCycle: LifeCycle, se
currentSessionId = session.getId()
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, (currentContext) => {
currentView = currentContext
})
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved

lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_CREATED, (currentContext) => {
currentAction = currentContext
})
Expand Down Expand Up @@ -100,7 +110,13 @@ export function startParentContexts(location: Location, lifeCycle: LifeCycle, se
}

function buildCurrentViewContext() {
return { sessionId: currentSessionId, view: { id: currentView!.id, url: location.href } }
return {
sessionId: currentSessionId,
view: {
id: currentView!.id,
url: currentView!.location.href,
},
}
}

function buildCurrentActionContext() {
Expand All @@ -110,7 +126,7 @@ export function startParentContexts(location: Location, lifeCycle: LifeCycle, se
function findContext<T>(
buildContext: () => T,
previousContexts: Array<PreviousContext<T>>,
currentContext?: CurrentContext,
currentContext?: { startTime: number },
startTime?: number
) {
if (!startTime) {
Expand Down
2 changes: 1 addition & 1 deletion packages/rum/src/rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export function startRum(
): Omit<RumGlobal, 'init'> {
let globalContext: Context = {}

const parentContexts = startParentContexts(window.location, lifeCycle, session)
const parentContexts = startParentContexts(lifeCycle, session)

internalMonitoring.setExternalContextProvider(() => {
return deepMerge(
Expand Down
28 changes: 16 additions & 12 deletions packages/rum/src/viewCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,26 @@ export const THROTTLE_VIEW_UPDATE_PERIOD = 3000
export const SESSION_KEEP_ALIVE_INTERVAL = 5 * ONE_MINUTE

export function startViewCollection(location: Location, lifeCycle: LifeCycle) {
let currentLocation = { ...location }
const startOrigin = 0
let currentView = newView(lifeCycle, currentLocation, ViewLoadingType.INITIAL_LOAD, startOrigin)
let currentView = newView(lifeCycle, location, ViewLoadingType.INITIAL_LOAD, startOrigin)

// Renew view on history changes
trackHistory(() => {
if (areDifferentViews(currentLocation, location)) {
currentLocation = { ...location }
if (currentView.isDifferentView(location)) {
currentView.triggerUpdate()
currentView.end()
currentView = newView(lifeCycle, currentLocation, ViewLoadingType.ROUTE_CHANGE)
currentView = newView(lifeCycle, location, ViewLoadingType.ROUTE_CHANGE)
} else {
currentView.updateLocation(location)
currentView.triggerUpdate()
}
})

// Renew view on session renewal
lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => {
// do not trigger view update to avoid wrong data
currentView.end()
currentView = newView(lifeCycle, currentLocation, ViewLoadingType.ROUTE_CHANGE)
currentView = newView(lifeCycle, location, ViewLoadingType.ROUTE_CHANGE)
})

// End the current view on page unload
Expand All @@ -82,7 +83,7 @@ export function startViewCollection(location: Location, lifeCycle: LifeCycle) {

function newView(
lifeCycle: LifeCycle,
location: Location,
initialLocation: Location,
loadingType: ViewLoadingType,
startTime: number = performance.now()
) {
Expand All @@ -96,8 +97,9 @@ function newView(
}
let documentVersion = 0
let loadingTime: number | undefined
let location: Location = { ...initialLocation }

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { id, startTime })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { id, startTime, location })

// Update the view every time the measures are changing
const { throttled: scheduleViewUpdate, stop: stopScheduleViewUpdate } = throttle(
Expand Down Expand Up @@ -145,9 +147,15 @@ function newView(
// prevent pending view updates execution
stopScheduleViewUpdate()
},
isDifferentView(otherLocation: Location) {
return location.pathname !== otherLocation.pathname
},
triggerUpdate() {
updateView()
},
updateLocation(newLocation: Location) {
location = { ...newLocation }
},
}
}

Expand All @@ -165,10 +173,6 @@ function trackHistory(onHistoryChange: () => void) {
window.addEventListener(DOM_EVENT.POP_STATE, monitor(onHistoryChange))
}

function areDifferentViews(previous: Location, current: Location) {
return previous.pathname !== current.pathname
}

interface Timings {
domComplete?: number
domContentLoaded?: number
Expand Down
48 changes: 21 additions & 27 deletions packages/rum/test/parentContexts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
VIEW_CONTEXT_TIME_OUT_DELAY,
} from '../src/parentContexts'
import { AutoUserAction } from '../src/userActionCollection'
import { View } from '../src/viewCollection'
import { setup, TestSetupBuilder } from './specHelper'

function stubActionWithDuration(duration: number): AutoUserAction {
Expand All @@ -16,20 +17,13 @@ describe('parentContexts', () => {
const FAKE_ID = 'fake'
const startTime = 10

let fakeUrl: string
let sessionId: string
let setupBuilder: TestSetupBuilder

beforeEach(() => {
fakeUrl = 'fake-url'
sessionId = 'fake-session'
const fakeLocation = {
get href() {
return fakeUrl
},
}
setupBuilder = setup()
.withFakeLocation(fakeLocation)
.withFakeLocation('http://fake-url.com')
.withSession({
getId: () => sessionId,
isTracked: () => true,
Expand All @@ -52,7 +46,7 @@ describe('parentContexts', () => {
it('should return the current view context when there is no start time', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime, id: FAKE_ID })

expect(parentContexts.findView()).toBeDefined()
expect(parentContexts.findView()!.view.id).toEqual(FAKE_ID)
Expand All @@ -61,9 +55,9 @@ describe('parentContexts', () => {
it('should return the view context corresponding to startTime', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 20, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 30, id: 'view 3' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 20, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 30, id: 'view 3' })

expect(parentContexts.findView(15)!.view.id).toEqual('view 1')
expect(parentContexts.findView(20)!.view.id).toEqual('view 2')
Expand All @@ -73,43 +67,43 @@ describe('parentContexts', () => {
it('should return undefined when no view context corresponding to startTime', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 20, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 20, id: 'view 2' })

expect(parentContexts.findView(5)).not.toBeDefined()
})

it('should replace the current view context on VIEW_CREATED', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime, id: FAKE_ID })
const newViewId = 'fake 2'
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: newViewId })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime, id: newViewId })

expect(parentContexts.findView()!.view.id).toEqual(newViewId)
})

it('should return the current url with the current view', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()
const { lifeCycle, parentContexts, fakeLocation } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: FAKE_ID })
expect(parentContexts.findView()!.view.url).toBe('fake-url')
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: FAKE_ID, location: fakeLocation as Location })
expect(parentContexts.findView()!.view.url).toBe('http://fake-url.com/')

fakeUrl = 'other-url'
history.pushState({}, '', '/foo')

expect(parentContexts.findView()!.view.url).toBe('other-url')
expect(parentContexts.findView()!.view.url).toBe('http://fake-url.com/foo')
})

it('should update session id only on VIEW_CREATED', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime, id: FAKE_ID })
expect(parentContexts.findView()!.sessionId).toBe('fake-session')

sessionId = 'other-session'
expect(parentContexts.findView()!.sessionId).toBe('fake-session')

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime, id: 'fake 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime, id: 'fake 2' })
expect(parentContexts.findView()!.sessionId).toBe('other-session')
})
})
Expand Down Expand Up @@ -181,8 +175,8 @@ describe('parentContexts', () => {
it('should be cleared on SESSION_RENEWED', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: 20, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 10, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: 20, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime: 10, id: 'action 1' })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, stubActionWithDuration(10))
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime: 20, id: 'action 2' })
Expand All @@ -206,10 +200,10 @@ describe('parentContexts', () => {
const originalTime = performance.now()
const targetTime = originalTime + 5

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: originalTime, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: originalTime, id: 'view 1' })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime: originalTime, id: 'action 1' })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, stubActionWithDuration(10))
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startTime: originalTime + 10, id: 'view 2' })
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { location, startTime: originalTime + 10, id: 'view 2' })

clock.tick(10)
expect(parentContexts.findView(targetTime)).toBeDefined()
Expand Down
39 changes: 39 additions & 0 deletions packages/rum/test/rum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,3 +633,42 @@ describe('rum internal context', () => {
})
})
})

describe('rum event assembly', () => {
const FAKE_ERROR: Partial<ErrorMessage> = { message: 'test' }

let setupBuilder: TestSetupBuilder

beforeEach(() => {
setupBuilder = setup()
.withFakeServer()
.withRum()
})

afterEach(() => {
setupBuilder.cleanup()
})

it('should sets the view URL on long task events', () => {
const { server, lifeCycle } = setupBuilder.withFakeLocation('http://foo.com/').build()

server.requests = []

lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, FAKE_ERROR as ErrorMessage)

expect(server.requests.length).toEqual(1)
expect(getRumMessage(server, 0).view.url).toEqual('http://foo.com/')
})

it('should keep the same URL when updating a view ended by a URL change', () => {
const { server } = setupBuilder.withFakeLocation('http://foo.com/').build()

server.requests = []

history.pushState({}, '', '/bar')

expect(server.requests.length).toEqual(2)
expect(getRumMessage(server, 0).view.url).toEqual('http://foo.com/')
expect(getRumMessage(server, 1).view.url).toEqual('http://foo.com/bar')
})
})
Loading