diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index b54604d999cb..2a4f14cae541 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -10,6 +10,11 @@ export class AppController { return this.appService.testTransaction(); } + @Get('test-middleware-instrumentation') + testMiddlewareInstrumentation() { + return this.appService.testMiddleware(); + } + @Get('test-exception/:id') async testException(@Param('id') id: string) { return this.appService.testException(id); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index f4c5ceb0cc5a..b2aad014c745 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -1,12 +1,17 @@ -import { Module } from '@nestjs/common'; +import { MiddlewareConsumer, Module } from '@nestjs/common'; import { ScheduleModule } from '@nestjs/schedule'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], controllers: [AppController], providers: [AppService], }) -export class AppModule {} +export class AppModule { + configure(consumer: MiddlewareConsumer): void { + consumer.apply(ExampleMiddleware).forRoutes('test-middleware-instrumentation'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts index 3afb7b5147bd..9a47f0e08e7a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts @@ -21,6 +21,11 @@ export class AppService { }); } + testMiddleware() { + // span that should not be a child span of the middleware span + Sentry.startSpan({ name: 'test-controller-span' }, () => {}); + } + testException(id: string) { throw new Error(`This is an exception with id ${id}`); } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts new file mode 100644 index 000000000000..31d15c9372ea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts @@ -0,0 +1,12 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import * as Sentry from '@sentry/nestjs'; +import { NextFunction, Request, Response } from 'express'; + +@Injectable() +export class ExampleMiddleware implements NestMiddleware { + use(req: Request, res: Response, next: NextFunction) { + // span that should be a child span of the middleware span + Sentry.startSpan({ name: 'test-middleware-span' }, () => {}); + next(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts index ae5d8b63b16d..b7017b72dbf5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts @@ -121,3 +121,82 @@ test('Sends an API route transaction', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes nest middleware span. Spans created in and after middleware are nested correctly', async ({ + baseURL, +}) => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-middleware-instrumentation' + ); + }); + + await fetch(`${baseURL}/test-middleware-instrumentation`); + + const transactionEvent = await pageloadTransactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleMiddleware', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); + + const exampleMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'ExampleMiddleware'); + const exampleMiddlewareSpanId = exampleMiddlewareSpan?.span_id; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-controller-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-middleware-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + ]), + }), + ); + + // verify correct span parent-child relationships + const testMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'test-middleware-span'); + const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span'); + + // 'ExampleMiddleware' is the parent of 'test-middleware-span' + expect(testMiddlewareSpan.parent_span_id).toBe(exampleMiddlewareSpanId); + + // 'ExampleMiddleware' is NOT the parent of 'test-controller-span' + expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts index b54604d999cb..2a4f14cae541 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts @@ -10,6 +10,11 @@ export class AppController { return this.appService.testTransaction(); } + @Get('test-middleware-instrumentation') + testMiddlewareInstrumentation() { + return this.appService.testMiddleware(); + } + @Get('test-exception/:id') async testException(@Param('id') id: string) { return this.appService.testException(id); diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts index ceb7199a99cf..567dbefeadb7 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts @@ -1,11 +1,16 @@ -import { Module } from '@nestjs/common'; +import { MiddlewareConsumer, Module } from '@nestjs/common'; import { ScheduleModule } from '@nestjs/schedule'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [ScheduleModule.forRoot()], controllers: [AppController], providers: [AppService], }) -export class AppModule {} +export class AppModule { + configure(consumer: MiddlewareConsumer): void { + consumer.apply(ExampleMiddleware).forRoutes('test-middleware-instrumentation'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts index 3afb7b5147bd..9a47f0e08e7a 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts @@ -21,6 +21,11 @@ export class AppService { }); } + testMiddleware() { + // span that should not be a child span of the middleware span + Sentry.startSpan({ name: 'test-controller-span' }, () => {}); + } + testException(id: string) { throw new Error(`This is an exception with id ${id}`); } diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts new file mode 100644 index 000000000000..31d15c9372ea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts @@ -0,0 +1,12 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import * as Sentry from '@sentry/nestjs'; +import { NextFunction, Request, Response } from 'express'; + +@Injectable() +export class ExampleMiddleware implements NestMiddleware { + use(req: Request, res: Response, next: NextFunction) { + // span that should be a child span of the middleware span + Sentry.startSpan({ name: 'test-middleware-span' }, () => {}); + next(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts index ae5d8b63b16d..b7017b72dbf5 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts @@ -121,3 +121,82 @@ test('Sends an API route transaction', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes nest middleware span. Spans created in and after middleware are nested correctly', async ({ + baseURL, +}) => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-middleware-instrumentation' + ); + }); + + await fetch(`${baseURL}/test-middleware-instrumentation`); + + const transactionEvent = await pageloadTransactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleMiddleware', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); + + const exampleMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'ExampleMiddleware'); + const exampleMiddlewareSpanId = exampleMiddlewareSpan?.span_id; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-controller-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-middleware-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + ]), + }), + ); + + // verify correct span parent-child relationships + const testMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'test-middleware-span'); + const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span'); + + // 'ExampleMiddleware' is the parent of 'test-middleware-span' + expect(testMiddlewareSpan.parent_span_id).toBe(exampleMiddlewareSpanId); + + // 'ExampleMiddleware' is NOT the parent of 'test-controller-span' + expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId); +}); diff --git a/packages/node/src/integrations/tracing/nest.ts b/packages/node/src/integrations/tracing/nest.ts index ab6a66fdb895..2ec5fa840387 100644 --- a/packages/node/src/integrations/tracing/nest.ts +++ b/packages/node/src/integrations/tracing/nest.ts @@ -1,16 +1,27 @@ +import { isWrapped } from '@opentelemetry/core'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { + InstrumentationBase, + InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, +} from '@opentelemetry/instrumentation'; import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core'; import { + SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, captureException, defineIntegration, + getActiveSpan, getClient, getDefaultIsolationScope, getIsolationScope, spanToJSON, + startSpanManual, + withActiveSpan, } from '@sentry/core'; import type { IntegrationFn, Span } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { addNonEnumerableProperty, logger } from '@sentry/utils'; import { generateInstrumentOnce } from '../../otel/instrument'; interface MinimalNestJsExecutionContext { @@ -44,7 +55,155 @@ interface MinimalNestJsApp { const INTEGRATION_NAME = 'Nest'; -export const instrumentNest = generateInstrumentOnce(INTEGRATION_NAME, () => new NestInstrumentation()); +const supportedVersions = ['>=8.0.0 <11']; + +const sentryPatched = 'sentryPatched'; + +/** + * Represents an injectable target class in NestJS. + */ +export interface InjectableTarget { + name: string; + sentryPatched?: boolean; + prototype: { + use?: (req: unknown, res: unknown, next: () => void) => void; + }; +} + +/** + * Helper checking if a concrete target class is already patched. + * + * We already guard duplicate patching with isWrapped. However, isWrapped checks whether a file has been patched, whereas we use this check for concrete target classes. + * This check might not be necessary, but better to play it safe. + */ +export function isPatched(target: InjectableTarget): boolean { + if (target.sentryPatched) { + return true; + } + + addNonEnumerableProperty(target, sentryPatched, true); + return false; +} + +/** + * Custom instrumentation for nestjs. + * + * This hooks into the @Injectable decorator, which is applied on class middleware, interceptors and guards. + */ +export class SentryNestInstrumentation extends InstrumentationBase { + public static readonly COMPONENT = '@nestjs/common'; + public static readonly COMMON_ATTRIBUTES = { + component: SentryNestInstrumentation.COMPONENT, + }; + + public constructor(config: InstrumentationConfig = {}) { + super('sentry-nestjs', SDK_VERSION, config); + } + + /** + * Initializes the instrumentation by defining the modules to be patched. + */ + public init(): InstrumentationNodeModuleDefinition { + const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions); + + moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions)); + return moduleDef; + } + + /** + * Wraps the @Injectable decorator. + */ + private _getInjectableFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { + return new InstrumentationNodeModuleFile( + '@nestjs/common/decorators/core/injectable.decorator.js', + versions, + (moduleExports: { Injectable: InjectableTarget }) => { + if (isWrapped(moduleExports.Injectable)) { + this._unwrap(moduleExports, 'Injectable'); + } + this._wrap(moduleExports, 'Injectable', this._createWrapInjectable()); + return moduleExports; + }, + (moduleExports: { Injectable: InjectableTarget }) => { + this._unwrap(moduleExports, 'Injectable'); + }, + ); + } + + /** + * Creates a wrapper function for the @Injectable decorator. + * + * Wraps the use method to instrument nest class middleware. + */ + private _createWrapInjectable() { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrapInjectable(original: any) { + return function wrappedInjectable(options?: unknown) { + return function (target: InjectableTarget) { + // patch middleware + if (typeof target.prototype.use === 'function') { + // patch only once + if (isPatched(target)) { + return original(options)(target); + } + + target.prototype.use = new Proxy(target.prototype.use, { + apply: (originalUse, thisArgUse, argsUse) => { + const [req, res, next, ...args] = argsUse; + const prevSpan = getActiveSpan(); + + startSpanManual( + { + name: target.name, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.nestjs', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.middleware.nestjs', + }, + }, + (span: Span) => { + const nextProxy = new Proxy(next, { + apply: (originalNext, thisArgNext, argsNext) => { + span.end(); + + if (prevSpan) { + withActiveSpan(prevSpan, () => { + Reflect.apply(originalNext, thisArgNext, argsNext); + }); + } else { + Reflect.apply(originalNext, thisArgNext, argsNext); + } + }, + }); + + originalUse.apply(thisArgUse, [req, res, nextProxy, args]); + }, + ); + }, + }); + } + + return original(options)(target); + }; + }; + }; + } +} + +const instrumentNestCore = generateInstrumentOnce('Nest-Core', () => { + return new NestInstrumentation(); +}); + +const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { + return new SentryNestInstrumentation(); +}); + +export const instrumentNest = Object.assign( + (): void => { + instrumentNestCore(); + instrumentNestCommon(); + }, + { id: INTEGRATION_NAME }, +); const _nestIntegration = (() => { return { diff --git a/packages/node/test/integrations/tracing/nest.test.ts b/packages/node/test/integrations/tracing/nest.test.ts new file mode 100644 index 000000000000..3dc321f28008 --- /dev/null +++ b/packages/node/test/integrations/tracing/nest.test.ts @@ -0,0 +1,17 @@ +import type { InjectableTarget } from '../../../src/integrations/tracing/nest'; +import { isPatched } from '../../../src/integrations/tracing/nest'; + +describe('Nest', () => { + describe('isPatched', () => { + it('should return true if target is already patched', () => { + const target = { name: 'TestTarget', sentryPatched: true, prototype: {} }; + expect(isPatched(target)).toBe(true); + }); + + it('should add the sentryPatched property and return false if target is not patched', () => { + const target: InjectableTarget = { name: 'TestTarget', prototype: {} }; + expect(isPatched(target)).toBe(false); + expect(target.sentryPatched).toBe(true); + }); + }); +});