From 29452e5b700388a145b04b63cc163c93d75c1f80 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 15 May 2025 12:37:15 +0200 Subject: [PATCH 1/5] add e2e test --- .../src/events.controller.ts | 7 +++++++ .../src/events.service.ts | 7 +++++++ .../src/listeners/test-event.listener.ts | 6 ++++++ .../tests/events.test.ts | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.controller.ts index cb5ddebcc3ae..5c4c92ac5f7d 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.controller.ts @@ -11,4 +11,11 @@ export class EventsController { return { message: 'Events emitted' }; } + + @Get('emit-multiple') + async emitMultipleEvents() { + await this.eventsService.emitMultipleEvents(); + + return { message: 'Events emitted' }; + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.service.ts index 4a9f36ddaf5c..ad119106ef08 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/events.service.ts @@ -11,4 +11,11 @@ export class EventsService { return { message: 'Events emitted' }; } + + async emitMultipleEvents() { + this.eventEmitter.emit('multiple.first', { data: 'test-first' }); + this.eventEmitter.emit('multiple.second', { data: 'test-second' }); + + return { message: 'Events emitted' }; + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts index c1a3237f1f0c..d626ae1a8698 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts @@ -13,4 +13,10 @@ export class TestEventListener { await new Promise(resolve => setTimeout(resolve, 100)); throw new Error('Test error from event handler'); } + + @OnEvent('multiple.first') + @OnEvent('multiple.second') + async handleMultipleEvents(payload: any): Promise { + await new Promise(resolve => setTimeout(resolve, 100)); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts index ed4a36303efa..8df6461b89b5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts @@ -40,3 +40,21 @@ test('Event emitter', async () => { status: 'ok', }); }); + +test('Multiple OnEvent decorators', async () => { + const firstTxPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => { + return transactionEvent.transaction === 'event multiple.first'; + }); + const secondTxPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => { + return transactionEvent.transaction === 'event multiple.second'; + }); + + const eventsUrl = `http://localhost:3050/events/emit-multiple`; + await fetch(eventsUrl); + + const firstTx = await firstTxPromise; + const secondTx = await secondTxPromise; + + expect(firstTx).toBeDefined(); + expect(secondTx).toBeDefined(); +}); From 7ba1fbdb5899e11f38860f2ac2189e98cb88801a Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 15 May 2025 12:51:22 +0200 Subject: [PATCH 2/5] test payload tags --- .../src/listeners/test-event.listener.ts | 2 ++ .../nestjs-distributed-tracing/tests/events.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts index d626ae1a8698..67dccb7fc9a4 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common'; import { OnEvent } from '@nestjs/event-emitter'; +import * as Sentry from '@sentry/nestjs'; @Injectable() export class TestEventListener { @@ -17,6 +18,7 @@ export class TestEventListener { @OnEvent('multiple.first') @OnEvent('multiple.second') async handleMultipleEvents(payload: any): Promise { + Sentry.setTag('payload', payload); await new Promise(resolve => setTimeout(resolve, 100)); } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts index 8df6461b89b5..bed1293013b0 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts @@ -55,6 +55,6 @@ test('Multiple OnEvent decorators', async () => { const firstTx = await firstTxPromise; const secondTx = await secondTxPromise; - expect(firstTx).toBeDefined(); - expect(secondTx).toBeDefined(); + expect(firstTx.tags).toMatchObject({ payload: { data: 'test-first' } }); + expect(secondTx.tags).toMatchObject({ payload: { data: 'test-second' } }); }); From 6c90d23093c232a8b40c69ca1d498ff921862637 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 15 May 2025 16:22:25 +0200 Subject: [PATCH 3/5] merge event names --- .../src/listeners/test-event.listener.ts | 2 +- .../tests/events.test.ts | 14 ++++-- .../sentry-nest-event-instrumentation.ts | 46 +++++++++++++------ 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts index 67dccb7fc9a4..26d934ba384c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/src/listeners/test-event.listener.ts @@ -18,7 +18,7 @@ export class TestEventListener { @OnEvent('multiple.first') @OnEvent('multiple.second') async handleMultipleEvents(payload: any): Promise { - Sentry.setTag('payload', payload); + Sentry.setTag(payload.data, true); await new Promise(resolve => setTimeout(resolve, 100)); } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts index bed1293013b0..62781e32e37c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts @@ -43,10 +43,13 @@ test('Event emitter', async () => { test('Multiple OnEvent decorators', async () => { const firstTxPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => { - return transactionEvent.transaction === 'event multiple.first'; + return transactionEvent.transaction === 'event multiple.first|multiple.second'; }); const secondTxPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => { - return transactionEvent.transaction === 'event multiple.second'; + return transactionEvent.transaction === 'event multiple.first|multiple.second'; + }); + const rootPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => { + return transactionEvent.transaction === 'GET /events/emit-multiple'; }); const eventsUrl = `http://localhost:3050/events/emit-multiple`; @@ -54,7 +57,10 @@ test('Multiple OnEvent decorators', async () => { const firstTx = await firstTxPromise; const secondTx = await secondTxPromise; + const rootTx = await rootPromise; - expect(firstTx.tags).toMatchObject({ payload: { data: 'test-first' } }); - expect(secondTx.tags).toMatchObject({ payload: { data: 'test-second' } }); + expect(firstTx).toBeDefined(); + expect(secondTx).toBeDefined(); + // assert that the correct payloads were added + expect(rootTx.tags).toMatchObject({ 'test-first': true, 'test-second': true }); }); diff --git a/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts b/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts index 05e319e8d774..0787fea5a545 100644 --- a/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts +++ b/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts @@ -58,31 +58,46 @@ export class SentryNestEventInstrumentation extends InstrumentationBase { private _createWrapOnEvent() { // eslint-disable-next-line @typescript-eslint/no-explicit-any return function wrapOnEvent(original: any) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return function wrappedOnEvent(event: any, options?: any) { - const eventName = Array.isArray(event) - ? event.join(',') - : typeof event === 'string' || typeof event === 'symbol' - ? event.toString() - : ''; - + return function wrappedOnEvent(event: unknown, options?: unknown) { // Get the original decorator result const decoratorResult = original(event, options); // Return a new decorator function that wraps the handler - return function (target: OnEventTarget, propertyKey: string | symbol, descriptor: PropertyDescriptor) { - if (!descriptor.value || typeof descriptor.value !== 'function' || target.__SENTRY_INTERNAL__) { + return (target: OnEventTarget, propertyKey: string | symbol, descriptor: PropertyDescriptor) => { + if ( + !descriptor.value || + typeof descriptor.value !== 'function' || + target.__SENTRY_INTERNAL__ || + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + descriptor.value.__SENTRY_INSTRUMENTED__ + ) { return decoratorResult(target, propertyKey, descriptor); } - // Get the original handler const originalHandler = descriptor.value; // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const handlerName = originalHandler.name || propertyKey; + let eventName = typeof event === 'string' ? event : String(event); + + // Instrument the actual handler + descriptor.value = async function (...args: unknown[]) { + // When multiple @OnEvent decorators are used on a single method, we need to get all event names + // from the reflector metadata as there is no information during execution which event triggered it + if (Reflect.getMetadataKeys(descriptor.value).includes('EVENT_LISTENER_METADATA')) { + const eventData = Reflect.getMetadata('EVENT_LISTENER_METADATA', descriptor.value); + if (Array.isArray(eventData) && eventData.length > 0) { + eventName = eventData + .map((data: unknown) => { + if (data && typeof data === 'object' && 'event' in data && data.event) { + return data.event; + } + return ''; + }) + .reverse() // decorators are evaluated bottom to top + .join('|'); + } + } - // Instrument the handler - // eslint-disable-next-line @typescript-eslint/no-explicit-any - descriptor.value = async function (...args: any[]) { return startSpan(getEventSpanOptions(eventName), async () => { try { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -96,6 +111,9 @@ export class SentryNestEventInstrumentation extends InstrumentationBase { }); }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + descriptor.value.__SENTRY_INSTRUMENTED__ = true; + // Preserve the original function name Object.defineProperty(descriptor.value, 'name', { value: handlerName, From 42bf5bacd47bcb1368239a869b59866d6258be7b Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 16 May 2025 00:43:00 +0200 Subject: [PATCH 4/5] add import --- packages/nestjs/test/integrations/nest.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nestjs/test/integrations/nest.test.ts b/packages/nestjs/test/integrations/nest.test.ts index 58b004232449..2eecfbc6b240 100644 --- a/packages/nestjs/test/integrations/nest.test.ts +++ b/packages/nestjs/test/integrations/nest.test.ts @@ -1,3 +1,4 @@ +import 'reflect-metadata'; import * as core from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { isPatched } from '../../src/integrations/helpers'; From 9df2e343e2fc545aefa2a4700c1cf3615767a7fd Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 16 May 2025 18:27:59 +0200 Subject: [PATCH 5/5] simplify --- .../src/integrations/sentry-nest-event-instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts b/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts index 0787fea5a545..968c24a469e4 100644 --- a/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts +++ b/packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts @@ -85,7 +85,7 @@ export class SentryNestEventInstrumentation extends InstrumentationBase { // from the reflector metadata as there is no information during execution which event triggered it if (Reflect.getMetadataKeys(descriptor.value).includes('EVENT_LISTENER_METADATA')) { const eventData = Reflect.getMetadata('EVENT_LISTENER_METADATA', descriptor.value); - if (Array.isArray(eventData) && eventData.length > 0) { + if (Array.isArray(eventData)) { eventName = eventData .map((data: unknown) => { if (data && typeof data === 'object' && 'event' in data && data.event) {