From 17c5c6c8a4d70f376b0aad906dea34ef8ad3ba99 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 28 Apr 2025 16:57:24 +0200 Subject: [PATCH 1/4] add instrumentation --- packages/react-router/package.json | 1 + .../src/server/instrumentation/reactRouter.ts | 103 ++++++++++++++++++ .../src/server/instrumentation/util.ts | 25 +++++ .../server/integration/reactRouterServer.ts | 28 +++++ packages/react-router/src/server/sdk.ts | 15 ++- 5 files changed, 170 insertions(+), 2 deletions(-) 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 diff --git a/packages/react-router/package.json b/packages/react-router/package.json index 6305ec1da8ef..585e4c2f82ff 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.14.0", "@sentry/cli": "^2.43.0", "@sentry/core": "9.14.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..63893ea6de31 --- /dev/null +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -0,0 +1,103 @@ +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { + 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 { isActionRequest, isDataRequest, isLoaderRequest } 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 wrappedCreateRequestHandler(this: unknown, ...args: any[]) { + const originalRequestHandler = original.apply(this, args); + + return async function wrappedRequestHandler(request: Request, initialContext?: any) { + 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); + } + + // All data requests end with .data + let txName = url.pathname.replace(/\.data$/, ''); + + if (isLoaderRequest(url.pathname, request.method)) { + txName = `Loader ${txName}`; + } else if (isActionRequest(url.pathname, request.method)) { + txName = `Action ${txName}`; + } + + return startSpan( + { + name: txName, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + url: url.pathname, + method: 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..a727948bfdfb --- /dev/null +++ b/packages/react-router/src/server/instrumentation/util.ts @@ -0,0 +1,25 @@ +/** + * 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'); +} 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..eda1495ad682 --- /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 = 'React Router Server'; + +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 d1e6b32b1d96..9cdea595a512 100644 --- a/packages/react-router/src/server/sdk.ts +++ b/packages/react-router/src/server/sdk.ts @@ -1,13 +1,16 @@ +import type { Integration } from '@sentry/core'; import { applySdkMetadata, logger, setTag } from '@sentry/core'; import type { NodeClient, NodeOptions } from '@sentry/node'; -import { init as initNodeSdk } from '@sentry/node'; +import { getDefaultIntegrations, init as initNodeSdk } from '@sentry/node'; import { DEBUG_BUILD } from '../common/debug-build'; +import { reactRouterServerIntegration } from './integration/reactRouterServer'; /** * Initializes the server side of the React Router SDK */ export function init(options: NodeOptions): NodeClient | undefined { - const opts = { + const opts: NodeOptions = { + defaultIntegrations: [...getDefaultReactRouterServerIntegrations(options)], ...options, }; @@ -22,3 +25,11 @@ export function init(options: NodeOptions): NodeClient | undefined { DEBUG_BUILD && logger.log('SDK successfully initialized'); return client; } + +/** + * Returns the default integrations for the React Router SDK. + * @param options The options for the SDK. + */ +export function getDefaultReactRouterServerIntegrations(options: NodeOptions): Integration[] { + return [...getDefaultIntegrations(options), reactRouterServerIntegration()]; +} From 926200805e8c94b62987ba159a7cef120f2d9793 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 29 Apr 2025 14:18:36 +0200 Subject: [PATCH 2/4] some attribute cleanup --- .../src/server/instrumentation/reactRouter.ts | 25 +++++++++++-------- .../src/server/instrumentation/util.ts | 13 ++++++++++ .../server/integration/reactRouterServer.ts | 2 +- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/react-router/src/server/instrumentation/reactRouter.ts b/packages/react-router/src/server/instrumentation/reactRouter.ts index 63893ea6de31..7152ea161e10 100644 --- a/packages/react-router/src/server/instrumentation/reactRouter.ts +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -1,6 +1,9 @@ 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, @@ -8,7 +11,8 @@ import { startSpan, } from '@sentry/core'; import type * as reactRouter from 'react-router'; -import { isActionRequest, isDataRequest, isLoaderRequest } from './util'; +import { getSpanName, isDataRequest } from './util'; +import { DEBUG_BUILD } from '../../common/debug-build'; type ReactRouterModuleExports = typeof reactRouter; @@ -53,10 +57,10 @@ export class ReactRouterInstrumentation extends InstrumentationBase { return new ReactRouterInstrumentation(); From 01abd59de7e3e292d4e805376ac86e58ae1c074e Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 29 Apr 2025 14:27:27 +0200 Subject: [PATCH 3/4] lint --- packages/react-router/src/server/instrumentation/reactRouter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router/src/server/instrumentation/reactRouter.ts b/packages/react-router/src/server/instrumentation/reactRouter.ts index 7152ea161e10..d96dcb476a87 100644 --- a/packages/react-router/src/server/instrumentation/reactRouter.ts +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -11,8 +11,8 @@ import { startSpan, } from '@sentry/core'; import type * as reactRouter from 'react-router'; -import { getSpanName, isDataRequest } from './util'; import { DEBUG_BUILD } from '../../common/debug-build'; +import { getSpanName, isDataRequest } from './util'; type ReactRouterModuleExports = typeof reactRouter; From 24a3c3ff737312ec7b4f073eb262f511e85ab763 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 5 May 2025 16:57:34 +0200 Subject: [PATCH 4/4] unit test for instrumentation --- .../instrumentation/reactRouterServer.test.ts | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 packages/react-router/test/server/instrumentation/reactRouterServer.test.ts 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..ca0ad6bc71a8 --- /dev/null +++ b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts @@ -0,0 +1,114 @@ +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 }), +}; + +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); + }); +});