From 37833e0f3b76f4aad78db66005981740015f3233 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 5 Aug 2024 14:03:23 +0200 Subject: [PATCH 1/8] Implement --- .../src/integrations/tracing/nest/helpers.ts | 6 +- .../nest/sentry-nest-instrumentation.ts | 55 ++++++++++++++++++- .../src/integrations/tracing/nest/types.ts | 12 ++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/helpers.ts b/packages/node/src/integrations/tracing/nest/helpers.ts index 32eb3a0d5a39..43a0a3d9ed95 100644 --- a/packages/node/src/integrations/tracing/nest/helpers.ts +++ b/packages/node/src/integrations/tracing/nest/helpers.ts @@ -1,6 +1,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { addNonEnumerableProperty } from '@sentry/utils'; -import type { InjectableTarget } from './types'; +import type {CatchTarget, InjectableTarget} from './types'; const sentryPatched = 'sentryPatched'; @@ -10,7 +10,7 @@ const sentryPatched = 'sentryPatched'; * 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 { +export function isPatched(target: InjectableTarget | CatchTarget): boolean { if (target.sentryPatched) { return true; } @@ -23,7 +23,7 @@ export function isPatched(target: InjectableTarget): boolean { * Returns span options for nest middleware spans. */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function getMiddlewareSpanOptions(target: InjectableTarget) { +export function getMiddlewareSpanOptions(target: InjectableTarget | CatchTarget) { return { name: target.name, attributes: { diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index 52c3a4ad6b40..fdf87d5b67df 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -9,7 +9,7 @@ import { getActiveSpan, startSpan, startSpanManual, withActiveSpan } from '@sent import type { Span } from '@sentry/types'; import { SDK_VERSION } from '@sentry/utils'; import { getMiddlewareSpanOptions, isPatched } from './helpers'; -import type { InjectableTarget } from './types'; +import type {CatchTarget, InjectableTarget} from './types'; const supportedVersions = ['>=8.0.0 <11']; @@ -58,10 +58,30 @@ export class SentryNestInstrumentation extends InstrumentationBase { ); } + /** + * Wraps the @Catch decorator. + */ + private _getCatchFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { + return new InstrumentationNodeModuleFile( + '@nestjs/common/decorators/core/catch.decorator.js', + versions, + (moduleExports: { Catch: CatchTarget }) => { + if (isWrapped(moduleExports.Catch)) { + this._unwrap(moduleExports, 'Catch'); + } + this._wrap(moduleExports, 'Catch', this._createWrapCatch()); + return moduleExports; + }, + (moduleExports: { Catch: CatchTarget }) => { + this._unwrap(moduleExports, 'Catch'); + } + ) + } + + + /** * 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 @@ -177,4 +197,33 @@ export class SentryNestInstrumentation extends InstrumentationBase { }; }; } + + /** + * Creates a wrapper function for the @Catch decorator. Used to instrument exception filters. + */ + private _createWrapCatch() { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrapCatch(original: any) { + return function wrappedCatch(...exceptions: unknown[]) { + return function (target: CatchTarget) { + if (typeof target.prototype.catch === 'function' && !target.__SENTRY_INTERNAL__) { + // patch only once + if (isPatched(target)) { + return original(exceptions)(target); + } + + target.prototype.catch = new Proxy(target.prototype.catch, { + apply: (originalCatch, thisArgCatch, argsCatch) => { + return startSpan(getMiddlewareSpanOptions(target), () => { + return originalCatch.apply(thisArgCatch, argsCatch); + }) + } + }) + } + + return original(exceptions)(target); + } + } + } + } } diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 2cdd1b6aefaf..e948ba3bff93 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -55,3 +55,15 @@ export interface InjectableTarget { intercept?: (context: unknown, next: CallHandler, ...args: any[]) => Observable; }; } + +/** + * Represents a target class in NestJS annotated with @Catch. + */ +export interface CatchTarget { + name: string, + sentryPatched?: boolean; + __SENTRY_INTERNAL__?: boolean; + prototype: { + catch?: (...args: any[]) => any; + } +} From 36a199fdc6e1a30a7b9a234c57ea4470c86a293b Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 5 Aug 2024 14:33:38 +0200 Subject: [PATCH 2/8] Lint + logs --- .../nestjs-basic/tests/transactions.test.ts | 24 +++++++++++++++++++ .../tests/transactions.test.ts | 24 +++++++++++++++++++ .../src/integrations/tracing/nest/helpers.ts | 2 +- .../nest/sentry-nest-instrumentation.ts | 20 +++++++--------- .../src/integrations/tracing/nest/types.ts | 4 ++-- 5 files changed, 60 insertions(+), 14 deletions(-) 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 555b6357ade8..563e39c45b3b 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 @@ -11,8 +11,12 @@ test('Sends an API route transaction', async ({ baseURL }) => { await fetch(`${baseURL}/test-transaction`); + console.log('fetch returned'); + const transactionEvent = await pageloadTransactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent.contexts?.trace).toEqual({ data: { 'sentry.source': 'route', @@ -135,8 +139,12 @@ test('API route transaction includes nest middleware span. Spans created in and const response = await fetch(`${baseURL}/test-middleware-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await pageloadTransactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -215,8 +223,12 @@ test('API route transaction includes nest guard span and span started in guard i const response = await fetch(`${baseURL}/test-guard-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -279,8 +291,12 @@ test('API route transaction includes nest pipe span for valid request', async ({ const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -316,8 +332,12 @@ test('API route transaction includes nest pipe span for invalid request', async const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`); expect(response.status).toBe(400); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -354,8 +374,12 @@ test('API route transaction includes nest interceptor span. Spans created in and const response = await fetch(`${baseURL}/test-interceptor-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await pageloadTransactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ 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 cb04bc06839e..fcf7bdf91d7a 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 @@ -11,8 +11,12 @@ test('Sends an API route transaction', async ({ baseURL }) => { await fetch(`${baseURL}/test-transaction`); + console.log('fetch returned'); + const transactionEvent = await pageloadTransactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent.contexts?.trace).toEqual({ data: { 'sentry.source': 'route', @@ -135,8 +139,12 @@ test('API route transaction includes nest middleware span. Spans created in and const response = await fetch(`${baseURL}/test-middleware-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -215,8 +223,12 @@ test('API route transaction includes nest guard span and span started in guard i const response = await fetch(`${baseURL}/test-guard-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -278,8 +290,12 @@ test('API route transaction includes nest pipe span for valid request', async ({ const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -314,8 +330,12 @@ test('API route transaction includes nest pipe span for invalid request', async const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`); expect(response.status).toBe(400); + console.log('fetch returned'); + const transactionEvent = await transactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -352,8 +372,12 @@ test('API route transaction includes nest interceptor span. Spans created in and const response = await fetch(`${baseURL}/test-interceptor-instrumentation`); expect(response.status).toBe(200); + console.log('fetch returned'); + const transactionEvent = await pageloadTransactionEventPromise; + console.log('transaction returned'); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ diff --git a/packages/node/src/integrations/tracing/nest/helpers.ts b/packages/node/src/integrations/tracing/nest/helpers.ts index 43a0a3d9ed95..babf80022c1f 100644 --- a/packages/node/src/integrations/tracing/nest/helpers.ts +++ b/packages/node/src/integrations/tracing/nest/helpers.ts @@ -1,6 +1,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { addNonEnumerableProperty } from '@sentry/utils'; -import type {CatchTarget, InjectableTarget} from './types'; +import type { CatchTarget, InjectableTarget } from './types'; const sentryPatched = 'sentryPatched'; diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index fdf87d5b67df..b243a713c3ea 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -9,7 +9,7 @@ import { getActiveSpan, startSpan, startSpanManual, withActiveSpan } from '@sent import type { Span } from '@sentry/types'; import { SDK_VERSION } from '@sentry/utils'; import { getMiddlewareSpanOptions, isPatched } from './helpers'; -import type {CatchTarget, InjectableTarget} from './types'; +import type { CatchTarget, InjectableTarget } from './types'; const supportedVersions = ['>=8.0.0 <11']; @@ -74,12 +74,10 @@ export class SentryNestInstrumentation extends InstrumentationBase { }, (moduleExports: { Catch: CatchTarget }) => { this._unwrap(moduleExports, 'Catch'); - } - ) + }, + ); } - - /** * Creates a wrapper function for the @Injectable decorator. */ @@ -216,14 +214,14 @@ export class SentryNestInstrumentation extends InstrumentationBase { apply: (originalCatch, thisArgCatch, argsCatch) => { return startSpan(getMiddlewareSpanOptions(target), () => { return originalCatch.apply(thisArgCatch, argsCatch); - }) - } - }) + }); + }, + }); } return original(exceptions)(target); - } - } - } + }; + }; + }; } } diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index e948ba3bff93..42aa0b003315 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -60,10 +60,10 @@ export interface InjectableTarget { * Represents a target class in NestJS annotated with @Catch. */ export interface CatchTarget { - name: string, + name: string; sentryPatched?: boolean; __SENTRY_INTERNAL__?: boolean; prototype: { catch?: (...args: any[]) => any; - } + }; } From a50026df83c9f907eee2a4f2db32ea07e8c34bb8 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 11:34:17 +0200 Subject: [PATCH 3/8] Fix --- .../integrations/tracing/nest/sentry-nest-instrumentation.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index b243a713c3ea..d4c551bdad66 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -34,7 +34,7 @@ export class SentryNestInstrumentation extends InstrumentationBase { public init(): InstrumentationNodeModuleDefinition { const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions); - moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions)); + moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions), this._getCatchFileInstrumentation(supportedVersions)); return moduleDef; } @@ -205,6 +205,7 @@ export class SentryNestInstrumentation extends InstrumentationBase { return function wrappedCatch(...exceptions: unknown[]) { return function (target: CatchTarget) { if (typeof target.prototype.catch === 'function' && !target.__SENTRY_INTERNAL__) { + console.log('patching exception filter!'); // patch only once if (isPatched(target)) { return original(exceptions)(target); @@ -212,6 +213,7 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.catch = new Proxy(target.prototype.catch, { apply: (originalCatch, thisArgCatch, argsCatch) => { + console.log('exception filter!'); return startSpan(getMiddlewareSpanOptions(target), () => { return originalCatch.apply(thisArgCatch, argsCatch); }); From 1ad6c4f036689b4e4bf2862f2700d765f5e7874d Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 12:17:21 +0200 Subject: [PATCH 4/8] Remove logs --- .../nestjs-basic/tests/transactions.test.ts | 24 ------------------- .../tests/transactions.test.ts | 24 ------------------- 2 files changed, 48 deletions(-) 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 563e39c45b3b..555b6357ade8 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 @@ -11,12 +11,8 @@ test('Sends an API route transaction', async ({ baseURL }) => { await fetch(`${baseURL}/test-transaction`); - console.log('fetch returned'); - const transactionEvent = await pageloadTransactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent.contexts?.trace).toEqual({ data: { 'sentry.source': 'route', @@ -139,12 +135,8 @@ test('API route transaction includes nest middleware span. Spans created in and const response = await fetch(`${baseURL}/test-middleware-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await pageloadTransactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -223,12 +215,8 @@ test('API route transaction includes nest guard span and span started in guard i const response = await fetch(`${baseURL}/test-guard-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -291,12 +279,8 @@ test('API route transaction includes nest pipe span for valid request', async ({ const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -332,12 +316,8 @@ test('API route transaction includes nest pipe span for invalid request', async const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`); expect(response.status).toBe(400); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -374,12 +354,8 @@ test('API route transaction includes nest interceptor span. Spans created in and const response = await fetch(`${baseURL}/test-interceptor-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await pageloadTransactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ 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 fcf7bdf91d7a..cb04bc06839e 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 @@ -11,12 +11,8 @@ test('Sends an API route transaction', async ({ baseURL }) => { await fetch(`${baseURL}/test-transaction`); - console.log('fetch returned'); - const transactionEvent = await pageloadTransactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent.contexts?.trace).toEqual({ data: { 'sentry.source': 'route', @@ -139,12 +135,8 @@ test('API route transaction includes nest middleware span. Spans created in and const response = await fetch(`${baseURL}/test-middleware-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -223,12 +215,8 @@ test('API route transaction includes nest guard span and span started in guard i const response = await fetch(`${baseURL}/test-guard-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -290,12 +278,8 @@ test('API route transaction includes nest pipe span for valid request', async ({ const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -330,12 +314,8 @@ test('API route transaction includes nest pipe span for invalid request', async const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`); expect(response.status).toBe(400); - console.log('fetch returned'); - const transactionEvent = await transactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -372,12 +352,8 @@ test('API route transaction includes nest interceptor span. Spans created in and const response = await fetch(`${baseURL}/test-interceptor-instrumentation`); expect(response.status).toBe(200); - console.log('fetch returned'); - const transactionEvent = await pageloadTransactionEventPromise; - console.log('transaction returned'); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ From 420f47ff3ed68b25497d555c7aadc8497592cd28 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 13:39:56 +0200 Subject: [PATCH 5/8] Fix --- .../example.filter.ts | 1 + .../tests/transactions.test.ts | 17 +++++++++++++++++ .../tracing/nest/sentry-nest-instrumentation.ts | 6 +++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts index 848441caf855..a85802c0f6a2 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts @@ -6,6 +6,7 @@ import { ExampleException } from './example.exception'; export class ExampleExceptionFilter extends BaseExceptionFilter { catch(exception: unknown, host: ArgumentsHost) { if (exception instanceof ExampleException) { + console.log('catch example exception!'); return super.catch(new BadRequestException(exception.message), host); } return super.catch(exception, host); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index 887284585ae1..bdfee9361edb 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -121,3 +121,20 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes exception filter span', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /expected-exception' && + transactionEvent?.request?.url?.includes('/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + console.log(transactionEvent); +}); diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index d4c551bdad66..58fd68b18522 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -208,20 +208,20 @@ export class SentryNestInstrumentation extends InstrumentationBase { console.log('patching exception filter!'); // patch only once if (isPatched(target)) { - return original(exceptions)(target); + return original(...exceptions)(target); } target.prototype.catch = new Proxy(target.prototype.catch, { apply: (originalCatch, thisArgCatch, argsCatch) => { - console.log('exception filter!'); return startSpan(getMiddlewareSpanOptions(target), () => { + console.log('exception filter!'); return originalCatch.apply(thisArgCatch, argsCatch); }); }, }); } - return original(exceptions)(target); + return original(...exceptions)(target); }; }; }; From c4296d04b6cf8c43b3e8b6dbcc9f007b3adf7b98 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 14:01:12 +0200 Subject: [PATCH 6/8] Fix tests --- .../example.filter.ts | 1 - .../tests/transactions.test.ts | 26 +++++++++++++++++-- .../nest/sentry-nest-instrumentation.ts | 7 ++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts index a85802c0f6a2..848441caf855 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter/example.filter.ts @@ -6,7 +6,6 @@ import { ExampleException } from './example.exception'; export class ExampleExceptionFilter extends BaseExceptionFilter { catch(exception: unknown, host: ArgumentsHost) { if (exception instanceof ExampleException) { - console.log('catch example exception!'); return super.catch(new BadRequestException(exception.message), host); } return super.catch(exception, host); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index bdfee9361edb..fc561512a681 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -126,8 +126,8 @@ test('API route transaction includes exception filter span', async ({ baseURL }) const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && - transactionEvent?.transaction === 'GET /expected-exception' && - transactionEvent?.request?.url?.includes('/expected-exception') + transactionEvent?.transaction === 'GET /example-module/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module/expected-exception') ); }); @@ -137,4 +137,26 @@ test('API route transaction includes exception filter span', async ({ baseURL }) const transactionEvent = await transactionEventPromise; console.log(transactionEvent); + + 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: 'ExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); }); diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index 58fd68b18522..28d5a74ef63d 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -34,7 +34,10 @@ export class SentryNestInstrumentation extends InstrumentationBase { public init(): InstrumentationNodeModuleDefinition { const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions); - moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions), this._getCatchFileInstrumentation(supportedVersions)); + moduleDef.files.push( + this._getInjectableFileInstrumentation(supportedVersions), + this._getCatchFileInstrumentation(supportedVersions), + ); return moduleDef; } @@ -205,7 +208,6 @@ export class SentryNestInstrumentation extends InstrumentationBase { return function wrappedCatch(...exceptions: unknown[]) { return function (target: CatchTarget) { if (typeof target.prototype.catch === 'function' && !target.__SENTRY_INTERNAL__) { - console.log('patching exception filter!'); // patch only once if (isPatched(target)) { return original(...exceptions)(target); @@ -214,7 +216,6 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.catch = new Proxy(target.prototype.catch, { apply: (originalCatch, thisArgCatch, argsCatch) => { return startSpan(getMiddlewareSpanOptions(target), () => { - console.log('exception filter!'); return originalCatch.apply(thisArgCatch, argsCatch); }); }, From 3f14f43b5921ce8941cc0d49e4b50a1d36679495 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 14:18:19 +0200 Subject: [PATCH 7/8] Add test --- .../tests/transactions.test.ts | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index fc561512a681..9217249faad0 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -122,7 +122,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { ); }); -test('API route transaction includes exception filter span', async ({ baseURL }) => { +test('API route transaction includes exception filter span for global filter', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && @@ -136,8 +136,6 @@ test('API route transaction includes exception filter span', async ({ baseURL }) const transactionEvent = await transactionEventPromise; - console.log(transactionEvent); - expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ @@ -160,3 +158,40 @@ test('API route transaction includes exception filter span', async ({ baseURL }) }), ); }); + +test('API route transaction includes exception filter span for local filter', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module-local-filter/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + 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: 'LocalExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); From 0cf602438bc79d351b4dab661503b343aab0b92f Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 6 Aug 2024 16:56:59 +0200 Subject: [PATCH 8/8] sentry internals --- packages/nestjs/src/setup.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index b068ed052a91..5e76f5cbe912 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -64,6 +64,8 @@ export { SentryTracingInterceptor }; * Global filter to handle exceptions and report them to Sentry. */ class SentryGlobalFilter extends BaseExceptionFilter { + public static readonly __SENTRY_INTERNAL__ = true; + /** * Catches exceptions and reports them to Sentry unless they are expected errors. */ @@ -84,6 +86,8 @@ export { SentryGlobalFilter }; * Service to set up Sentry performance tracing for Nest.js applications. */ class SentryService implements OnModuleInit { + public static readonly __SENTRY_INTERNAL__ = true; + /** * Initializes the Sentry service and registers span attributes. */