fix(instrumentation-fastify): fix span attributes and avoid FSTDEP017…

… FastifyDeprecation warning for 404 request (#1763)

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.

Also add a test that Fastify instrumentation works for ESM usage.

Fixes: #1757
Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>

plugins/node/opentelemetry-instrumentation-fastify/.tav.yml

@@ -1,5 +1,9 @@
 "fastify":
-  - versions: "4.23.2"
+  # Sanity check the first 4.x release, instead of all releases, plus recent
+  # releases.
+  - versions: "4.0.0 || >=4.24.3 <5"
     commands: npm run test
+
+# Fastify versions after 4.18.0 require a typescript greater than 4.4.4.
 "typescript":
   - versions: "4.7.4"

plugins/node/opentelemetry-instrumentation-fastify/package.json

@@ -46,16 +46,19 @@
     "@fastify/express": "^2.0.2",
     "@opentelemetry/api": "^1.3.0",
     "@opentelemetry/context-async-hooks": "^1.8.0",
+    "@opentelemetry/contrib-test-utils": "^0.35.0",
     "@opentelemetry/instrumentation-http": "^0.45.1",
     "@opentelemetry/sdk-trace-base": "^1.8.0",
     "@opentelemetry/sdk-trace-node": "^1.8.0",
     "@types/express": "4.17.18",
     "@types/mocha": "7.0.2",
     "@types/node": "18.15.3",
+    "@types/semver": "7.5.5",
     "fastify": "4.18.0",
     "mocha": "7.2.0",
     "nyc": "15.1.0",
     "rimraf": "5.0.5",
+    "semver": "^7.5.4",
     "test-all-versions": "5.0.1",
     "ts-mocha": "10.0.0",
     "typescript": "4.4.4"

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 // since fastify@4.10.0
+        : 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 // since fastify@4.10.0
+          : request.routerPath,
       };
       if (handlerName) {
         spanAttributes[AttributeNames.FASTIFY_NAME] = handlerName;

plugins/node/opentelemetry-instrumentation-fastify/test/fixtures/use-fastify.mjs

@@ -0,0 +1,54 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Use fastify from an ES module:
+//    node --experimental-loader=@opentelemetry/instrumentation/hook.mjs use-fastify.mjs
+
+import { trace } from '@opentelemetry/api';
+import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';
+
+import { FastifyInstrumentation } from '../../build/src/index.js';
+
+const sdk = createTestNodeSdk({
+  serviceName: 'use-fastify',
+  instrumentations: [
+    new FastifyInstrumentation()
+  ]
+})
+sdk.start();
+
+import Fastify from 'fastify';
+import http from 'http';
+
+// Start a fastify server.
+const app = Fastify();
+app.get('/a-route', function aRoute(_request, reply) {
+  reply.send({ hello: 'world' });
+})
+const addr = await app.listen({ port: 0 });
+
+// Make a single request to it.
+await new Promise(resolve => {
+  http.get(addr + '/a-route', (res) => {
+    res.resume();
+    res.on('end', () => {
+      resolve();
+    });
+  })
+})
+
+await app.close();
+await sdk.shutdown();

plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts

@@ -25,13 +25,21 @@ import {
   SimpleSpanProcessor,
 } from '@opentelemetry/sdk-trace-base';
 import { Span } from '@opentelemetry/api';
+import {
+  getPackageVersion,
+  runTestFixture,
+  TestCollector,
+} from '@opentelemetry/contrib-test-utils';
+import * as semver from 'semver';
 import * as http from 'http';
 import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
 import { AttributeNames, FastifyInstrumentation } from '../src';
 import { FastifyRequestInfo } from '../src/types';
 
 const URL = require('url').URL;
 
+const fastifyVersion = getPackageVersion('fastify');
+
 const httpRequest = {
   get: (options: http.ClientRequestArgs | string) => {
     return new Promise((resolve, reject) => {
@@ -183,6 +191,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();
@@ -424,12 +449,17 @@ describe('fastify', () => {
         await startServer();
       });
 
-      it('preClose is not instrumented', async () => {
-        app.addHook('preClose', () => {
-          assertRootContextActive();
-        });
+      it('preClose is not instrumented', async function () {
+        // 'preClose' was added in fastify@4.16.0.
+        if (semver.lt(fastifyVersion, '4.16.0')) {
+          this.skip();
+        } else {
+          app.addHook('preClose', () => {
+            assertRootContextActive();
+          });
 
-        await startServer();
+          await startServer();
+        }
       });
 
       it('onClose is not instrumented', async () => {
@@ -507,4 +537,30 @@ describe('fastify', () => {
       });
     });
   });
+
+  it('should work with ESM usage', async () => {
+    await runTestFixture({
+      cwd: __dirname,
+      argv: ['fixtures/use-fastify.mjs'],
+      env: {
+        NODE_OPTIONS:
+          '--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
+        NODE_NO_WARNINGS: '1',
+      },
+      checkResult: (err, stdout, stderr) => {
+        assert.ifError(err);
+      },
+      checkCollector: (collector: TestCollector) => {
+        const spans = collector.sortedSpans;
+        assert.strictEqual(spans.length, 1);
+        assert.strictEqual(spans[0].name, 'request handler - aRoute');
+        assert.strictEqual(
+          spans[0].attributes.filter(a => a.key === 'plugin.name')[0]?.value
+            ?.stringValue,
+          'fastify',
+          'attribute plugin.name'
+        );
+      },
+    });
+  });
 });