From c64c4fdab68a2a06058545dc2396733447911218 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Mon, 12 May 2025 17:07:26 +0200 Subject: [PATCH 1/8] ci: Improve size limit tree shaking handling (#16267) By us providing an empty terser plugin we actually get weird results (the browser variant with tree shaking was actually larger then the default one :O). We can just use the defaults here. I also updated the GH action to use `bytes-iec` instead of `bytes` package for rendering the sizes - this is also what size-limit itself uses, so this should be better aligned now. Values may _look_ larger now because it formats as `kB` now (so factors of 1000) instead of `KiB` (factors of 1024). --- .size-limit.js | 6 ------ dev-packages/size-limit-gh-action/package.json | 2 +- .../size-limit-gh-action/utils/SizeLimitFormatter.mjs | 2 +- yarn.lock | 2 +- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index bbe29ceded7c..4452272b00f3 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -18,7 +18,6 @@ module.exports = [ limit: '24.1 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); - const TerserPlugin = require('terser-webpack-plugin'); config.plugins.push( new webpack.DefinePlugin({ @@ -30,7 +29,6 @@ module.exports = [ ); config.optimization.minimize = true; - config.optimization.minimizer = [new TerserPlugin()]; return config; }, @@ -57,7 +55,6 @@ module.exports = [ limit: '70.1 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); - const TerserPlugin = require('terser-webpack-plugin'); config.plugins.push( new webpack.DefinePlugin({ @@ -69,7 +66,6 @@ module.exports = [ ); config.optimization.minimize = true; - config.optimization.minimizer = [new TerserPlugin()]; return config; }, @@ -239,7 +235,6 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], modifyWebpackConfig: function (config) { const webpack = require('webpack'); - const TerserPlugin = require('terser-webpack-plugin'); config.plugins.push( new webpack.DefinePlugin({ @@ -248,7 +243,6 @@ module.exports = [ ); config.optimization.minimize = true; - config.optimization.minimizer = [new TerserPlugin()]; return config; }, diff --git a/dev-packages/size-limit-gh-action/package.json b/dev-packages/size-limit-gh-action/package.json index 9b2b8ae7f99b..ef8075abaca0 100644 --- a/dev-packages/size-limit-gh-action/package.json +++ b/dev-packages/size-limit-gh-action/package.json @@ -20,7 +20,7 @@ "@actions/github": "^5.0.0", "@actions/glob": "0.4.0", "@actions/io": "1.1.3", - "bytes": "3.1.2", + "bytes-iec": "3.1.1", "markdown-table": "3.0.3" }, "volta": { diff --git a/dev-packages/size-limit-gh-action/utils/SizeLimitFormatter.mjs b/dev-packages/size-limit-gh-action/utils/SizeLimitFormatter.mjs index d7b3a444ff7c..ff1f40c6a716 100644 --- a/dev-packages/size-limit-gh-action/utils/SizeLimitFormatter.mjs +++ b/dev-packages/size-limit-gh-action/utils/SizeLimitFormatter.mjs @@ -1,5 +1,5 @@ import * as core from '@actions/core'; -import bytes from 'bytes'; +import bytes from 'bytes-iec'; const SIZE_RESULTS_HEADER = ['Path', 'Size', '% Change', 'Change']; diff --git a/yarn.lock b/yarn.lock index 1acd6df25742..b8ed2f69b728 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11367,7 +11367,7 @@ byte-size@8.1.1: resolved "https://registry.yarnpkg.com/byte-size/-/byte-size-8.1.1.tgz#3424608c62d59de5bfda05d31e0313c6174842ae" integrity sha512-tUkzZWK0M/qdoLEqikxBWe4kumyuwjl3HO6zHTr4yEI23EojPtLYXdG1+AQY7MN0cGyNDvEaJ8wiYQm6P2bPxg== -bytes-iec@^3.1.1: +bytes-iec@3.1.1, bytes-iec@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/bytes-iec/-/bytes-iec-3.1.1.tgz#94cd36bf95c2c22a82002c247df8772d1d591083" integrity sha512-fey6+4jDK7TFtFg/klGSvNKJctyU7n2aQdnM+CO0ruLPbqqMOM8Tio0Pc+deqUeVKX1tL5DQep1zQ7+37aTAsA== From f55018fbf6bfd83c770517e2b14c727e0a0803cb Mon Sep 17 00:00:00 2001 From: Sid Date: Tue, 13 May 2025 13:57:34 -0700 Subject: [PATCH 2/8] fix(react): Handle nested parameterized routes in reactrouterv3 transaction normalization (#16274) We noticed that routes like `/teams/:teamId/details` were normalized to `/teams:teamIddetails` which is missing `/` and is incorrect. This PR updates the tests for the React Router v3 integration to ensure correct transaction name normalization for nested parameterized routes. Specifically, it modifies the `normalizes transaction name` test case to include navigation to a route like `/teams/:teamId/details` and verifies that the resulting transaction name is correctly normalized. This addresses an issue where similar nested route patterns might not be handled correctly. --- packages/react/src/reactrouterv3.ts | 9 ++++----- packages/react/test/reactrouterv3.test.tsx | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/react/src/reactrouterv3.ts b/packages/react/src/reactrouterv3.ts index 73058307353f..f49948a2a74b 100644 --- a/packages/react/src/reactrouterv3.ts +++ b/packages/react/src/reactrouterv3.ts @@ -150,9 +150,8 @@ function getRouteStringFromRoutes(routes: Route[]): string { } } - return routesWithPaths - .slice(index) - .filter(({ path }) => !!path) - .map(({ path }) => path) - .join(''); + return routesWithPaths.slice(index).reduce((acc, { path }) => { + const pathSegment = acc === '/' || acc === '' ? path : `/${path}`; + return `${acc}${pathSegment}`; + }, ''); } diff --git a/packages/react/test/reactrouterv3.test.tsx b/packages/react/test/reactrouterv3.test.tsx index 841cbf018647..18fc34527c3f 100644 --- a/packages/react/test/reactrouterv3.test.tsx +++ b/packages/react/test/reactrouterv3.test.tsx @@ -64,6 +64,11 @@ describe('browserTracingReactRouterV3', () => {
OrgId
} />
Team
} /> + + +
Team Details
} /> + + ); const history = createMemoryHistory(); @@ -192,6 +197,22 @@ describe('browserTracingReactRouterV3', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', }, }); + expect(getCurrentScope().getScopeData().transactionName).toEqual('/users/:userid'); + + act(() => { + history.push('/teams/456/details'); + }); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/teams/:teamId/details', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(getCurrentScope().getScopeData().transactionName).toEqual('/teams/:teamId/details'); }); it("updates the scope's `transactionName` on a navigation", () => { From b9984338be761ea1ef785fa0c23c170e5e01faff Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 13 May 2025 21:58:41 +0100 Subject: [PATCH 3/8] feat(remix): Vendor in `opentelemetry-instrumentation-remix` (#16145) Resolves: #15739 - Removed Remix v1-related instrumentation code from the original implementation while vendoring in. - Also, FormData entries for manual instrumentation are aligned with OTEL use. Resolves: https://github.com/getsentry/sentry-javascript/issues/16140 --- packages/remix/package.json | 3 +- packages/remix/src/server/errors.ts | 2 +- .../src/server/integrations/opentelemetry.ts | 2 +- packages/remix/src/utils/utils.ts | 19 +- packages/remix/src/vendor/instrumentation.ts | 375 ++++++++++++++++++ yarn.lock | 39 +- 6 files changed, 402 insertions(+), 38 deletions(-) create mode 100644 packages/remix/src/vendor/instrumentation.ts diff --git a/packages/remix/package.json b/packages/remix/package.json index 122b1dfdf329..f2c8d2c4f2a2 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -65,6 +65,8 @@ }, "dependencies": { "@opentelemetry/api": "^1.9.0", + "@opentelemetry/instrumentation": "^0.57.2", + "@opentelemetry/semantic-conventions": "^1.30.0", "@remix-run/router": "1.x", "@sentry/cli": "^2.43.0", "@sentry/core": "9.18.0", @@ -72,7 +74,6 @@ "@sentry/opentelemetry": "9.18.0", "@sentry/react": "9.18.0", "glob": "^10.3.4", - "opentelemetry-instrumentation-remix": "0.8.0", "yargs": "^17.6.0" }, "devDependencies": { diff --git a/packages/remix/src/server/errors.ts b/packages/remix/src/server/errors.ts index 90359212300d..0e26242a0164 100644 --- a/packages/remix/src/server/errors.ts +++ b/packages/remix/src/server/errors.ts @@ -134,7 +134,7 @@ export async function errorHandleDataFunction( const options = getClient()?.getOptions() as RemixOptions | undefined; if (options?.sendDefaultPii && options.captureActionFormDataKeys) { - await storeFormDataKeys(args, span); + await storeFormDataKeys(args, span, options.captureActionFormDataKeys); } } diff --git a/packages/remix/src/server/integrations/opentelemetry.ts b/packages/remix/src/server/integrations/opentelemetry.ts index 42654201da18..b4ad1d28bb6a 100644 --- a/packages/remix/src/server/integrations/opentelemetry.ts +++ b/packages/remix/src/server/integrations/opentelemetry.ts @@ -1,8 +1,8 @@ import type { Client, IntegrationFn, Span } from '@sentry/core'; import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { generateInstrumentOnce, getClient, spanToJSON } from '@sentry/node'; -import { RemixInstrumentation } from 'opentelemetry-instrumentation-remix'; import type { RemixOptions } from '../../utils/remixOptions'; +import { RemixInstrumentation } from '../../vendor/instrumentation'; const INTEGRATION_NAME = 'Remix'; diff --git a/packages/remix/src/utils/utils.ts b/packages/remix/src/utils/utils.ts index a1d878ac1314..62fee4b20d61 100644 --- a/packages/remix/src/utils/utils.ts +++ b/packages/remix/src/utils/utils.ts @@ -10,7 +10,11 @@ type ServerRouteManifest = ServerBuild['routes']; /** * */ -export async function storeFormDataKeys(args: LoaderFunctionArgs | ActionFunctionArgs, span: Span): Promise { +export async function storeFormDataKeys( + args: LoaderFunctionArgs | ActionFunctionArgs, + span: Span, + formDataKeys?: Record | undefined, +): Promise { try { // We clone the request for Remix be able to read the FormData later. const clonedRequest = args.request.clone(); @@ -21,7 +25,18 @@ export async function storeFormDataKeys(args: LoaderFunctionArgs | ActionFunctio const formData = await clonedRequest.formData(); formData.forEach((value, key) => { - span.setAttribute(`remix.action_form_data.${key}`, typeof value === 'string' ? value : '[non-string value]'); + let attrKey = key; + + if (formDataKeys?.[key]) { + if (typeof formDataKeys[key] === 'string') { + attrKey = formDataKeys[key] as string; + } + + span.setAttribute( + `remix.action_form_data.${attrKey}`, + typeof value === 'string' ? value : '[non-string value]', + ); + } }); } catch (e) { DEBUG_BUILD && logger.warn('Failed to read FormData from request', e); diff --git a/packages/remix/src/vendor/instrumentation.ts b/packages/remix/src/vendor/instrumentation.ts new file mode 100644 index 000000000000..317a17da663d --- /dev/null +++ b/packages/remix/src/vendor/instrumentation.ts @@ -0,0 +1,375 @@ +/* eslint-disable deprecation/deprecation */ +/* eslint-disable max-lines */ +/* eslint-disable jsdoc/require-jsdoc */ + +// Vendored and modified from: +// https://github.com/justindsmith/opentelemetry-instrumentations-js/blob/3b1e8c3e566e5cc3389e9c28cafce6a5ebb39600/packages/instrumentation-remix/src/instrumentation.ts + +/* + * Copyright Justin Smith + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Span } from '@opentelemetry/api'; +import opentelemetry, { SpanStatusCode } from '@opentelemetry/api'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { + InstrumentationBase, + InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, + isWrapped, +} from '@opentelemetry/instrumentation'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import type { Params } from '@remix-run/router'; +import type * as remixRunServerRuntime from '@remix-run/server-runtime'; +import type * as remixRunServerRuntimeData from '@remix-run/server-runtime/dist/data'; +import type * as remixRunServerRuntimeRouteMatching from '@remix-run/server-runtime/dist/routeMatching'; +import type { RouteMatch } from '@remix-run/server-runtime/dist/routeMatching'; +import type { ServerRoute } from '@remix-run/server-runtime/dist/routes'; +import { SDK_VERSION } from '@sentry/core'; + +const RemixSemanticAttributes = { + MATCH_PARAMS: 'match.params', + MATCH_ROUTE_ID: 'match.route.id', +}; + +const VERSION = SDK_VERSION; + +export interface RemixInstrumentationConfig extends InstrumentationConfig { + /** + * Mapping of FormData field to span attribute names. Appends attribute as `formData.${name}`. + * + * Provide `true` value to use the FormData field name as the attribute name, or provide + * a `string` value to map the field name to a custom attribute name. + * + * @default { _action: "actionType" } + */ + actionFormDataAttributes?: Record; +} + +const DEFAULT_CONFIG: RemixInstrumentationConfig = { + actionFormDataAttributes: { + _action: 'actionType', + }, +}; + +export class RemixInstrumentation extends InstrumentationBase { + public constructor(config: RemixInstrumentationConfig = {}) { + super('RemixInstrumentation', VERSION, Object.assign({}, DEFAULT_CONFIG, config)); + } + + public getConfig(): RemixInstrumentationConfig { + return this._config; + } + + public setConfig(config: RemixInstrumentationConfig = {}): void { + this._config = Object.assign({}, DEFAULT_CONFIG, config); + } + + // eslint-disable-next-line @typescript-eslint/naming-convention + protected init(): InstrumentationNodeModuleDefinition { + const remixRunServerRuntimeRouteMatchingFile = new InstrumentationNodeModuleFile( + '@remix-run/server-runtime/dist/routeMatching.js', + ['2.x'], + (moduleExports: typeof remixRunServerRuntimeRouteMatching) => { + // createRequestHandler + if (isWrapped(moduleExports['matchServerRoutes'])) { + this._unwrap(moduleExports, 'matchServerRoutes'); + } + this._wrap(moduleExports, 'matchServerRoutes', this._patchMatchServerRoutes()); + + return moduleExports; + }, + (moduleExports: typeof remixRunServerRuntimeRouteMatching) => { + this._unwrap(moduleExports, 'matchServerRoutes'); + }, + ); + + const remixRunServerRuntimeData_File = new InstrumentationNodeModuleFile( + '@remix-run/server-runtime/dist/data.js', + ['2.9.0 - 2.x'], + (moduleExports: typeof remixRunServerRuntimeData) => { + // callRouteLoader + if (isWrapped(moduleExports['callRouteLoader'])) { + this._unwrap(moduleExports, 'callRouteLoader'); + } + this._wrap(moduleExports, 'callRouteLoader', this._patchCallRouteLoader()); + + // callRouteAction + if (isWrapped(moduleExports['callRouteAction'])) { + this._unwrap(moduleExports, 'callRouteAction'); + } + this._wrap(moduleExports, 'callRouteAction', this._patchCallRouteAction()); + return moduleExports; + }, + (moduleExports: typeof remixRunServerRuntimeData) => { + this._unwrap(moduleExports, 'callRouteLoader'); + this._unwrap(moduleExports, 'callRouteAction'); + }, + ); + + /* + * In Remix 2.9.0, the `callXXLoaderRR` functions were renamed to `callXXLoader`. + */ + const remixRunServerRuntimeDataPre_2_9_File = new InstrumentationNodeModuleFile( + '@remix-run/server-runtime/dist/data.js', + ['2.0.0 - 2.8.x'], + ( + moduleExports: typeof remixRunServerRuntimeData & { + callRouteLoaderRR: typeof remixRunServerRuntimeData.callRouteLoader; + callRouteActionRR: typeof remixRunServerRuntimeData.callRouteAction; + }, + ) => { + // callRouteLoader + if (isWrapped(moduleExports['callRouteLoaderRR'])) { + this._unwrap(moduleExports, 'callRouteLoaderRR'); + } + this._wrap(moduleExports, 'callRouteLoaderRR', this._patchCallRouteLoader()); + + // callRouteAction + if (isWrapped(moduleExports['callRouteActionRR'])) { + this._unwrap(moduleExports, 'callRouteActionRR'); + } + this._wrap(moduleExports, 'callRouteActionRR', this._patchCallRouteAction()); + return moduleExports; + }, + ( + moduleExports: typeof remixRunServerRuntimeData & { + callRouteLoaderRR: typeof remixRunServerRuntimeData.callRouteLoader; + callRouteActionRR: typeof remixRunServerRuntimeData.callRouteAction; + }, + ) => { + this._unwrap(moduleExports, 'callRouteLoaderRR'); + this._unwrap(moduleExports, 'callRouteActionRR'); + }, + ); + + const remixRunServerRuntimeModule = new InstrumentationNodeModuleDefinition( + '@remix-run/server-runtime', + ['2.x'], + (moduleExports: typeof remixRunServerRuntime) => { + // createRequestHandler + if (isWrapped(moduleExports['createRequestHandler'])) { + this._unwrap(moduleExports, 'createRequestHandler'); + } + this._wrap(moduleExports, 'createRequestHandler', this._patchCreateRequestHandler()); + + return moduleExports; + }, + (moduleExports: typeof remixRunServerRuntime) => { + this._unwrap(moduleExports, 'createRequestHandler'); + }, + [remixRunServerRuntimeRouteMatchingFile, remixRunServerRuntimeData_File, remixRunServerRuntimeDataPre_2_9_File], + ); + + return remixRunServerRuntimeModule; + } + + private _patchMatchServerRoutes(): (original: typeof remixRunServerRuntimeRouteMatching.matchServerRoutes) => any { + return function matchServerRoutes(original) { + return function patchMatchServerRoutes( + this: any, + ...args: Parameters + ): RouteMatch[] | null { + const result = original.apply(this, args) as RouteMatch[] | null; + + const span = opentelemetry.trace.getSpan(opentelemetry.context.active()); + + const route = (result || []).slice(-1)[0]?.route; + + const routePath = route?.path; + if (span && routePath) { + span.setAttribute(SemanticAttributes.HTTP_ROUTE, routePath); + span.updateName(`remix.request ${routePath}`); + } + + const routeId = route?.id; + if (span && routeId) { + span.setAttribute(RemixSemanticAttributes.MATCH_ROUTE_ID, routeId); + } + + return result; + }; + }; + } + + private _patchCreateRequestHandler(): (original: typeof remixRunServerRuntime.createRequestHandler) => any { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const plugin = this; + return function createRequestHandler(original) { + return function patchCreateRequestHandler( + this: any, + ...args: Parameters + ): remixRunServerRuntime.RequestHandler { + const originalRequestHandler: remixRunServerRuntime.RequestHandler = original.apply(this, args); + + return (request: Request, loadContext?: remixRunServerRuntime.AppLoadContext) => { + const span = plugin.tracer.startSpan( + 'remix.request', + { + attributes: { [SemanticAttributes.CODE_FUNCTION]: 'requestHandler' }, + }, + opentelemetry.context.active(), + ); + addRequestAttributesToSpan(span, request); + + const originalResponsePromise = opentelemetry.context.with( + opentelemetry.trace.setSpan(opentelemetry.context.active(), span), + () => originalRequestHandler(request, loadContext), + ); + return originalResponsePromise + .then(response => { + addResponseAttributesToSpan(span, response); + return response; + }) + .catch(error => { + plugin._addErrorToSpan(span, error); + throw error; + }) + .finally(() => { + span.end(); + }); + }; + }; + }; + } + + private _patchCallRouteLoader(): (original: typeof remixRunServerRuntimeData.callRouteLoader) => any { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const plugin = this; + return function callRouteLoader(original) { + return function patchCallRouteLoader(this: any, ...args: Parameters): Promise { + const [params] = args; + + const span = plugin.tracer.startSpan( + `LOADER ${params.routeId}`, + { attributes: { [SemanticAttributes.CODE_FUNCTION]: 'loader' } }, + opentelemetry.context.active(), + ); + + addRequestAttributesToSpan(span, params.request); + addMatchAttributesToSpan(span, { routeId: params.routeId, params: params.params }); + + return opentelemetry.context.with(opentelemetry.trace.setSpan(opentelemetry.context.active(), span), () => { + const originalResponsePromise: Promise = original.apply(this, args); + return originalResponsePromise + .then(response => { + addResponseAttributesToSpan(span, response); + return response; + }) + .catch(error => { + plugin._addErrorToSpan(span, error); + throw error; + }) + .finally(() => { + span.end(); + }); + }); + }; + }; + } + + private _patchCallRouteAction(): (original: typeof remixRunServerRuntimeData.callRouteAction) => any { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const plugin = this; + return function callRouteAction(original) { + return async function patchCallRouteAction(this: any, ...args: Parameters): Promise { + const [params] = args; + const clonedRequest = params.request.clone(); + const span = plugin.tracer.startSpan( + `ACTION ${params.routeId}`, + { attributes: { [SemanticAttributes.CODE_FUNCTION]: 'action' } }, + opentelemetry.context.active(), + ); + + addRequestAttributesToSpan(span, clonedRequest); + addMatchAttributesToSpan(span, { routeId: params.routeId, params: params.params }); + + return opentelemetry.context.with( + opentelemetry.trace.setSpan(opentelemetry.context.active(), span), + async () => { + const originalResponsePromise: Promise = original.apply(this, args); + + return originalResponsePromise + .then(async response => { + addResponseAttributesToSpan(span, response); + + try { + const formData = await clonedRequest.formData(); + const { actionFormDataAttributes: actionFormAttributes } = plugin.getConfig(); + + formData.forEach((value: unknown, key: string) => { + if ( + actionFormAttributes?.[key] && + actionFormAttributes[key] !== false && + typeof value === 'string' + ) { + const keyName = actionFormAttributes[key] === true ? key : actionFormAttributes[key]; + span.setAttribute(`formData.${keyName}`, value.toString()); + } + }); + } catch { + // Silently continue on any error. Typically happens because the action body cannot be processed + // into FormData, in which case we should just continue. + } + + return response; + }) + .catch(async error => { + plugin._addErrorToSpan(span, error); + throw error; + }) + .finally(() => { + span.end(); + }); + }, + ); + }; + }; + } + + private _addErrorToSpan(span: Span, error: Error): void { + addErrorEventToSpan(span, error); + } +} + +const addRequestAttributesToSpan = (span: Span, request: Request): void => { + span.setAttributes({ + [SemanticAttributes.HTTP_METHOD]: request.method, + [SemanticAttributes.HTTP_URL]: request.url, + }); +}; + +const addMatchAttributesToSpan = (span: Span, match: { routeId: string; params: Params }): void => { + span.setAttributes({ + [RemixSemanticAttributes.MATCH_ROUTE_ID]: match.routeId, + }); + + Object.keys(match.params).forEach(paramName => { + span.setAttribute(`${RemixSemanticAttributes.MATCH_PARAMS}.${paramName}`, match.params[paramName] || '(undefined)'); + }); +}; + +const addResponseAttributesToSpan = (span: Span, response: Response | null): void => { + if (response) { + span.setAttributes({ + [SemanticAttributes.HTTP_STATUS_CODE]: response.status, + }); + } +}; + +const addErrorEventToSpan = (span: Span, error: Error): void => { + span.recordException(error); + span.setStatus({ code: SpanStatusCode.ERROR, message: error.message }); +}; diff --git a/yarn.lock b/yarn.lock index b8ed2f69b728..ebee6dfc068a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5427,13 +5427,6 @@ dependencies: "@opentelemetry/api" "^1.3.0" -"@opentelemetry/api-logs@0.52.1": - version "0.52.1" - resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.52.1.tgz#52906375da4d64c206b0c4cb8ffa209214654ecc" - integrity sha512-qnSqB2DQ9TPP96dl8cDubDvrUyWc0/sK81xHTK8eSUspzDM3bsewX903qclQFvVhgStjRWdC5bLb3kQqMkfV5A== - dependencies: - "@opentelemetry/api" "^1.0.0" - "@opentelemetry/api-logs@0.57.2": version "0.57.2" resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.57.2.tgz#d4001b9aa3580367b40fe889f3540014f766cc87" @@ -5441,7 +5434,7 @@ dependencies: "@opentelemetry/api" "^1.3.0" -"@opentelemetry/api@1.9.0", "@opentelemetry/api@^1.0.0", "@opentelemetry/api@^1.3.0", "@opentelemetry/api@^1.9.0": +"@opentelemetry/api@1.9.0", "@opentelemetry/api@^1.3.0", "@opentelemetry/api@^1.9.0": version "1.9.0" resolved "https://registry.yarnpkg.com/@opentelemetry/api/-/api-1.9.0.tgz#d03eba68273dc0f7509e2a3d5cba21eae10379fe" integrity sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg== @@ -5711,18 +5704,6 @@ require-in-the-middle "^7.1.1" shimmer "^1.2.1" -"@opentelemetry/instrumentation@^0.52.1": - version "0.52.1" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.52.1.tgz#2e7e46a38bd7afbf03cf688c862b0b43418b7f48" - integrity sha512-uXJbYU/5/MBHjMp1FqrILLRuiJCs3Ofk0MeRDk8g1S1gD47U8X3JnSwcMO1rtRo1x1a7zKaQHaoYu49p/4eSKw== - dependencies: - "@opentelemetry/api-logs" "0.52.1" - "@types/shimmer" "^1.0.2" - import-in-the-middle "^1.8.1" - require-in-the-middle "^7.1.1" - semver "^7.5.2" - shimmer "^1.2.1" - "@opentelemetry/propagation-utils@^0.30.16": version "0.30.16" resolved "https://registry.yarnpkg.com/@opentelemetry/propagation-utils/-/propagation-utils-0.30.16.tgz#6715d0225b618ea66cf34cc3800fa3452a8475fa" @@ -5772,10 +5753,10 @@ resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.28.0.tgz#337fb2bca0453d0726696e745f50064411f646d6" integrity sha512-lp4qAiMTD4sNWW4DbKLBkfiMZ4jbAboJIGOQr5DvciMRI494OapieI9qiODpOt0XBr1LjIDy1xAGAnVs5supTA== -"@opentelemetry/semantic-conventions@^1.25.1", "@opentelemetry/semantic-conventions@^1.27.0", "@opentelemetry/semantic-conventions@^1.28.0", "@opentelemetry/semantic-conventions@^1.29.0", "@opentelemetry/semantic-conventions@^1.30.0": - version "1.33.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.33.0.tgz#ec8ebd2ac768ab366aff94e0e7f27e8ae24fa49f" - integrity sha512-TIpZvE8fiEILFfTlfPnltpBaD3d9/+uQHVCyC3vfdh6WfCXKhNFzoP5RyDDIndfvZC5GrA4pyEDNyjPloJud+w== +"@opentelemetry/semantic-conventions@^1.27.0", "@opentelemetry/semantic-conventions@^1.28.0", "@opentelemetry/semantic-conventions@^1.29.0", "@opentelemetry/semantic-conventions@^1.30.0": + version "1.32.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.32.0.tgz#a15e8f78f32388a7e4655e7f539570e40958ca3f" + integrity sha512-s0OpmpQFSfMrmedAn9Lhg4KWJELHCU6uU9dtIJ28N8UGhf9Y55im5X8fEzwhwDwiSqN+ZPSNrDJF7ivf/AuRPQ== "@opentelemetry/sql-common@^0.40.1": version "0.40.1" @@ -8302,7 +8283,7 @@ "@types/mime" "*" "@types/node" "*" -"@types/shimmer@^1.0.2", "@types/shimmer@^1.2.0": +"@types/shimmer@^1.2.0": version "1.2.0" resolved "https://registry.yarnpkg.com/@types/shimmer/-/shimmer-1.2.0.tgz#9b706af96fa06416828842397a70dfbbf1c14ded" integrity sha512-UE7oxhQLLd9gub6JKIAhDq06T0F6FnztwMNRvYgjeQSBeMc1ZG/tA47EwfduvkuQS8apbkM/lpLpWsaCeYsXVg== @@ -22496,14 +22477,6 @@ opener@^1.5.2: resolved "https://registry.yarnpkg.com/opener/-/opener-1.5.2.tgz#5d37e1f35077b9dcac4301372271afdeb2a13598" integrity sha512-ur5UIdyw5Y7yEj9wLzhqXiy6GZ3Mwx0yGI+5sMn2r0N0v3cKJvUmFH5yPP+WXh9e0xfyzyJX95D8l088DNFj7A== -opentelemetry-instrumentation-remix@0.8.0: - version "0.8.0" - resolved "https://registry.yarnpkg.com/opentelemetry-instrumentation-remix/-/opentelemetry-instrumentation-remix-0.8.0.tgz#cf917395f82b2c995ee46068d85d9fa1c95eb36f" - integrity sha512-2XhIEWfzHeQmxnzv9HzklwkgYMx4NuWwloZuVIwjUb9R28gH5j3rJPqjErTvYSyz0fLbw0gyI+gfYHKHn/v/1Q== - dependencies: - "@opentelemetry/instrumentation" "^0.52.1" - "@opentelemetry/semantic-conventions" "^1.25.1" - optional-require@1.0.x: version "1.0.3" resolved "https://registry.yarnpkg.com/optional-require/-/optional-require-1.0.3.tgz#275b8e9df1dc6a17ad155369c2422a440f89cb07" From e83658f23ae627b14ad1ca63ae3fc90c9cd5cdb8 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 14 May 2025 09:04:00 +0200 Subject: [PATCH 4/8] fix(browser): Ensure spans auto-ended for navigations have `cancelled` reason (#16277) Noticed that this now has `externalFinish` as reason, which is not really helpful. With this, you should be able to see that the pageload/navigation was ended because a new one started. --- .size-limit.js | 2 +- .../navigation-aborting-pageload/init.js | 12 ++++ .../navigation-aborting-pageload/test.ts | 58 +++++++++++++++++++ .../navigation/test.ts | 21 +++++++ .../pageload/test.ts | 1 + .../src/tracing/browserTracingIntegration.ts | 1 + 6 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts diff --git a/.size-limit.js b/.size-limit.js index 4452272b00f3..0c03c0ff1b8b 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -135,7 +135,7 @@ module.exports = [ path: 'packages/vue/build/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '40 KB', + limit: '41 KB', }, // Svelte SDK (ESM) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js new file mode 100644 index 000000000000..c0424a9b743f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, +}); + +// Immediately navigate to a new page to abort the pageload +window.location.href = '#foo'; diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts new file mode 100644 index 000000000000..ad224aa6d1d9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts @@ -0,0 +1,58 @@ +import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers'; + +sentryTest( + 'should create a navigation transaction that aborts an ongoing pageload', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + const navigationRequestPromise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation', + ); + + await page.goto(url); + + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + + expect(pageloadRequest.contexts?.trace?.op).toBe('pageload'); + expect(navigationRequest.contexts?.trace?.op).toBe('navigation'); + + expect(navigationRequest.transaction_info?.source).toEqual('url'); + + const pageloadTraceId = pageloadRequest.contexts?.trace?.trace_id; + const navigationTraceId = navigationRequest.contexts?.trace?.trace_id; + + expect(pageloadTraceId).toBeDefined(); + expect(navigationTraceId).toBeDefined(); + expect(pageloadTraceId).not.toEqual(navigationTraceId); + + expect(pageloadRequest.contexts?.trace?.data).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'cancelled', + }); + expect(navigationRequest.contexts?.trace?.data).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + ['sentry.idle_span_finish_reason']: 'idleTimeout', + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts index 2f170c7b7c3e..503aa73ba4ff 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts @@ -1,5 +1,11 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -25,6 +31,21 @@ sentryTest('should create a navigation transaction on page navigation', async ({ expect(navigationTraceId).toBeDefined(); expect(pageloadTraceId).not.toEqual(navigationTraceId); + expect(pageloadRequest.contexts?.trace?.data).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'idleTimeout', + }); + expect(navigationRequest.contexts?.trace?.data).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + ['sentry.idle_span_finish_reason']: 'idleTimeout', + }); + const pageloadSpans = pageloadRequest.spans; const navigationSpans = navigationRequest.spans; diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts index 0146be46057c..9dd0086ba411 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts @@ -30,6 +30,7 @@ sentryTest('creates a pageload transaction with url as source', async ({ getLoca [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'idleTimeout', }); expect(eventData.contexts?.trace?.op).toBe('pageload'); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 99036353d4d7..643b561af583 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -386,6 +386,7 @@ export const browserTracingIntegration = ((_options: Partial Date: Wed, 14 May 2025 09:04:47 +0200 Subject: [PATCH 5/8] chore: Add external contributor to CHANGELOG.md (#16282) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #16274 Co-authored-by: AbhiPrasad <18689448+AbhiPrasad@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d117a6907bf4..b8ca7f9a2e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @sidx1024. Thank you for your contribution! + ## 9.18.0 ### Important changes From 26731edc9932a358f8e5af8424577211b45f2a19 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 14 May 2025 10:07:04 +0200 Subject: [PATCH 6/8] feat(react-router): Add otel instrumentation for server requests (#16147) This PR adds OpenTelemetry instrumentation for server-side requests in React Router by tracing server requests for data loaders and actions. --- .../react-router-7-framework/app/routes.ts | 1 + .../app/routes/performance/dynamic-param.tsx | 5 + .../app/routes/performance/index.tsx | 1 + .../app/routes/performance/server-loader.tsx | 16 +++ .../performance/performance.server.test.ts | 28 +++++ packages/react-router/package.json | 1 + .../src/server/instrumentation/reactRouter.ts | 111 +++++++++++++++++ .../src/server/instrumentation/util.ts | 53 ++++++++ .../server/integration/reactRouterServer.ts | 28 +++++ packages/react-router/src/server/sdk.ts | 48 +++++++- .../src/server/wrapSentryHandleRequest.ts | 11 +- .../instrumentation/reactRouterServer.test.ts | 115 ++++++++++++++++++ .../server/wrapSentryHandleRequest.test.ts | 2 + 13 files changed, 413 insertions(+), 7 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/server-loader.tsx create mode 100644 packages/react-router/src/server/instrumentation/reactRouter.ts create mode 100644 packages/react-router/src/server/instrumentation/util.ts create mode 100644 packages/react-router/src/server/integration/reactRouterServer.ts create mode 100644 packages/react-router/test/server/instrumentation/reactRouterServer.test.ts diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts index 1d8ab1b24a74..c1aacf4e5ce2 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts @@ -15,5 +15,6 @@ export default [ route('ssr', 'routes/performance/ssr.tsx'), route('with/:param', 'routes/performance/dynamic-param.tsx'), route('static', 'routes/performance/static.tsx'), + route('server-loader', 'routes/performance/server-loader.tsx'), ]), ] satisfies RouteConfig; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/dynamic-param.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/dynamic-param.tsx index 39cf7bd5dbf6..1ac02775f2ff 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/dynamic-param.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/dynamic-param.tsx @@ -1,5 +1,10 @@ import type { Route } from './+types/dynamic-param'; +export async function loader() { + await new Promise(resolve => setTimeout(resolve, 500)); + return { data: 'burritos' }; +} + export default function DynamicParamPage({ params }: Route.ComponentProps) { const { param } = params; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx index 99086aadfeae..e5383306625a 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx @@ -7,6 +7,7 @@ export default function PerformancePage() { ); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/server-loader.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/server-loader.tsx new file mode 100644 index 000000000000..e5c222ff4c05 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/server-loader.tsx @@ -0,0 +1,16 @@ +import type { Route } from './+types/server-loader'; + +export async function loader() { + await new Promise(resolve => setTimeout(resolve, 500)); + return { data: 'burritos' }; +} + +export default function ServerLoaderPage({ loaderData }: Route.ComponentProps) { + const { data } = loaderData; + return ( +
+

Server Loader Page

+
{data}
+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/performance.server.test.ts index 4f570beca144..36e37f1ff288 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/performance.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/performance.server.test.ts @@ -104,4 +104,32 @@ test.describe('servery - performance', () => { }, }); }); + + test('should automatically instrument server loader', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/server-loader.data'; + }); + + await page.goto(`/performance`); // initial ssr pageloads do not contain .data requests + await page.waitForTimeout(500); // quick breather before navigation + await page.getByRole('link', { name: 'Server Loader' }).click(); // this will actually trigger a .data request + + const transaction = await txPromise; + + expect(transaction?.spans?.[transaction.spans?.length - 1]).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.http.react-router', + 'sentry.op': 'function.react-router.loader', + }, + description: 'Executing Server Loader', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'function.react-router.loader', + origin: 'auto.http.react-router', + }); + }); }); diff --git a/packages/react-router/package.json b/packages/react-router/package.json index 3f5cfdcd3011..356affde67c8 100644 --- a/packages/react-router/package.json +++ b/packages/react-router/package.json @@ -37,6 +37,7 @@ "@opentelemetry/api": "^1.9.0", "@opentelemetry/core": "^1.30.1", "@opentelemetry/semantic-conventions": "^1.30.0", + "@opentelemetry/instrumentation": "0.57.2", "@sentry/browser": "9.18.0", "@sentry/cli": "^2.43.0", "@sentry/core": "9.18.0", diff --git a/packages/react-router/src/server/instrumentation/reactRouter.ts b/packages/react-router/src/server/instrumentation/reactRouter.ts new file mode 100644 index 000000000000..5bfc0b62e352 --- /dev/null +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -0,0 +1,111 @@ +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { + getActiveSpan, + getRootSpan, + logger, + SDK_VERSION, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + startSpan, +} from '@sentry/core'; +import type * as reactRouter from 'react-router'; +import { DEBUG_BUILD } from '../../common/debug-build'; +import { getOpName, getSpanName, isDataRequest, SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE } from './util'; + +type ReactRouterModuleExports = typeof reactRouter; + +const supportedVersions = ['>=7.0.0']; +const COMPONENT = 'react-router'; + +/** + * Instrumentation for React Router's server request handler. + * This patches the requestHandler function to add Sentry performance monitoring for data loaders. + */ +export class ReactRouterInstrumentation extends InstrumentationBase { + public constructor(config: InstrumentationConfig = {}) { + super('ReactRouterInstrumentation', SDK_VERSION, config); + } + + /** + * Initializes the instrumentation by defining the React Router server modules to be patched. + */ + // eslint-disable-next-line @typescript-eslint/naming-convention + protected init(): InstrumentationNodeModuleDefinition { + const reactRouterServerModule = new InstrumentationNodeModuleDefinition( + COMPONENT, + supportedVersions, + (moduleExports: ReactRouterModuleExports) => { + return this._createPatchedModuleProxy(moduleExports); + }, + (_moduleExports: unknown) => { + // nothing to unwrap here + return _moduleExports; + }, + ); + + return reactRouterServerModule; + } + + /** + * Creates a proxy around the React Router module exports that patches the createRequestHandler function. + * This allows us to wrap the request handler to add performance monitoring for data loaders and actions. + */ + private _createPatchedModuleProxy(moduleExports: ReactRouterModuleExports): ReactRouterModuleExports { + return new Proxy(moduleExports, { + get(target, prop, receiver) { + if (prop === 'createRequestHandler') { + const original = target[prop]; + return function sentryWrappedCreateRequestHandler(this: unknown, ...args: unknown[]) { + const originalRequestHandler = original.apply(this, args); + + return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) { + let url: URL; + try { + url = new URL(request.url); + } catch (error) { + return originalRequestHandler(request, initialContext); + } + + // We currently just want to trace loaders and actions + if (!isDataRequest(url.pathname)) { + return originalRequestHandler(request, initialContext); + } + + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + + if (!rootSpan) { + DEBUG_BUILD && logger.debug('No active root span found, skipping tracing for data request'); + return originalRequestHandler(request, initialContext); + } + + // Set the source and overwrite attributes on the root span to ensure the transaction name + // is derived from the raw URL pathname rather than any parameterized route that may be set later + // TODO: try to set derived parameterized route from build here (args[0]) + rootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE]: `${request.method} ${url.pathname}`, + }); + + return startSpan( + { + name: getSpanName(url.pathname, request.method), + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getOpName(url.pathname, request.method), + }, + }, + () => { + return originalRequestHandler(request, initialContext); + }, + ); + }; + }; + } + return Reflect.get(target, prop, receiver); + }, + }); + } +} diff --git a/packages/react-router/src/server/instrumentation/util.ts b/packages/react-router/src/server/instrumentation/util.ts new file mode 100644 index 000000000000..19aec91999fc --- /dev/null +++ b/packages/react-router/src/server/instrumentation/util.ts @@ -0,0 +1,53 @@ +/** + * Gets the op name for a request based on whether it's a loader or action request. + * @param pathName The URL pathname to check + * @param requestMethod The HTTP request method + */ +export function getOpName(pathName: string, requestMethod: string): string { + return isLoaderRequest(pathName, requestMethod) + ? 'function.react-router.loader' + : isActionRequest(pathName, requestMethod) + ? 'function.react-router.action' + : 'function.react-router'; +} + +/** + * Gets the span name for a request based on whether it's a loader or action request. + * @param pathName The URL pathname to check + * @param requestMethod The HTTP request method + */ +export function getSpanName(pathName: string, requestMethod: string): string { + return isLoaderRequest(pathName, requestMethod) + ? 'Executing Server Loader' + : isActionRequest(pathName, requestMethod) + ? 'Executing Server Action' + : 'Unknown Data Request'; +} + +/** + * Checks if the request is a server loader request + * @param pathname The URL pathname to check + * @param requestMethod The HTTP request method + */ +export function isLoaderRequest(pathname: string, requestMethod: string): boolean { + return isDataRequest(pathname) && requestMethod === 'GET'; +} + +/** + * Checks if the request is a server action request + * @param pathname The URL pathname to check + * @param requestMethod The HTTP request method + */ +export function isActionRequest(pathname: string, requestMethod: string): boolean { + return isDataRequest(pathname) && requestMethod === 'POST'; +} + +/** + * Checks if the request is a react-router data request + * @param pathname The URL pathname to check + */ +export function isDataRequest(pathname: string): boolean { + return pathname.endsWith('.data'); +} + +export const SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE = 'sentry.overwrite-route'; diff --git a/packages/react-router/src/server/integration/reactRouterServer.ts b/packages/react-router/src/server/integration/reactRouterServer.ts new file mode 100644 index 000000000000..548b21f6f039 --- /dev/null +++ b/packages/react-router/src/server/integration/reactRouterServer.ts @@ -0,0 +1,28 @@ +import { defineIntegration } from '@sentry/core'; +import { generateInstrumentOnce } from '@sentry/node'; +import { ReactRouterInstrumentation } from '../instrumentation/reactRouter'; + +const INTEGRATION_NAME = 'ReactRouterServer'; + +const instrumentReactRouter = generateInstrumentOnce('React-Router-Server', () => { + return new ReactRouterInstrumentation(); +}); + +export const instrumentReactRouterServer = Object.assign( + (): void => { + instrumentReactRouter(); + }, + { id: INTEGRATION_NAME }, +); + +/** + * Integration capturing tracing data for React Router server functions. + */ +export const reactRouterServerIntegration = defineIntegration(() => { + return { + name: INTEGRATION_NAME, + setupOnce() { + instrumentReactRouterServer(); + }, + }; +}); diff --git a/packages/react-router/src/server/sdk.ts b/packages/react-router/src/server/sdk.ts index c980078ac7b5..b0ca0e79bd49 100644 --- a/packages/react-router/src/server/sdk.ts +++ b/packages/react-router/src/server/sdk.ts @@ -1,21 +1,32 @@ -import type { Integration } from '@sentry/core'; -import { applySdkMetadata, logger, setTag } from '@sentry/core'; +import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; +import type { EventProcessor, Integration } from '@sentry/core'; +import { applySdkMetadata, getGlobalScope, logger, setTag } from '@sentry/core'; import type { NodeClient, NodeOptions } from '@sentry/node'; import { getDefaultIntegrations as getNodeDefaultIntegrations, init as initNodeSdk } from '@sentry/node'; import { DEBUG_BUILD } from '../common/debug-build'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE } from './instrumentation/util'; +import { reactRouterServerIntegration } from './integration/reactRouterServer'; import { lowQualityTransactionsFilterIntegration } from './lowQualityTransactionsFilterIntegration'; -function getDefaultIntegrations(options: NodeOptions): Integration[] { - return [...getNodeDefaultIntegrations(options), lowQualityTransactionsFilterIntegration(options)]; +/** + * Returns the default integrations for the React Router SDK. + * @param options The options for the SDK. + */ +export function getDefaultReactRouterServerIntegrations(options: NodeOptions): Integration[] { + return [ + ...getNodeDefaultIntegrations(options), + lowQualityTransactionsFilterIntegration(options), + reactRouterServerIntegration(), + ]; } /** * Initializes the server side of the React Router SDK */ export function init(options: NodeOptions): NodeClient | undefined { - const opts = { + const opts: NodeOptions = { ...options, - defaultIntegrations: getDefaultIntegrations(options), + defaultIntegrations: getDefaultReactRouterServerIntegrations(options), }; DEBUG_BUILD && logger.log('Initializing SDK...'); @@ -26,6 +37,31 @@ export function init(options: NodeOptions): NodeClient | undefined { setTag('runtime', 'node'); + // Overwrite the transaction name for instrumented data loaders because the trace data gets overwritten at a later point. + // We only update the tx in case SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE got set in our instrumentation before. + getGlobalScope().addEventProcessor( + Object.assign( + (event => { + const overwrite = event.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE]; + if ( + event.type === 'transaction' && + event.transaction === 'GET *' && + event.contexts?.trace?.data?.[ATTR_HTTP_ROUTE] === '*' && + overwrite + ) { + event.transaction = overwrite; + event.contexts.trace.data[ATTR_HTTP_ROUTE] = 'url'; + } + + // always yeet this attribute into the void, as this should not reach the server + delete event.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE]; + + return event; + }) satisfies EventProcessor, + { id: 'ReactRouterTransactionEnhancer' }, + ), + ); + DEBUG_BUILD && logger.log('SDK successfully initialized'); return client; diff --git a/packages/react-router/src/server/wrapSentryHandleRequest.ts b/packages/react-router/src/server/wrapSentryHandleRequest.ts index ab13ea30ff0b..40a336a40fbd 100644 --- a/packages/react-router/src/server/wrapSentryHandleRequest.ts +++ b/packages/react-router/src/server/wrapSentryHandleRequest.ts @@ -1,7 +1,13 @@ import { context } from '@opentelemetry/api'; import { getRPCMetadata, RPCType } from '@opentelemetry/core'; import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; -import { getActiveSpan, getRootSpan, getTraceMetaTags, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { + getActiveSpan, + getRootSpan, + getTraceMetaTags, + SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; import type { AppLoadContext, EntryContext } from 'react-router'; import type { PassThrough } from 'stream'; import { Transform } from 'stream'; @@ -30,6 +36,7 @@ export function wrapSentryHandleRequest(originalHandle: OriginalHandleRequest): ) { const parameterizedPath = routerContext?.staticHandlerContext?.matches?.[routerContext.staticHandlerContext.matches.length - 1]?.route.path; + if (parameterizedPath) { const activeSpan = getActiveSpan(); if (activeSpan) { @@ -38,6 +45,7 @@ export function wrapSentryHandleRequest(originalHandle: OriginalHandleRequest): // The express instrumentation writes on the rpcMetadata and that ends up stomping on the `http.route` attribute. const rpcMetadata = getRPCMetadata(context.active()); + if (rpcMetadata?.type === RPCType.HTTP) { rpcMetadata.route = routeName; } @@ -46,6 +54,7 @@ export function wrapSentryHandleRequest(originalHandle: OriginalHandleRequest): rootSpan.setAttributes({ [ATTR_HTTP_ROUTE]: routeName, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]: `${request.method} ${routeName}`, }); } } diff --git a/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts new file mode 100644 index 000000000000..ddcb856c68b9 --- /dev/null +++ b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts @@ -0,0 +1,115 @@ +import type { Span } from '@sentry/core'; +import * as SentryCore from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter'; +import * as Util from '../../../src/server/instrumentation/util'; + +vi.mock('@sentry/core', async () => { + return { + getActiveSpan: vi.fn(), + getRootSpan: vi.fn(), + logger: { + debug: vi.fn(), + }, + SDK_VERSION: '1.0.0', + SEMANTIC_ATTRIBUTE_SENTRY_OP: 'sentry.op', + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', + startSpan: vi.fn((opts, fn) => fn({})), + }; +}); + +vi.mock('./util', async () => { + return { + getSpanName: vi.fn((pathname: string, method: string) => `span:${pathname}:${method}`), + isDataRequest: vi.fn(), + }; +}); + +const mockSpan = { + spanContext: () => ({ traceId: '1', spanId: '2', traceFlags: 1 }), + setAttributes: vi.fn(), +}; + +function createRequest(url: string, method = 'GET') { + return { url, method } as unknown as Request; +} + +describe('ReactRouterInstrumentation', () => { + let instrumentation: ReactRouterInstrumentation; + let mockModule: any; + let originalHandler: any; + + beforeEach(() => { + instrumentation = new ReactRouterInstrumentation(); + originalHandler = vi.fn(); + mockModule = { + createRequestHandler: vi.fn(() => originalHandler), + }; + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should patch createRequestHandler', () => { + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + expect(typeof proxy.createRequestHandler).toBe('function'); + expect(proxy.createRequestHandler).not.toBe(mockModule.createRequestHandler); + }); + + it('should call original handler for non-data requests', async () => { + vi.spyOn(Util, 'isDataRequest').mockReturnValue(false); + + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + const wrappedHandler = proxy.createRequestHandler(); + const req = createRequest('https://test.com/page'); + await wrappedHandler(req); + + expect(Util.isDataRequest).toHaveBeenCalledWith('/page'); + expect(originalHandler).toHaveBeenCalledWith(req, undefined); + }); + + it('should call original handler if no active root span', async () => { + vi.spyOn(Util, 'isDataRequest').mockReturnValue(true); + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(undefined); + + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + const wrappedHandler = proxy.createRequestHandler(); + const req = createRequest('https://test.com/data'); + await wrappedHandler(req); + + expect(SentryCore.logger.debug).toHaveBeenCalledWith( + 'No active root span found, skipping tracing for data request', + ); + expect(originalHandler).toHaveBeenCalledWith(req, undefined); + }); + + it('should start a span for data requests with active root span', async () => { + vi.spyOn(Util, 'isDataRequest').mockReturnValue(true); + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(mockSpan as Span); + vi.spyOn(SentryCore, 'getRootSpan').mockReturnValue(mockSpan as Span); + vi.spyOn(Util, 'getSpanName').mockImplementation((pathname, method) => `span:${pathname}:${method}`); + vi.spyOn(SentryCore, 'startSpan').mockImplementation((_opts, fn) => fn(mockSpan as Span)); + + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + const wrappedHandler = proxy.createRequestHandler(); + const req = createRequest('https://test.com/data', 'POST'); + await wrappedHandler(req); + + expect(Util.isDataRequest).toHaveBeenCalledWith('/data'); + expect(Util.getSpanName).toHaveBeenCalledWith('/data', 'POST'); + expect(SentryCore.startSpan).toHaveBeenCalled(); + expect(originalHandler).toHaveBeenCalledWith(req, undefined); + }); + + it('should handle invalid URLs gracefully', async () => { + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + const wrappedHandler = proxy.createRequestHandler(); + const req = { url: 'not a url', method: 'GET' } as any; + await wrappedHandler(req); + + expect(originalHandler).toHaveBeenCalledWith(req, undefined); + }); +}); diff --git a/packages/react-router/test/server/wrapSentryHandleRequest.test.ts b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts index ced113261709..61a92e7b6546 100644 --- a/packages/react-router/test/server/wrapSentryHandleRequest.test.ts +++ b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts @@ -12,6 +12,7 @@ vi.mock('@opentelemetry/core', () => ({ vi.mock('@sentry/core', () => ({ SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', + SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME: 'sentry.custom-span-name', getActiveSpan: vi.fn(), getRootSpan: vi.fn(), getTraceMetaTags: vi.fn(), @@ -69,6 +70,7 @@ describe('wrapSentryHandleRequest', () => { expect(getRootSpan).toHaveBeenCalledWith(mockActiveSpan); expect(mockRootSpan.setAttributes).toHaveBeenCalledWith({ [ATTR_HTTP_ROUTE]: '/some-path', + 'sentry.custom-span-name': 'GET /some-path', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', }); expect(mockRpcMetadata.route).toBe('/some-path'); From 4b039f8a6e123ff1548c83e22a3d26c9c387c677 Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Wed, 14 May 2025 11:54:53 +0200 Subject: [PATCH 7/8] fix(node): Pin `@fastify/otel` fork to direct url to allow installing without git (#16287) The previous approach of specifying the git repo directly breaks systems that don't have git installed (e.g. slim linux distros). I tested this works on a ubuntu vm without git. **Important:** When making changes to the fork, the linked commit sha in `packages/node/package.json` has to be updated and the yarn lockfile regenerated. Resolves: #16281 --- packages/node/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 991397d9012f..637b9e2534f8 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -65,7 +65,7 @@ "access": "public" }, "dependencies": { - "@fastify/otel": "getsentry/fastify-otel#otel-v1", + "@fastify/otel": "https://codeload.github.com/getsentry/fastify-otel/tar.gz/ae3088d65e286bdc94ac5d722573537d6a6671bb", "@opentelemetry/api": "^1.9.0", "@opentelemetry/context-async-hooks": "^1.30.1", "@opentelemetry/core": "^1.30.1", diff --git a/yarn.lock b/yarn.lock index ebee6dfc068a..78c0f0fb30a3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3912,9 +3912,9 @@ resolved "https://registry.yarnpkg.com/@fastify/busboy/-/busboy-2.0.0.tgz#f22824caff3ae506b18207bad4126dbc6ccdb6b8" integrity sha512-JUFJad5lv7jxj926GPgymrWQxxjPYuJNiNjNMzqT+HiuP6Vl3dk5xzG+8sTX96np0ZAluvaMzPsjhHZ5rNuNQQ== -"@fastify/otel@getsentry/fastify-otel#otel-v1": +"@fastify/otel@https://codeload.github.com/getsentry/fastify-otel/tar.gz/ae3088d65e286bdc94ac5d722573537d6a6671bb": version "0.8.0" - resolved "https://codeload.github.com/getsentry/fastify-otel/tar.gz/d6bb1756c3db3d00d4d82c39c93ee3316e06d305" + resolved "https://codeload.github.com/getsentry/fastify-otel/tar.gz/ae3088d65e286bdc94ac5d722573537d6a6671bb#1632d3df7ebf8cd86996a50e9e42721aea05b39c" dependencies: "@opentelemetry/core" "^1.30.1" "@opentelemetry/instrumentation" "^0.57.2" From ed276633ce093e6e49149789d0561d7c5fe1162f Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Wed, 14 May 2025 11:56:15 +0200 Subject: [PATCH 8/8] meta(changelog): Update changelog for 9.19.0 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8ca7f9a2e5d..78064f48920a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 9.19.0 + +- feat(react-router): Add otel instrumentation for server requests ([#16147](https://github.com/getsentry/sentry-javascript/pull/16147)) +- feat(remix): Vendor in `opentelemetry-instrumentation-remix` ([#16145](https://github.com/getsentry/sentry-javascript/pull/16145)) +- fix(browser): Ensure spans auto-ended for navigations have `cancelled` reason ([#16277](https://github.com/getsentry/sentry-javascript/pull/16277)) +- fix(node): Pin `@fastify/otel` fork to direct url to allow installing without git ([#16287](https://github.com/getsentry/sentry-javascript/pull/16287)) +- fix(react): Handle nested parameterized routes in reactrouterv3 transaction normalization ([#16274](https://github.com/getsentry/sentry-javascript/pull/16274)) + Work in this release was contributed by @sidx1024. Thank you for your contribution! ## 9.18.0