Skip to content

Commit bf682c8

Browse files
andreiborzaandreiborza
committed
feat(solid): Add route parametrization for Solid Router
This PR adds route parametrization for the solidRouterBrowserTracingIntegration. It replaces raw URLs (e.g. /users/5) with parametrized routes (e.g. /users/:id) in transaction names.
1 parent 94534e6 commit bf682c8

File tree

7 files changed

+176
-115
lines changed

7 files changed

+176
-115
lines changed

‎dev-packages/e2e-tests/test-applications/solidstart-dynamic-import/tests/performance.client.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ test('sends a pageload transaction', async ({ page }) => {
1818
},
1919
transaction: '/',
2020
transaction_info: {
21-
source: 'url',
21+
source: 'route',
2222
},
2323
});
2424
});
2525

26-
test('sends a navigation transaction', async ({ page }) => {
26+
test('sends a navigation transaction with parametrized route', async ({ page }) => {
2727
const transactionPromise = waitForTransaction('solidstart-dynamic-import', async transactionEvent => {
28-
return transactionEvent?.transaction === '/users/5' && transactionEvent.contexts?.trace?.op === 'navigation';
28+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
2929
});
3030

3131
await page.goto(`/`);
@@ -39,9 +39,9 @@ test('sends a navigation transaction', async ({ page }) => {
3939
origin: 'auto.navigation.solidstart.solidrouter',
4040
},
4141
},
42-
transaction: '/users/5',
42+
transaction: '/users/:id',
4343
transaction_info: {
44-
source: 'url',
44+
source: 'route',
4545
},
4646
});
4747
});
@@ -51,7 +51,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
5151
// The sentry solidRouterBrowserTracingIntegration tries to update such
5252
// transactions with the proper name once the `useLocation` hook triggers.
5353
const navigationTxnPromise = waitForTransaction('solidstart-dynamic-import', async transactionEvent => {
54-
return transactionEvent?.transaction === '/users/6' && transactionEvent.contexts?.trace?.op === 'navigation';
54+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
5555
});
5656

5757
await page.goto(`/back-navigation`);
@@ -65,9 +65,9 @@ test('updates the transaction when using the back button', async ({ page }) => {
6565
origin: 'auto.navigation.solidstart.solidrouter',
6666
},
6767
},
68-
transaction: '/users/6',
68+
transaction: '/users/:id',
6969
transaction_info: {
70-
source: 'url',
70+
source: 'route',
7171
},
7272
});
7373

@@ -89,7 +89,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
8989
},
9090
transaction: '/back-navigation',
9191
transaction_info: {
92-
source: 'url',
92+
source: 'route',
9393
},
9494
});
9595
});

‎dev-packages/e2e-tests/test-applications/solidstart-spa/tests/performance.client.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ test('sends a pageload transaction', async ({ page }) => {
1818
},
1919
transaction: '/',
2020
transaction_info: {
21-
source: 'url',
21+
source: 'route',
2222
},
2323
});
2424
});
2525

26-
test('sends a navigation transaction', async ({ page }) => {
26+
test('sends a navigation transaction with parametrized route', async ({ page }) => {
2727
const transactionPromise = waitForTransaction('solidstart-spa', async transactionEvent => {
28-
return transactionEvent?.transaction === '/users/5' && transactionEvent.contexts?.trace?.op === 'navigation';
28+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
2929
});
3030

3131
await page.goto(`/`);
@@ -39,9 +39,9 @@ test('sends a navigation transaction', async ({ page }) => {
3939
origin: 'auto.navigation.solidstart.solidrouter',
4040
},
4141
},
42-
transaction: '/users/5',
42+
transaction: '/users/:id',
4343
transaction_info: {
44-
source: 'url',
44+
source: 'route',
4545
},
4646
});
4747
});
@@ -51,7 +51,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
5151
// The sentry solidRouterBrowserTracingIntegration tries to update such
5252
// transactions with the proper name once the `useLocation` hook triggers.
5353
const navigationTxnPromise = waitForTransaction('solidstart-spa', async transactionEvent => {
54-
return transactionEvent?.transaction === '/users/6' && transactionEvent.contexts?.trace?.op === 'navigation';
54+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
5555
});
5656

5757
await page.goto(`/back-navigation`);
@@ -65,9 +65,9 @@ test('updates the transaction when using the back button', async ({ page }) => {
6565
origin: 'auto.navigation.solidstart.solidrouter',
6666
},
6767
},
68-
transaction: '/users/6',
68+
transaction: '/users/:id',
6969
transaction_info: {
70-
source: 'url',
70+
source: 'route',
7171
},
7272
});
7373

@@ -89,7 +89,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
8989
},
9090
transaction: '/back-navigation',
9191
transaction_info: {
92-
source: 'url',
92+
source: 'route',
9393
},
9494
});
9595
});

‎dev-packages/e2e-tests/test-applications/solidstart-top-level-import/tests/performance.client.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ test('sends a pageload transaction', async ({ page }) => {
1818
},
1919
transaction: '/',
2020
transaction_info: {
21-
source: 'url',
21+
source: 'route',
2222
},
2323
});
2424
});
2525

26-
test('sends a navigation transaction', async ({ page }) => {
26+
test('sends a navigation transaction with parametrized route', async ({ page }) => {
2727
const transactionPromise = waitForTransaction('solidstart-top-level-import', async transactionEvent => {
28-
return transactionEvent?.transaction === '/users/5' && transactionEvent.contexts?.trace?.op === 'navigation';
28+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
2929
});
3030

3131
await page.goto(`/`);
@@ -39,9 +39,9 @@ test('sends a navigation transaction', async ({ page }) => {
3939
origin: 'auto.navigation.solidstart.solidrouter',
4040
},
4141
},
42-
transaction: '/users/5',
42+
transaction: '/users/:id',
4343
transaction_info: {
44-
source: 'url',
44+
source: 'route',
4545
},
4646
});
4747
});
@@ -51,7 +51,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
5151
// The sentry solidRouterBrowserTracingIntegration tries to update such
5252
// transactions with the proper name once the `useLocation` hook triggers.
5353
const navigationTxnPromise = waitForTransaction('solidstart-top-level-import', async transactionEvent => {
54-
return transactionEvent?.transaction === '/users/6' && transactionEvent.contexts?.trace?.op === 'navigation';
54+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
5555
});
5656

5757
await page.goto(`/back-navigation`);
@@ -65,9 +65,9 @@ test('updates the transaction when using the back button', async ({ page }) => {
6565
origin: 'auto.navigation.solidstart.solidrouter',
6666
},
6767
},
68-
transaction: '/users/6',
68+
transaction: '/users/:id',
6969
transaction_info: {
70-
source: 'url',
70+
source: 'route',
7171
},
7272
});
7373

@@ -89,7 +89,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
8989
},
9090
transaction: '/back-navigation',
9191
transaction_info: {
92-
source: 'url',
92+
source: 'route',
9393
},
9494
});
9595
});

‎dev-packages/e2e-tests/test-applications/solidstart/tests/performance.client.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ test('sends a pageload transaction', async ({ page }) => {
1818
},
1919
transaction: '/',
2020
transaction_info: {
21-
source: 'url',
21+
source: 'route',
2222
},
2323
});
2424
});
2525

26-
test('sends a navigation transaction', async ({ page }) => {
26+
test('sends a navigation transaction with parametrized route', async ({ page }) => {
2727
const transactionPromise = waitForTransaction('solidstart', async transactionEvent => {
28-
return transactionEvent?.transaction === '/users/5' && transactionEvent.contexts?.trace?.op === 'navigation';
28+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
2929
});
3030

3131
await page.goto(`/`);
@@ -39,9 +39,9 @@ test('sends a navigation transaction', async ({ page }) => {
3939
origin: 'auto.navigation.solidstart.solidrouter',
4040
},
4141
},
42-
transaction: '/users/5',
42+
transaction: '/users/:id',
4343
transaction_info: {
44-
source: 'url',
44+
source: 'route',
4545
},
4646
});
4747
});
@@ -51,7 +51,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
5151
// The sentry solidRouterBrowserTracingIntegration tries to update such
5252
// transactions with the proper name once the `useLocation` hook triggers.
5353
const navigationTxnPromise = waitForTransaction('solidstart', async transactionEvent => {
54-
return transactionEvent?.transaction === '/users/6' && transactionEvent.contexts?.trace?.op === 'navigation';
54+
return transactionEvent?.transaction === '/users/:id' && transactionEvent.contexts?.trace?.op === 'navigation';
5555
});
5656

5757
await page.goto(`/back-navigation`);
@@ -65,9 +65,9 @@ test('updates the transaction when using the back button', async ({ page }) => {
6565
origin: 'auto.navigation.solidstart.solidrouter',
6666
},
6767
},
68-
transaction: '/users/6',
68+
transaction: '/users/:id',
6969
transaction_info: {
70-
source: 'url',
70+
source: 'route',
7171
},
7272
});
7373

@@ -89,7 +89,7 @@ test('updates the transaction when using the back button', async ({ page }) => {
8989
},
9090
transaction: '/back-navigation',
9191
transaction_info: {
92-
source: 'url',
92+
source: 'route',
9393
},
9494
});
9595
});

‎packages/solid/src/solidrouter.ts‎

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type {
2020
RouteSectionProps,
2121
StaticRouter,
2222
} from '@solidjs/router';
23-
import { useBeforeLeave, useLocation } from '@solidjs/router';
23+
import { useBeforeLeave, useCurrentMatches, useLocation } from '@solidjs/router';
2424
import type { Component, JSX, ParentProps } from 'solid-js';
2525
import { createEffect, mergeProps, splitProps } from 'solid-js';
2626
import { createComponent } from 'solid-js/web';
@@ -66,31 +66,60 @@ function SentryDefaultRoot(props: ParentProps): JSX.Element {
6666
*/
6767
function withSentryRouterRoot(Root: Component<RouteSectionProps>): Component<RouteSectionProps> {
6868
const SentryRouterRoot = (props: RouteSectionProps): JSX.Element => {
69-
// TODO: This is a rudimentary first version of handling navigation spans
70-
// It does not
71-
// - use query params
72-
// - parameterize the route
69+
// Tracks the target of a pending navigation, so the effect can skip
70+
// stale updates during <Navigate> redirects where the location signal
71+
// hasn't caught up to the navigation span yet.
72+
let pendingNavigationTarget: string | undefined;
7373

7474
useBeforeLeave(({ to }: BeforeLeaveEventArgs) => {
75-
// `to` could be `-1` if the browser back-button was used
76-
handleNavigation(to.toString());
75+
const target = to.toString();
76+
pendingNavigationTarget = target;
77+
handleNavigation(target);
7778
});
7879

7980
const location = useLocation();
81+
const matches = useCurrentMatches();
82+
8083
createEffect(() => {
8184
const name = location.pathname;
8285
const rootSpan = getActiveRootSpan();
86+
if (!rootSpan) {
87+
return;
88+
}
8389

84-
if (rootSpan) {
90+
// During <Navigate> redirects, the effect can fire before the router
91+
// transition completes. In that case, location.pathname still points
92+
// to the old route while the active span is already the navigation span.
93+
// Skip the update to avoid overwriting the span with stale route data.
94+
// `-1` is solid router's representation of a browser back-button
95+
// navigation, where we don't know the target URL upfront.
96+
if (pendingNavigationTarget && pendingNavigationTarget !== '-1' && name !== pendingNavigationTarget) {
97+
return;
98+
}
99+
pendingNavigationTarget = undefined;
100+
101+
const currentMatches = matches();
102+
const lastMatch = currentMatches[currentMatches.length - 1];
103+
104+
if (lastMatch) {
105+
const parametrizedRoute = lastMatch.route.pattern || name;
106+
rootSpan.updateName(parametrizedRoute);
107+
rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
108+
109+
const params = lastMatch.params;
110+
for (const [key, value] of Object.entries(params)) {
111+
if (value !== undefined) {
112+
rootSpan.setAttribute(`url.path.parameter.${key}`, value);
113+
rootSpan.setAttribute(`params.${key}`, value);
114+
}
115+
}
116+
} else {
117+
// No matched route - update back-button navigations and set source to url
85118
const { op, description } = spanToJSON(rootSpan);
86-
87-
// We only need to update navigation spans that have been created by
88-
// a browser back-button navigation (stored as `-1` by solid router)
89-
// everything else was already instrumented correctly in `useBeforeLeave`
90119
if (op === 'navigation' && description === '-1') {
91120
rootSpan.updateName(name);
92-
rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url');
93121
}
122+
rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url');
94123
}
95124
});
96125

0 commit comments

Comments
 (0)