Skip to content

Commit 256b9b9

Browse files
Lms24cursoragent
andauthored
feat(ember): Set url.template, url.path and url.full on router spans (#22095)
Emit `url.template`, `url.path`, and `url.full` on Ember pageload and navigation root spans so Ember aligns with the other framework router instrumentations in this stack. Navigation spans set URL attributes at `routeDidChange` via `routerService.currentURL`, once the destination route is finalized. Dynamic path segments are parameterized into `url.template` (e.g. `/users/123` → `/users/:user_id`). E2E coverage was added for both `ember-classic` and `ember-embroider`, plus unit tests for the URL attribute helpers. Stacked on #22091. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 73aabe4 commit 256b9b9

4 files changed

Lines changed: 182 additions & 3 deletions

File tree

dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) =
1515
trace: {
1616
op: 'pageload',
1717
origin: 'auto.pageload.ember',
18+
data: {
19+
'sentry.origin': 'auto.pageload.ember',
20+
'sentry.source': 'route',
21+
'url.template': '/',
22+
'url.path': '/',
23+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
24+
},
1825
},
1926
},
2027
transaction: 'route:index',
@@ -43,6 +50,13 @@ test('sends a navigation transaction with a parameterized URL', async ({ page })
4350
trace: {
4451
op: 'navigation',
4552
origin: 'auto.navigation.ember',
53+
data: {
54+
'sentry.origin': 'auto.navigation.ember',
55+
'sentry.source': 'route',
56+
'url.template': '/tracing',
57+
'url.path': '/tracing',
58+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/),
59+
},
4660
},
4761
},
4862
transaction: 'route:tracing',
@@ -75,6 +89,13 @@ test('sends a navigation transaction even if the pageload span is still active',
7589
trace: {
7690
op: 'pageload',
7791
origin: 'auto.pageload.ember',
92+
data: {
93+
'sentry.origin': 'auto.pageload.ember',
94+
'sentry.source': 'route',
95+
'url.template': '/',
96+
'url.path': '/',
97+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
98+
},
7899
},
79100
},
80101
transaction: 'route:index',
@@ -88,6 +109,13 @@ test('sends a navigation transaction even if the pageload span is still active',
88109
trace: {
89110
op: 'navigation',
90111
origin: 'auto.navigation.ember',
112+
data: {
113+
'sentry.origin': 'auto.navigation.ember',
114+
'sentry.source': 'route',
115+
'url.template': '/tracing',
116+
'url.path': '/tracing',
117+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/),
118+
},
91119
},
92120
},
93121
transaction: 'route:tracing',
@@ -124,6 +152,13 @@ test('captures correct spans for navigation', async ({ page }) => {
124152
trace: {
125153
op: 'navigation',
126154
origin: 'auto.navigation.ember',
155+
data: {
156+
'sentry.origin': 'auto.navigation.ember',
157+
'sentry.source': 'route',
158+
'url.template': '/slow-loading-route',
159+
'url.path': '/slow-loading-route',
160+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/),
161+
},
127162
},
128163
},
129164
transaction: 'route:slow-loading-route.index',

dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) =
1515
trace: {
1616
op: 'pageload',
1717
origin: 'auto.pageload.ember',
18+
data: {
19+
'sentry.origin': 'auto.pageload.ember',
20+
'sentry.source': 'route',
21+
'url.template': '/',
22+
'url.path': '/',
23+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
24+
},
1825
},
1926
},
2027
transaction: 'route:index',
@@ -43,6 +50,13 @@ test('sends a navigation transaction with a parameterized URL', async ({ page })
4350
trace: {
4451
op: 'navigation',
4552
origin: 'auto.navigation.ember',
53+
data: {
54+
'sentry.origin': 'auto.navigation.ember',
55+
'sentry.source': 'route',
56+
'url.template': '/tracing',
57+
'url.path': '/tracing',
58+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/),
59+
},
4660
},
4761
},
4862
transaction: 'route:tracing',
@@ -75,6 +89,13 @@ test('sends a navigation transaction even if the pageload span is still active',
7589
trace: {
7690
op: 'pageload',
7791
origin: 'auto.pageload.ember',
92+
data: {
93+
'sentry.origin': 'auto.pageload.ember',
94+
'sentry.source': 'route',
95+
'url.template': '/',
96+
'url.path': '/',
97+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
98+
},
7899
},
79100
},
80101
transaction: 'route:index',
@@ -88,6 +109,13 @@ test('sends a navigation transaction even if the pageload span is still active',
88109
trace: {
89110
op: 'navigation',
90111
origin: 'auto.navigation.ember',
112+
data: {
113+
'sentry.origin': 'auto.navigation.ember',
114+
'sentry.source': 'route',
115+
'url.template': '/tracing',
116+
'url.path': '/tracing',
117+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/),
118+
},
91119
},
92120
},
93121
transaction: 'route:tracing',
@@ -124,6 +152,13 @@ test('captures correct spans for navigation', async ({ page }) => {
124152
trace: {
125153
op: 'navigation',
126154
origin: 'auto.navigation.ember',
155+
data: {
156+
'sentry.origin': 'auto.navigation.ember',
157+
'sentry.source': 'route',
158+
'url.template': '/slow-loading-route',
159+
'url.path': '/slow-loading-route',
160+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/),
161+
},
127162
},
128163
},
129164
transaction: 'route:slow-loading-route.index',

packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,22 @@ import type {
55
startBrowserTracingNavigationSpan as startBrowserTracingNavigationSpanType,
66
startBrowserTracingPageLoadSpan as startBrowserTracingPageLoadSpanType,
77
} from '@sentry/browser';
8-
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startInactiveSpan } from '@sentry/browser';
8+
import {
9+
getAbsoluteUrl,
10+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
11+
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
12+
startInactiveSpan,
13+
} from '@sentry/browser';
914
import type { Client, Span } from '@sentry/core';
1015
import type { EmberRouterMain } from '../types';
1116
import { getBackburner } from './performance';
1217

18+
const URL_FULL = 'url.full';
19+
const URL_PATH = 'url.path';
20+
const URL_TEMPLATE = 'url.template';
21+
22+
type TransitionWithIntent = Transition & { intent?: { url?: string } };
23+
1324
export function instrumentEmberAppInstanceForPerformance(
1425
client: Client,
1526
appInstance: ApplicationInstance,
@@ -22,6 +33,7 @@ export function instrumentEmberAppInstanceForPerformance(
2233
let routerService = appInstance.lookup('service:router') as RouterService & {
2334
externalRouter?: RouterService;
2435
_hasMountedSentryPerformanceRouting?: boolean;
36+
currentURL?: string;
2537
};
2638

2739
if (routerService.externalRouter) {
@@ -60,6 +72,52 @@ function getTransitionInformation(
6072
};
6173
}
6274

75+
function getUrlPathFromEmberLocation(url: string): string {
76+
if (!url) {
77+
return '/';
78+
}
79+
80+
const withoutQuery = url.split('?')[0] ?? url;
81+
82+
if (withoutQuery.includes('#')) {
83+
const hashPart = withoutQuery.substring(withoutQuery.indexOf('#') + 1);
84+
return hashPart.startsWith('/') ? hashPart : `/${hashPart}`;
85+
}
86+
87+
return withoutQuery.startsWith('/') ? withoutQuery : `/${withoutQuery}`;
88+
}
89+
90+
function buildUrlTemplate(path: string, params: Record<string, unknown> = {}): string {
91+
let template = path;
92+
93+
const paramEntries = Object.entries(params)
94+
.filter((entry): entry is [string, string] => typeof entry[1] === 'string' && entry[1].length > 0)
95+
.sort(([, a], [, b]) => b.length - a.length);
96+
97+
for (const [key, value] of paramEntries) {
98+
template = template.replace(`/${value}`, `/:${key}`);
99+
}
100+
101+
return template;
102+
}
103+
104+
// Only exported for testing
105+
export function _getRouteUrlAttributes(
106+
url: string,
107+
params: Record<string, unknown> = {},
108+
fullUrl: string = url,
109+
): Record<string, string> {
110+
const path = getUrlPathFromEmberLocation(url);
111+
112+
// `url.full` is derived from the unstripped URL so that hash-location apps keep their `#/...`
113+
// fragment (e.g. `https://host/#/tracing`), which would otherwise be lost by `getUrlPathFromEmberLocation`.
114+
return {
115+
[URL_PATH]: path,
116+
[URL_FULL]: getAbsoluteUrl(fullUrl),
117+
[URL_TEMPLATE]: buildUrlTemplate(path, params),
118+
};
119+
}
120+
63121
// Only exported for testing
64122
export function _getLocationURL(location: EmberRouterMain['location']): string {
65123
if (!location?.getURL || !location?.formatURL) {
@@ -76,7 +134,7 @@ export function _getLocationURL(location: EmberRouterMain['location']): string {
76134

77135
function _instrumentEmberRouter(
78136
client: Client,
79-
routerService: RouterService,
137+
routerService: RouterService & { currentURL?: string },
80138
routerMain: EmberRouterMain,
81139
config: { disableRunloopPerformance?: boolean; instrumentPageLoad?: boolean; instrumentNavigation?: boolean },
82140
startBrowserTracingPageLoadSpan: typeof startBrowserTracingPageLoadSpanType,
@@ -96,6 +154,7 @@ function _instrumentEmberRouter(
96154
attributes: {
97155
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
98156
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.ember',
157+
..._getRouteUrlAttributes(url, routeInfo.params),
99158
url,
100159
toRoute: routeInfo.name,
101160
},
@@ -124,11 +183,19 @@ function _instrumentEmberRouter(
124183

125184
activeRootSpan?.end();
126185

186+
// Only `intent.url` reliably reflects the *destination* URL at `routeWillChange` time. The
187+
// router's location still points at the current (pre-transition) route here, so falling back to
188+
// it would tag the navigation span with the previous route's `url.*` attributes. When we don't
189+
// have a trustworthy target URL, we omit them and let `routeDidChange` set them from `currentURL`.
190+
const targetUrl = (transition as TransitionWithIntent).intent?.url;
191+
const urlAttributes = targetUrl ? _getRouteUrlAttributes(targetUrl, transition.to?.params) : {};
192+
127193
activeRootSpan = startBrowserTracingNavigationSpan(client, {
128194
name: `route:${toRoute}`,
129195
attributes: {
130196
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
131197
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.ember',
198+
...urlAttributes,
132199
fromRoute,
133200
toRoute,
134201
},
@@ -150,6 +217,15 @@ function _instrumentEmberRouter(
150217
}
151218
transitionSpan.end();
152219

220+
const url = routerService.currentURL ?? _getLocationURL(location);
221+
if (url) {
222+
const routeInfo = routerService.recognize(url);
223+
// `currentURL` is the normalized route path and never includes the hash fragment, so we source
224+
// `url.full` from the location URL (which preserves `#/...` for hash-location apps) when available.
225+
const fullUrl = _getLocationURL(location) || url;
226+
activeRootSpan.setAttributes(_getRouteUrlAttributes(url, routeInfo.params ?? transition.to?.params, fullUrl));
227+
}
228+
153229
if (disableRunloopPerformance) {
154230
activeRootSpan.end();
155231
return;

packages/ember/tests/unit/instrument-router-location-test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { EmberRouterMain } from '@sentry/ember/addon/types';
2-
import { _getLocationURL } from '@sentry/ember/utils/instrumentEmberAppInstanceForPerformance';
2+
import { _getLocationURL, _getRouteUrlAttributes } from '@sentry/ember/utils/instrumentEmberAppInstanceForPerformance';
33
import { setupTest } from 'ember-qunit';
44
import { module, test } from 'qunit';
55
import type { SentryTestContext } from '../helpers/setup-sentry';
@@ -97,4 +97,37 @@ module('Unit | Utility | instrument-router-location', function (hooks) {
9797
const result = _getLocationURL(mockLocation);
9898
assert.strictEqual(result, '', 'Should return empty string when formatURL is not available');
9999
});
100+
101+
test('_getRouteUrlAttributes handles history location paths', function (this: SentryTestContext, assert) {
102+
const result = _getRouteUrlAttributes('/tracing');
103+
104+
assert.strictEqual(result['url.path'], '/tracing');
105+
assert.strictEqual(result['url.template'], '/tracing');
106+
assert.ok(result['url.full']?.includes('/tracing'), 'url.full includes the path');
107+
});
108+
109+
test('_getRouteUrlAttributes parameterizes dynamic segments', function (this: SentryTestContext, assert) {
110+
const result = _getRouteUrlAttributes('/users/123', { user_id: '123' });
111+
112+
assert.strictEqual(result['url.path'], '/users/123');
113+
assert.strictEqual(result['url.template'], '/users/:user_id');
114+
assert.ok(result['url.full']?.includes('/users/123'), 'url.full includes the path');
115+
});
116+
117+
test('_getRouteUrlAttributes handles hash location paths', function (this: SentryTestContext, assert) {
118+
const result = _getRouteUrlAttributes('/#/tracing');
119+
120+
assert.strictEqual(result['url.path'], '/tracing');
121+
assert.strictEqual(result['url.template'], '/tracing');
122+
assert.ok(result['url.full']?.includes('/#/tracing'), 'url.full preserves the hash fragment');
123+
});
124+
125+
test('_getRouteUrlAttributes sources url.full from an explicit fullUrl', function (this: SentryTestContext, assert) {
126+
// Mirrors `routeDidChange`, where the route path (`currentURL`) has no hash but the location URL does.
127+
const result = _getRouteUrlAttributes('/tracing', {}, '/#/tracing');
128+
129+
assert.strictEqual(result['url.path'], '/tracing');
130+
assert.strictEqual(result['url.template'], '/tracing');
131+
assert.ok(result['url.full']?.includes('/#/tracing'), 'url.full preserves the hash fragment from fullUrl');
132+
});
100133
});

0 commit comments

Comments
 (0)