Skip to content

Commit f703ae9

Browse files
committed
Use 100ms window for skipping cross-usage duplicates
1 parent 79ccaaa commit f703ae9

File tree

3 files changed

+141
-47
lines changed

3 files changed

+141
-47
lines changed

packages/react/src/reactrouter-compat-utils/instrumentation.tsx

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ let _enableAsyncRouteHandlers: boolean = false;
5353
const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet<Client>();
5454

5555
/**
56-
* Adds resolved routes as children to the parent route.
57-
* Prevents duplicate routes by checking if they already exist.
56+
* Tracks last navigation per client to prevent duplicate spans in cross-usage scenarios.
57+
* Uses 100ms window to deduplicate when multiple wrappers handle the same navigation.
5858
*/
59+
const LAST_NAVIGATION_PER_CLIENT = new WeakMap<Client, { key: string; timestamp: number }>();
60+
5961
export function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void {
6062
const existingChildren = parentRoute.children || [];
6163

@@ -618,6 +620,69 @@ function wrapPatchRoutesOnNavigation(
618620
};
619621
}
620622

623+
function getNavigationKey(location: Location): string {
624+
return `${location.pathname}${location.search}${location.hash}`;
625+
}
626+
627+
function tryUpdateSpanName(
628+
activeSpan: Span,
629+
currentSpanName: string | undefined,
630+
newName: string,
631+
newSource: string,
632+
): void {
633+
const isNewNameBetter = newName !== currentSpanName && newName.includes(':');
634+
if (isNewNameBetter) {
635+
activeSpan.updateName(newName);
636+
activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, newSource as 'route' | 'url' | 'custom');
637+
}
638+
}
639+
640+
function isDuplicateNavigation(client: Client, navigationKey: string): boolean {
641+
const lastNavigation = LAST_NAVIGATION_PER_CLIENT.get(client);
642+
const now = Date.now();
643+
return !!(lastNavigation && lastNavigation.key === navigationKey && now - lastNavigation.timestamp < 100);
644+
}
645+
646+
function createNavigationSpan(opts: {
647+
client: Client;
648+
name: string;
649+
source: string;
650+
version: string;
651+
location: Location;
652+
routes: RouteObject[];
653+
basename?: string;
654+
allRoutes?: RouteObject[];
655+
navigationKey: string;
656+
}): Span | undefined {
657+
const { client, name, source, version, location, routes, basename, allRoutes, navigationKey } = opts;
658+
659+
LAST_NAVIGATION_PER_CLIENT.set(client, {
660+
key: navigationKey,
661+
timestamp: Date.now(),
662+
});
663+
664+
const navigationSpan = startBrowserTracingNavigationSpan(client, {
665+
name,
666+
attributes: {
667+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source as 'route' | 'url' | 'custom',
668+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
669+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
670+
},
671+
});
672+
673+
if (navigationSpan) {
674+
patchNavigationSpanEnd(navigationSpan, location, routes, basename, allRoutes);
675+
676+
client.on('spanEnd', endedSpan => {
677+
if (endedSpan === navigationSpan) {
678+
LAST_NAVIGATION_PER_CLIENT.delete(client);
679+
}
680+
});
681+
}
682+
683+
return navigationSpan;
684+
}
685+
621686
export function handleNavigation(opts: {
622687
location: Location;
623688
routes: RouteObject[];
@@ -628,16 +693,13 @@ export function handleNavigation(opts: {
628693
allRoutes?: RouteObject[];
629694
}): void {
630695
const { location, routes, navigationType, version, matches, basename, allRoutes } = opts;
631-
// Use allRoutes for matching to include lazy-loaded routes
632696
const branches = Array.isArray(matches) ? matches : _matchRoutes(allRoutes || routes, location, basename);
633697

634698
const client = getClient();
635699
if (!client || !CLIENTS_WITH_INSTRUMENT_NAVIGATION.has(client)) {
636700
return;
637701
}
638702

639-
// Avoid starting a navigation span on initial load when a pageload root span is active.
640-
// This commonly happens when lazy routes resolve during the first render and React Router emits a POP.
641703
const activeRootSpan = getActiveRootSpan();
642704
if (activeRootSpan && spanToJSON(activeRootSpan).op === 'pageload' && navigationType === 'POP') {
643705
return;
@@ -655,25 +717,25 @@ export function handleNavigation(opts: {
655717
const activeSpan = getActiveSpan();
656718
const spanJson = activeSpan && spanToJSON(activeSpan);
657719
const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';
658-
659-
// Only skip creating a new span if we're already in a navigation span AND the route name matches.
660-
// This handles cross-usage (multiple wrappers for same navigation) while allowing consecutive navigations.
661720
const isSpanForSameRoute = isAlreadyInNavigationSpan && spanJson?.description === name;
662721

663-
if (!isSpanForSameRoute) {
664-
const navigationSpan = startBrowserTracingNavigationSpan(client, {
722+
const currentNavigationKey = getNavigationKey(location);
723+
const isNavDuplicate = isDuplicateNavigation(client, currentNavigationKey);
724+
725+
if (!isSpanForSameRoute && !isNavDuplicate) {
726+
createNavigationSpan({
727+
client,
665728
name,
666-
attributes: {
667-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
668-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
669-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
670-
},
729+
source,
730+
version,
731+
location,
732+
routes,
733+
basename,
734+
allRoutes,
735+
navigationKey: currentNavigationKey,
671736
});
672-
673-
// Patch navigation span to handle early cancellation (e.g., document.hidden)
674-
if (navigationSpan) {
675-
patchNavigationSpanEnd(navigationSpan, location, routes, basename, allRoutes);
676-
}
737+
} else if (isNavDuplicate && isAlreadyInNavigationSpan && activeSpan) {
738+
tryUpdateSpanName(activeSpan, spanJson?.description, name, source);
677739
}
678740
}
679741
}

packages/react/test/reactrouter-cross-usage.test.tsx

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,10 @@ describe('React Router cross usage of wrappers', () => {
220220

221221
expect(container.innerHTML).toContain('Details');
222222

223-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
224-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
225-
name: '/second-level/:id/third-level/:id',
226-
attributes: {
227-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
228-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
229-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
230-
},
231-
});
223+
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
224+
// In cross-usage scenarios, the first wrapper creates the span and the second updates it
225+
expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/second-level/:id/third-level/:id');
226+
expect(mockNavigationSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
232227
});
233228
});
234229

@@ -465,16 +460,12 @@ describe('React Router cross usage of wrappers', () => {
465460

466461
expect(container.innerHTML).toContain('Details');
467462

468-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
463+
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
469464

470-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
471-
name: '/second-level/:id/third-level/:id',
472-
attributes: {
473-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
474-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
475-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
476-
},
477-
});
465+
// Cross-usage deduplication: Span created once with initial route name
466+
// With nested lazy routes, initial name may be raw path, updated to parameterized by later wrapper
467+
expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/second-level/:id/third-level/:id');
468+
expect(mockNavigationSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
478469
});
479470
});
480471

@@ -595,15 +586,10 @@ describe('React Router cross usage of wrappers', () => {
595586
);
596587

597588
expect(container.innerHTML).toContain('Details');
598-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
599-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
600-
name: '/second-level/:id/third-level/:id',
601-
attributes: {
602-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
603-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
604-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
605-
},
606-
});
589+
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
590+
// Cross-usage with all three wrappers: span created once, then updated
591+
expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/second-level/:id/third-level/:id');
592+
expect(mockNavigationSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
607593
});
608594
});
609595

yarn.lock

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6943,6 +6943,20 @@
69436943
"@angular-devkit/schematics" "14.2.13"
69446944
jsonc-parser "3.1.0"
69456945

6946+
"@sentry-internal/browser-utils@10.23.0":
6947+
version "10.23.0"
6948+
resolved "https://registry.yarnpkg.com/@sentry-internal/browser-utils/-/browser-utils-10.23.0.tgz#738a07ed99168cdf69d0cdb5a152289ed049de81"
6949+
integrity sha512-FUak8FH51TnGrx2i31tgqun0VsbDCVQS7dxWnUZHdi+0hpnFoq9+wBHY+qrOQjaInZSz3crIifYv3z7SEzD0Jg==
6950+
dependencies:
6951+
"@sentry/core" "10.23.0"
6952+
6953+
"@sentry-internal/feedback@10.23.0":
6954+
version "10.23.0"
6955+
resolved "https://registry.yarnpkg.com/@sentry-internal/feedback/-/feedback-10.23.0.tgz#4b9ade29f1d96309eea83cc513c4a73e3992c4d7"
6956+
integrity sha512-+HWC9VTPICsFX/lIPoBU9GxTaJZVXJcukP+qGxj+j/8q/Dy1w22JHDWcJbZiaW4kWWlz7VbA0KVKS3grD+e9aA==
6957+
dependencies:
6958+
"@sentry/core" "10.23.0"
6959+
69466960
"@sentry-internal/node-cpu-profiler@^2.2.0":
69476961
version "2.2.0"
69486962
resolved "https://registry.yarnpkg.com/@sentry-internal/node-cpu-profiler/-/node-cpu-profiler-2.2.0.tgz#0640d4aebb4d36031658ccff83dc22b76f437ede"
@@ -6959,6 +6973,22 @@
69596973
detect-libc "^2.0.4"
69606974
node-abi "^3.73.0"
69616975

6976+
"@sentry-internal/replay-canvas@10.23.0":
6977+
version "10.23.0"
6978+
resolved "https://registry.yarnpkg.com/@sentry-internal/replay-canvas/-/replay-canvas-10.23.0.tgz#236916fb9d40637d8c9f86c52b2b1619b1170854"
6979+
integrity sha512-GLNY8JPcMI6xhQ5FHiYO/W/3flrwZMt4CI/E3jDRNujYWbCrca60MRke6k7Zm1qi9rZ1FuhVWZ6BAFc4vwXnSg==
6980+
dependencies:
6981+
"@sentry-internal/replay" "10.23.0"
6982+
"@sentry/core" "10.23.0"
6983+
6984+
"@sentry-internal/replay@10.23.0":
6985+
version "10.23.0"
6986+
resolved "https://registry.yarnpkg.com/@sentry-internal/replay/-/replay-10.23.0.tgz#7a6075e2c2e1d0a371764d7c2e5dad578bb7b1fe"
6987+
integrity sha512-5yPD7jVO2JY8+JEHXep0Bf/ugp4rmxv5BkHIcSAHQsKSPhziFks2x+KP+6M8hhbF1WydqAaDYlGjrkL2yspHqA==
6988+
dependencies:
6989+
"@sentry-internal/browser-utils" "10.23.0"
6990+
"@sentry/core" "10.23.0"
6991+
69626992
"@sentry-internal/rrdom@2.34.0":
69636993
version "2.34.0"
69646994
resolved "https://registry.yarnpkg.com/@sentry-internal/rrdom/-/rrdom-2.34.0.tgz#fccc9fe211c3995d4200abafbe8d75b671961ee9"
@@ -7032,6 +7062,17 @@
70327062
resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-4.3.0.tgz#c5b6cbb986952596d3ad233540a90a1fd18bad80"
70337063
integrity sha512-OuxqBprXRyhe8Pkfyz/4yHQJc5c3lm+TmYWSSx8u48g5yKewSQDOxkiLU5pAk3WnbLPy8XwU/PN+2BG0YFU9Nw==
70347064

7065+
"@sentry/browser@10.23.0":
7066+
version "10.23.0"
7067+
resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-10.23.0.tgz#aa85f9c21c9a6c80b8952ee15307997fb34edbb3"
7068+
integrity sha512-9hViLfYONxRJykOhJQ3ZHQ758t1wQIsxEC7mTsydbDm+m12LgbBtXbfgcypWHlom5Yvb+wg6W+31bpdGnATglw==
7069+
dependencies:
7070+
"@sentry-internal/browser-utils" "10.23.0"
7071+
"@sentry-internal/feedback" "10.23.0"
7072+
"@sentry-internal/replay" "10.23.0"
7073+
"@sentry-internal/replay-canvas" "10.23.0"
7074+
"@sentry/core" "10.23.0"
7075+
70357076
"@sentry/bundler-plugin-core@4.3.0", "@sentry/bundler-plugin-core@^4.3.0":
70367077
version "4.3.0"
70377078
resolved "https://registry.yarnpkg.com/@sentry/bundler-plugin-core/-/bundler-plugin-core-4.3.0.tgz#cf302522a3e5b8a3bf727635d0c6a7bece981460"
@@ -7106,6 +7147,11 @@
71067147
"@sentry/cli-win32-i686" "2.56.0"
71077148
"@sentry/cli-win32-x64" "2.56.0"
71087149

7150+
"@sentry/core@10.23.0":
7151+
version "10.23.0"
7152+
resolved "https://registry.yarnpkg.com/@sentry/core/-/core-10.23.0.tgz#7d4eb4d2c7b9ecc88872975a916f44e0b9fec78a"
7153+
integrity sha512-4aZwu6VnSHWDplY5eFORcVymhfvS/P6BRfK81TPnG/ReELaeoykKjDwR+wC4lO7S0307Vib9JGpszjsEZw245g==
7154+
71097155
"@sentry/rollup-plugin@^4.3.0":
71107156
version "4.3.0"
71117157
resolved "https://registry.yarnpkg.com/@sentry/rollup-plugin/-/rollup-plugin-4.3.0.tgz#d23fe49e48fa68dafa2b0933a8efabcc964b1df9"

0 commit comments

Comments
 (0)