From a7c937238f4ff60a5738ae6116328719667d5740 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 25 Apr 2025 12:25:50 +0100 Subject: [PATCH 1/4] fix(remix): Avoid rewrapping root loader. --- packages/remix/src/server/instrumentServer.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/remix/src/server/instrumentServer.ts b/packages/remix/src/server/instrumentServer.ts index f4634805e1e1..0103f8e207ad 100644 --- a/packages/remix/src/server/instrumentServer.ts +++ b/packages/remix/src/server/instrumentServer.ts @@ -364,6 +364,18 @@ function instrumentBuildCallback( for (const [id, route] of Object.entries(build.routes)) { const wrappedRoute = { ...route, module: { ...route.module } }; + // Entry module should have a loader function to provide `sentry-trace` and `baggage` + // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage` + if (!wrappedRoute.parentId) { + if (!wrappedRoute.module.loader) { + wrappedRoute.module.loader = () => ({}); + } + + if (!(wrappedRoute.module.loader as WrappedFunction).__sentry_original__) { + wrappedRoute.module.loader = makeWrappedRootLoader()(wrappedRoute.module.loader); + } + } + const routeAction = wrappedRoute.module.action as undefined | WrappedFunction; if (routeAction && !routeAction.__sentry_original__) { fill(wrappedRoute.module, 'action', makeWrappedAction(id, options?.instrumentTracing)); @@ -374,17 +386,6 @@ function instrumentBuildCallback( fill(wrappedRoute.module, 'loader', makeWrappedLoader(id, options?.instrumentTracing)); } - // Entry module should have a loader function to provide `sentry-trace` and `baggage` - // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage` - if (!wrappedRoute.parentId) { - if (!wrappedRoute.module.loader) { - wrappedRoute.module.loader = () => ({}); - } - - // We want to wrap the root loader regardless of whether it's already wrapped before. - fill(wrappedRoute.module, 'loader', makeWrappedRootLoader()); - } - routes[id] = wrappedRoute; } From c0a03121831adf6cf9fc972d6b562013f1db8e3f Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 25 Apr 2025 12:32:12 +0100 Subject: [PATCH 2/4] Bump `remix` to `2.16.5` in e2e test app. --- .../create-remix-app-v2/package.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/package.json b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/package.json index e8a03932560a..63f8b0faf722 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/package.json +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/package.json @@ -12,10 +12,10 @@ }, "dependencies": { "@sentry/remix": "latest || *", - "@remix-run/css-bundle": "2.7.2", - "@remix-run/node": "2.7.2", - "@remix-run/react": "2.7.2", - "@remix-run/serve": "2.7.2", + "@remix-run/css-bundle": "2.16.5", + "@remix-run/node": "2.16.5", + "@remix-run/react": "2.16.5", + "@remix-run/serve": "2.16.5", "isbot": "^3.6.8", "react": "^18.2.0", "react-dom": "^18.2.0" @@ -23,8 +23,8 @@ "devDependencies": { "@playwright/test": "~1.50.0", "@sentry-internal/test-utils": "link:../../../test-utils", - "@remix-run/dev": "2.7.2", - "@remix-run/eslint-config": "2.7.2", + "@remix-run/dev": "2.16.5", + "@remix-run/eslint-config": "2.16.5", "@sentry/core": "latest || *", "@types/react": "^18.0.35", "@types/react-dom": "^18.0.11", From f9fced831915f2674d572ac29191eb6b062b6b65 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 25 Apr 2025 12:33:38 +0100 Subject: [PATCH 3/4] Match previous wrapper usage --- packages/remix/src/server/instrumentServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix/src/server/instrumentServer.ts b/packages/remix/src/server/instrumentServer.ts index 0103f8e207ad..63ae39a037b2 100644 --- a/packages/remix/src/server/instrumentServer.ts +++ b/packages/remix/src/server/instrumentServer.ts @@ -372,7 +372,7 @@ function instrumentBuildCallback( } if (!(wrappedRoute.module.loader as WrappedFunction).__sentry_original__) { - wrappedRoute.module.loader = makeWrappedRootLoader()(wrappedRoute.module.loader); + fill(wrappedRoute.module, 'loader', makeWrappedRootLoader()); } } From 9df73ea33d1a21ddba09417733ca459ab69eb58b Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 25 Apr 2025 13:52:17 +0100 Subject: [PATCH 4/4] Align tests --- .../remix-hydrogen/tests/server-transactions.test.ts | 2 -- .../integration/test/server/instrumentation/action.test.ts | 4 ++-- .../integration/test/server/instrumentation/loader.test.ts | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/server-transactions.test.ts index 2a13d40e2c54..5153b57a4d90 100644 --- a/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/server-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/server-transactions.test.ts @@ -40,7 +40,6 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id; const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id; - const loaderSpanId = httpServerTransaction?.spans?.find(span => span.op === 'function.remix.loader')?.span_id; const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id; const pageLoadSpanId = pageloadTransaction.contexts?.trace?.span_id; @@ -50,7 +49,6 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page expect(httpServerTraceId).toBeDefined(); expect(httpServerSpanId).toBeDefined(); - expect(loaderSpanId).toBeDefined(); expect(pageLoadTraceId).toEqual(httpServerTraceId); expect(pageLoadSpanId).not.toEqual(httpServerSpanId); diff --git a/packages/remix/test/integration/test/server/instrumentation/action.test.ts b/packages/remix/test/integration/test/server/instrumentation/action.test.ts index e0a07ac82eae..3681e43807b8 100644 --- a/packages/remix/test/integration/test/server/instrumentation/action.test.ts +++ b/packages/remix/test/integration/test/server/instrumentation/action.test.ts @@ -28,7 +28,7 @@ describe('Remix API Actions', () => { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', - 'match.route.id': 'routes/action-json-response.$id', + 'match.route.id': 'root', 'match.params.id': '123123', }, }, @@ -36,7 +36,7 @@ describe('Remix API Actions', () => { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', - 'match.route.id': 'root', + 'match.route.id': 'routes/action-json-response.$id', 'match.params.id': '123123', }, }, diff --git a/packages/remix/test/integration/test/server/instrumentation/loader.test.ts b/packages/remix/test/integration/test/server/instrumentation/loader.test.ts index baec26b3eb67..a1e257681fa0 100644 --- a/packages/remix/test/integration/test/server/instrumentation/loader.test.ts +++ b/packages/remix/test/integration/test/server/instrumentation/loader.test.ts @@ -242,14 +242,14 @@ describe('Remix API Loaders', () => { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', - 'match.route.id': 'routes/loader-defer-response.$id', + 'match.route.id': 'root', }, }, { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', - 'match.route.id': 'root', + 'match.route.id': 'routes/loader-defer-response.$id', }, }, ],