From de6156aea1db7a7a018ad34f08cfc9f7ff7752b8 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Mon, 6 Nov 2023 00:56:47 -0800 Subject: [PATCH] fix(instrumentation-fastify): do not wrap preClose and onRequestAbort hooks (#1764) Fixes: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1762 --- .../src/constants.ts | 21 +++++++++++++------ .../src/instrumentation.ts | 6 +++--- .../test/instrumentation.test.ts | 8 +++++++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-fastify/src/constants.ts b/plugins/node/opentelemetry-instrumentation-fastify/src/constants.ts index e6e0ebedae1..4f2efd02d51 100644 --- a/plugins/node/opentelemetry-instrumentation-fastify/src/constants.ts +++ b/plugins/node/opentelemetry-instrumentation-fastify/src/constants.ts @@ -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', +]); diff --git a/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts index ba96a020dd0..e410b93badd 100644 --- a/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts @@ -33,7 +33,7 @@ import type { FastifyRequest, FastifyReply, } from 'fastify'; -import { applicationHookNames } from './constants'; +import { hooksNamesToWrap } from './constants'; import { AttributeNames, FastifyNames, @@ -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 = diff --git a/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts b/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts index 90a881a3213..2efccaf724b 100644 --- a/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts +++ b/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts @@ -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();