Skip to content

Commit

Permalink
feat(nestjs): Automatic instrumentation of nestjs pipes (#13137)
Browse files Browse the repository at this point in the history
Adds automatic instrumentation of pipes to `@sentry/nestjs`. Pipes in
nest have a `@Injectable` decorator and implement a `transform`
function. So we can simply extend the existing instrumentation to add a
proxy for `transform`.
  • Loading branch information
nicohrubec authored Aug 1, 2024
1 parent 2306458 commit bfb7ec4
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Controller, Get, Param, UseGuards } from '@nestjs/common';
import { Controller, Get, Param, ParseIntPipe, UseGuards } from '@nestjs/common';
import { AppService } from './app.service';
import { ExampleGuard } from './example.guard';

Expand All @@ -22,6 +22,11 @@ export class AppController {
return {};
}

@Get('test-pipe-instrumentation/:id')
testPipeInstrumentation(@Param('id', ParseIntPipe) id: number) {
return { value: id };
}

@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
Expand Up @@ -266,3 +266,75 @@ test('API route transaction includes nest guard span and span started in guard i
// 'ExampleGuard' is the parent of 'test-guard-span'
expect(testGuardSpan.parent_span_id).toBe(exampleGuardSpanId);
});

test('API route transaction includes nest pipe span for valid request', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id'
);
});

const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`);
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: 'ParseIntPipe',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
]),
}),
);
});

test('API route transaction includes nest pipe span for invalid request', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id'
);
});

const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`);
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: 'ParseIntPipe',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'unknown_error',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
]),
}),
);
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Controller, Get, Param, UseGuards } from '@nestjs/common';
import { Controller, Get, Param, ParseIntPipe, UseGuards } from '@nestjs/common';
import { AppService } from './app.service';
import { ExampleGuard } from './example.guard';

Expand All @@ -22,6 +22,11 @@ export class AppController {
return {};
}

@Get('test-pipe-instrumentation/:id')
testPipeInstrumentation(@Param('id', ParseIntPipe) id: number) {
return { value: id };
}

@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
Expand Up @@ -266,3 +266,75 @@ test('API route transaction includes nest guard span and span started in guard i
// 'ExampleGuard' is the parent of 'test-guard-span'
expect(testGuardSpan.parent_span_id).toBe(exampleGuardSpanId);
});

test('API route transaction includes nest pipe span for valid request', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id'
);
});

const response = await fetch(`${baseURL}/test-pipe-instrumentation/123`);
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: 'ParseIntPipe',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
]),
}),
);
});

test('API route transaction includes nest pipe span for invalid request', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id'
);
});

const response = await fetch(`${baseURL}/test-pipe-instrumentation/abc`);
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: 'ParseIntPipe',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'unknown_error',
op: 'middleware.nestjs',
origin: 'auto.middleware.nestjs',
},
]),
}),
);
});
26 changes: 26 additions & 0 deletions packages/node/src/integrations/tracing/nest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export interface InjectableTarget {
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>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
transform?: (...args: any[]) => any;
};
}

Expand Down Expand Up @@ -212,6 +214,30 @@ export class SentryNestInstrumentation extends InstrumentationBase {
});
}

// patch pipes
if (typeof target.prototype.transform === 'function') {
if (isPatched(target)) {
return original(options)(target);
}

target.prototype.transform = new Proxy(target.prototype.transform, {
apply: (originalTransform, thisArgTransform, argsTransform) => {
return startSpan(
{
name: target.name,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.nestjs',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.middleware.nestjs',
},
},
() => {
return originalTransform.apply(thisArgTransform, argsTransform);
},
);
},
});
}

return original(options)(target);
};
};
Expand Down

0 comments on commit bfb7ec4

Please sign in to comment.