feat(instrumentation-hapi): support v21 (#1985)

* update hapi bindings to v21 * fix(instrumentation-hapi): restore support for v17 to v20 * fix(instrumentation-hapi): route.options function not patched * fix: update package-lock.json * style: remove newline in test * style: convert ternary to if statements * fix: remove joi devDependency * test: test hapi 21 against node 14 * chore: add joi dev dependency to compile auto-instrumentation-node * chore: remove skipLibCheck in tsconfig.json * chore: fix package-json.lock * chore: insert missing newline Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com> --------- Co-authored-by: toddtarsi <tarsitodd@gmail.com> Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
open-telemetry · Apr 29, 2024 · eb6e8ef · eb6e8ef
1 parent 405472d
commit eb6e8ef
Show file tree

Hide file tree

Showing 8 changed files with 1,220 additions and 510 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/plugins/node/opentelemetry-instrumentation-hapi/.tav.yml b/plugins/node/opentelemetry-instrumentation-hapi/.tav.yml
@@ -5,3 +5,7 @@
   - versions: "^20.3.0"
     node: ">=14"
     commands: npm test
+  - versions: "^21.3.3"
+    node: ">=14.15.0"
+    commands: npm test
+
diff --git a/plugins/node/opentelemetry-instrumentation-hapi/README.md b/plugins/node/opentelemetry-instrumentation-hapi/README.md
@@ -21,7 +21,7 @@ npm install --save @opentelemetry/instrumentation-hapi
 
 ### Supported Versions
 
-- `>=17.0.0 <21`
+- `>=17.0.0 <22`
 
 ## Usage
 

diff --git a/plugins/node/opentelemetry-instrumentation-hapi/package.json b/plugins/node/opentelemetry-instrumentation-hapi/package.json
@@ -43,13 +43,14 @@
     "@opentelemetry/api": "^1.3.0"
   },
   "devDependencies": {
-    "@hapi/hapi": "20.1.5",
+    "@hapi/hapi": "21.3.3",
     "@opentelemetry/api": "^1.3.0",
     "@opentelemetry/context-async-hooks": "^1.8.0",
     "@opentelemetry/sdk-trace-base": "^1.8.0",
     "@opentelemetry/sdk-trace-node": "^1.8.0",
     "@types/mocha": "7.0.2",
     "@types/node": "18.6.5",
+    "joi": "17.12.2",
     "mocha": "7.2.0",
     "nyc": "15.1.0",
     "rimraf": "5.0.5",
@@ -60,8 +61,7 @@
   "dependencies": {
     "@opentelemetry/core": "^1.8.0",
     "@opentelemetry/instrumentation": "^0.51.0",
-    "@opentelemetry/semantic-conventions": "^1.0.0",
-    "@types/hapi__hapi": "20.0.13"
+    "@opentelemetry/semantic-conventions": "^1.0.0"
   },
   "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-hapi#readme"
 }
diff --git a/plugins/node/opentelemetry-instrumentation-hapi/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-hapi/src/instrumentation.ts
@@ -23,8 +23,7 @@ import {
   isWrapped,
 } from '@opentelemetry/instrumentation';
 
-// types for @hapi/hapi are published under @types/hapi__hapi
-import type * as Hapi from 'hapi__hapi';
+import type * as Hapi from '@hapi/hapi';
 import { VERSION } from './version';
 import {
   HapiComponentName,
@@ -45,6 +44,7 @@ import {
   getExtMetadata,
   isDirectExtInput,
   isPatchableExtMethod,
+  getPluginFromInput,
 } from './utils';
 
 /** Hapi instrumentation for OpenTelemetry */
@@ -56,23 +56,21 @@ export class HapiInstrumentation extends InstrumentationBase {
   protected init() {
     return new InstrumentationNodeModuleDefinition(
       HapiComponentName,
-      ['>=17 <21'],
+      ['>=17 <22'],
       (moduleExports: typeof Hapi) => {
         if (!isWrapped(moduleExports.server)) {
-          this._wrap(moduleExports, 'server', this._getServerPatch.bind(this));
+          this._wrap(
+            moduleExports,
+            'server',
+            this._getServerPatch.bind(this) as any
+          );
         }
 
-        // Casting as any is necessary here due to an issue with the @types/hapi__hapi
-        // type definition for Hapi.Server. Hapi.Server (note the uppercase) can also function
-        // as a factory function, similarly to Hapi.server (lowercase), and so should
-        // also be supported and instrumented. This is an issue with the DefinitelyTyped repo.
-        // Function is defined at: https://github.com/hapijs/hapi/blob/main/lib/index.js#L9
         if (!isWrapped(moduleExports.Server)) {
           this._wrap(
-            // eslint-disable-next-line @typescript-eslint/no-explicit-any
-            moduleExports as any,
+            moduleExports,
             'Server',
-            this._getServerPatch.bind(this)
+            this._getServerPatch.bind(this) as any
           );
         }
         return moduleExports;
@@ -145,14 +143,12 @@ export class HapiInstrumentation extends InstrumentationBase {
     ) {
       if (Array.isArray(pluginInput)) {
         for (const pluginObj of pluginInput) {
-          instrumentation._wrapRegisterHandler(
-            pluginObj.plugin?.plugin ?? pluginObj.plugin ?? pluginObj
-          );
+          const plugin = getPluginFromInput(pluginObj);
+          instrumentation._wrapRegisterHandler(plugin);
         }
       } else {
-        instrumentation._wrapRegisterHandler(
-          pluginInput.plugin?.plugin ?? pluginInput.plugin ?? pluginInput
-        );
+        const plugin = getPluginFromInput(pluginInput);
+        instrumentation._wrapRegisterHandler(plugin);
       }
       return original.apply(this, [pluginInput, options]);
     };
@@ -272,7 +268,6 @@ export class HapiInstrumentation extends InstrumentationBase {
     const oldHandler = plugin.register;
     const self = this;
     const newRegisterHandler = function (server: Hapi.Server, options: T) {
-      server.route;
       self._wrap(server, 'route', original => {
         return instrumentation._getServerRoutePatch.bind(instrumentation)(
           original,
@@ -312,7 +307,6 @@ export class HapiInstrumentation extends InstrumentationBase {
     pluginName?: string
   ): T {
     const instrumentation: HapiInstrumentation = this;
-
     if (method instanceof Array) {
       for (let i = 0; i < method.length; i++) {
         method[i] = instrumentation._wrapExtMethods(
@@ -375,13 +369,13 @@ export class HapiInstrumentation extends InstrumentationBase {
     const instrumentation: HapiInstrumentation = this;
     if (route[handlerPatched] === true) return route;
     route[handlerPatched] = true;
-    const oldHandler = route.options?.handler ?? route.handler;
-    if (typeof oldHandler === 'function') {
-      const newHandler: Hapi.Lifecycle.Method = async function (
-        ...params: Parameters<Hapi.Lifecycle.Method>
-      ) {
+
+    const wrapHandler: (
+      oldHandler: Hapi.Lifecycle.Method
+    ) => Hapi.Lifecycle.Method = oldHandler => {
+      return async function (...params: Parameters<Hapi.Lifecycle.Method>) {
         if (api.trace.getSpan(api.context.active()) === undefined) {
-          return await oldHandler(...params);
+          return await oldHandler.call(this, ...params);
         }
         const rpcMetadata = getRPCMetadata(api.context.active());
         if (rpcMetadata?.type === RPCType.HTTP) {
@@ -394,7 +388,7 @@ export class HapiInstrumentation extends InstrumentationBase {
         try {
           return await api.context.with(
             api.trace.setSpan(api.context.active(), span),
-            () => oldHandler(...params)
+            () => oldHandler.call(this, ...params)
           );
         } catch (err: any) {
           span.recordException(err);
@@ -407,11 +401,25 @@ export class HapiInstrumentation extends InstrumentationBase {
           span.end();
         }
       };
-      if (route.options?.handler) {
-        route.options.handler = newHandler;
-      } else {
-        route.handler = newHandler;
-      }
+    };
+
+    if (typeof route.handler === 'function') {
+      route.handler = wrapHandler(route.handler as Hapi.Lifecycle.Method);
+    } else if (typeof route.options === 'function') {
+      const oldOptions = route.options;
+      route.options = function (server) {
+        const options = oldOptions(server);
+        if (typeof options.handler === 'function') {
+          options.handler = wrapHandler(
+            options.handler as Hapi.Lifecycle.Method
+          );
+        }
+        return options;
+      };
+    } else if (typeof route.options?.handler === 'function') {
+      route.options.handler = wrapHandler(
+        route.options.handler as Hapi.Lifecycle.Method
+      );
     }
     return route;
   }

diff --git a/plugins/node/opentelemetry-instrumentation-hapi/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-hapi/src/internal-types.ts
@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-// types for @hapi/hapi are published under @types/hapi__hapi
-import type * as Hapi from 'hapi__hapi';
+import type * as Hapi from '@hapi/hapi';
 
 export const HapiComponentName = '@hapi/hapi';
 
@@ -35,14 +34,9 @@ export type HapiServerRouteInput =
 
 export type PatchableServerRoute = Hapi.ServerRoute<any> & {
   [handlerPatched]?: boolean;
-  options?: {
-    handler?: Hapi.Lifecycle.Method;
-  };
 };
 
-export type HapiPluginObject<T> = Hapi.ServerRegisterPluginObject<T> & {
-  plugin: Hapi.ServerRegisterPluginObject<T>;
-};
+export type HapiPluginObject<T> = Hapi.ServerRegisterPluginObject<T>;
 
 export type HapiPluginInput<T> =
   | HapiPluginObject<T>

diff --git a/plugins/node/opentelemetry-instrumentation-hapi/src/utils.ts b/plugins/node/opentelemetry-instrumentation-hapi/src/utils.ts
@@ -20,6 +20,7 @@ import type * as Hapi from '@hapi/hapi';
 import {
   HapiLayerType,
   HapiLifecycleMethodNames,
+  HapiPluginObject,
   PatchableExtMethod,
   ServerExtDirectInput,
 } from './internal-types';
@@ -119,3 +120,15 @@ export const getExtMetadata = (
     name: `ext - ${extPoint}`,
   };
 };
+
+export const getPluginFromInput = <T>(
+  pluginObj: HapiPluginObject<T>
+): Hapi.Plugin<T, void> => {
+  if ('plugin' in pluginObj) {
+    if ('plugin' in pluginObj.plugin) {
+      return pluginObj.plugin.plugin;
+    }
+    return pluginObj.plugin;
+  }
+  return pluginObj;
+};
diff --git a/plugins/node/opentelemetry-instrumentation-hapi/test/hapi.test.ts b/plugins/node/opentelemetry-instrumentation-hapi/test/hapi.test.ts
@@ -153,6 +153,52 @@ describe('Hapi Instrumentation - Core Tests', () => {
       });
     });
 
+    describe('when handler is returned by route.options function', () => {
+      it('should create a child span for single routes', async () => {
+        const rootSpan = tracer.startSpan('rootSpan');
+        server.route({
+          method: 'GET',
+          path: '/',
+          options: () => ({
+            handler: (request, h) => {
+              return 'Hello World!';
+            },
+          }),
+        });
+
+        await server.start();
+        assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+
+        await context.with(
+          trace.setSpan(context.active(), rootSpan),
+          async () => {
+            const res = await server.inject({
+              method: 'GET',
+              url: '/',
+            });
+            assert.strictEqual(res.statusCode, 200);
+
+            rootSpan.end();
+            assert.deepStrictEqual(memoryExporter.getFinishedSpans().length, 2);
+
+            const requestHandlerSpan = memoryExporter
+              .getFinishedSpans()
+              .find(span => span.name === 'route - /');
+            assert.notStrictEqual(requestHandlerSpan, undefined);
+            assert.strictEqual(
+              requestHandlerSpan?.attributes[AttributeNames.HAPI_TYPE],
+              HapiLayerType.ROUTER
+            );
+
+            const exportedRootSpan = memoryExporter
+              .getFinishedSpans()
+              .find(span => span.name === 'rootSpan');
+            assert.notStrictEqual(exportedRootSpan, undefined);
+          }
+        );
+      });
+    });
+
     it('should instrument the Hapi.Server (note: uppercase) method', async () => {
       const rootSpan = tracer.startSpan('rootSpan');
       server = new hapi.Server({