diff --git a/packages/browser-utils/src/web-vitals/softNavs.ts b/packages/browser-utils/src/web-vitals/softNavs.ts index 9e397a60d4f6..addab70c50b0 100644 --- a/packages/browser-utils/src/web-vitals/softNavs.ts +++ b/packages/browser-utils/src/web-vitals/softNavs.ts @@ -29,8 +29,15 @@ interface PendingNavigation { interactionTimestamp: number; } +interface PendingInteraction { + interactionId: number; + interactionTimestamp: number; +} + // The navigation span whose triggering interaction we haven't identified yet. let _pendingNavigation: PendingNavigation | undefined; +// The interaction whose Event Timing entry arrived before any navigation span claimed it. +let _pendingInteraction: PendingInteraction | undefined; // The timestamp of the most recent trusted click/keydown, i.e. our best guess at the interaction // that a history change happening right now was driven by. let _lastInteractionTimestamp: number | undefined; @@ -40,6 +47,14 @@ const _navigationIdToNavigationSpan = new LRUMap(MAX_TRACKED_NAVIG let _correlationStarted = false; +/** + * Whether an Event Timing entry's `startTime` and a DOM event's `timeStamp` name the same + * interaction. + */ +function interactionMatches(entryStartTime: number, interactionTimestamp: number): boolean { + return Math.abs(entryStartTime - interactionTimestamp) <= INTERACTION_MATCH_TOLERANCE_MS; +} + /** * Whether the browser can report web vitals for soft navigations. * @@ -104,23 +119,50 @@ export function startSoftNavigationCorrelation(client: Client): void { // A navigation with no preceding interaction can't produce a soft navigation, so there is // nothing to wait for. Dropping the pending span here also keeps us from binding a stale one. - _pendingNavigation = - _lastInteractionTimestamp != null ? { span, interactionTimestamp: _lastInteractionTimestamp } : undefined; + _pendingNavigation = undefined; + const interactionTimestamp = _lastInteractionTimestamp; + if (interactionTimestamp == null) { + return; + } + + // The interaction's entry may already be here: the router code that starts this span races the + // paint that flushes the entry, so either one can win. + if (_pendingInteraction?.interactionTimestamp === interactionTimestamp) { + _interactionIdToNavigationSpan.set(_pendingInteraction.interactionId, span); + _pendingInteraction = undefined; + return; + } + + _pendingNavigation = { span, interactionTimestamp }; }); const bindInteractionToNavigationSpan = ({ entries }: { entries: PerformanceEntry[] }): void => { for (const entry of entries) { + if (!isPerformanceEventTiming(entry) || !entry.interactionId) { + continue; + } + const pending = _pendingNavigation; - if (!pending || !isPerformanceEventTiming(entry) || !entry.interactionId) { + if (pending && interactionMatches(entry.startTime, pending.interactionTimestamp)) { + _interactionIdToNavigationSpan.set(entry.interactionId, pending.span); + _pendingNavigation = undefined; continue; } - if (Math.abs(entry.startTime - pending.interactionTimestamp) > INTERACTION_MATCH_TOLERANCE_MS) { + // Once a navigation span has claimed this interaction, only a span that is still waiting can + // rebind it, which the check above already allows. Holding on to the interaction's remaining + // entries would instead let an unrelated later navigation claim it. + if (_interactionIdToNavigationSpan.get(entry.interactionId)) { continue; } - _interactionIdToNavigationSpan.set(entry.interactionId, pending.span); - _pendingNavigation = undefined; + // The navigation span this interaction drove may still be on its way, so hold on to the + // interaction instead of dropping it. Only the most recent one is worth keeping: + // `_lastInteractionTimestamp` is what `spanStart` matches against and it only moves forward, + // so an entry that doesn't match it now can never match it later. + if (_lastInteractionTimestamp != null && interactionMatches(entry.startTime, _lastInteractionTimestamp)) { + _pendingInteraction = { interactionId: entry.interactionId, interactionTimestamp: _lastInteractionTimestamp }; + } } }; diff --git a/packages/browser-utils/test/web-vitals/softNavs.test.ts b/packages/browser-utils/test/web-vitals/softNavs.test.ts index 8ccc70f89e07..a6b621b8a3b3 100644 --- a/packages/browser-utils/test/web-vitals/softNavs.test.ts +++ b/packages/browser-utils/test/web-vitals/softNavs.test.ts @@ -56,6 +56,7 @@ describe('soft navigation correlation', () => { afterEach(() => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); vi.clearAllMocks(); }); @@ -76,6 +77,92 @@ describe('soft navigation correlation', () => { expect(getNavigationSpanForMetric({ navigationType: 'soft-navigation', navigationId: 7 })).toBe(navigationSpan); }); + it('correlates when the interaction entry is delivered before the navigation span starts', async () => { + const { getNavigationSpanForMetric, startSoftNavigationCorrelation } = await loadSoftNavs(); + const { client, startSpan } = createMockClient(); + + startSoftNavigationCorrelation(client as never); + + windowListeners.get('click')?.({ isTrusted: true, timeStamp: 1234 }); + // The router code that starts the span has not run yet, so the entry gets here first. + performanceHandlers.get('event')?.({ entries: [{ duration: 8, startTime: 1234, interactionId: 42 }] }); + + const navigationSpan = createMockSpan('navigation'); + startSpan(navigationSpan); + + performanceHandlers.get('soft-navigation')?.({ entries: [{ navigationId: 7, interactionId: 42 }] }); + + expect(navigationSpan.setAttribute).toHaveBeenCalledWith(BROWSER_NAVIGATION_ID, 7); + expect(getNavigationSpanForMetric({ navigationType: 'soft-navigation', navigationId: 7 })).toBe(navigationSpan); + }); + + it('does not let a later navigation steal an interaction a navigation already claimed', async () => { + const { getNavigationSpanForMetric, startSoftNavigationCorrelation } = await loadSoftNavs(); + const { client, startSpan } = createMockClient(); + + startSoftNavigationCorrelation(client as never); + + windowListeners.get('click')?.({ isTrusted: true, timeStamp: 1000 }); + const navigationSpan = createMockSpan('navigation'); + startSpan(navigationSpan); + + // One interaction produces several entries. The first binds; the rest are delivered after the + // span is no longer pending. + performanceHandlers.get('event')?.({ + entries: [ + { duration: 8, startTime: 1000, interactionId: 42 }, + { duration: 8, startTime: 999, interactionId: 42 }, + ], + }); + + // A programmatic navigation, with no interaction of its own, must not claim interaction 42. + startSpan(createMockSpan('navigation')); + + expect( + getNavigationSpanForMetric({ navigationType: 'soft-navigation', navigationId: 7, navigationInteractionId: 42 }), + ).toBe(navigationSpan); + }); + + it('correlates when the interaction handler ran long before the navigation span started', async () => { + const { getNavigationSpanForMetric, startSoftNavigationCorrelation } = await loadSoftNavs(); + const { client, startSpan } = createMockClient(); + + startSoftNavigationCorrelation(client as never); + + // A click whose handler blocks for seconds. These are the worst INP values on the page, so + // they're the ones that matter most, and the span still starts before the entry is delivered. + windowListeners.get('click')?.({ isTrusted: true, timeStamp: 1000 }); + vi.spyOn(performance, 'now').mockReturnValue(3500); + + const navigationSpan = createMockSpan('navigation'); + startSpan(navigationSpan); + performanceHandlers.get('event')?.({ entries: [{ duration: 2500, startTime: 1000, interactionId: 42 }] }); + + expect( + getNavigationSpanForMetric({ navigationType: 'soft-navigation', navigationId: 7, navigationInteractionId: 42 }), + ).toBe(navigationSpan); + }); + + it('does not bind an early entry to a navigation from a different interaction', async () => { + const { getNavigationSpanForMetric, startSoftNavigationCorrelation } = await loadSoftNavs(); + const { client, startSpan } = createMockClient(); + + startSoftNavigationCorrelation(client as never); + + windowListeners.get('click')?.({ isTrusted: true, timeStamp: 500 }); + performanceHandlers.get('event')?.({ entries: [{ duration: 8, startTime: 500, interactionId: 1 }] }); + + // A second click, whose own entry has not arrived, is what this navigation happened during. + windowListeners.get('click')?.({ isTrusted: true, timeStamp: 1234 }); + const navigationSpan = createMockSpan('navigation'); + startSpan(navigationSpan); + + performanceHandlers.get('soft-navigation')?.({ entries: [{ navigationId: 7, interactionId: 1 }] }); + + expect(navigationSpan.setAttribute).not.toHaveBeenCalled(); + expect(getNavigationSpanForMetric({ navigationType: 'soft-navigation', navigationId: 7 })).toBeUndefined(); + }); + it('falls back to the interaction id when the soft navigation entry has not been observed yet', async () => { const { getNavigationSpanForMetric, startSoftNavigationCorrelation } = await loadSoftNavs(); const { client, startSpan } = createMockClient();