diff --git a/.oxlintrc.base.json b/.oxlintrc.base.json index e5c35bf49d5c..37b0b47b2fc0 100644 --- a/.oxlintrc.base.json +++ b/.oxlintrc.base.json @@ -146,7 +146,6 @@ "**/integrations/tracing/knex/vendored/**/*.ts", "**/integrations/tracing/mongo/vendored/**/*.ts", "**/integrations/tracing/graphql/vendored/**/*.ts", - "**/integrations/tracing/koa/vendored/**/*.ts", "**/integrations/tracing/mysql2/vendored/**/*.ts", "**/integration/aws/vendored/**/*.ts", "**/integrations/tracing/kafka/vendored/**/*.ts", diff --git a/dev-packages/e2e-tests/test-applications/node-koa/index.js b/dev-packages/e2e-tests/test-applications/node-koa/index.js index 9e800a4fcc99..629c9bb82c04 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/index.js +++ b/dev-packages/e2e-tests/test-applications/node-koa/index.js @@ -115,8 +115,30 @@ router1.post('/test-post', async ctx => { ctx.body = { status: 'ok', body: ctx.request.body }; }); +// RegExp route - exercises the @koa/router dispatch patch with a non-string layer path. +router1.get(/^\/test-regexp/, ctx => { + ctx.body = { matched: 'regexp' }; +}); + +// Same middleware instance passed twice - the second occurrence must be skipped (kLayerPatched dedup). +const sharedRouteMiddleware = async (ctx, next) => { + await next(); +}; +router1.get('/test-dedup', sharedRouteMiddleware, sharedRouteMiddleware, ctx => { + ctx.body = { ok: true }; +}); + app1.use(router1.routes()).use(router1.allowedMethods()); +// Nested router - the routed span's http.route is the composed parent + child path. +const nestedRouter = new Router(); +nestedRouter.get('/details/:id', ctx => { + ctx.body = { id: ctx.params.id }; +}); +const outerRouter = new Router(); +outerRouter.use('/:first', nestedRouter.routes()); +app1.use(outerRouter.routes()).use(outerRouter.allowedMethods()); + app1.listen(port1); const app2 = new Koa(); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/instrumented-paths.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/instrumented-paths.test.ts new file mode 100644 index 000000000000..ebf42c7f8738 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/instrumented-paths.test.ts @@ -0,0 +1,99 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('instruments RegExp router routes', async ({ baseURL }) => { + const transactionPromise = waitForTransaction('node-koa', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && !!transactionEvent.transaction?.includes('test-regexp') + ); + }); + + await fetch(`${baseURL}/test-regexp`); + + const transactionEvent = await transactionPromise; + + expect(transactionEvent.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + op: 'router.koa', + origin: 'auto.http.otel.koa', + data: expect.objectContaining({ + 'koa.type': 'router', + 'sentry.op': 'router.koa', + 'sentry.origin': 'auto.http.otel.koa', + 'http.route': '/^\\/test-regexp/', + }), + }), + ]), + ); +}); + +test('instruments nested routers with the composed http.route', async ({ baseURL }) => { + const transactionPromise = waitForTransaction('node-koa', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.transaction === 'GET /:first/details/:id' + ); + }); + + await fetch(`${baseURL}/shop/details/1`); + + const transactionEvent = await transactionPromise; + + expect(transactionEvent.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + op: 'router.koa', + description: '/:first/details/:id', + data: expect.objectContaining({ + 'koa.type': 'router', + 'http.route': '/:first/details/:id', + 'sentry.op': 'router.koa', + 'sentry.origin': 'auto.http.otel.koa', + }), + }), + ]), + ); +}); + +test('does not instrument the same middleware twice', async ({ baseURL }) => { + const transactionPromise = waitForTransaction('node-koa', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent.transaction === 'GET /test-dedup' + ); + }); + + await fetch(`${baseURL}/test-dedup`); + + const transactionEvent = await transactionPromise; + + // The route stack is [sharedRouteMiddleware, sharedRouteMiddleware, handler]; the repeated + // middleware instance is skipped, leaving one span for it plus the handler span. + const dedupSpans = transactionEvent.spans?.filter( + span => span.op === 'router.koa' && span.description === '/test-dedup', + ); + expect(dedupSpans).toHaveLength(2); +}); + +test('marks the layer span as errored when a handler throws', async ({ baseURL }) => { + const transactionPromise = waitForTransaction('node-koa', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.transaction === 'GET /test-exception/:id' + ); + }); + + await fetch(`${baseURL}/test-exception/123`); + + const transactionEvent = await transactionPromise; + + expect(transactionEvent.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + op: 'router.koa', + origin: 'auto.http.otel.koa', + status: 'internal_error', + }), + ]), + ); +}); diff --git a/packages/node/src/integrations/tracing/koa/index.ts b/packages/node/src/integrations/tracing/koa/index.ts index 11d8e01bf669..d40ec05d36e0 100644 --- a/packages/node/src/integrations/tracing/koa/index.ts +++ b/packages/node/src/integrations/tracing/koa/index.ts @@ -1,18 +1,8 @@ import type { KoaInstrumentationConfig, KoaLayerType } from './vendored/types'; import { KoaInstrumentation } from './vendored/instrumentation'; -import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; import type { IntegrationFn } from '@sentry/core'; -import { - captureException, - debug, - defineIntegration, - getDefaultIsolationScope, - getIsolationScope, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - spanToJSON, -} from '@sentry/core'; -import { addOriginToSpan, ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; -import { DEBUG_BUILD } from '../../../debug-build'; +import { captureException, defineIntegration } from '@sentry/core'; +import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; interface KoaOptions { /** @@ -29,36 +19,6 @@ export const instrumentKoa = generateInstrumentOnce( (options: KoaOptions = {}) => { return { ignoreLayersType: options.ignoreLayersType as KoaLayerType[], - requestHook(span, info) { - addOriginToSpan(span, 'auto.http.otel.koa'); - - const attributes = spanToJSON(span).data; - - // this is one of: middleware, router - const type = attributes['koa.type']; - if (type) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.koa`); - } - - // Also update the name - const name = attributes['koa.name']; - if (typeof name === 'string') { - // Somehow, name is sometimes `''` for middleware spans - // See: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2220 - span.updateName(name || '< unknown >'); - } - - if (getIsolationScope() === getDefaultIsolationScope()) { - DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName'); - return; - } - const route = attributes[ATTR_HTTP_ROUTE]; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const method = info.context?.request?.method?.toUpperCase() || 'GET'; - if (route) { - getIsolationScope().setTransactionName(`${method} ${route}`); - } - }, } satisfies KoaInstrumentationConfig; }, ); diff --git a/packages/node/src/integrations/tracing/koa/vendored/enums/AttributeNames.ts b/packages/node/src/integrations/tracing/koa/vendored/enums/AttributeNames.ts index 28dd9a00fed2..935534e7792a 100644 --- a/packages/node/src/integrations/tracing/koa/vendored/enums/AttributeNames.ts +++ b/packages/node/src/integrations/tracing/koa/vendored/enums/AttributeNames.ts @@ -17,7 +17,6 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-koa * - Upstream version: @opentelemetry/instrumentation-koa@0.66.0 */ -/* eslint-disable */ export enum AttributeNames { KOA_TYPE = 'koa.type', diff --git a/packages/node/src/integrations/tracing/koa/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/koa/vendored/instrumentation.ts index 062f5329e084..fd1199a0949f 100644 --- a/packages/node/src/integrations/tracing/koa/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/koa/vendored/instrumentation.ts @@ -17,25 +17,43 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-koa * - Upstream version: @opentelemetry/instrumentation-koa@0.66.0 * - Minor TypeScript strictness adjustments for this repository's compiler settings + * - Span creation migrated to the @sentry/core API; op/origin/name and transaction name folded into + * span creation (previously set via a Sentry requestHook) */ -/* eslint-disable */ import * as api from '@opentelemetry/api'; +import { isWrapped, InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; + +import { KoaLayerType, type KoaInstrumentationConfig } from './types'; import { - isWrapped, - InstrumentationBase, - InstrumentationNodeModuleDefinition, - safeExecuteInTheMiddle, -} from '@opentelemetry/instrumentation'; - -import { KoaLayerType, KoaInstrumentationConfig } from './types'; -import { SDK_VERSION } from '@sentry/core'; + debug, + getDefaultIsolationScope, + getIsolationScope, + SDK_VERSION, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + startSpan, +} from '@sentry/core'; +import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; import { getMiddlewareMetadata, isLayerIgnored } from './utils'; import { setHttpServerSpanRouteAttribute } from '../../../../utils/setHttpServerSpanRouteAttribute'; -import { Next, kLayerPatched, KoaContext, KoaMiddleware, KoaPatchedMiddleware } from './internal-types'; +import { DEBUG_BUILD } from '../../../../debug-build'; +import { AttributeNames } from './enums/AttributeNames'; +import { + kLayerPatched, + type Next, + type KoaContext, + type KoaMiddleware, + type KoaPatchedMiddleware, +} from './internal-types'; const PACKAGE_NAME = '@sentry/instrumentation-koa'; +interface KoaModuleExports { + prototype: { use: KoaMiddleware }; +} + +type KoaModule = KoaModuleExports & { [Symbol.toStringTag]?: string; default?: KoaModuleExports }; + /** Koa instrumentation for OpenTelemetry */ export class KoaInstrumentation extends InstrumentationBase { constructor(config: KoaInstrumentationConfig = {}) { @@ -46,8 +64,8 @@ export class KoaInstrumentation extends InstrumentationBase=2.0.0 <4'], - (module: any) => { - const moduleExports: any = + (module: KoaModule) => { + const moduleExports = module[Symbol.toStringTag] === 'Module' ? module.default // ESM : module; // CommonJS @@ -60,12 +78,12 @@ export class KoaInstrumentation extends InstrumentationBase { - const moduleExports: any = + (module: KoaModule) => { + const moduleExports = module[Symbol.toStringTag] === 'Module' ? module.default // ESM : module; // CommonJS - if (isWrapped(moduleExports.prototype.use)) { + if (moduleExports && isWrapped(moduleExports.prototype.use)) { this._unwrap(moduleExports.prototype, 'use'); } }, @@ -77,15 +95,13 @@ export class KoaInstrumentation extends InstrumentationBase any) { - const plugin = this; - return function use(this: any, middlewareFunction: KoaMiddleware) { - let patchedFunction: KoaMiddleware; - if (middlewareFunction.router) { - patchedFunction = plugin._patchRouterDispatch(middlewareFunction); - } else { - patchedFunction = plugin._patchLayer(middlewareFunction, false); - } + private _getKoaUsePatch(original: (middleware: KoaMiddleware) => unknown) { + const patchRouterDispatch = this._patchRouterDispatch.bind(this); + const patchLayer = this._patchLayer.bind(this); + return function use(this: unknown, middlewareFunction: KoaMiddleware) { + const patchedFunction = middlewareFunction.router + ? patchRouterDispatch(middlewareFunction) + : patchLayer(middlewareFunction, false); return original.apply(this, [patchedFunction]); }; } @@ -98,18 +114,14 @@ export class KoaInstrumentation extends InstrumentationBase { + return (context: KoaContext, next: Next) => { const parent = api.trace.getSpan(api.context.active()); if (parent === undefined) { return middlewareLayer(context, next); } const metadata = getMiddlewareMetadata(context, middlewareLayer, isRouter, layerPath); - const span = this.tracer.startSpan(metadata.name, { - attributes: metadata.attributes, - }); if (context._matchedRoute) { setHttpServerSpanRouteAttribute(context._matchedRoute.toString()); } - const { requestHook } = this.getConfig(); - if (requestHook) { - safeExecuteInTheMiddle( - () => - requestHook(span, { - context, - middlewareLayer, - layerType, - }), - e => { - if (e) { - api.diag.error('koa instrumentation: request hook failed', e); - } + const koaName = metadata.attributes[AttributeNames.KOA_NAME]; + // Somehow, name is sometimes `''` for middleware spans + // See: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2220 + const name = typeof koaName === 'string' ? koaName || '< unknown >' : metadata.name; + + return startSpan( + { + name, + op: `${layerType}.koa`, + attributes: { + ...metadata.attributes, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.koa', }, - true, - ); - } - - const newContext = api.trace.setSpan(api.context.active(), span); - return api.context.with(newContext, async () => { - try { - return await middlewareLayer(context, next); - } catch (err: any) { - span.recordException(err); - throw err; - } finally { - span.end(); - } - }); + }, + () => { + const route = metadata.attributes[ATTR_HTTP_ROUTE]; + if (getIsolationScope() === getDefaultIsolationScope()) { + DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName'); + } else if (route) { + const method = (context.request as { method?: string } | undefined)?.method?.toUpperCase() || 'GET'; + getIsolationScope().setTransactionName(`${method} ${route}`); + } + return middlewareLayer(context, next); + }, + ); }; } } diff --git a/packages/node/src/integrations/tracing/koa/vendored/internal-types.ts b/packages/node/src/integrations/tracing/koa/vendored/internal-types.ts index a2039dc009cb..5eff68df669e 100644 --- a/packages/node/src/integrations/tracing/koa/vendored/internal-types.ts +++ b/packages/node/src/integrations/tracing/koa/vendored/internal-types.ts @@ -18,20 +18,19 @@ * - Upstream version: @opentelemetry/instrumentation-koa@0.66.0 * - Some types vendored from @types/koa, @types/koa-compose, and @types/koa__router with simplifications */ -/* eslint-disable */ interface DefaultState {} -export type Next = () => Promise; +export type Next = () => Promise; type ParameterizedContext<_StateT = DefaultState, ContextT = {}, _ResponseBodyT = unknown> = { - [key: string]: any; + [key: string]: unknown; } & ContextT; -type Middleware = ( +type Middleware = ( context: ParameterizedContext, next: Next, -) => any; +) => unknown; interface RouterParamContext { params: Record; @@ -42,7 +41,7 @@ interface RouterParamContext { interface Layer { path: string | RegExp; - stack: any[]; + stack: KoaMiddleware[]; } export interface Router<_StateT = DefaultState, _ContextT = {}> { diff --git a/packages/node/src/integrations/tracing/koa/vendored/types.ts b/packages/node/src/integrations/tracing/koa/vendored/types.ts index 54a79d68e252..d7a24cc75d0c 100644 --- a/packages/node/src/integrations/tracing/koa/vendored/types.ts +++ b/packages/node/src/integrations/tracing/koa/vendored/types.ts @@ -17,58 +17,18 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-koa * - Upstream version: @opentelemetry/instrumentation-koa@0.66.0 */ -/* eslint-disable */ -import { Span } from '@opentelemetry/api'; -import { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; export enum KoaLayerType { ROUTER = 'router', MIDDLEWARE = 'middleware', } -/** - * Information about the current Koa middleware layer - * The middleware layer type is any by default. - * One can install koa types packages `@types/koa` and `@types/koa__router` - * with compatible versions to the koa version used in the project - * to get more specific types for the middleware layer property. - * - * Example use in a custom attribute function: - * ```ts - * import type { Middleware, ParameterizedContext, DefaultState } from 'koa'; - * import type { RouterParamContext } from '@koa/router'; - * - * type KoaContext = ParameterizedContext; - * type KoaMiddleware = Middleware; - * - * const koaConfig: KoaInstrumentationConfig = { - * requestHook: (span: Span, info: KoaRequestInfo) => { - * // custom typescript code that can access the typed into.middlewareLayer and info.context - * } - * - */ -export type KoaRequestInfo = { - context: KoaContextType; - middlewareLayer: KoaMiddlewareType; - layerType: KoaLayerType; -}; - -/** - * Function that can be used to add custom attributes to the current span - * @param span - The Express middleware layer span. - * @param context - The current KoaContext. - */ -export interface KoaRequestCustomAttributeFunction { - (span: Span, info: KoaRequestInfo): void; -} - /** * Options available for the Koa Instrumentation (see [documentation](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-Instrumentation-koa#koa-Instrumentation-options)) */ -export interface KoaInstrumentationConfig extends InstrumentationConfig { +export interface KoaInstrumentationConfig extends InstrumentationConfig { /** Ignore specific layers based on their type */ ignoreLayersType?: KoaLayerType[]; - /** Function for adding custom attributes to each middleware layer span */ - requestHook?: KoaRequestCustomAttributeFunction; } diff --git a/packages/node/src/integrations/tracing/koa/vendored/utils.ts b/packages/node/src/integrations/tracing/koa/vendored/utils.ts index 2066afde33c5..bcbcf3de357c 100644 --- a/packages/node/src/integrations/tracing/koa/vendored/utils.ts +++ b/packages/node/src/integrations/tracing/koa/vendored/utils.ts @@ -17,12 +17,11 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-koa * - Upstream version: @opentelemetry/instrumentation-koa@0.66.0 */ -/* eslint-disable */ -import { KoaLayerType, KoaInstrumentationConfig } from './types'; -import { KoaContext, KoaMiddleware } from './internal-types'; +import { KoaLayerType, type KoaInstrumentationConfig } from './types'; +import type { KoaContext, KoaMiddleware } from './internal-types'; import { AttributeNames } from './enums/AttributeNames'; -import { Attributes } from '@opentelemetry/api'; +import type { Attributes } from '@opentelemetry/api'; import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; export const getMiddlewareMetadata = ( diff --git a/packages/node/test/integrations/tracing/koa.test.ts b/packages/node/test/integrations/tracing/koa.test.ts index 4961b0c9b82c..337c5f51a700 100644 --- a/packages/node/test/integrations/tracing/koa.test.ts +++ b/packages/node/test/integrations/tracing/koa.test.ts @@ -2,6 +2,8 @@ import { KoaInstrumentation } from '../../../src/integrations/tracing/koa/vendor import { INSTRUMENTED } from '@sentry/node-core'; import { beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest'; import { instrumentKoa, koaIntegration } from '../../../src/integrations/tracing/koa'; +import { isLayerIgnored } from '../../../src/integrations/tracing/koa/vendored/utils'; +import { KoaLayerType, type KoaInstrumentationConfig } from '../../../src/integrations/tracing/koa/vendored/types'; vi.mock('../../../src/integrations/tracing/koa/vendored/instrumentation'); @@ -27,7 +29,6 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: undefined, - requestHook: expect.any(Function), }); }); @@ -37,7 +38,6 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: ['middleware'], - requestHook: expect.any(Function), }); }); @@ -47,7 +47,6 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: ['middleware', 'router'], - requestHook: expect.any(Function), }); }); @@ -57,7 +56,6 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: undefined, - requestHook: expect.any(Function), }); }); @@ -67,7 +65,6 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: ['middleware'], - requestHook: expect.any(Function), }); }); @@ -77,7 +74,20 @@ describe('Koa', () => { expect(KoaInstrumentation).toHaveBeenCalledTimes(1); expect(KoaInstrumentation).toHaveBeenCalledWith({ ignoreLayersType: ['router', 'middleware'], - requestHook: expect.any(Function), }); }); }); + +describe('isLayerIgnored', () => { + it('does not fail with invalid config', () => { + expect(isLayerIgnored(KoaLayerType.MIDDLEWARE)).toBe(false); + expect(isLayerIgnored(KoaLayerType.MIDDLEWARE, {} as KoaInstrumentationConfig)).toBe(false); + expect(isLayerIgnored(KoaLayerType.MIDDLEWARE, { ignoreLayersType: {} } as KoaInstrumentationConfig)).toBe(false); + expect(isLayerIgnored(KoaLayerType.ROUTER, { ignoreLayersType: {} } as KoaInstrumentationConfig)).toBe(false); + }); + + it('ignores based on type', () => { + expect(isLayerIgnored(KoaLayerType.MIDDLEWARE, { ignoreLayersType: [KoaLayerType.MIDDLEWARE] })).toBe(true); + expect(isLayerIgnored(KoaLayerType.ROUTER, { ignoreLayersType: [KoaLayerType.MIDDLEWARE] })).toBe(false); + }); +});