Skip to content

🐛 Fix #3935: Include pre-start context in first view event #4304

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.

Sign up for GitHub

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

Open
allspain wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 Fix #3935: Include pre-start context in first view event #4304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions packages/rum-core/src/boot/preStartRum.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,69 @@ describe('preStartRum', () => {
})
})

describe('passes pre-start contexts to doStartRum', () => {
function mockStartRumResult(): StartRumResult {
return {
globalContext: { setContext: noop } as any,
userContext: { setContext: noop } as any,
accountContext: { setContext: noop } as any,
} as unknown as StartRumResult
}

it('should pass global context set before init to doStartRum', () => {
const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
doStartRumSpy.and.returnValue(mockStartRumResult())

strategy.globalContext.setContextProperty('foo', 'bar')
strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API)

expect(doStartRumSpy).toHaveBeenCalledTimes(1)
const initialContexts = doStartRumSpy.calls.mostRecent().args[5]
expect(initialContexts).toBeDefined()
expect(initialContexts.globalContext).toEqual({ foo: 'bar' })
})

it('should pass user context set before init to doStartRum', () => {
const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
doStartRumSpy.and.returnValue(mockStartRumResult())

strategy.userContext.setContextProperty('id', 'user-123')
strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API)

expect(doStartRumSpy).toHaveBeenCalledTimes(1)
const initialContexts = doStartRumSpy.calls.mostRecent().args[5]
expect(initialContexts).toBeDefined()
expect(initialContexts.userContext).toEqual({ id: 'user-123' })
})

it('should pass account context set before init to doStartRum', () => {
const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
doStartRumSpy.and.returnValue(mockStartRumResult())

strategy.accountContext.setContextProperty('id', 'account-456')
strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API)

expect(doStartRumSpy).toHaveBeenCalledTimes(1)
const initialContexts = doStartRumSpy.calls.mostRecent().args[5]
expect(initialContexts).toBeDefined()
expect(initialContexts.accountContext).toEqual({ id: 'account-456' })
})

it('should pass empty contexts when nothing is set before init', () => {
const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
doStartRumSpy.and.returnValue(mockStartRumResult())

strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API)

expect(doStartRumSpy).toHaveBeenCalledTimes(1)
const initialContexts = doStartRumSpy.calls.mostRecent().args[5]
expect(initialContexts).toBeDefined()
expect(initialContexts.globalContext).toEqual({})
expect(initialContexts.userContext).toEqual({})
expect(initialContexts.accountContext).toEqual({})
})
})

describe('buffers API calls before starting RUM', () => {
let strategy: Strategy
let doStartRumSpy: jasmine.Spy<DoStartRum>
Expand Down
24 changes: 22 additions & 2 deletions packages/rum-core/src/boot/preStartRum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,19 @@ import { callPluginsMethod } from '../domain/plugins'
import type { StartRumResult } from './startRum'
import type { RumPublicApiOptions, Strategy } from './rumPublicApi'

export interface InitialContexts {
globalContext: Context
userContext: Context
accountContext: Context
}

export type DoStartRum = (
configuration: RumConfiguration,
deflateWorker: DeflateWorker | undefined,
initialViewOptions: ViewOptions | undefined,
telemetry: Telemetry,
hooks: Hooks
hooks: Hooks,
initialContexts: InitialContexts
) => StartRumResult

export function createPreStartStrategy(
Expand Down Expand Up @@ -117,7 +124,20 @@ export function createPreStartStrategy(
initialViewOptions = firstStartViewCall.options
}

const startRumResult = doStartRum(cachedConfiguration, deflateWorker, initialViewOptions, telemetry, hooks)
const initialContexts: InitialContexts = {
globalContext: globalContext.getContext(),
userContext: userContext.getContext(),
accountContext: accountContext.getContext(),
}

const startRumResult = doStartRum(
cachedConfiguration,
deflateWorker,
initialViewOptions,
telemetry,
hooks,
initialContexts
)

bufferApiCalls.drain(startRumResult)
}
Expand Down
5 changes: 3 additions & 2 deletions packages/rum-core/src/boot/rumPublicApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ export function makeRumPublicApi(
options,
trackingConsentState,
customVitalsState,
(configuration, deflateWorker, initialViewOptions, telemetry, hooks) => {
(configuration, deflateWorker, initialViewOptions, telemetry, hooks, initialContexts) => {
const createEncoder =
deflateWorker && options.createDeflateEncoder
? (streamId: DeflateEncoderStreamId) => options.createDeflateEncoder!(configuration, deflateWorker, streamId)
Expand All @@ -607,7 +607,8 @@ export function makeRumPublicApi(
bufferedDataObservable,
telemetry,
hooks,
options.sdkName
options.sdkName,
initialContexts
)

recorderApi.onRumStart(
Expand Down
69 changes: 69 additions & 0 deletions packages/rum-core/src/boot/startRum.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,75 @@ describe('rum session', () => {
})
})

describe('initial view event with pre-start contexts', () => {
it('should include global context on the first view event when initial contexts are provided', () => {
const lifeCycle = new LifeCycle()
const sessionManager = createRumSessionManagerMock().setId('42')
const hooks = createHooks()
const serverRumEvents = collectServerEvents(lifeCycle)

const initialContexts = {
globalContext: { foo: 'bar' },
userContext: {},
accountContext: {},
}

const { stop } = startRumEventCollection(
lifeCycle,
hooks,
mockRumConfiguration(),
sessionManager,
noopRecorderApi,
undefined,
createCustomVitalsState(),
new Observable(),
undefined,
noop,
initialContexts
)

registerCleanupTask(stop)

// The first event should be a view with global context
expect(serverRumEvents.length).toBeGreaterThanOrEqual(1)
expect(serverRumEvents[0].type).toEqual('view')
expect(serverRumEvents[0].context).toEqual({ foo: 'bar' })
})

it('should include user context on the first view event when initial contexts are provided', () => {
const lifeCycle = new LifeCycle()
const sessionManager = createRumSessionManagerMock().setId('42')
const hooks = createHooks()
const serverRumEvents = collectServerEvents(lifeCycle)

const initialContexts = {
globalContext: {},
userContext: { id: 'user-123', name: 'Test User' },
accountContext: {},
}

const { stop } = startRumEventCollection(
lifeCycle,
hooks,
mockRumConfiguration(),
sessionManager,
noopRecorderApi,
undefined,
createCustomVitalsState(),
new Observable(),
undefined,
noop,
initialContexts
)

registerCleanupTask(stop)

expect(serverRumEvents.length).toBeGreaterThanOrEqual(1)
expect(serverRumEvents[0].type).toEqual('view')
expect(serverRumEvents[0].usr).toEqual(jasmine.objectContaining({ id: 'user-123', name: 'Test User' }))
})
})

describe('rum session keep alive', () => {
let lifeCycle: LifeCycle
let clock: Clock
Expand Down
21 changes: 18 additions & 3 deletions packages/rum-core/src/boot/startRum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
createPageMayExitObservable,
canUseEventBridge,
addTelemetryDebug,
isEmptyObject,
startAccountContext,
startGlobalContext,
startUserContext,
Expand Down Expand Up @@ -55,6 +56,7 @@ import { startEventCollection } from '../domain/event/eventCollection'
import { startInitialViewMetricsTelemetry } from '../domain/view/viewMetrics/startInitialViewMetricsTelemetry'
import { startSourceCodeContext } from '../domain/contexts/sourceCodeContext'
import type { RecorderApi, ProfilerApi } from './rumPublicApi'
import type { InitialContexts } from './preStartRum'

export type StartRum = typeof startRum
export type StartRumResult = ReturnType<StartRum>
Expand All @@ -74,7 +76,8 @@ export function startRum(
bufferedDataObservable: BufferedObservable<BufferedData>,
telemetry: Telemetry,
hooks: Hooks,
sdkName?: SdkName
sdkName?: SdkName,
initialContexts?: InitialContexts
) {
const cleanupTasks: Array<() => void> = []
const lifeCycle = new LifeCycle()
Expand Down Expand Up @@ -127,7 +130,8 @@ export function startRum(
customVitalsState,
bufferedDataObservable,
sdkName,
reportError
reportError,
initialContexts
)
cleanupTasks.push(stopRumEventCollection)
bufferedDataObservable.unbuffer()
Expand Down Expand Up @@ -158,7 +162,8 @@ export function startRumEventCollection(
customVitalsState: CustomVitalsState,
bufferedDataObservable: Observable<BufferedData>,
sdkName: SdkName | undefined,
reportError: (error: RawError) => void
reportError: (error: RawError) => void,
initialContexts?: InitialContexts
) {
const cleanupTasks: Array<() => void> = []

Expand All @@ -181,6 +186,16 @@ export function startRumEventCollection(
const userContext = startUserContext(hooks, configuration, session, 'rum')
const accountContext = startAccountContext(hooks, configuration, 'rum')

// Initialize context managers with pre-start values so the first view event
// includes any context set before init() was called (see #3935)
if (initialContexts) {
globalContext.setContext(initialContexts.globalContext)
Copy link
Copy Markdown
Contributor

@mormubis mormubis Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a regression here for storeContextsAcrossPages users.

When that flag is on, startGlobalContext calls storeContextManager internally, which synchronously loads the previous session's context from localStorage before returning. So by the time we get here, the managers already have that data.

If nothing was set before init(), initialContexts.globalContext and initialContexts.userContext are both {}. Calling setContext({}) wipes what was just loaded — and since changeObservable fires right away, dumpToStorage runs and writes {} back to localStorage. The stored data is gone permanently.

accountContext is fine because the isEmptyObject guard below protects it, but the other two aren't guarded.

For a user with storeContextsAcrossPages who doesn't set any pre-start context:

  • before this PR: first view = {} (the bug), subsequent events = { storedKey: 'value' }
  • after this PR: first view = {} (still wrong), subsequent = {}, localStorage wiped

Adding the same guard to all three should fix it:

if (!isEmptyObject(initialContexts.globalContext)) {
  globalContext.setContext(initialContexts.globalContext)
}
if (!isEmptyObject(initialContexts.userContext)) {
  userContext.setContext(initialContexts.userContext)
}
if (!isEmptyObject(initialContexts.accountContext)) {
  accountContext.setContext(initialContexts.accountContext)
}

userContext.setContext(initialContexts.userContext)
if (!isEmptyObject(initialContexts.accountContext)) {
accountContext.setContext(initialContexts.accountContext)
}
}

const actionCollection = startActionCollection(
lifeCycle,
hooks,
Expand Down
Loading