From d3757287100d27fce2f86374a561ef15f83da0f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Lilensten?= Date: Fri, 13 Feb 2026 11:32:13 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9C=A8=20feat(rum):=20add=20addViewLoadi?= =?UTF-8?q?ngTime()=20public=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new public API `DD_RUM.addViewLoadingTime(options?)` that allows developers to manually report when a view has finished loading. - Computes loading time as elapsed time since view start - Suppresses auto-detected loading time on first manual call - First-call-wins by default; `{ overwrite: true }` to replace - Pre-start buffering with preserved call timestamps - Telemetry usage tracking - Gracefully no-ops on ended/expired views - Unit tests + E2E test --- .gitignore | 1 + .../domain/telemetry/telemetryEvent.types.ts | 2 + .../rum-core/src/boot/preStartRum.spec.ts | 27 ++++ packages/rum-core/src/boot/preStartRum.ts | 4 + .../rum-core/src/boot/rumPublicApi.spec.ts | 37 +++++ packages/rum-core/src/boot/rumPublicApi.ts | 30 ++++ packages/rum-core/src/boot/startRum.ts | 2 + .../domain/view/setupViewTest.specHelper.ts | 3 +- .../src/domain/view/trackViews.spec.ts | 143 ++++++++++++++++++ .../rum-core/src/domain/view/trackViews.ts | 18 +++ .../viewMetrics/trackCommonViewMetrics.ts | 28 +++- test/e2e/scenario/rum/views.scenario.ts | 20 +++ 12 files changed, 312 insertions(+), 3 deletions(-) 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..2437c4505c 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: () => ({ overwritten: false as const }), 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..39378e5a5a 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,20 @@ 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. + * + * See [Override RUM View Loading Time](https://docs.datadoghq.com/real_user_monitoring/browser/advanced_configuration/) for further information. + * + * @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 +548,7 @@ export interface Strategy { getInternalContext: StartRumResult['getInternalContext'] stopSession: StartRumResult['stopSession'] addTiming: StartRumResult['addTiming'] + addLoadingTime: StartRumResult['addLoadingTime'] startView: StartRumResult['startView'] setViewName: StartRumResult['setViewName'] @@ -727,6 +743,20 @@ 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) + + // For pre-start buffered calls, result is undefined so we emit best-guess default values. + const isResult = result && typeof result === 'object' + addTelemetryUsage({ + feature: 'addViewLoadingTime', + no_view: false, + no_active_view: isResult && 'no_active_view' in result, + overwritten: isResult && 'overwritten' in result && 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..16ea9ab848 100644 --- a/packages/rum-core/src/domain/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/view/trackViews.spec.ts @@ -783,6 +783,149 @@ 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) + }) +}) + 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..a5c951287f 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,19 @@ function newView( customTimings[sanitizeTiming(name)] = relativeTime scheduleViewUpdate() }, + addLoadingTime(callTimestamp?: TimeStamp, overwrite = false) { + if (endClocks) { + return { no_active_view: true as const } + } + if (hasManualLoadingTime && !overwrite) { + return { already_set: true as const } + } + const loadingTime = elapsed(startClocks.timeStamp, callTimestamp ?? timeStampNow()) + const result = setManualLoadingTime(loadingTime) + hasManualLoadingTime = true + scheduleViewUpdate() + return result + }, 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..bccde0b1cf 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 isLoadingTimeSetManually = false const { stop: stopLoadingTimeTracking, setLoadEvent } = trackLoadingTime( lifeCycle, @@ -37,8 +38,10 @@ export function trackCommonViewMetrics( loadingType, viewStart, (newLoadingTime) => { - commonViewMetrics.loadingTime = newLoadingTime - scheduleViewUpdate() + if (!isLoadingTimeSetManually) { + commonViewMetrics.loadingTime = newLoadingTime + scheduleViewUpdate() + } } ) @@ -74,5 +77,26 @@ export function trackCommonViewMetrics( commonViewMetrics.interactionToNextPaint = getInteractionToNextPaint() return commonViewMetrics }, + setManualLoadingTime: (loadingTime: Duration) => { + const hadPreviousManual = isLoadingTimeSetManually + const hadAutoValue = !isLoadingTimeSetManually && commonViewMetrics.loadingTime !== undefined + + // Stop auto-detection entirely on first manual call (CONTEXT decision) + if (!isLoadingTimeSetManually) { + stopLoadingTimeTracking() + } + + isLoadingTimeSetManually = true + commonViewMetrics.loadingTime = loadingTime + + // Return overwrite source for Phase 2 telemetry + if (hadPreviousManual) { + return { overwritten: 'manual' as const } + } + if (hadAutoValue) { + return { overwritten: 'auto' as const } + } + return { overwritten: false as const } + }, } } diff --git a/test/e2e/scenario/rum/views.scenario.ts b/test/e2e/scenario/rum/views.scenario.ts index 7b65010992..7d5f2c6997 100644 --- a/test/e2e/scenario/rum/views.scenario.ts +++ b/test/e2e/scenario/rum/views.scenario.ts @@ -66,6 +66,26 @@ 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('send performance first input delay') From 1bb569effe7f45033e93bd6664b976e71615a616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Lilensten?= Date: Mon, 16 Feb 2026 11:56:18 +0100 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A7=B9=20chore(05-01):=20remove=20int?= =?UTF-8?q?ernal=20process-artifact=20comments=20and=20broken=20JSDoc=20li?= =?UTF-8?q?nk?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove '(CONTEXT decision)' comment from trackCommonViewMetrics.ts - Remove 'Phase 2 telemetry' comment from trackCommonViewMetrics.ts - Remove broken JSDoc link to non-existent doc page from rumPublicApi.ts --- packages/rum-core/src/boot/rumPublicApi.ts | 2 -- .../src/domain/view/viewMetrics/trackCommonViewMetrics.ts | 2 -- 2 files changed, 4 deletions(-) diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 39378e5a5a..ce66e0c358 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -223,8 +223,6 @@ export interface RumPublicApi extends PublicApi { * 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. * - * See [Override RUM View Loading Time](https://docs.datadoghq.com/real_user_monitoring/browser/advanced_configuration/) for further information. - * * @category Data Collection * @param options - Options. Set `overwrite: true` to replace a previously set loading time. */ diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts index bccde0b1cf..9dfb311b7a 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts @@ -81,7 +81,6 @@ export function trackCommonViewMetrics( const hadPreviousManual = isLoadingTimeSetManually const hadAutoValue = !isLoadingTimeSetManually && commonViewMetrics.loadingTime !== undefined - // Stop auto-detection entirely on first manual call (CONTEXT decision) if (!isLoadingTimeSetManually) { stopLoadingTimeTracking() } @@ -89,7 +88,6 @@ export function trackCommonViewMetrics( isLoadingTimeSetManually = true commonViewMetrics.loadingTime = loadingTime - // Return overwrite source for Phase 2 telemetry if (hadPreviousManual) { return { overwritten: 'manual' as const } } From 7d86ef3587a5a4266d96c996e42e12c4d10ff0f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Lilensten?= Date: Wed, 18 Feb 2026 22:17:47 +0100 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9C=A8=20Add=20manual=20view=20loading?= =?UTF-8?q?=20time=20reporting=20with=20addViewLoadingTime=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refactor addLoadingTime to return void at newView and trackViews layers - Simplify telemetry to user intent, remove runtime assertion - Add unit and E2E tests for manual loading time coverage - Draft RFC for addViewLoadingTime() public API --- .../rum-core/src/boot/rumPublicApi.spec.ts | 2 +- packages/rum-core/src/boot/rumPublicApi.ts | 7 +- .../src/domain/view/trackViews.spec.ts | 75 +++++++++++++++++++ .../rum-core/src/domain/view/trackViews.ts | 9 ++- .../viewMetrics/trackCommonViewMetrics.ts | 20 +---- test/e2e/scenario/rum/views.scenario.ts | 25 +++++++ 6 files changed, 112 insertions(+), 26 deletions(-) diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index 2437c4505c..55949950de 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -14,7 +14,7 @@ const noopStartRum = (): ReturnType => ({ addError: () => undefined, addEvent: () => undefined, addTiming: () => undefined, - addLoadingTime: () => ({ overwritten: false as const }), + addLoadingTime: () => ({ noActiveView: false, overwritten: false }), addFeatureFlagEvaluation: () => undefined, startView: () => undefined, setViewContext: () => undefined, diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index ce66e0c358..f2879c6745 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -744,14 +744,11 @@ export function makeRumPublicApi( addViewLoadingTime: monitor((options?: { overwrite?: boolean }) => { const callTimestamp = timeStampNow() const result = strategy.addLoadingTime(callTimestamp, options?.overwrite ?? false) - - // For pre-start buffered calls, result is undefined so we emit best-guess default values. - const isResult = result && typeof result === 'object' addTelemetryUsage({ feature: 'addViewLoadingTime', no_view: false, - no_active_view: isResult && 'no_active_view' in result, - overwritten: isResult && 'overwritten' in result && result.overwritten !== false, + no_active_view: result?.noActiveView ?? false, + overwritten: result?.overwritten ?? false, }) }), diff --git a/packages/rum-core/src/domain/view/trackViews.spec.ts b/packages/rum-core/src/domain/view/trackViews.spec.ts index 16ea9ab848..1b449d5491 100644 --- a/packages/rum-core/src/domain/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/view/trackViews.spec.ts @@ -924,6 +924,81 @@ describe('manual loading time', () => { 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', () => { diff --git a/packages/rum-core/src/domain/view/trackViews.ts b/packages/rum-core/src/domain/view/trackViews.ts index a5c951287f..f0aaf2a244 100644 --- a/packages/rum-core/src/domain/view/trackViews.ts +++ b/packages/rum-core/src/domain/view/trackViews.ts @@ -385,16 +385,17 @@ function newView( }, addLoadingTime(callTimestamp?: TimeStamp, overwrite = false) { if (endClocks) { - return { no_active_view: true as const } + return { noActiveView: true, overwritten: false } } if (hasManualLoadingTime && !overwrite) { - return { already_set: true as const } + return { noActiveView: false, overwritten: false } } + const overwritten = hasManualLoadingTime const loadingTime = elapsed(startClocks.timeStamp, callTimestamp ?? timeStampNow()) - const result = setManualLoadingTime(loadingTime) + setManualLoadingTime(loadingTime) hasManualLoadingTime = true scheduleViewUpdate() - return result + return { noActiveView: false, overwritten } }, setViewName(updatedName: string) { name = updatedName diff --git a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts index 9dfb311b7a..4274565174 100644 --- a/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts +++ b/packages/rum-core/src/domain/view/viewMetrics/trackCommonViewMetrics.ts @@ -28,7 +28,7 @@ export function trackCommonViewMetrics( viewStart: ClocksState ) { const commonViewMetrics: CommonViewMetrics = {} - let isLoadingTimeSetManually = false + let hasManualLoadingTime = false const { stop: stopLoadingTimeTracking, setLoadEvent } = trackLoadingTime( lifeCycle, @@ -38,7 +38,7 @@ export function trackCommonViewMetrics( loadingType, viewStart, (newLoadingTime) => { - if (!isLoadingTimeSetManually) { + if (!hasManualLoadingTime) { commonViewMetrics.loadingTime = newLoadingTime scheduleViewUpdate() } @@ -78,23 +78,11 @@ export function trackCommonViewMetrics( return commonViewMetrics }, setManualLoadingTime: (loadingTime: Duration) => { - const hadPreviousManual = isLoadingTimeSetManually - const hadAutoValue = !isLoadingTimeSetManually && commonViewMetrics.loadingTime !== undefined - - if (!isLoadingTimeSetManually) { + if (!hasManualLoadingTime) { stopLoadingTimeTracking() } - - isLoadingTimeSetManually = true + hasManualLoadingTime = true commonViewMetrics.loadingTime = loadingTime - - if (hadPreviousManual) { - return { overwritten: 'manual' as const } - } - if (hadAutoValue) { - return { overwritten: 'auto' as const } - } - return { overwritten: false as const } }, } } diff --git a/test/e2e/scenario/rum/views.scenario.ts b/test/e2e/scenario/rum/views.scenario.ts index 7d5f2c6997..170b0910be 100644 --- a/test/e2e/scenario/rum/views.scenario.ts +++ b/test/e2e/scenario/rum/views.scenario.ts @@ -86,6 +86,31 @@ test.describe('rum views', () => { 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')