Skip to content

Commit

Permalink
feat(fastify): Update scope transactionName when handling request (g…
Browse files Browse the repository at this point in the history
…etsentry#11447)

This PR updates the isolation scope's `transactionName` by adding another
hook callback in our `setupFastifyErrorHandler` fastify plugin. This is
better than accessing the otel span for two reasons:

1. We can always update the transactionName, not just when a span is
sampled and recording
2. The `onRequest` hook is executed earlier in the lifecycle than the
`preHandler` hook that Otel uses to trigger the `requestHook` callback.
  • Loading branch information
Lms24 authored and cadesalaberry committed Apr 19, 2024
1 parent b429c86 commit 95b88ac
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ app.get('/test-error', async function (req, res) {
res.send({ exceptionId });
});

app.get('/test-exception', async function (req, res) {
throw new Error('This is an exception');
app.get('/test-exception/:id', async function (req, res) {
throw new Error(`This is an exception with id ${req.params.id}`);
});

app.get('/test-outgoing-fetch-external-allowed', async function (req, res) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,28 @@ test('Sends exception to Sentry', async ({ baseURL }) => {

test('Sends correct error event', async ({ baseURL }) => {
const errorEventPromise = waitForError('node-fastify-app', event => {
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception';
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
});

try {
await axios.get(`${baseURL}/test-exception`);
await axios.get(`${baseURL}/test-exception/123`);
} catch {
// this results in an error, but we don't care - we want to check the error event
}

const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception');
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
headers: expect.any(Object),
url: 'http://localhost:3030/test-exception',
url: 'http://localhost:3030/test-exception/123',
});

expect(errorEvent.transaction).toEqual('GET /test-exception');
expect(errorEvent.transaction).toEqual('GET /test-exception/:id');

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.any(String),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ test('Sends an API route transaction', async ({ baseURL }) => {
expect(transactionEvent).toEqual(
expect.objectContaining({
spans: [
{
data: {
'plugin.name': 'fastify -> sentry-fastify-error-handler',
'fastify.type': 'middleware',
'hook.name': 'onRequest',
'otel.kind': 'INTERNAL',
'sentry.origin': 'manual',
},
description: 'middleware - fastify -> sentry-fastify-error-handler',
parent_span_id: expect.any(String),
span_id: expect.any(String),
start_timestamp: expect.any(Number),
status: 'ok',
timestamp: expect.any(Number),
trace_id: expect.any(String),
origin: 'manual',
},
{
data: {
'plugin.name': 'fastify -> sentry-fastify-error-handler',
Expand Down
29 changes: 28 additions & 1 deletion packages/node/src/integrations/tracing/fastify.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { FastifyInstrumentation } from '@opentelemetry/instrumentation-fastify';
import { captureException, defineIntegration } from '@sentry/core';
import { captureException, defineIntegration, getIsolationScope } from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';

import { addOriginToSpan } from '../../utils/addOriginToSpan';
Expand Down Expand Up @@ -35,6 +35,19 @@ interface Fastify {
addHook: (hook: string, handler: (request: unknown, reply: unknown, error: Error) => void) => void;
}

/**
* Minimal request type containing properties around route information.
* Works for Fastify 3, 4 and presumably 5.
*/
interface FastifyRequestRouteInfo {
// since fastify@4.10.0
routeOptions?: {
url?: string;
method?: string;
};
routerPath?: string;
}

/**
* Setup an error handler for Fastify.
*/
Expand All @@ -45,6 +58,20 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
captureException(error);
});

// registering `onRequest` hook here instead of using Otel `onRequest` callback b/c `onRequest` hook
// is ironically called in the fastify `preHandler` hook which is called later in the lifecycle:
// https://fastify.dev/docs/latest/Reference/Lifecycle/
fastify.addHook('onRequest', async (request, _reply) => {
const reqWithRouteInfo = request as FastifyRequestRouteInfo;

// Taken from Otel Fastify instrumentation:
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts#L94-L96
const routeName = reqWithRouteInfo.routeOptions?.url || reqWithRouteInfo.routerPath;
const method = reqWithRouteInfo.routeOptions?.method || 'GET';

getIsolationScope().setTransactionName(`${method} ${routeName}`);
});

done();
},
{
Expand Down

0 comments on commit 95b88ac

Please sign in to comment.