Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nestjs): Automatic instrumentation of nestjs interceptors after route execution #13264

Merged
merged 24 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
import { flush } from '@sentry/nestjs';
import { AppService } from './app.service';
import { ExampleInterceptor1 } from './example-1.interceptor';
import { ExampleInterceptor2 } from './example-2.interceptor';
import { ExampleExceptionGlobalFilter } from './example-global-filter.exception';
import { ExampleExceptionLocalFilter } from './example-local-filter.exception';
import { ExampleLocalFilter } from './example-local.filter';
import { ExampleGuard } from './example.guard';
import { ExampleInterceptor } from './example.interceptor';

@Controller()
@UseFilters(ExampleLocalFilter)
Expand All @@ -29,7 +30,7 @@ export class AppController {
}

@Get('test-interceptor-instrumentation')
@UseInterceptors(ExampleInterceptor)
@UseInterceptors(ExampleInterceptor1, ExampleInterceptor2)
testInterceptorInstrumentation() {
return this.appService.testSpan();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common';
import * as Sentry from '@sentry/nestjs';
import { tap } from 'rxjs';

@Injectable()
export class ExampleInterceptor1 implements NestInterceptor {
intercept(context: ExecutionContext, next: CallHandler) {
Sentry.startSpan({ name: 'test-interceptor-span-1' }, () => {});
return next.handle().pipe(
tap(() => {
Sentry.startSpan({ name: 'test-interceptor-span-after-route' }, () => {});
}),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nes
import * as Sentry from '@sentry/nestjs';

@Injectable()
export class ExampleInterceptor implements NestInterceptor {
export class ExampleInterceptor2 implements NestInterceptor {
intercept(context: ExecutionContext, next: CallHandler) {
Sentry.startSpan({ name: 'test-interceptor-span' }, () => {});
Sentry.startSpan({ name: 'test-interceptor-span-2' }, () => {});
return next.handle().pipe();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ test('API route transaction includes nest pipe span for invalid request', async
);
});

test('API route transaction includes nest interceptor span. Spans created in and after interceptor are nested correctly', async ({
test('API route transaction includes nest interceptor spans before route execution. Spans created in and after interceptor are nested correctly', async ({
baseURL,
}) => {
const pageloadTransactionEventPromise = waitForTransaction('nestjs-basic', transactionEvent => {
Expand All @@ -356,6 +356,7 @@ test('API route transaction includes nest interceptor span. Spans created in and

const transactionEvent = await pageloadTransactionEventPromise;

// check if interceptor spans before route execution exist
expect(transactionEvent).toEqual(
expect.objectContaining({
spans: expect.arrayContaining([
Expand All @@ -366,7 +367,22 @@ test('API route transaction includes nest interceptor span. Spans created in and
'sentry.op': 'middleware.nestjs',
'sentry.origin': 'auto.middleware.nestjs',
},
description: 'ExampleInterceptor',
description: 'ExampleInterceptor1',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
{
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.op': 'middleware.nestjs',
'sentry.origin': 'auto.middleware.nestjs',
},
description: 'ExampleInterceptor2',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
Expand All @@ -378,9 +394,13 @@ test('API route transaction includes nest interceptor span. Spans created in and
}),
);

const exampleInterceptorSpan = transactionEvent.spans.find(span => span.description === 'ExampleInterceptor');
const exampleInterceptorSpanId = exampleInterceptorSpan?.span_id;
// get interceptor spans
const exampleInterceptor1Span = transactionEvent.spans.find(span => span.description === 'ExampleInterceptor1');
const exampleInterceptor1SpanId = exampleInterceptor1Span?.span_id;
const exampleInterceptor2Span = transactionEvent.spans.find(span => span.description === 'ExampleInterceptor2');
const exampleInterceptor2SpanId = exampleInterceptor2Span?.span_id;

// check if manually started spans exist
expect(transactionEvent).toEqual(
expect.objectContaining({
spans: expect.arrayContaining([
Expand All @@ -399,7 +419,18 @@ test('API route transaction includes nest interceptor span. Spans created in and
span_id: expect.any(String),
trace_id: expect.any(String),
data: expect.any(Object),
description: 'test-interceptor-span',
description: 'test-interceptor-span-1',
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-interceptor-span-2',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
Expand All @@ -411,12 +442,101 @@ test('API route transaction includes nest interceptor span. Spans created in and
);

// verify correct span parent-child relationships
const testInterceptorSpan = transactionEvent.spans.find(span => span.description === 'test-interceptor-span');
const testInterceptor1Span = transactionEvent.spans.find(span => span.description === 'test-interceptor-span-1');
const testInterceptor2Span = transactionEvent.spans.find(span => span.description === 'test-interceptor-span-2');
const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span');

// 'ExampleInterceptor1' is the parent of 'test-interceptor-span-1'
expect(testInterceptor1Span.parent_span_id).toBe(exampleInterceptor1SpanId);

// 'ExampleInterceptor1' is NOT the parent of 'test-controller-span'
expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptor1SpanId);

// 'ExampleInterceptor2' is the parent of 'test-interceptor-span-2'
expect(testInterceptor2Span.parent_span_id).toBe(exampleInterceptor2SpanId);

// 'ExampleInterceptor2' is NOT the parent of 'test-controller-span'
expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptor2SpanId);
});

test('API route transaction includes exactly one nest interceptor span after route execution. Spans created in controller and in interceptor are nested correctly', async ({
baseURL,
}) => {
const pageloadTransactionEventPromise = waitForTransaction('nestjs-basic', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-interceptor-instrumentation'
);
});

const response = await fetch(`${baseURL}/test-interceptor-instrumentation`);
expect(response.status).toBe(200);

const transactionEvent = await pageloadTransactionEventPromise;

// check if interceptor spans after route execution exist
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: 'Interceptor - After Route',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
]),
}),
);

// check that exactly one after route span is sent
const allInterceptorSpansAfterRoute = transactionEvent.spans.filter(
span => span.description === 'Interceptor - After Route',
);
expect(allInterceptorSpansAfterRoute.length).toBe(1);

// get interceptor span
const exampleInterceptorSpanAfterRoute = transactionEvent.spans.find(
span => span.description === 'Interceptor - After Route',
);
const exampleInterceptorSpanAfterRouteId = exampleInterceptorSpanAfterRoute?.span_id;

// check if manually started span in interceptor after route exists
expect(transactionEvent).toEqual(
expect.objectContaining({
spans: expect.arrayContaining([
{
span_id: expect.any(String),
trace_id: expect.any(String),
data: expect.any(Object),
description: 'test-interceptor-span-after-route',
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 testInterceptorSpanAfterRoute = transactionEvent.spans.find(
span => span.description === 'test-interceptor-span-after-route',
);
const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span');

// 'ExampleInterceptor' is the parent of 'test-interceptor-span'
expect(testInterceptorSpan.parent_span_id).toBe(exampleInterceptorSpanId);
// 'Interceptor - After Route' is the parent of 'test-interceptor-span-after-route'
expect(testInterceptorSpanAfterRoute.parent_span_id).toBe(exampleInterceptorSpanAfterRouteId);

// 'ExampleInterceptor' is NOT the parent of 'test-controller-span'
expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptorSpanId);
// 'Interceptor - After Route' is NOT the parent of 'test-controller-span'
expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptorSpanAfterRouteId);
});
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common';
import { flush } from '@sentry/nestjs';
import { AppService } from './app.service';
import { ExampleInterceptor1 } from './example-1.interceptor';
import { ExampleInterceptor2 } from './example-2.interceptor';
import { ExampleGuard } from './example.guard';
import { ExampleInterceptor } from './example.interceptor';

@Controller()
export class AppController {
Expand All @@ -25,7 +26,7 @@ export class AppController {
}

@Get('test-interceptor-instrumentation')
@UseInterceptors(ExampleInterceptor)
@UseInterceptors(ExampleInterceptor1, ExampleInterceptor2)
testInterceptorInstrumentation() {
return this.appService.testSpan();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common';
import * as Sentry from '@sentry/nestjs';
import { tap } from 'rxjs';

@Injectable()
export class ExampleInterceptor1 implements NestInterceptor {
intercept(context: ExecutionContext, next: CallHandler) {
Sentry.startSpan({ name: 'test-interceptor-span-1' }, () => {});
return next.handle().pipe(
tap(() => {
Sentry.startSpan({ name: 'test-interceptor-span-after-route' }, () => {});
}),
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common';
import * as Sentry from '@sentry/nestjs';

@Injectable()
export class ExampleInterceptor2 implements NestInterceptor {
intercept(context: ExecutionContext, next: CallHandler) {
Sentry.startSpan({ name: 'test-interceptor-span-2' }, () => {});
return next.handle().pipe();
}
}

This file was deleted.

Loading
Loading