diff --git a/packages/remix/package.json b/packages/remix/package.json index b912ddb301ac..46b8c9e349bc 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -57,8 +57,6 @@ "access": "public" }, "dependencies": { - "@opentelemetry/api": "^1.9.1", - "@opentelemetry/instrumentation": "^0.220.0", "@remix-run/router": "^1.23.3", "@sentry/cli": "^2.58.6", "@sentry/conventions": "^0.16.0", diff --git a/packages/remix/src/server/integrations/RemixIntegration.ts b/packages/remix/src/server/integrations/RemixIntegration.ts index f70926fa9add..7593c4ea5153 100644 --- a/packages/remix/src/server/integrations/RemixIntegration.ts +++ b/packages/remix/src/server/integrations/RemixIntegration.ts @@ -1,8 +1,6 @@ import type { IntegrationFn } from '@sentry/core'; import { defineIntegration, getClient } from '@sentry/core'; -import { isOrchestrionInjected } from '@sentry/server-utils/orchestrion'; import { instrumentRemix } from './tracing-channel'; -import { addRemixSpanAttributes, instrumentRemixWithOpenTelemetry } from './opentelemetry'; import type { RemixOptions } from '../../utils/remixOptions'; const INTEGRATION_NAME = 'Remix' as const; @@ -17,18 +15,7 @@ const _remixIntegration = (() => { ? options?.captureActionFormDataKeys : undefined; - if (isOrchestrionInjected()) { - instrumentRemix(actionFormDataAttributes); - } else { - instrumentRemixWithOpenTelemetry({ actionFormDataAttributes }); - } - }, - setup(client) { - if (!isOrchestrionInjected()) { - client.on('spanStart', span => { - addRemixSpanAttributes(span); - }); - } + instrumentRemix(actionFormDataAttributes); }, }; }) satisfies IntegrationFn; diff --git a/packages/remix/src/server/integrations/opentelemetry.ts b/packages/remix/src/server/integrations/opentelemetry.ts deleted file mode 100644 index ceaa9349c706..000000000000 --- a/packages/remix/src/server/integrations/opentelemetry.ts +++ /dev/null @@ -1,40 +0,0 @@ -import type { Span } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { generateInstrumentOnce, spanToJSON } from '@sentry/node'; -import { RemixInstrumentation } from '../../vendor/instrumentation'; - -const INTEGRATION_NAME = 'Remix'; - -interface RemixInstrumentationOptions { - actionFormDataAttributes?: Record; -} - -export const instrumentRemixWithOpenTelemetry = generateInstrumentOnce( - INTEGRATION_NAME, - (options?: RemixInstrumentationOptions) => { - return new RemixInstrumentation(options); - }, -); - -export function addRemixSpanAttributes(span: Span): void { - const attributes = spanToJSON(span).data; - - // this is one of: loader, action, requestHandler - const type = attributes['code.function']; - - // If this is already set, or we have no remix span, no need to process again... - if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) { - return; - } - - // `requestHandler` span from `opentelemetry-instrumentation-remix` is the main server span. - // It should be marked as the `http.server` operation. - // The incoming requests are skipped by the custom `RemixHttpIntegration` package. - // All other spans are marked as `remix` operations with their specific type [loader, action] - const op = type === 'requestHandler' ? 'http.server' : `${type}.remix`; - - span.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.remix', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, - }); -} diff --git a/packages/remix/src/server/integrations/tracing-channel.ts b/packages/remix/src/server/integrations/tracing-channel.ts index 7051c9cbc2c8..26837beaa3ab 100644 --- a/packages/remix/src/server/integrations/tracing-channel.ts +++ b/packages/remix/src/server/integrations/tracing-channel.ts @@ -26,8 +26,6 @@ const ORIGIN = 'auto.http.orchestrion.remix'; const NOOP = (): void => {}; -// `match.route.id` / `match.params.*` mirror `RemixSemanticAttributes` from the vendored -// `RemixInstrumentation` this integration replaces. const MATCH_ROUTE_ID = 'match.route.id'; const MATCH_PARAMS = 'match.params'; @@ -103,8 +101,7 @@ function setResponseStatus(span: Span, result: unknown): void { /** * `matchServerRoutes` opens no span of its own; it enriches the enclosing request span with the - * matched route (used to derive the `http.server` transaction name), mirroring the vendored - * instrumentation's patch. + * matched route (used to derive the `http.server` transaction name). */ function enrichActiveSpanWithRoute(result: unknown): void { const span = getActiveSpan(); diff --git a/packages/remix/src/vendor/instrumentation.ts b/packages/remix/src/vendor/instrumentation.ts deleted file mode 100644 index d33341507723..000000000000 --- a/packages/remix/src/vendor/instrumentation.ts +++ /dev/null @@ -1,362 +0,0 @@ -/* eslint-disable typescript/no-deprecated */ -/* eslint-disable jsdoc/require-jsdoc */ -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable import/no-named-as-default-member */ -/* eslint-disable import/no-duplicates */ - -// Vendored and modified from: -// https://github.com/justindsmith/opentelemetry-instrumentations-js/blob/3b1e8c3e566e5cc3389e9c28cafce6a5ebb39600/packages/instrumentation-remix/src/instrumentation.ts - -/* - * Copyright Justin Smith - * SPDX-License-Identifier: Apache-2.0 - */ - -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 { CODE_FUNCTION, HTTP_METHOD, HTTP_ROUTE, HTTP_STATUS_CODE, URL_FULL } from '@sentry/conventions/attributes'; -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(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: { [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: { [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: { [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] && 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({ - [HTTP_METHOD]: request.method, - [URL_FULL]: 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({ - [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/packages/remix/test/server/remix-integration-otel.test.ts b/packages/remix/test/server/remix-integration-otel.test.ts deleted file mode 100644 index c2ef63d0a678..000000000000 --- a/packages/remix/test/server/remix-integration-otel.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import * as SentryCore from '@sentry/core'; -import type { NodeClient } from '@sentry/node'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -// Force the OpenTelemetry branch of `remixIntegration`. `isOrchestrionInjected` is the only export -// the loaded graph needs (the tracing-channel module is mocked below), so a minimal mock suffices. -vi.mock('@sentry/server-utils/orchestrion', () => ({ - isOrchestrionInjected: () => false, -})); - -// Replace both instrument helpers so we can assert the call shape without OTel/channel side effects. -vi.mock('../../src/server/integrations/opentelemetry', () => ({ - instrumentRemixWithOpenTelemetry: vi.fn(), - addRemixSpanAttributes: vi.fn(), -})); -vi.mock('../../src/server/integrations/tracing-channel', () => ({ - instrumentRemix: vi.fn(), -})); - -import { remixIntegration } from '../../src/server/integrations/RemixIntegration'; -import { instrumentRemixWithOpenTelemetry } from '../../src/server/integrations/opentelemetry'; -import { instrumentRemix } from '../../src/server/integrations/tracing-channel'; - -function mockClient( - captureActionFormDataKeys: Record | undefined, - httpBodies: string[], -): void { - vi.spyOn(SentryCore, 'getClient').mockReturnValue({ - getOptions: () => ({ captureActionFormDataKeys }), - getDataCollectionOptions: () => ({ httpBodies }), - } as unknown as NodeClient); -} - -describe('remixIntegration (OpenTelemetry-based)', () => { - afterEach(() => { - vi.clearAllMocks(); - vi.restoreAllMocks(); - }); - - it('wraps the opted-in form-data keys in the RemixInstrumentation options object', () => { - mockClient({ username: true }, ['incomingRequest']); - - remixIntegration().setupOnce?.(); - - // Must be wrapped as `{ actionFormDataAttributes }` — passing the bare map would leave - // RemixInstrumentation's default `{ _action: 'actionType' }` mapping in place. - expect(instrumentRemixWithOpenTelemetry).toHaveBeenCalledWith({ actionFormDataAttributes: { username: true } }); - expect(instrumentRemix).not.toHaveBeenCalled(); - }); - - it('passes undefined attributes when form-data capture is not opted into', () => { - // `httpBodies` without `incomingRequest` means capture is off, regardless of the configured keys. - mockClient({ username: true }, []); - - remixIntegration().setupOnce?.(); - - expect(instrumentRemixWithOpenTelemetry).toHaveBeenCalledWith({ actionFormDataAttributes: undefined }); - }); -}); diff --git a/packages/remix/test/server/remix-integration.test.ts b/packages/remix/test/server/remix-integration.test.ts new file mode 100644 index 000000000000..c08d9283b15e --- /dev/null +++ b/packages/remix/test/server/remix-integration.test.ts @@ -0,0 +1,44 @@ +import * as SentryCore from '@sentry/core'; +import type { NodeClient } from '@sentry/node'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../src/server/integrations/tracing-channel', () => ({ + instrumentRemix: vi.fn(), +})); + +import { remixIntegration } from '../../src/server/integrations/RemixIntegration'; +import { instrumentRemix } from '../../src/server/integrations/tracing-channel'; + +function mockClient( + captureActionFormDataKeys: Record | undefined, + httpBodies: string[], +): void { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ captureActionFormDataKeys }), + getDataCollectionOptions: () => ({ httpBodies }), + } as unknown as NodeClient); +} + +describe('remixIntegration', () => { + afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); + }); + + it('passes the opted-in form-data keys through to the channel instrumentation', () => { + mockClient({ username: true }, ['incomingRequest']); + + remixIntegration().setupOnce?.(); + + expect(instrumentRemix).toHaveBeenCalledWith({ username: true }); + }); + + it('passes undefined attributes when form-data capture is not opted into', () => { + // `httpBodies` without `incomingRequest` means capture is off, regardless of the configured keys. + mockClient({ username: true }, []); + + remixIntegration().setupOnce?.(); + + expect(instrumentRemix).toHaveBeenCalledWith(undefined); + }); +});