feat: support async and promise-returning handlers

plugins/node/opentelemetry-instrumentation-restify/src/instrumentation.ts

@@ -27,7 +27,7 @@ import {
   isWrapped,
 } from '@opentelemetry/instrumentation';
 import { HttpAttribute } from '@opentelemetry/semantic-conventions';
-import { types as checkType } from 'util';
+import { isPromise, isAsyncFunction } from './utils';
 
 const { diag } = api;
 
@@ -56,7 +56,9 @@ export class RestifyInstrumentation extends InstrumentationBase<
         'restify/lib/server.js',
         constants.SUPPORTED_VERSIONS,
         (moduleExports, moduleVersion) => {
-          diag.debug(`Applying patch for ${constants.MODULE_NAME}@${moduleVersion}`);
+          diag.debug(
+            `Applying patch for ${constants.MODULE_NAME}@${moduleVersion}`
+          );
           this._isDisabled = false;
           const Server: any = moduleExports;
           for (const name of constants.RESTIFY_METHODS) {
@@ -82,7 +84,9 @@ export class RestifyInstrumentation extends InstrumentationBase<
           return moduleExports;
         },
         (moduleExports, moduleVersion) => {
-          diag.debug(`Removing patch for ${constants.MODULE_NAME}@${moduleVersion}`);
+          diag.debug(
+            `Removing patch for ${constants.MODULE_NAME}@${moduleVersion}`
+          );
           this._isDisabled = true;
           if (moduleExports) {
             const Server: any = moduleExports;
@@ -200,24 +204,26 @@ export class RestifyInstrumentation extends InstrumentationBase<
 
         const wrapPromise = (promise: Promise<unknown>) => {
           return promise
-            .catch((err) => {
-              span.recordException(err);
-              throw err;
+            .then(value => {
+              span.end();
+              return value;
             })
-            .finally(() => {
+            .catch(err => {
+              span.recordException(err);
               span.end();
+              throw err;
             });
-        }
+        };
 
         return api.context.with(
           api.setSpan(api.context.active(), span),
           (req: types.Request, res: restify.Response, next: restify.Next) => {
-            if (checkType.isAsyncFunction(handler)) {
+            if (isAsyncFunction(handler)) {
               return wrapPromise(handler(req, res, next));
             }
             try {
               const result = handler(req, res, next);
-              if (checkType.isPromise(result)) {
+              if (isPromise(result)) {
                 return wrapPromise(result);
               }
               span.end();

plugins/node/opentelemetry-instrumentation-restify/src/utils.ts

@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+// util.types.isPromise is supported from 10.0.0
+export const isPromise = (value: any): value is Promise<unknown> => {
+  if (
+    typeof value.then === 'function' &&
+    typeof value.catch === 'function' &&
+    value.toString() === '[object Promise]'
+  ) {
+    return true;
+  }
+  return false;
+};
+
+// util.types.isAsyncFunction is supported from 10.0.0
+export const isAsyncFunction = (value: unknown) => {
+  if (
+    typeof value === 'function' &&
+    value.constructor?.name === 'AsyncFunction'
+  ) {
+    return true;
+  }
+  return false;
+};

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

@@ -50,7 +50,11 @@ const httpRequest = {
   },
 };
 const noop = (value: unknown) => {};
-const defer = (): { promise: Promise<unknown>, resolve: Function, reject: Function } => {
+const defer = (): {
+  promise: Promise<unknown>;
+  resolve: Function;
+  reject: Function;
+} => {
   let resolve = noop;
   let reject = noop;
   const promise = new Promise((res, rej) => {
@@ -301,14 +305,8 @@ describe('Restify Instrumentation', () => {
     });
 
     it('should work with async handlers', async () => {
-      const {
-        promise: work,
-        resolve: resolveWork,
-      } = defer();
-      const {
-        promise: started,
-        resolve: resolveStarted,
-      } = defer();
+      const { promise: work, resolve: resolveWork } = defer();
+      const { promise: started, resolve: resolveStarted } = defer();
       // status to assert the correctness of the test
       let status = 'uninit';
       const asyncHandler: restify.RequestHandler = async (req, res, next) => {
@@ -319,20 +317,17 @@ describe('Restify Instrumentation', () => {
         return getHandler(req, res, next);
       };
       const testLocalServer = await createServer((server: restify.Server) => {
-        server.get(
-          '/route/:param',
-          asyncHandler
-        );
+        server.get('/route/:param', asyncHandler);
       });
       const testLocalPort = testLocalServer.address().port;
 
       try {
-        const requestPromise = httpRequest.get(
-          `http://localhost:${testLocalPort}/route/hello`
-        ).then((res) => {
-          // assert request results
-          assert.strictEqual(res, '{"route":"hello"}');
-        });
+        const requestPromise = httpRequest
+          .get(`http://localhost:${testLocalPort}/route/hello`)
+          .then(res => {
+            // assert request results
+            assert.strictEqual(res, '{"route":"hello"}');
+          });
 
         // assert pre request state
         assert.strictEqual(status, 'uninit');
@@ -367,39 +362,34 @@ describe('Restify Instrumentation', () => {
     });
 
     it('should work with promise-returning handlers', async () => {
-      const {
-        promise: work,
-        resolve: resolveWork,
-      } = defer();
-      const {
-        promise: started,
-        resolve: resolveStarted,
-      } = defer();
+      const { promise: work, resolve: resolveWork } = defer();
+      const { promise: started, resolve: resolveStarted } = defer();
       // status to assert the correctness of the test
       let status = 'uninit';
-      const promiseReturningHandler: restify.RequestHandler = (req, res, next) => {
+      const promiseReturningHandler: restify.RequestHandler = (
+        req,
+        res,
+        next
+      ) => {
         status = 'started';
         resolveStarted();
         return work.then(() => {
           status = 'done';
-          return getHandler(req, res, next)
+          return getHandler(req, res, next);
         });
       };
       const testLocalServer = await createServer((server: restify.Server) => {
-        server.get(
-          '/route/:param',
-          promiseReturningHandler
-        );
+        server.get('/route/:param', promiseReturningHandler);
       });
       const testLocalPort = testLocalServer.address().port;
 
       try {
-        const requestPromise = httpRequest.get(
-          `http://localhost:${testLocalPort}/route/hello`
-        ).then((res) => {
-          // assert request results
-          assert.strictEqual(res, '{"route":"hello"}');
-        });
+        const requestPromise = httpRequest
+          .get(`http://localhost:${testLocalPort}/route/hello`)
+          .then(res => {
+            // assert request results
+            assert.strictEqual(res, '{"route":"hello"}');
+          });
 
         // assert pre request state
         assert.strictEqual(status, 'uninit');
@@ -425,7 +415,10 @@ describe('Restify Instrumentation', () => {
             span.attributes['restify.type'],
             'request_handler'
           );
-          assert.strictEqual(span.attributes['restify.name'], 'promiseReturningHandler');
+          assert.strictEqual(
+            span.attributes['restify.name'],
+            'promiseReturningHandler'
+          );
           assert.strictEqual(span.attributes['restify.version'], 'n/a');
         }
       } finally {