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 middleware #13065

Merged
merged 28 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
545ea04
First non-working implementation of injectable patch
nicohrubec Jul 26, 2024
743d358
Merge branch 'develop' into nh/nestjs-middleware-instrumentation
nicohrubec Jul 26, 2024
5aade8a
Add global class middleware in nest application for testing
nicohrubec Jul 26, 2024
6e35eb1
make work lol
lforst Jul 26, 2024
3a68e89
Patching use works
nicohrubec Jul 29, 2024
283a7a0
.
nicohrubec Jul 29, 2024
131caf0
Improve span attributes
nicohrubec Jul 29, 2024
f1e8218
Update description of span
nicohrubec Jul 29, 2024
4f20e34
Add test in nestjs-basic to check if middleware span exists
nicohrubec Jul 29, 2024
42c7731
Add test for node-nestjs-basic
nicohrubec Jul 29, 2024
7f8ccf0
Cleaning logs
nicohrubec Jul 29, 2024
d320894
Add docstrings
nicohrubec Jul 29, 2024
9957d5b
Improve typing
nicohrubec Jul 29, 2024
ef1d0d0
Linting
nicohrubec Jul 29, 2024
3fdcfef
.
nicohrubec Jul 29, 2024
2392da7
Do not nest created spans
nicohrubec Jul 29, 2024
b28601d
Check for proper nesting in node-nestjs-basic middleware test
nicohrubec Jul 29, 2024
453a3fd
Tests
nicohrubec Jul 29, 2024
f70c606
Check that class is not patched multiple times
nicohrubec Jul 29, 2024
30b9791
Address pr comments
nicohrubec Jul 30, 2024
02a62ee
Lint
nicohrubec Jul 30, 2024
eaac26d
Simplify generation of nest instrumentatino
nicohrubec Jul 30, 2024
c47590c
Wrap next in proxy
nicohrubec Jul 30, 2024
c3ed4ca
Revert to Object.assign
nicohrubec Jul 30, 2024
c0c3195
Update use to be proxy
nicohrubec Jul 30, 2024
7fdc402
Pass all arguments to use
nicohrubec Jul 30, 2024
72a9068
Add unit tests for isPatched
nicohrubec Jul 30, 2024
0648265
.
nicohrubec Jul 30, 2024
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
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Loading
Loading