Skip to content

Commit 40c1793

Browse files
committed
Extract all update decision logic to shouldUpdateWildcardSpanName
1 parent fa120d4 commit 40c1793

File tree

1 file changed

+55
-24
lines changed

1 file changed

+55
-24
lines changed

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

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -753,8 +753,8 @@ export function handleNavigation(opts: {
753753

754754
// If we're already in a navigation span, check if we should update its name
755755
if (isAlreadyInNavigationSpan && activeSpan) {
756-
// Only update if the new name is better (doesn't have wildcards or is more complete)
757-
const shouldUpdate = currentName && transactionNameHasWildcard(currentName) && !transactionNameHasWildcard(name);
756+
const currentSource = spanJson?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
757+
const shouldUpdate = shouldUpdateWildcardSpanName(currentName, currentSource, name, source);
758758

759759
if (shouldUpdate) {
760760
activeSpan.updateName(name);
@@ -843,6 +843,54 @@ function updatePageloadTransaction({
843843
}
844844
}
845845

846+
/**
847+
* Determines if a span name should be updated during wildcard route resolution.
848+
*
849+
* This handles cases where:
850+
* 1. A wildcard route name (e.g., "/users/*") should be resolved to a specific route
851+
* 2. A URL-based name should be upgraded to a parameterized route name
852+
* 3. No current name exists and a route name is available (pageload transactions)
853+
*
854+
* @param currentName - The current span name (may be undefined)
855+
* @param currentSource - The current span source ('route', 'url', or undefined)
856+
* @param newName - The proposed new span name
857+
* @param newSource - The proposed new span source
858+
* @param allowNoCurrentName - If true, allow updates when there's no current name (for pageload spans)
859+
* @returns true if the span name should be updated
860+
*/
861+
function shouldUpdateWildcardSpanName(
862+
currentName: string | undefined,
863+
currentSource: string | undefined,
864+
newName: string,
865+
newSource: string,
866+
allowNoCurrentName = false,
867+
): boolean {
868+
if (!newName) {
869+
return false;
870+
}
871+
872+
// Allow update if no current name exists and allowNoCurrentName is true (pageloads)
873+
if (!currentName && allowNoCurrentName) {
874+
return true;
875+
}
876+
877+
// Check if current name has wildcard
878+
const hasWildcard = currentName && transactionNameHasWildcard(currentName);
879+
880+
// Current has wildcard, new is non-wildcard route
881+
if (hasWildcard && newSource === 'route' && !transactionNameHasWildcard(newName)) {
882+
return true;
883+
}
884+
885+
// Source upgrade - URL → route (but never route → URL)
886+
if (currentSource !== 'route' && newSource === 'route') {
887+
return true;
888+
}
889+
890+
// Otherwise, don't update (prevents route→url downgrade)
891+
return false;
892+
}
893+
846894
/** Updates span name before end using latest route information. */
847895
function tryUpdateSpanNameBeforeEnd(
848896
span: Span,
@@ -856,10 +904,9 @@ function tryUpdateSpanNameBeforeEnd(
856904
): void {
857905
try {
858906
const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
859-
const hasWildcard = currentName && transactionNameHasWildcard(currentName);
860907

861908
// Only attempt update if source is not 'route' or if the name has wildcards
862-
if (currentSource === 'route' && !hasWildcard) {
909+
if (currentSource === 'route' && currentName && !transactionNameHasWildcard(currentName)) {
863910
return;
864911
}
865912

@@ -873,16 +920,9 @@ function tryUpdateSpanNameBeforeEnd(
873920

874921
const [name, source] = resolveRouteNameAndSource(location, routesToUse, routesToUse, branches, basename);
875922

876-
// Only update if we have a valid name and it's better than current
877-
// Upgrade conditions:
878-
// 1. No current name exists
879-
// 2. Current name has wildcards and new name is non-wildcard route (wildcard resolution)
880-
// 3. Upgrading from non-route source to route source (e.g., URL -> parameterized route)
881-
const isImprovement =
882-
name &&
883-
(!currentName || // No current name - always set
884-
(hasWildcard && source === 'route' && !transactionNameHasWildcard(name)) || // Wildcard → non-wildcard route
885-
(currentSource !== 'route' && source === 'route')); // URL → route upgrade
923+
// Check if the new name is an improvement over the current name
924+
// For pageload spans, allow updates when there's no current name
925+
const isImprovement = shouldUpdateWildcardSpanName(currentName, currentSource, name, source, true);
886926
const spanNotEnded = spanType === 'pageload' || !spanJson.timestamp;
887927

888928
if (isImprovement && spanNotEnded) {
@@ -938,16 +978,7 @@ function patchSpanEnd(
938978
// Special case: 0 means don't wait at all (legacy behavior)
939979
if (_lazyRouteTimeout === 0) {
940980
// Don't wait - immediately update and end span
941-
tryUpdateSpanNameBeforeEnd(
942-
span,
943-
spanJson,
944-
currentName,
945-
location,
946-
routes,
947-
basename,
948-
spanType,
949-
allRoutes,
950-
);
981+
tryUpdateSpanNameBeforeEnd(span, spanJson, currentName, location, routes, basename, spanType, allRoutes);
951982
return originalEnd(...args);
952983
}
953984

0 commit comments

Comments
 (0)