Skip to content

fix(webapp): report full route path on Remix request spans #4309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
carderne wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −6
Draft

fix(webapp): report full route path on Remix request spans #4309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions patches/@sentry__remix@9.46.0.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
diff --git a/build/cjs/vendor/instrumentation.js b/build/cjs/vendor/instrumentation.js
index 84e18d1051f57d5807e65c8b8ce858ceee7d4557..640a5253d565650338fe33e0ea52c8dffc63e4e7 100644
index 84e18d1051f57d5807e65c8b8ce858ceee7d4557..098a18992044ff4e52de986782bfb52761e50ab7 100644
--- a/build/cjs/vendor/instrumentation.js
+++ b/build/cjs/vendor/instrumentation.js
@@ -238,7 +238,7 @@ class RemixInstrumentation extends instrumentation.InstrumentationBase {
@@ -140,7 +140,14 @@ class RemixInstrumentation extends instrumentation.InstrumentationBase {

const route = (result || []).slice(-1)[0]?.route;

- const routePath = route?.path;
+ // Join every matched route segment so nested routes report their
+ // full URL pattern on http.route (and the span name), not just the
+ // leaf route's own path segment.
+ const routePath =
+ (result || [])
+ .map((match) => match?.route?.path)
+ .filter((segment) => typeof segment === 'string' && segment.length > 0)
+ .join('/') || route?.path;
Comment on lines +13 to +17

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize multiple slashes in joined route paths.

When a root route segment (e.g., '/') is joined with child segments (e.g., 'users'), the resulting path will contain double slashes (e.g., '//users'). Normalizing the joined string prevents incorrectly formatted span names.

  • patches/@sentry__remix@9.46.0.patch#L13-L17: Append .replace(/\/\/+/g, '/') to the joined string in the CJS build to safely clean up the path.
  • patches/@sentry__remix@9.46.0.patch#L68-L72: Append .replace(/\/\/+/g, '/') to the joined string in the ESM build to safely clean up the path.
💡 Proposed fixes

For the CJS build (patches/@sentry__remix@9.46.0.patch#L13-L17):

-        const routePath =
-          (result || [])
-            .map((match) => match?.route?.path)
-            .filter((segment) => typeof segment === 'string' && segment.length > 0)
-            .join('/') || route?.path;
+        const routePath =
+          (result || [])
+            .map((match) => match?.route?.path)
+            .filter((segment) => typeof segment === 'string' && segment.length > 0)
+            .join('/')
+            .replace(/\/\/+/g, '/') || route?.path;

For the ESM build (patches/@sentry__remix@9.46.0.patch#L68-L72):

-        const routePath =
-          (result || [])
-            .map((match) => match?.route?.path)
-            .filter((segment) => typeof segment === 'string' && segment.length > 0)
-            .join('/') || route?.path;
+        const routePath =
+          (result || [])
+            .map((match) => match?.route?.path)
+            .filter((segment) => typeof segment === 'string' && segment.length > 0)
+            .join('/')
+            .replace(/\/\/+/g, '/') || route?.path;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
+ const routePath =
+ (result || [])
+ .map((match) => match?.route?.path)
+ .filter((segment) => typeof segment === 'string' && segment.length > 0)
+ .join('/') || route?.path;
const routePath =
(result || [])
.map((match) => match?.route?.path)
.filter((segment) => typeof segment === 'string' && segment.length > 0)
.join('/')
.replace(/\/\/+/g, '/') || route?.path;
📍 Affects 1 file
  • patches/@sentry__remix@9.46.0.patch#L13-L17 (this comment)
  • patches/@sentry__remix@9.46.0.patch#L68-L72

if (span && routePath) {
span.setAttribute(semanticConventions.SemanticAttributes.HTTP_ROUTE, routePath);
span.updateName(`remix.request ${routePath}`);
@@ -238,7 +245,7 @@ class RemixInstrumentation extends instrumentation.InstrumentationBase {
return function callRouteAction(original) {
return async function patchCallRouteAction( ...args) {
const [params] = args;
Expand All @@ -11,7 +27,7 @@ index 84e18d1051f57d5807e65c8b8ce858ceee7d4557..640a5253d565650338fe33e0ea52c8df
const span = plugin.tracer.startSpan(
`ACTION ${params.routeId}`,
{ attributes: { [semanticConventions.SemanticAttributes.CODE_FUNCTION]: 'action' } },
@@ -257,25 +257,6 @@ class RemixInstrumentation extends instrumentation.InstrumentationBase {
@@ -257,25 +264,6 @@ class RemixInstrumentation extends instrumentation.InstrumentationBase {
.then(async response => {
addResponseAttributesToSpan(span, response);

Expand All @@ -37,3 +53,23 @@ index 84e18d1051f57d5807e65c8b8ce858ceee7d4557..640a5253d565650338fe33e0ea52c8df
return response;
})
.catch(async error => {
diff --git a/build/esm/vendor/instrumentation.js b/build/esm/vendor/instrumentation.js
index e33b84eecdffe3b10a5e50f13bafaf90775747c5..652a89c9194ea1d0bb0d748fb8f7f060ca2cf5ba 100644
--- a/build/esm/vendor/instrumentation.js
+++ b/build/esm/vendor/instrumentation.js
@@ -138,7 +138,14 @@ class RemixInstrumentation extends InstrumentationBase {

const route = (result || []).slice(-1)[0]?.route;

- const routePath = route?.path;
+ // Join every matched route segment so nested routes report their
+ // full URL pattern on http.route (and the span name), not just the
+ // leaf route's own path segment.
+ const routePath =
+ (result || [])
+ .map((match) => match?.route?.path)
+ .filter((segment) => typeof segment === 'string' && segment.length > 0)
+ .join('/') || route?.path;
if (span && routePath) {
span.setAttribute(SemanticAttributes.HTTP_ROUTE, routePath);
span.updateName(`remix.request ${routePath}`);
Comment on lines +56 to +75

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Missing action processing changes for the ESM build.

The patch applies the nested-route joining logic to the ESM build but entirely omits the callRouteAction modifications that were successfully applied to the CJS build (lines 24-54). Consequently, ESM consumers will still experience request cloning and form-data parsing, which breaks the intended bundle consistency and could result in request-body-consumption bugs.

Please add the missing patch hunks targeting build/esm/vendor/instrumentation.js to remove params.request.clone() and the formData() span enrichment, matching the CJS changes.

6 changes: 3 additions & 3 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading