feat(ember): Set url.template, url.path and url.full on router spans#22095
Conversation
size-limit report 📦
|
ff5adcc to
be1c420
Compare
aa52df6 to
45fa82b
Compare
be1c420 to
ff5adcc
Compare
45fa82b to
659f478
Compare
ff5adcc to
be1c420
Compare
659f478 to
45fa82b
Compare
be1c420 to
ff5adcc
Compare
45fa82b to
659f478
Compare
ff5adcc to
a40f18b
Compare
659f478 to
610bc80
Compare
610bc80 to
cfee70c
Compare
6265a7c to
ffde4cb
Compare
6a4e140 to
afa64b0
Compare
ffde4cb to
c2ee1fa
Compare
a020c02 to
acde345
Compare
219838a to
7b29fa4
Compare
acde345 to
a698c2c
Compare
7c681a3 to
e96660f
Compare
a698c2c to
9a2444e
Compare
e96660f to
1675c6e
Compare
9a2444e to
ae419a9
Compare
1675c6e to
cc62a5f
Compare
584269b to
cb61d0b
Compare
cc62a5f to
24c2c27
Compare
cb61d0b to
a0eea1a
Compare
24c2c27 to
638f200
Compare
|
|
||
| const targetUrl = | ||
| (transition as TransitionWithIntent).intent?.url ?? _getLocationURL(location); | ||
| const urlAttributes = _getRouteUrlAttributes(targetUrl, transition.to?.params); |
There was a problem hiding this comment.
Bug: For nested dynamic routes, transition.to?.params only provides leaf route parameters, resulting in an incomplete URL template for the transaction name.
Severity: MEDIUM
Suggested Fix
To fix this, the code should traverse the route hierarchy to collect parameters from all parent routes, not just the leaf route. This can be done by iterating through the transition's routes and aggregating all params objects before calling buildUrlTemplate.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts#L185
Potential issue: When a navigation transition occurs to a nested dynamic route (e.g.,
`/users/:user_id/posts/:post_id`), the code uses `transition.to?.params` to gather route
parameters. This object only contains parameters for the final leaf route (e.g., `{
post_id: '456' }`), excluding parameters from parent routes (e.g., `{ user_id: '123'
}`). This leads to the `buildUrlTemplate` function generating an incomplete URL
template, such as `/users/123/posts/:post_id` instead of the correct
`/users/:user_id/posts/:post_id`. This will result in incorrect transaction names for
applications using this common routing pattern.
a0eea1a to
e018d44
Compare
638f200 to
ee82dda
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ee82dda. Configure here.
Emit standard URL attributes on Ember pageload and navigation root spans, updating navigation attributes at routeDidChange once the destination URL is finalized. Co-Authored-By: Cursor <cursoragent@cursor.com>
At `routeWillChange` the router's location still points at the current (pre-transition) route, so falling back to it when `transition.intent.url` is missing tagged the navigation root span with the previous route's `url.path`, `url.template`, and `url.full` until `routeDidChange` corrected them. Only use `intent.url` when available and otherwise defer the url attributes to `routeDidChange`, which reads them from `currentURL`. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
738909d to
8c71aa7
Compare
`_getRouteUrlAttributes` built `url.full` from the hash-stripped route path, so hash-location apps lost their `#/...` fragment (turning `https://host/#/tracing` into `https://host/tracing`). On `routeDidChange` this was compounded by using `currentURL`, which is the normalized route path and never carries the hash. Derive `url.full` from the unstripped URL and, on `routeDidChange`, source it from the location URL while keeping `currentURL` for path/template. Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Cursor <cursoragent@cursor.com>

Emit
url.template,url.path, andurl.fullon Ember pageload and navigation root spans so Ember aligns with the other framework router instrumentations in this stack.Navigation spans set URL attributes at
routeDidChangeviarouterService.currentURL, once the destination route is finalized. Dynamic path segments are parameterized intourl.template(e.g./users/123→/users/:user_id).E2E coverage was added for both
ember-classicandember-embroider, plus unit tests for the URL attribute helpers.Stacked on #22091.