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鈥檒l 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
25 changes: 25 additions & 0 deletions packages/core/src/domain/session/sessionManager.spec.ts
Expand Up @@ -497,6 +497,31 @@ describe('startSessionManager', () => {
secondSessionTrackingType
)
})

it('should return the current session context in the renewObservable callback', () => {
const sessionManager = startSessionManagerWithDefaults()
let currentSession
sessionManager.renewObservable.subscribe(() => (currentSession = sessionManager.findActiveSession()))

// new session
expireSessionCookie()
document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK))
clock.tick(STORAGE_POLL_DELAY)

expect(currentSession).toBeDefined()
})

it('should return the current session context in the expireObservable callback', () => {
const sessionManager = startSessionManagerWithDefaults()
let currentSession
sessionManager.expireObservable.subscribe(() => (currentSession = sessionManager.findActiveSession()))

// new session
expireSessionCookie()
clock.tick(STORAGE_POLL_DELAY)

expect(currentSession).toBeDefined()
})
})

describe('tracking consent', () => {
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/domain/session/sessionManager.ts
@@ -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 Down Expand Up @@ -32,6 +32,9 @@ 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>()

// TODO - Improve configuration type and remove assertion
const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState)
stopCallbacks.push(() => sessionStore.stop())
Expand All @@ -41,8 +44,10 @@ 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(() => {
expireObservable.notify()
sessionContextHistory.closeActive(relativeNow())
})

Expand Down Expand Up @@ -75,8 +80,8 @@ export function startSessionManager<TrackingType extends string>(

return {
findActiveSession: (startTime) => sessionContextHistory.find(startTime),
renewObservable: sessionStore.renewObservable,
expireObservable: sessionStore.expireObservable,
renewObservable,
expireObservable,
expire: sessionStore.expire,
}
}
Expand Down
17 changes: 16 additions & 1 deletion packages/core/src/tools/instrumentMethod.spec.ts
Expand Up @@ -262,6 +262,20 @@ describe('instrumentSetter', () => {
stop()

object.foo = 2
clock.tick(0)

expect(instrumentationSetterSpy).not.toHaveBeenCalled()
})

it('does not call instrumentation pending in the event loop via setTimeout', () => {
const object = {} as { foo: number }
Object.defineProperty(object, 'foo', { set: noop, configurable: true })
const instrumentationSetterSpy = jasmine.createSpy()
const { stop } = instrumentSetter(object, 'foo', instrumentationSetterSpy)

object.foo = 2
stop()
clock.tick(0)

expect(instrumentationSetterSpy).not.toHaveBeenCalled()
})
Expand All @@ -284,13 +298,14 @@ describe('instrumentSetter', () => {
const object = {} as { foo: number }
Object.defineProperty(object, 'foo', { set: noop, configurable: true })
const instrumentationSetterSpy = jasmine.createSpy()
const { stop } = instrumentSetter(object, 'foo', noop)
const { stop } = instrumentSetter(object, 'foo', instrumentationSetterSpy)

thirdPartyInstrumentation(object)

stop()

object.foo = 2
clock.tick(0)

expect(instrumentationSetterSpy).not.toHaveBeenCalled()
})
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/tools/instrumentMethod.ts
Expand Up @@ -125,10 +125,13 @@ export function instrumentSetter<TARGET extends { [key: string]: any }, PROPERTY
return { stop: noop }
}

const stoppedInstrumentation = noop
let instrumentation = (target: TARGET, value: TARGET[PROPERTY]) => {
// put hooked setter into event loop to avoid of set latency
setTimeout(() => {
after(target, value)
if (instrumentation !== stoppedInstrumentation) {
after(target, value)
}
bcaudan marked this conversation as resolved.
Show resolved Hide resolved
}, 0)
}

Expand All @@ -145,9 +148,8 @@ export function instrumentSetter<TARGET extends { [key: string]: any }, PROPERTY
stop: () => {
if (Object.getOwnPropertyDescriptor(targetPrototype, property)?.set === instrumentationWrapper) {
Object.defineProperty(targetPrototype, property, originalDescriptor)
} else {
instrumentation = noop
}
instrumentation = stoppedInstrumentation
},
}
}
20 changes: 10 additions & 10 deletions packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts
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, {
lifeCycle.notify(LifeCycleEventType.AFTER_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,16 +149,16 @@ 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)

clock.tick(10)
featureFlagContexts.addFeatureFlagEvaluation('feature', 'one')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
lifeCycle.notify(LifeCycleEventType.AFTER_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
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.AFTER_VIEW_ENDED, ({ endClocks }) => {
featureFlagContexts.closeActive(endClocks.relative)
})

return {
findFeatureFlagEvaluations: (startTime?: RelativeTime) => featureFlagContexts.find(startTime),
addFeatureFlagEvaluation: (key: string, value: ContextValue) => {
Expand Down
24 changes: 12 additions & 12 deletions packages/rum-core/src/domain/contexts/urlContexts.spec.ts
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, {
lifeCycle.notify(LifeCycleEventType.AFTER_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,16 +82,16 @@ 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)

clock.tick(10)
changeLocation('/foo')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
lifeCycle.notify(LifeCycleEventType.AFTER_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 @@ -100,10 +100,10 @@ describe('urlContexts', () => {

clock.tick(10)
changeLocation('/qux')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
lifeCycle.notify(LifeCycleEventType.AFTER_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,10 +132,10 @@ 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, {
lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)

Expand Down
10 changes: 5 additions & 5 deletions packages/rum-core/src/domain/contexts/urlContexts.ts
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.AFTER_VIEW_ENDED, ({ endClocks }) => {
urlContextHistory.closeActive(endClocks.relative)
})

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