feat(nestjs): Automatic instrumentation of nestjs guards (#13129)

nicohrubec · web-flow · commit e2668f808a27 · 2024-07-31T13:28:52.000+02:00
Adds automatic instrumentation to `@sentry/nestjs`. Guards in nest have
a `@Injectable` decorator and implement a `canActivate` function. So we
can simply extend the existing instrumentation to add a proxy for
`canActivate`.

Also fixed a mistake with the middleware instrumentation (missing
return).
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
@@ -1,5 +1,6 @@
-import { Controller, Get, Param } from '@nestjs/common';
+import { Controller, Get, Param, UseGuards } from '@nestjs/common';
 import { AppService } from './app.service';
+import { ExampleGuard } from './example.guard';
 
 @Controller()
 export class AppController {
@@ -15,6 +16,12 @@ export class AppController {
     return this.appService.testMiddleware();
   }
 
+  @Get('test-guard-instrumentation')
+  @UseGuards(ExampleGuard)
+  testGuardInstrumentation() {
+    return {};
+  }
+
   @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/example.guard.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.guard.ts
@@ -0,0 +1,10 @@
+import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
+import * as Sentry from '@sentry/nestjs';
+
+@Injectable()
+export class ExampleGuard implements CanActivate {
+  canActivate(context: ExecutionContext): boolean {
+    Sentry.startSpan({ name: 'test-guard-span' }, () => {});
+    return true;
+  }
+}
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
@@ -132,7 +132,8 @@ test('API route transaction includes nest middleware span. Spans created in and
     );
   });
 
-  await fetch(`${baseURL}/test-middleware-instrumentation`);
+  const response = await fetch(`${baseURL}/test-middleware-instrumentation`);
+  expect(response.status).toBe(200);
 
   const transactionEvent = await pageloadTransactionEventPromise;
 
@@ -200,3 +201,68 @@ test('API route transaction includes nest middleware span. Spans created in and
   // 'ExampleMiddleware' is NOT the parent of 'test-controller-span'
   expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId);
 });
+
+test('API route transaction includes nest guard span and span started in guard is nested correctly', async ({
+  baseURL,
+}) => {
+  const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'http.server' &&
+      transactionEvent?.transaction === 'GET /test-guard-instrumentation'
+    );
+  });
+
+  const response = await fetch(`${baseURL}/test-guard-instrumentation`);
+  expect(response.status).toBe(200);
+
+  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: 'ExampleGuard',
+          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 exampleGuardSpan = transactionEvent.spans.find(span => span.description === 'ExampleGuard');
+  const exampleGuardSpanId = exampleGuardSpan?.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-guard-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 testGuardSpan = transactionEvent.spans.find(span => span.description === 'test-guard-span');
+
+  // 'ExampleGuard' is the parent of 'test-guard-span'
+  expect(testGuardSpan.parent_span_id).toBe(exampleGuardSpanId);
+});
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
@@ -1,5 +1,6 @@
-import { Controller, Get, Param } from '@nestjs/common';
+import { Controller, Get, Param, UseGuards } from '@nestjs/common';
 import { AppService } from './app.service';
+import { ExampleGuard } from './example.guard';
 
 @Controller()
 export class AppController {
@@ -15,6 +16,12 @@ export class AppController {
     return this.appService.testMiddleware();
   }
 
+  @Get('test-guard-instrumentation')
+  @UseGuards(ExampleGuard)
+  testGuardInstrumentation() {
+    return {};
+  }
+
   @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/example.guard.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.guard.ts
@@ -0,0 +1,10 @@
+import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
+import * as Sentry from '@sentry/nestjs';
+
+@Injectable()
+export class ExampleGuard implements CanActivate {
+  canActivate(context: ExecutionContext): boolean {
+    Sentry.startSpan({ name: 'test-guard-span' }, () => {});
+    return true;
+  }
+}
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
@@ -125,16 +125,17 @@ 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 => {
+  const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
     return (
       transactionEvent?.contexts?.trace?.op === 'http.server' &&
       transactionEvent?.transaction === 'GET /test-middleware-instrumentation'
     );
   });
 
-  await fetch(`${baseURL}/test-middleware-instrumentation`);
+  const response = await fetch(`${baseURL}/test-middleware-instrumentation`);
+  expect(response.status).toBe(200);
 
-  const transactionEvent = await pageloadTransactionEventPromise;
+  const transactionEvent = await transactionEventPromise;
 
   expect(transactionEvent).toEqual(
     expect.objectContaining({
@@ -200,3 +201,68 @@ test('API route transaction includes nest middleware span. Spans created in and
   // 'ExampleMiddleware' is NOT the parent of 'test-controller-span'
   expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId);
 });
+
+test('API route transaction includes nest guard span and span started in guard is nested correctly', async ({
+  baseURL,
+}) => {
+  const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'http.server' &&
+      transactionEvent?.transaction === 'GET /test-guard-instrumentation'
+    );
+  });
+
+  const response = await fetch(`${baseURL}/test-guard-instrumentation`);
+  expect(response.status).toBe(200);
+
+  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: 'ExampleGuard',
+          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 exampleGuardSpan = transactionEvent.spans.find(span => span.description === 'ExampleGuard');
+  const exampleGuardSpanId = exampleGuardSpan?.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-guard-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 testGuardSpan = transactionEvent.spans.find(span => span.description === 'test-guard-span');
+
+  // 'ExampleGuard' is the parent of 'test-guard-span'
+  expect(testGuardSpan.parent_span_id).toBe(exampleGuardSpanId);
+});
diff --git a/packages/node/src/integrations/tracing/nest.ts b/packages/node/src/integrations/tracing/nest.ts
@@ -17,11 +17,13 @@ import {
   getDefaultIsolationScope,
   getIsolationScope,
   spanToJSON,
+  startSpan,
   startSpanManual,
   withActiveSpan,
 } from '@sentry/core';
 import type { IntegrationFn, Span } from '@sentry/types';
 import { addNonEnumerableProperty, logger } from '@sentry/utils';
+import type { Observable } from 'rxjs';
 import { generateInstrumentOnce } from '../../otel/instrument';
 
 interface MinimalNestJsExecutionContext {
@@ -66,7 +68,10 @@ export interface InjectableTarget {
   name: string;
   sentryPatched?: boolean;
   prototype: {
-    use?: (req: unknown, res: unknown, next: () => void) => void;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    use?: (req: unknown, res: unknown, next: () => void, ...args: any[]) => void;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    canActivate?: (...args: any[]) => boolean | Promise<boolean> | Observable<boolean>;
   };
 }
 
@@ -152,7 +157,7 @@ export class SentryNestInstrumentation extends InstrumentationBase {
                 const [req, res, next, ...args] = argsUse;
                 const prevSpan = getActiveSpan();
 
-                startSpanManual(
+                return startSpanManual(
                   {
                     name: target.name,
                     attributes: {
@@ -167,15 +172,40 @@ export class SentryNestInstrumentation extends InstrumentationBase {
 
                         if (prevSpan) {
                           withActiveSpan(prevSpan, () => {
-                            Reflect.apply(originalNext, thisArgNext, argsNext);
+                            return Reflect.apply(originalNext, thisArgNext, argsNext);
                           });
                         } else {
-                          Reflect.apply(originalNext, thisArgNext, argsNext);
+                          return Reflect.apply(originalNext, thisArgNext, argsNext);
                         }
                       },
                     });
 
-                    originalUse.apply(thisArgUse, [req, res, nextProxy, args]);
+                    return originalUse.apply(thisArgUse, [req, res, nextProxy, args]);
+                  },
+                );
+              },
+            });
+          }
+
+          // patch guards
+          if (typeof target.prototype.canActivate === 'function') {
+            // patch only once
+            if (isPatched(target)) {
+              return original(options)(target);
+            }
+
+            target.prototype.canActivate = new Proxy(target.prototype.canActivate, {
+              apply: (originalCanActivate, thisArgCanActivate, argsCanActivate) => {
+                return startSpan(
+                  {
+                    name: target.name,
+                    attributes: {
+                      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.nestjs',
+                      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.middleware.nestjs',
+                    },
+                  },
+                  () => {
+                    return originalCanActivate.apply(thisArgCanActivate, argsCanActivate);
                   },
                 );
               },
