Skip to content

Commit

Permalink
fix(instrumentation-fastify): do not wrap preClose and onRequestAbort…
Browse files Browse the repository at this point in the history
… hooks (#1764)

Fixes: #1762
  • Loading branch information
trentm authored Nov 6, 2023
1 parent 1b0caa6 commit de6156a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,18 @@ export const spanRequestSymbol = Symbol(
'opentelemetry.instrumentation.fastify.request_active_span'
);

export const applicationHookNames = [
'onRegister',
'onRoute',
'onReady',
'onClose',
];
// The instrumentation creates a span for invocations of lifecycle hook handlers
// that take `(request, reply, ...[, done])` arguments. Currently this is all
// lifecycle hooks except `onRequestAbort`.
// https://fastify.dev/docs/latest/Reference/Hooks
export const hooksNamesToWrap = new Set([
'onTimeout',
'onRequest',
'preParsing',
'preValidation',
'preSerialization',
'preHandler',
'onSend',
'onResponse',
'onError',
]);
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import type {
FastifyRequest,
FastifyReply,
} from 'fastify';
import { applicationHookNames } from './constants';
import { hooksNamesToWrap } from './constants';
import {
AttributeNames,
FastifyNames,
Expand Down Expand Up @@ -178,8 +178,8 @@ export class FastifyInstrumentation extends InstrumentationBase {
const name = args[0] as string;
const handler = args[1] as HandlerOriginal;
const pluginName = this.pluginName;
if (applicationHookNames.includes(name)) {
return original.apply(this, [name, handler] as never);
if (!hooksNamesToWrap.has(name)) {
return original.apply(this, args);
}

const syncFunctionWithDone =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,14 @@ describe('fastify', () => {
await startServer();
});

it('preClose is not instrumented', async () => {
app.addHook('preClose', () => {
assertRootContextActive();
});

await startServer();
});

it('onClose is not instrumented', async () => {
app.addHook('onClose', () => {
assertRootContextActive();
Expand Down

0 comments on commit de6156a

Please sign in to comment.