Skip to content

Commit 4c1079e

Browse files
mydeamydea
authored
fix(react-router): Ensure that all browser spans have source=route (#16984)
This updates the browser tracing implementation in react router to ensure that we always have parametrized routes. In the previous implementation, if the parametrized route was the same as the "initial" one, we would just do nothing and the source remains URL. We would also not set the origin of the spans. this is not correct, because we actually ensured this is parametrized (there are simply no parameters in, but still!), so the source should still be `route` in this case.
1 parent e67ba06 commit 4c1079e

File tree

10 files changed

+101
-88
lines changed

10 files changed

+101
-88
lines changed

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/navigation.client.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ test.describe('client - navigation performance', () => {
2222
data: {
2323
'sentry.origin': 'auto.navigation.react-router',
2424
'sentry.op': 'navigation',
25-
'sentry.source': 'url',
25+
'sentry.source': 'route',
2626
},
2727
op: 'navigation',
2828
origin: 'auto.navigation.react-router',
@@ -33,7 +33,7 @@ test.describe('client - navigation performance', () => {
3333
timestamp: expect.any(Number),
3434
transaction: '/performance/ssr',
3535
type: 'transaction',
36-
transaction_info: { source: 'url' },
36+
transaction_info: { source: 'route' },
3737
platform: 'javascript',
3838
request: {
3939
url: expect.stringContaining('/performance/ssr'),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/pageload.client.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { APP_NAME } from '../constants';
55
test.describe('client - pageload performance', () => {
66
test('should send pageload transaction', async ({ page }) => {
77
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
8-
return transactionEvent.transaction === '/performance/';
8+
return transactionEvent.transaction === '/performance';
99
});
1010

1111
await page.goto(`/performance`);
@@ -18,20 +18,20 @@ test.describe('client - pageload performance', () => {
1818
span_id: expect.any(String),
1919
trace_id: expect.any(String),
2020
data: {
21-
'sentry.origin': 'auto.pageload.browser',
21+
'sentry.origin': 'auto.pageload.react-router',
2222
'sentry.op': 'pageload',
23-
'sentry.source': 'url',
23+
'sentry.source': 'route',
2424
},
2525
op: 'pageload',
26-
origin: 'auto.pageload.browser',
26+
origin: 'auto.pageload.react-router',
2727
},
2828
},
2929
spans: expect.any(Array),
3030
start_timestamp: expect.any(Number),
3131
timestamp: expect.any(Number),
32-
transaction: '/performance/',
32+
transaction: '/performance',
3333
type: 'transaction',
34-
transaction_info: { source: 'url' },
34+
transaction_info: { source: 'route' },
3535
measurements: expect.any(Object),
3636
platform: 'javascript',
3737
request: {
@@ -68,12 +68,12 @@ test.describe('client - pageload performance', () => {
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
7070
data: {
71-
'sentry.origin': 'auto.pageload.browser',
71+
'sentry.origin': 'auto.pageload.react-router',
7272
'sentry.op': 'pageload',
7373
'sentry.source': 'route',
7474
},
7575
op: 'pageload',
76-
origin: 'auto.pageload.browser',
76+
origin: 'auto.pageload.react-router',
7777
},
7878
},
7979
spans: expect.any(Array),
@@ -105,26 +105,26 @@ test.describe('client - pageload performance', () => {
105105

106106
test('should send pageload transaction for prerendered pages', async ({ page }) => {
107107
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
108-
return transactionEvent.transaction === '/performance/static/';
108+
return transactionEvent.transaction === '/performance/static';
109109
});
110110

111111
await page.goto(`/performance/static`);
112112

113113
const transaction = await txPromise;
114114

115115
expect(transaction).toMatchObject({
116-
transaction: '/performance/static/',
116+
transaction: '/performance/static',
117117
contexts: {
118118
trace: {
119119
span_id: expect.any(String),
120120
trace_id: expect.any(String),
121121
data: {
122-
'sentry.origin': 'auto.pageload.browser',
122+
'sentry.origin': 'auto.pageload.react-router',
123123
'sentry.op': 'pageload',
124-
'sentry.source': 'url',
124+
'sentry.source': 'route',
125125
},
126126
op: 'pageload',
127-
origin: 'auto.pageload.browser',
127+
origin: 'auto.pageload.react-router',
128128
},
129129
},
130130
});

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/navigation.client.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ test.describe('client - navigation performance', () => {
2222
data: {
2323
'sentry.origin': 'auto.navigation.react-router',
2424
'sentry.op': 'navigation',
25-
'sentry.source': 'url',
25+
'sentry.source': 'route',
2626
},
2727
op: 'navigation',
2828
origin: 'auto.navigation.react-router',
@@ -33,7 +33,7 @@ test.describe('client - navigation performance', () => {
3333
timestamp: expect.any(Number),
3434
transaction: '/performance/ssr',
3535
type: 'transaction',
36-
transaction_info: { source: 'url' },
36+
transaction_info: { source: 'route' },
3737
platform: 'javascript',
3838
request: {
3939
url: expect.stringContaining('/performance/ssr'),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/pageload.client.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { APP_NAME } from '../constants';
55
test.describe('client - pageload performance', () => {
66
test('should send pageload transaction', async ({ page }) => {
77
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
8-
return transactionEvent.transaction === '/performance/';
8+
return transactionEvent.transaction === '/performance';
99
});
1010

1111
await page.goto(`/performance`);
@@ -18,20 +18,20 @@ test.describe('client - pageload performance', () => {
1818
span_id: expect.any(String),
1919
trace_id: expect.any(String),
2020
data: {
21-
'sentry.origin': 'auto.pageload.browser',
21+
'sentry.origin': 'auto.pageload.react-router',
2222
'sentry.op': 'pageload',
23-
'sentry.source': 'url',
23+
'sentry.source': 'route',
2424
},
2525
op: 'pageload',
26-
origin: 'auto.pageload.browser',
26+
origin: 'auto.pageload.react-router',
2727
},
2828
},
2929
spans: expect.any(Array),
3030
start_timestamp: expect.any(Number),
3131
timestamp: expect.any(Number),
32-
transaction: '/performance/',
32+
transaction: '/performance',
3333
type: 'transaction',
34-
transaction_info: { source: 'url' },
34+
transaction_info: { source: 'route' },
3535
measurements: expect.any(Object),
3636
platform: 'javascript',
3737
request: {
@@ -68,12 +68,12 @@ test.describe('client - pageload performance', () => {
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
7070
data: {
71-
'sentry.origin': 'auto.pageload.browser',
71+
'sentry.origin': 'auto.pageload.react-router',
7272
'sentry.op': 'pageload',
7373
'sentry.source': 'route',
7474
},
7575
op: 'pageload',
76-
origin: 'auto.pageload.browser',
76+
origin: 'auto.pageload.react-router',
7777
},
7878
},
7979
spans: expect.any(Array),
@@ -105,26 +105,26 @@ test.describe('client - pageload performance', () => {
105105

106106
test('should send pageload transaction for prerendered pages', async ({ page }) => {
107107
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
108-
return transactionEvent.transaction === '/performance/static/';
108+
return transactionEvent.transaction === '/performance/static';
109109
});
110110

111111
await page.goto(`/performance/static`);
112112

113113
const transaction = await txPromise;
114114

115115
expect(transaction).toMatchObject({
116-
transaction: '/performance/static/',
116+
transaction: '/performance/static',
117117
contexts: {
118118
trace: {
119119
span_id: expect.any(String),
120120
trace_id: expect.any(String),
121121
data: {
122-
'sentry.origin': 'auto.pageload.browser',
122+
'sentry.origin': 'auto.pageload.react-router',
123123
'sentry.op': 'pageload',
124-
'sentry.source': 'url',
124+
'sentry.source': 'route',
125125
},
126126
op: 'pageload',
127-
origin: 'auto.pageload.browser',
127+
origin: 'auto.pageload.react-router',
128128
},
129129
},
130130
});

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/pageload.client.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@ test.describe('client - pageload performance', () => {
1818
span_id: expect.any(String),
1919
trace_id: expect.any(String),
2020
data: {
21-
'sentry.origin': 'auto.pageload.browser',
21+
'sentry.origin': 'auto.pageload.react-router',
2222
'sentry.op': 'pageload',
23-
'sentry.source': 'url',
23+
'sentry.source': 'route',
2424
},
2525
op: 'pageload',
26-
origin: 'auto.pageload.browser',
26+
origin: 'auto.pageload.react-router',
2727
},
2828
},
2929
spans: expect.any(Array),
3030
start_timestamp: expect.any(Number),
3131
timestamp: expect.any(Number),
3232
transaction: '/performance',
3333
type: 'transaction',
34-
transaction_info: { source: 'url' },
34+
transaction_info: { source: 'route' },
3535
measurements: expect.any(Object),
3636
platform: 'javascript',
3737
request: {
@@ -68,12 +68,12 @@ test.describe('client - pageload performance', () => {
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
7070
data: {
71-
'sentry.origin': 'auto.pageload.browser',
71+
'sentry.origin': 'auto.pageload.react-router',
7272
'sentry.op': 'pageload',
7373
'sentry.source': 'route',
7474
},
7575
op: 'pageload',
76-
origin: 'auto.pageload.browser',
76+
origin: 'auto.pageload.react-router',
7777
},
7878
},
7979
spans: expect.any(Array),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/pageload.client.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@ test.describe('client - pageload performance', () => {
1818
span_id: expect.any(String),
1919
trace_id: expect.any(String),
2020
data: {
21-
'sentry.origin': 'auto.pageload.browser',
21+
'sentry.origin': 'auto.pageload.react-router',
2222
'sentry.op': 'pageload',
23-
'sentry.source': 'url',
23+
'sentry.source': 'route',
2424
},
2525
op: 'pageload',
26-
origin: 'auto.pageload.browser',
26+
origin: 'auto.pageload.react-router',
2727
},
2828
},
2929
spans: expect.any(Array),
3030
start_timestamp: expect.any(Number),
3131
timestamp: expect.any(Number),
3232
transaction: '/performance',
3333
type: 'transaction',
34-
transaction_info: { source: 'url' },
34+
transaction_info: { source: 'route' },
3535
measurements: expect.any(Object),
3636
platform: 'javascript',
3737
request: {
@@ -68,12 +68,12 @@ test.describe('client - pageload performance', () => {
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
7070
data: {
71-
'sentry.origin': 'auto.pageload.browser',
71+
'sentry.origin': 'auto.pageload.react-router',
7272
'sentry.op': 'pageload',
7373
'sentry.source': 'route',
7474
},
7575
op: 'pageload',
76-
origin: 'auto.pageload.browser',
76+
origin: 'auto.pageload.react-router',
7777
},
7878
},
7979
spans: expect.any(Array),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ test.describe('client - navigation performance', () => {
2222
data: {
2323
'sentry.origin': 'auto.navigation.react-router',
2424
'sentry.op': 'navigation',
25-
'sentry.source': 'url',
25+
'sentry.source': 'route',
2626
},
2727
op: 'navigation',
2828
origin: 'auto.navigation.react-router',
@@ -33,7 +33,7 @@ test.describe('client - navigation performance', () => {
3333
timestamp: expect.any(Number),
3434
transaction: '/performance/ssr',
3535
type: 'transaction',
36-
transaction_info: { source: 'url' },
36+
transaction_info: { source: 'route' },
3737
platform: 'javascript',
3838
request: {
3939
url: expect.stringContaining('/performance/ssr'),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { APP_NAME } from '../constants';
55
test.describe('client - pageload performance', () => {
66
test('should send pageload transaction', async ({ page }) => {
77
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
8-
return transactionEvent.transaction === '/performance/';
8+
return transactionEvent.transaction === '/performance';
99
});
1010

1111
await page.goto(`/performance`);
@@ -18,20 +18,20 @@ test.describe('client - pageload performance', () => {
1818
span_id: expect.any(String),
1919
trace_id: expect.any(String),
2020
data: {
21-
'sentry.origin': 'auto.pageload.browser',
21+
'sentry.origin': 'auto.pageload.react-router',
2222
'sentry.op': 'pageload',
23-
'sentry.source': 'url',
23+
'sentry.source': 'route',
2424
},
2525
op: 'pageload',
26-
origin: 'auto.pageload.browser',
26+
origin: 'auto.pageload.react-router',
2727
},
2828
},
2929
spans: expect.any(Array),
3030
start_timestamp: expect.any(Number),
3131
timestamp: expect.any(Number),
32-
transaction: '/performance/',
32+
transaction: '/performance',
3333
type: 'transaction',
34-
transaction_info: { source: 'url' },
34+
transaction_info: { source: 'route' },
3535
measurements: expect.any(Object),
3636
platform: 'javascript',
3737
request: {
@@ -68,12 +68,12 @@ test.describe('client - pageload performance', () => {
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
7070
data: {
71-
'sentry.origin': 'auto.pageload.browser',
71+
'sentry.origin': 'auto.pageload.react-router',
7272
'sentry.op': 'pageload',
7373
'sentry.source': 'route',
7474
},
7575
op: 'pageload',
76-
origin: 'auto.pageload.browser',
76+
origin: 'auto.pageload.react-router',
7777
},
7878
},
7979
spans: expect.any(Array),
@@ -105,26 +105,26 @@ test.describe('client - pageload performance', () => {
105105

106106
test('should send pageload transaction for prerendered pages', async ({ page }) => {
107107
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
108-
return transactionEvent.transaction === '/performance/static/';
108+
return transactionEvent.transaction === '/performance/static';
109109
});
110110

111111
await page.goto(`/performance/static`);
112112

113113
const transaction = await txPromise;
114114

115115
expect(transaction).toMatchObject({
116-
transaction: '/performance/static/',
116+
transaction: '/performance/static',
117117
contexts: {
118118
trace: {
119119
span_id: expect.any(String),
120120
trace_id: expect.any(String),
121121
data: {
122-
'sentry.origin': 'auto.pageload.browser',
122+
'sentry.origin': 'auto.pageload.react-router',
123123
'sentry.op': 'pageload',
124-
'sentry.source': 'url',
124+
'sentry.source': 'route',
125125
},
126126
op: 'pageload',
127-
origin: 'auto.pageload.browser',
127+
origin: 'auto.pageload.react-router',
128128
},
129129
},
130130
});

0 commit comments

Comments
 (0)