fix(fastify): fix span attributes and avoid FSTDEP017 FastifyDeprecat…

…ion warning for 404 request For a 404 `request.routeOptions.url` is undefined. Since fastify@4.10.0 when routeOptions was added, we shouldn't fallback to the deprecated request.routerPath. This also corrects the assumption that the handler name is "bound ..." in all cases. E.g. for a 404 it is Fastify's core "basic404" internal function. Fixes: open-telemetry#1757
trentm · Nov 1, 2023 · 22793d3 · 22793d3
1 parent 1b0caa6
commit 22793d3
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 7 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-fastify/package.json b/plugins/node/opentelemetry-instrumentation-fastify/package.json
@@ -55,7 +55,7 @@
     "@types/express": "4.17.18",
     "@types/mocha": "7.0.2",
     "@types/node": "18.15.3",
-    "fastify": "4.18.0",
+    "fastify": "4.24.3",
     "mocha": "7.2.0",
     "nyc": "15.1.0",
     "rimraf": "5.0.5",

diff --git a/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts
@@ -96,8 +96,9 @@ export class FastifyInstrumentation extends InstrumentationBase {
       const anyRequest = request as any;
 
       const rpcMetadata = getRPCMetadata(context.active());
-      const routeName =
-        anyRequest.routeOptions?.config?.url || request.routerPath;
+      const routeName = anyRequest.routeOptions
+        ? anyRequest.routeOptions.url
+        : request.routerPath;
       if (routeName && rpcMetadata?.type === RPCType.HTTP) {
         rpcMetadata.route = routeName;
       }
@@ -265,18 +266,21 @@ export class FastifyInstrumentation extends InstrumentationBase {
       const anyRequest = request as any;
 
       const handler =
-        anyRequest.routeOptions?.handler || anyRequest.context?.handler || {};
+        anyRequest.routeOptions?.handler || anyRequest.context?.handler;
 
-      const handlerName = handler?.name.substr(6);
+      const handlerName = handler?.name.startsWith('bound ')
+        ? handler.name.substr(6)
+        : handler?.name;
       const spanName = `${FastifyNames.REQUEST_HANDLER} - ${
         handlerName || this.pluginName || ANONYMOUS_NAME
       }`;
 
       const spanAttributes: SpanAttributes = {
         [AttributeNames.PLUGIN_NAME]: this.pluginName,
         [AttributeNames.FASTIFY_TYPE]: FastifyTypes.REQUEST_HANDLER,
-        [SemanticAttributes.HTTP_ROUTE]:
-          anyRequest.routeOptions?.config?.url || request.routerPath,
+        [SemanticAttributes.HTTP_ROUTE]: anyRequest.routeOptions
+          ? anyRequest.routeOptions.url
+          : request.routerPath,
       };
       if (handlerName) {
         spanAttributes[AttributeNames.FASTIFY_NAME] = handlerName;

diff --git a/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts b/plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts
@@ -183,6 +183,23 @@ describe('fastify', () => {
       assert.strictEqual(span.parentSpanId, baseSpan.spanContext().spanId);
     });
 
+    it('should generate span for 404 request', async () => {
+      await startServer();
+      await httpRequest.get(`http://localhost:${PORT}/no-such-route`);
+
+      const spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 5);
+      const span = spans[2];
+      assert.deepStrictEqual(span.attributes, {
+        'fastify.name': 'basic404',
+        'fastify.type': 'request_handler',
+        'plugin.name': 'fastify -> @fastify/express',
+      });
+      assert.strictEqual(span.name, 'request handler - basic404');
+      const baseSpan = spans[1];
+      assert.strictEqual(span.parentSpanId, baseSpan.spanContext().spanId);
+    });
+
     describe('when subsystem is registered', () => {
       beforeEach(async () => {
         httpInstrumentation.enable();