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

🐛 [RUM-3039] Fix missing pending mutations at view end #2598

Merged
merged 14 commits into from
Feb 19, 2024
23 changes: 9 additions & 14 deletions packages/core/src/domain/session/sessionManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,28 +242,23 @@ describe('startSessionManager', () => {

it('should notify each expire and renew observables', () => {
const firstSessionManager = startSessionManagerWithDefaults({ productKey: FIRST_PRODUCT_KEY })
const expireSessionASpy = jasmine.createSpy()
firstSessionManager.expireObservable.subscribe(expireSessionASpy)
const renewSessionASpy = jasmine.createSpy()
firstSessionManager.renewObservable.subscribe(renewSessionASpy)
const calls: string[] = []
firstSessionManager.beforeExpireObservable.subscribe(() => calls.push('beforeExpireA'))
firstSessionManager.expireObservable.subscribe(() => calls.push('expireA'))
firstSessionManager.renewObservable.subscribe(() => calls.push('renewA'))

const secondSessionManager = startSessionManagerWithDefaults({ productKey: SECOND_PRODUCT_KEY })
const expireSessionBSpy = jasmine.createSpy()
secondSessionManager.expireObservable.subscribe(expireSessionBSpy)
const renewSessionBSpy = jasmine.createSpy()
secondSessionManager.renewObservable.subscribe(renewSessionBSpy)
secondSessionManager.beforeExpireObservable.subscribe(() => calls.push('beforeExpireB'))
secondSessionManager.expireObservable.subscribe(() => calls.push('expireB'))
secondSessionManager.renewObservable.subscribe(() => calls.push('renewB'))

expireSessionCookie()

expect(expireSessionASpy).toHaveBeenCalled()
expect(expireSessionBSpy).toHaveBeenCalled()
expect(renewSessionASpy).not.toHaveBeenCalled()
expect(renewSessionBSpy).not.toHaveBeenCalled()
expect(calls).toEqual(['beforeExpireA', 'expireA', 'beforeExpireB', 'expireB'])

document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK))

expect(renewSessionASpy).toHaveBeenCalled()
expect(renewSessionBSpy).toHaveBeenCalled()
expect(calls).toEqual(['beforeExpireA', 'expireA', 'beforeExpireB', 'expireB', 'renewA', 'renewB'])
})
})

Expand Down
15 changes: 12 additions & 3 deletions packages/core/src/domain/session/sessionManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Observable } from '../../tools/observable'
import { Observable } from '../../tools/observable'
import type { Context } from '../../tools/serialisation/context'
import { ValueHistory } from '../../tools/valueHistory'
import type { RelativeTime } from '../../tools/utils/timeUtils'
Expand All @@ -13,6 +13,7 @@ import { startSessionStore } from './sessionStore'
export interface SessionManager<TrackingType extends string> {
findActiveSession: (startTime?: RelativeTime) => SessionContext<TrackingType> | undefined
renewObservable: Observable<void>
beforeExpireObservable: Observable<void>
expireObservable: Observable<void>
expire: () => void
}
Expand All @@ -32,6 +33,10 @@ export function startSessionManager<TrackingType extends string>(
computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean },
trackingConsentState: TrackingConsentState
): SessionManager<TrackingType> {
const renewObservable = new Observable<void>()
const expireObservable = new Observable<void>()
const beforeExpireObservable = new Observable<void>()

// TODO - Improve configuration type and remove assertion
const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState)
stopCallbacks.push(() => sessionStore.stop())
Expand All @@ -41,9 +46,12 @@ export function startSessionManager<TrackingType extends string>(

sessionStore.renewObservable.subscribe(() => {
sessionContextHistory.add(buildSessionContext(), relativeNow())
renewObservable.notify()
bcaudan marked this conversation as resolved.
Show resolved Hide resolved
})
sessionStore.expireObservable.subscribe(() => {
beforeExpireObservable.notify()
sessionContextHistory.closeActive(relativeNow())
expireObservable.notify()
})

// We expand/renew session unconditionally as tracking consent is always granted when the session
Expand Down Expand Up @@ -75,8 +83,9 @@ export function startSessionManager<TrackingType extends string>(

return {
findActiveSession: (startTime) => sessionContextHistory.find(startTime),
renewObservable: sessionStore.renewObservable,
expireObservable: sessionStore.expireObservable,
renewObservable,
expireObservable,
beforeExpireObservable,
expire: sessionStore.expire,
}
}
Expand Down
11 changes: 6 additions & 5 deletions packages/rum-core/src/boot/startRum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('rum session', () => {
expect(serverRumEvents[0].type).toEqual('view')
expect(serverRumEvents[0].session.id).toEqual('42')

lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED)
lifeCycle.notify(LifeCycleEventType.BEFORE_SESSION_EXPIRED)
expect(serverRumEvents.length).toEqual(2)

session.setId('43')
Expand Down Expand Up @@ -314,10 +314,10 @@ describe('rum events url', () => {
describe('view events', () => {
let setupBuilder: TestSetupBuilder
let interceptor: ReturnType<typeof interceptRequests>

let startRumResult: ReturnType<typeof startRum>
beforeEach(() => {
setupBuilder = setup().beforeBuild(({ configuration, customerDataTrackerManager }) =>
startRum(
setupBuilder = setup().beforeBuild(({ configuration, customerDataTrackerManager }) => {
startRumResult = startRum(
{} as RumInitConfiguration,
configuration,
noopRecorderApi,
Expand All @@ -327,7 +327,8 @@ describe('view events', () => {
createIdentityEncoder,
createTrackingConsentState(TrackingConsent.GRANTED)
)
)
return startRumResult
})
interceptor = interceptRequests()
})

Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export function startRum(

const pageExitObservable = createPageExitObservable(configuration)
const pageExitSubscription = pageExitObservable.subscribe((event) => {
lifeCycle.notify(LifeCycleEventType.BEFORE_PAGE_EXITED, event)
lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, event)
})
cleanupTasks.push(() => pageExitSubscription.unsubscribe())
Expand Down
5 changes: 2 additions & 3 deletions packages/rum-core/src/domain/action/trackClickActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ describe('trackClickActions', () => {
let input: HTMLInputElement
let setupBuilder: TestSetupBuilder
let findActionId: ActionContexts['findActionId']

beforeEach(() => {
button = document.createElement('button')
button.type = 'button'
Expand Down Expand Up @@ -176,13 +175,13 @@ describe('trackClickActions', () => {
expect(findActionId()).toEqual([])
})

it('ongoing click action is stopped on view end', () => {
it('ongoing click action is stopped before the view is ended', () => {
const { lifeCycle, clock } = setupBuilder.build()
emulateClick({ activity: { delay: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY } })

clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)

lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_ENDED, {
endClocks: clocksNow(),
})

Expand Down
4 changes: 2 additions & 2 deletions packages/rum-core/src/domain/action/trackClickActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function trackClickActions(
history.reset()
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, stopClickChain)
lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_ENDED, stopClickChain)

const { stop: stopActionEventsListener } = listenActionEvents<{
clickActionBase: ClickActionBase
Expand Down Expand Up @@ -192,7 +192,7 @@ function startClickAction(
CLICK_ACTION_MAX_DURATION
)

const viewEndedSubscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
const viewEndedSubscription = lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_ENDED, ({ endClocks }) => {
click.stop(endClocks.timeStamp)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('featureFlagContexts', () => {

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -67,7 +67,7 @@ describe('featureFlagContexts', () => {

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -83,7 +83,7 @@ describe('featureFlagContexts', () => {
it('should not add feature flag evaluation when the ff feature_flags is disabled', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -101,7 +101,7 @@ describe('featureFlagContexts', () => {

const updateCustomerDataSpy = spyOn(customerDataTracker, 'updateCustomerData')

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand Down Expand Up @@ -129,14 +129,14 @@ describe('featureFlagContexts', () => {

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)
featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -149,7 +149,7 @@ describe('featureFlagContexts', () => {

const { lifeCycle, clock } = setupBuilder.withFakeClock().build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -158,7 +158,7 @@ describe('featureFlagContexts', () => {
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

Expand Down
10 changes: 5 additions & 5 deletions packages/rum-core/src/domain/contexts/featureFlagContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ export function startFeatureFlagContexts(

const featureFlagContexts = new ValueHistory<FeatureFlagContext>(FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY)

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
featureFlagContexts.closeActive(endClocks.relative)
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => {
lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, ({ startClocks }) => {
featureFlagContexts.add({}, startClocks.relative)
customerDataTracker.resetCustomerData()
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
featureFlagContexts.closeActive(endClocks.relative)
})

return {
findFeatureFlagEvaluations: (startTime?: RelativeTime) => featureFlagContexts.find(startTime),
addFeatureFlagEvaluation: (key: string, value: ContextValue) => {
Expand Down
16 changes: 8 additions & 8 deletions packages/rum-core/src/domain/contexts/urlContexts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('urlContexts', () => {
it('should return current url and document referrer for initial view', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -50,7 +50,7 @@ describe('urlContexts', () => {
it('should update url context on location change', () => {
const { lifeCycle, changeLocation } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)
changeLocation('/foo')
Expand All @@ -63,14 +63,14 @@ describe('urlContexts', () => {
it('should update url context on new view', () => {
const { lifeCycle, changeLocation } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)
changeLocation('/foo')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -82,7 +82,7 @@ describe('urlContexts', () => {
it('should return the url context corresponding to the start time', () => {
const { lifeCycle, changeLocation, clock } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -91,7 +91,7 @@ describe('urlContexts', () => {
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

Expand All @@ -103,7 +103,7 @@ describe('urlContexts', () => {
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(30 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(30 as RelativeTime),
} as ViewCreatedEvent)

Expand Down Expand Up @@ -132,7 +132,7 @@ describe('urlContexts', () => {
it('should return undefined when no current view', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
Expand Down
10 changes: 5 additions & 5 deletions packages/rum-core/src/domain/contexts/urlContexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ export function startUrlContexts(

let previousViewUrl: string | undefined

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
urlContextHistory.closeActive(endClocks.relative)
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => {
lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, ({ startClocks }) => {
const viewUrl = location.href
urlContextHistory.add(
buildUrlContext({
Expand All @@ -47,6 +43,10 @@ export function startUrlContexts(
previousViewUrl = viewUrl
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
urlContextHistory.closeActive(endClocks.relative)
})

const locationChangeSubscription = locationChangeObservable.subscribe(({ newLocation }) => {
const current = urlContextHistory.find()
if (current) {
Expand Down
Loading