diff --git a/.gitignore b/.gitignore index d8f567b13a..92a83b69a8 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,4 @@ playwright-report/ # Claude Code local files *.local.md .claude/settings.local.json +.planning/ diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index b7b9039b89..f1a65fb6c1 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -532,11 +532,13 @@ export type TelemetryCommonFeaturesUsage = /** * Schema of browser specific features usage */ +// [MANUAL] AddViewLoadingTime added pending upstream rum-events-format schema sync export type TelemetryBrowserFeaturesUsage = | StartSessionReplayRecording | StartDurationVital | StopDurationVital | AddDurationVital + | AddViewLoadingTime /** * Schema of mobile specific features usage */ diff --git a/packages/rum-core/src/boot/preStartRum.spec.ts b/packages/rum-core/src/boot/preStartRum.spec.ts index 0d58b90ac3..ab570608ad 100644 --- a/packages/rum-core/src/boot/preStartRum.spec.ts +++ b/packages/rum-core/src/boot/preStartRum.spec.ts @@ -607,6 +607,33 @@ describe('preStartRum', () => { expect(addTimingSpy).toHaveBeenCalledOnceWith(name, time) }) + it('addLoadingTime', () => { + const addLoadingTimeSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ addLoadingTime: addLoadingTimeSpy } as unknown as StartRumResult) + + const timestamp = 123 as TimeStamp + strategy.addLoadingTime(timestamp, false) + strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API) + expect(addLoadingTimeSpy).toHaveBeenCalledOnceWith(timestamp, false) + }) + + it('addLoadingTime should preserve call timestamp', () => { + const clock = mockClock() + const addLoadingTimeSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ addLoadingTime: addLoadingTimeSpy } as unknown as StartRumResult) + + clock.tick(10) + strategy.addLoadingTime() + + clock.tick(20) + strategy.init(DEFAULT_INIT_CONFIGURATION, PUBLIC_API) + + expect(addLoadingTimeSpy).toHaveBeenCalledOnceWith(jasmine.any(Number), false) + // Verify the timestamp was captured at call time (tick 10), not at drain time (tick 30) + const capturedTimestamp = addLoadingTimeSpy.calls.argsFor(0)[0] as number + expect(capturedTimestamp).toBeLessThan(clock.timeStamp(30)) + }) + it('setViewContext', () => { const setViewContextSpy = jasmine.createSpy() doStartRumSpy.and.returnValue({ setViewContext: setViewContextSpy } as unknown as StartRumResult) diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index faa73de506..0ac30de8bc 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -241,6 +241,10 @@ export function createPreStartStrategy( bufferApiCalls.add((startRumResult) => startRumResult.addTiming(name, time)) }, + addLoadingTime: ((callTimestamp = timeStampNow(), overwrite = false) => { + bufferApiCalls.add((startRumResult) => startRumResult.addLoadingTime(callTimestamp, overwrite)) + }) as Strategy['addLoadingTime'], + startView(options, startClocks = clocksNow()) { const callback = (startRumResult: StartRumResult) => { startRumResult.startView(options, startClocks) diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index 349be1eb57..55949950de 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -14,6 +14,7 @@ const noopStartRum = (): ReturnType => ({ addError: () => undefined, addEvent: () => undefined, addTiming: () => undefined, + addLoadingTime: () => ({ noActiveView: false, overwritten: false }), addFeatureFlagEvaluation: () => undefined, startView: () => undefined, setViewContext: () => undefined, @@ -544,6 +545,42 @@ describe('rum public api', () => { }) }) + describe('addViewLoadingTime', () => { + let addLoadingTimeSpy: jasmine.Spy['addLoadingTime']> + let rumPublicApi: RumPublicApi + + beforeEach(() => { + addLoadingTimeSpy = jasmine.createSpy() + ;({ rumPublicApi } = makeRumPublicApiWithDefaults({ + startRumResult: { + addLoadingTime: addLoadingTimeSpy, + }, + })) + }) + + it('should call addLoadingTime with timestamp and no overwrite by default', () => { + rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) + + rumPublicApi.addViewLoadingTime() + + expect(addLoadingTimeSpy).toHaveBeenCalledOnceWith(jasmine.any(Number), false) + }) + + it('should pass overwrite true when specified', () => { + rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) + + rumPublicApi.addViewLoadingTime({ overwrite: true }) + + expect(addLoadingTimeSpy).toHaveBeenCalledOnceWith(jasmine.any(Number), true) + }) + + it('should not throw when called before init', () => { + expect(() => rumPublicApi.addViewLoadingTime()).not.toThrow() + + expect(addLoadingTimeSpy).not.toHaveBeenCalled() + }) + }) + describe('addFeatureFlagEvaluation', () => { let addFeatureFlagEvaluationSpy: jasmine.Spy['addFeatureFlagEvaluation']> let displaySpy: jasmine.Spy<() => void> diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index d0b9aa7c83..f2879c6745 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -35,6 +35,7 @@ import { startBufferingData, isExperimentalFeatureEnabled, ExperimentalFeature, + timeStampNow, } from '@datadog/browser-core' import type { LifeCycle } from '../domain/lifeCycle' @@ -215,6 +216,18 @@ export interface RumPublicApi extends PublicApi { */ addTiming: (name: string, time?: number) => void + /** + * Manually set the current view's loading time. + * + * Call this method when the view has finished loading. The loading time is computed as the + * elapsed time since the view started. By default, the first call sets the loading time and + * subsequent calls are no-ops. Use `{ overwrite: true }` to replace a previously set value. + * + * @category Data Collection + * @param options - Options. Set `overwrite: true` to replace a previously set loading time. + */ + addViewLoadingTime: (options?: { overwrite?: boolean }) => void + /** * Set the global context information to all events, stored in `@context` * See [Global context](https://docs.datadoghq.com/real_user_monitoring/browser/advanced_configuration/#global-context) for further information. @@ -533,6 +546,7 @@ export interface Strategy { getInternalContext: StartRumResult['getInternalContext'] stopSession: StartRumResult['stopSession'] addTiming: StartRumResult['addTiming'] + addLoadingTime: StartRumResult['addLoadingTime'] startView: StartRumResult['startView'] setViewName: StartRumResult['setViewName'] @@ -727,6 +741,17 @@ export function makeRumPublicApi( strategy.addTiming(sanitize(name)!, time as RelativeTime | TimeStamp | undefined) }), + addViewLoadingTime: monitor((options?: { overwrite?: boolean }) => { + const callTimestamp = timeStampNow() + const result = strategy.addLoadingTime(callTimestamp, options?.overwrite ?? false) + addTelemetryUsage({ + feature: 'addViewLoadingTime', + no_view: false, + no_active_view: result?.noActiveView ?? false, + overwritten: result?.overwritten ?? false, + }) + }), + setGlobalContext: defineContextMethod( getStrategy, CustomerContextKey.globalContext, diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 623ca080d0..903bf846fb 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -202,6 +202,7 @@ export function startRumEventCollection( const { addTiming, + addLoadingTime, startView, setViewName, setViewContext, @@ -252,6 +253,7 @@ export function startRumEventCollection( addEvent: eventCollection.addEvent, addError, addTiming, + addLoadingTime, addFeatureFlagEvaluation: featureFlagContexts.addFeatureFlagEvaluation, startView, setViewContext, diff --git a/packages/rum-core/src/domain/view/setupViewTest.specHelper.ts b/packages/rum-core/src/domain/view/setupViewTest.specHelper.ts index 08721f2f39..0e79a1a7fc 100644 --- a/packages/rum-core/src/domain/view/setupViewTest.specHelper.ts +++ b/packages/rum-core/src/domain/view/setupViewTest.specHelper.ts @@ -45,7 +45,7 @@ export function setupViewTest( } = spyOnViews() lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, viewEndHandler) - const { stop, startView, setViewName, setViewContext, setViewContextProperty, getViewContext, addTiming } = + const { stop, startView, setViewName, setViewContext, setViewContextProperty, getViewContext, addTiming, addLoadingTime } = trackViews( fakeLocation, lifeCycle, @@ -65,6 +65,7 @@ export function setupViewTest( changeLocation, setViewName, addTiming, + addLoadingTime, getViewUpdate, getViewUpdateCount, getViewCreate, diff --git a/packages/rum-core/src/domain/view/trackViews.spec.ts b/packages/rum-core/src/domain/view/trackViews.spec.ts index d8a57aa688..1b449d5491 100644 --- a/packages/rum-core/src/domain/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/view/trackViews.spec.ts @@ -783,6 +783,224 @@ describe('view custom timings', () => { }) }) +describe('manual loading time', () => { + const lifeCycle = new LifeCycle() + let clock: Clock + let viewTest: ViewTest + + beforeEach(() => { + clock = mockClock() + viewTest = setupViewTest({ lifeCycle }) + + registerCleanupTask(() => { + viewTest.stop() + }) + }) + + it('should set loading time on the current view', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(500) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.commonViewMetrics.loadingTime).toBe(clock.relative(500)) + }) + + it('should not overwrite loading time by default on subsequent calls', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const firstValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(200) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.commonViewMetrics.loadingTime).toBe(firstValue) + }) + + it('should overwrite loading time when overwrite is true', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const firstValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(200) + addLoadingTime(undefined, true) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.commonViewMetrics.loadingTime).not.toBe(firstValue) + }) + + it('should not set loading time when the session has expired', () => { + clock.tick(0) // run immediate timeouts (mostly for `trackNavigationTimings`) + const { getViewUpdateCount, addLoadingTime } = viewTest + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + const previousCount = getViewUpdateCount() + + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toBe(previousCount) + }) + + it('should compute loading time relative to route-change view start', () => { + const { getViewUpdate, getViewUpdateCount, startView, addLoadingTime } = viewTest + + clock.tick(2000) + startView() + + clock.tick(500) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.loadingType).toBe(ViewLoadingType.ROUTE_CHANGE) + expect(lastUpdate.commonViewMetrics.loadingTime).toBe(500 as Duration) + }) + + it('should suppress auto-detected loading time after manual call', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const manualValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(PAGE_ACTIVITY_END_DELAY) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.commonViewMetrics.loadingTime).toBe(manualValue) + }) + + it('should start with clean overwrite state on new view', () => { + const { getViewUpdate, getViewUpdateCount, startView, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + startView() + + clock.tick(200) + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.loadingType).toBe(ViewLoadingType.ROUTE_CHANGE) + expect(lastUpdate.commonViewMetrics.loadingTime).toBeDefined() + }) + + it('should trigger a view update after addLoadingTime', () => { + const { getViewUpdateCount, addLoadingTime } = viewTest + + const countBefore = getViewUpdateCount() + + addLoadingTime() + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toBeGreaterThan(countBefore) + }) + + it('should stop auto-detection tracking after first manual loading time', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() // first call -- should stop auto-detection tracking + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const firstValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(200) + addLoadingTime(undefined, true) // overwrite + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const overwrittenValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + // Let page activity end fire (would set auto-detected loading time if tracking wasn't stopped) + clock.tick(PAGE_ACTIVITY_END_DELAY) + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + // Auto-detection should not have replaced the overwritten value + const finalUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(finalUpdate.commonViewMetrics.loadingTime).toBe(overwrittenValue) + expect(finalUpdate.commonViewMetrics.loadingTime).not.toBe(firstValue) + }) + + it('should overwrite loading time on a route-change view with correct elapsed time', () => { + const { getViewUpdate, getViewUpdateCount, startView, addLoadingTime } = viewTest + + clock.tick(2000) // 2s into session + startView() // new route-change view starts + + clock.tick(300) // 300ms into new view + addLoadingTime() // first manual set: 300ms from view start + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const firstValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(200) // 200ms later (500ms + THROTTLE total from view start) + addLoadingTime(undefined, true) // overwrite + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + const lastUpdate = getViewUpdate(getViewUpdateCount() - 1) + expect(lastUpdate.loadingType).toBe(ViewLoadingType.ROUTE_CHANGE) + expect(lastUpdate.commonViewMetrics.loadingTime).not.toBe(firstValue) + // Loading time should be relative to view start, not time origin + // Value = 300 + THROTTLE_VIEW_UPDATE_PERIOD + 200 (all ms from view start) + expect(lastUpdate.commonViewMetrics.loadingTime).toBe((300 + THROTTLE_VIEW_UPDATE_PERIOD + 200) as Duration) + }) + + it('should allow multiple overwrites, each replacing the previous value', () => { + const { getViewUpdate, getViewUpdateCount, addLoadingTime } = viewTest + + clock.tick(100) + addLoadingTime() // first call: 100ms + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const firstValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(200) + addLoadingTime(undefined, true) // overwrite #1 + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const secondValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + clock.tick(300) + addLoadingTime(undefined, true) // overwrite #2 + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + const thirdValue = getViewUpdate(getViewUpdateCount() - 1).commonViewMetrics.loadingTime + + // Each value should be larger than the previous (more time elapsed) + expect(secondValue).toBeGreaterThan(firstValue as number) + expect(thirdValue).toBeGreaterThan(secondValue as number) + // Final value should be the cumulative elapsed time from time origin (initial view uses clocksOrigin) + expect(thirdValue).toBe(clock.relative(100 + THROTTLE_VIEW_UPDATE_PERIOD + 200 + THROTTLE_VIEW_UPDATE_PERIOD + 300)) + }) +}) + describe('start view', () => { const lifeCycle = new LifeCycle() let clock: Clock diff --git a/packages/rum-core/src/domain/view/trackViews.ts b/packages/rum-core/src/domain/view/trackViews.ts index 356678de73..f0aaf2a244 100644 --- a/packages/rum-core/src/domain/view/trackViews.ts +++ b/packages/rum-core/src/domain/view/trackViews.ts @@ -178,6 +178,9 @@ export function trackViews( addTiming: (name: string, time: RelativeTime | TimeStamp = timeStampNow()) => { currentView.addTiming(name, time) }, + addLoadingTime: (callTimestamp?: TimeStamp, overwrite?: boolean) => { + return currentView.addLoadingTime(callTimestamp, overwrite) + }, startView: (options?: ViewOptions, startClocks?: ClocksState) => { currentView.end({ endClocks: startClocks }) currentView = startNewView(ViewLoadingType.ROUTE_CHANGE, startClocks, options) @@ -226,6 +229,7 @@ function newView( const contextManager = createContextManager() let sessionIsActive = true + let hasManualLoadingTime = false let name = viewOptions?.name const service = viewOptions?.service || configuration.service const version = viewOptions?.version || configuration.version @@ -258,6 +262,7 @@ function newView( stop: stopCommonViewMetricsTracking, stopINPTracking, getCommonViewMetrics, + setManualLoadingTime, } = trackCommonViewMetrics( lifeCycle, domMutationObservable, @@ -378,6 +383,20 @@ function newView( customTimings[sanitizeTiming(name)] = relativeTime scheduleViewUpdate() }, + addLoadingTime(callTimestamp?: TimeStamp, overwrite = false) { + if (endClocks) { + return { noActiveView: true, overwritten: false } + } + if (hasManualLoadingTime && !overwrite) { + return { noActiveView: false, overwritten: false } + } + const overwritten = hasManualLoadingTime + const loadingTime = elapsed(startClocks.timeStamp, callTimestamp ?? timeStampNow()) + setManualLoadingTime(loadingTime) + hasManualLoadingTime = true + scheduleViewUpdate() + return { noActiveView: false, overwritten } + }, setViewName(updatedName: string) { name = updatedName triggerViewUpdate() diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts index 4f29990ccd..4274565174 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts @@ -28,6 +28,7 @@ export function trackCommonViewMetrics( viewStart: ClocksState ) { const commonViewMetrics: CommonViewMetrics = {} + let hasManualLoadingTime = false const { stop: stopLoadingTimeTracking, setLoadEvent } = trackLoadingTime( lifeCycle, @@ -37,8 +38,10 @@ export function trackCommonViewMetrics( loadingType, viewStart, (newLoadingTime) => { - commonViewMetrics.loadingTime = newLoadingTime - scheduleViewUpdate() + if (!hasManualLoadingTime) { + commonViewMetrics.loadingTime = newLoadingTime + scheduleViewUpdate() + } } ) @@ -74,5 +77,12 @@ export function trackCommonViewMetrics( commonViewMetrics.interactionToNextPaint = getInteractionToNextPaint() return commonViewMetrics }, + setManualLoadingTime: (loadingTime: Duration) => { + if (!hasManualLoadingTime) { + stopLoadingTimeTracking() + } + hasManualLoadingTime = true + commonViewMetrics.loadingTime = loadingTime + }, } } diff --git a/test/e2e/scenario/rum/views.scenario.ts b/test/e2e/scenario/rum/views.scenario.ts index 7b65010992..170b0910be 100644 --- a/test/e2e/scenario/rum/views.scenario.ts +++ b/test/e2e/scenario/rum/views.scenario.ts @@ -66,6 +66,51 @@ test.describe('rum views', () => { expect(viewEvent).toBeDefined() expect(viewEvent.view.loading_time).toBeGreaterThan(0) }) + + createTest('has a manual loading time') + .withRum() + .withBody(SPINNER) + .run(async ({ flushEvents, intakeRegistry, page }) => { + await page.evaluate( + () => + new Promise((resolve) => { + setTimeout(() => { + window.DD_RUM!.addViewLoadingTime() + resolve() + }, 200) + }) + ) + + await flushEvents() + const viewEvent = intakeRegistry.rumViewEvents.at(-1) + expect(viewEvent).toBeDefined() + expect(viewEvent!.view.loading_time).toBeGreaterThanOrEqual(200 * 1e6) + }) + + createTest('overwrites manual loading time when called with overwrite option') + .withRum() + .withBody(SPINNER) + .run(async ({ flushEvents, intakeRegistry, page }) => { + await page.evaluate( + () => + new Promise((resolve) => { + setTimeout(() => { + window.DD_RUM!.addViewLoadingTime() + }, 200) + + setTimeout(() => { + window.DD_RUM!.addViewLoadingTime({ overwrite: true }) + resolve() + }, 500) + }) + ) + + await flushEvents() + const viewEvent = intakeRegistry.rumViewEvents.at(-1) + expect(viewEvent).toBeDefined() + // Should reflect the second (overwritten) value (~500ms), not the first (~200ms) + expect(viewEvent!.view.loading_time).toBeGreaterThanOrEqual(500 * 1e6) + }) }) createTest('send performance first input delay')