feat(restify): add requestHook support (#1312)

* feat(restify): add requestHook support

The `requestHook` config option allows custom span handling per request
layer.

* fix(restify): pass config to super class

As mentioned in the review, pass the instrumentation config to the
parent class. That way the config is also stored when given to the
initializer, rather only when using the `setConfig` function.

* fix(restify): fix comment referencing restify type

Update comment to reference to correct type from the `@types/restify`
package.

* fix(restify): import missing Span type

Add the missing import reported by the linter.

* fix(restify): fix issues reported by linter

* fix(restify): fix comment referencing restify type

Mention the package name exactly.

* fix(restify): fix comment referencing restify type

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>

* fix(restify): add missing import in restify test

* feat(restify): add layer argument to requestHook

Add the layerType argument to the requestHook function. This is like the
following PR but for restify:
#1226

Move the LayerType from internal-types to types, because it's now used
in a function used by users of the instrumentation package.

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>

plugins/node/opentelemetry-instrumentation-restify/README.md

@@ -41,6 +41,27 @@ registerInstrumentations({
 
 See [examples/restify](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples/restify) for a short example.
 
+## Restify Instrumentation Options
+
+| Options | Type | Example | Description |
+| `requestHook` | `RestifyCustomAttributeFunction` | `(span, requestInfo) => {}` | Function for adding custom attributes to restify requests. Receives params: `Span, RestifyRequestInfo`. |
+
+### Using `requestHook`
+
+Instrumentation configuration accepts a custom "hook" function which will be called for every instrumented restify request. Custom attributes can be set on the span or run any custom logic per request.
+
+```javascript
+import { RestifyInstrumentation } from "@opentelemetry/instrumentation-restify"
+const restifyInstrumentation = new RestifyInstrumentation({
+  requestHook: function (span: Span, info: RestifyRequestInfo) {
+    span.setAttribute(
+      'http.method',
+      info.request.method,
+    )
+  }
+});
+```
+
 ## Useful links
 
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

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

@@ -20,3 +20,4 @@ export * from './instrumentation';
 export default RestifyInstrumentation;
 
 export * from './enums/AttributeNames';
+export * from './types';

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

@@ -19,31 +19,44 @@ import type * as restify from 'restify';
 
 import * as api from '@opentelemetry/api';
 import type { Server } from 'restify';
-import { LayerType } from './internal-types';
+import { LayerType } from './types';
 import * as AttributeNames from './enums/AttributeNames';
 import { VERSION } from './version';
 import * as constants from './constants';
 import {
   InstrumentationBase,
-  InstrumentationConfig,
   InstrumentationNodeModuleDefinition,
   InstrumentationNodeModuleFile,
   isWrapped,
+  safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { isPromise, isAsyncFunction } from './utils';
 import { getRPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
+import type { RestifyInstrumentationConfig } from './types';
 
 const { diag } = api;
 
 export class RestifyInstrumentation extends InstrumentationBase<any> {
-  constructor(config: InstrumentationConfig = {}) {
-    super(`@opentelemetry/instrumentation-${constants.MODULE_NAME}`, VERSION);
+  constructor(config: RestifyInstrumentationConfig = {}) {
+    super(
+      `@opentelemetry/instrumentation-${constants.MODULE_NAME}`,
+      VERSION,
+      Object.assign({}, config)
+    );
   }
 
   private _moduleVersion?: string;
   private _isDisabled = false;
 
+  override setConfig(config: RestifyInstrumentationConfig = {}) {
+    this._config = Object.assign({}, config);
+  }
+
+  override getConfig(): RestifyInstrumentationConfig {
+    return this._config as RestifyInstrumentationConfig;
+  }
+
   init() {
     const module = new InstrumentationNodeModuleDefinition<any>(
       constants.MODULE_NAME,
@@ -185,6 +198,26 @@ export class RestifyInstrumentation extends InstrumentationBase<any> {
           },
           api.context.active()
         );
+
+        const instrumentation = this;
+        const requestHook = instrumentation.getConfig().requestHook;
+        if (requestHook) {
+          safeExecuteInTheMiddle(
+            () => {
+              return requestHook!(span, {
+                request: req,
+                layerType: metadata.type,
+              });
+            },
+            e => {
+              if (e) {
+                instrumentation._diag.error('request hook failed', e);
+              }
+            },
+            true
+          );
+        }
+
         const patchedNext = (err?: any) => {
           span.end();
           next(err);

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

@@ -15,11 +15,7 @@
  */
 import { Span } from '@opentelemetry/api';
 import type * as restify from 'restify';
-
-export enum LayerType {
-  MIDDLEWARE = 'middleware',
-  REQUEST_HANDLER = 'request_handler',
-}
+import { LayerType } from './types';
 
 declare interface RequestWithRoute extends restify.Request {
   route: { path: string };

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

@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+import { Span } from '@opentelemetry/api';
+import { InstrumentationConfig } from '@opentelemetry/instrumentation';
+
+export enum LayerType {
+  MIDDLEWARE = 'middleware',
+  REQUEST_HANDLER = 'request_handler',
+}
+
+export interface RestifyRequestInfo {
+  request: any; // Request type from @types/restify package
+  layerType: LayerType;
+}
+
+/**
+ * Function that can be used to add custom attributes to the current span
+ * @param span - The restify handler span.
+ * @param info - The restify request info object.
+ */
+export interface RestifyCustomAttributeFunction {
+  (span: Span, info: RestifyRequestInfo): void;
+}
+
+/**
+ * Options available for the restify Instrumentation
+ */
+export interface RestifyInstrumentationConfig extends InstrumentationConfig {
+  /** Function for adding custom attributes to each handler span */
+  requestHook?: RestifyCustomAttributeFunction;
+}

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

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import { context, trace } from '@opentelemetry/api';
+import { context, trace, Span } from '@opentelemetry/api';
+import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { RPCType, setRPCMetadata } from '@opentelemetry/core';
 import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
@@ -25,6 +26,7 @@ import {
 
 import RestifyInstrumentation from '../src';
 import * as types from '../src/internal-types';
+import { RestifyRequestInfo } from '../src/types';
 const plugin = new RestifyInstrumentation();
 
 import * as semver from 'semver';
@@ -487,6 +489,85 @@ describe('Restify Instrumentation', () => {
       assert.strictEqual(memoryExporter.getFinishedSpans().length, 3);
       assert.strictEqual(res, '{"route":"bar"}');
     });
+
+    describe('using requestHook in config', () => {
+      it('calls requestHook provided function when set in config', async () => {
+        const requestHook = (span: Span, info: RestifyRequestInfo) => {
+          span.setAttribute(
+            SemanticAttributes.HTTP_METHOD,
+            info.request.method
+          );
+          span.setAttribute('restify.layer', info.layerType);
+        };
+
+        plugin.setConfig({
+          ...plugin.getConfig(),
+          requestHook,
+        });
+
+        const rootSpan = tracer.startSpan('clientSpan');
+
+        await context.with(
+          trace.setSpan(context.active(), rootSpan),
+          async () => {
+            await httpRequest.get(`http://localhost:${port}/route/foo`);
+            rootSpan.end();
+            assert.strictEqual(memoryExporter.getFinishedSpans().length, 4);
+
+            {
+              // span from get
+              const span = memoryExporter.getFinishedSpans()[2];
+              assert.notStrictEqual(span, undefined);
+              assert.strictEqual(
+                span.attributes[SemanticAttributes.HTTP_METHOD],
+                'GET'
+              );
+              assert.strictEqual(
+                span.attributes['restify.layer'],
+                'request_handler'
+              );
+            }
+          }
+        );
+      });
+
+      it('does not propagate an error from a requestHook that throws exception', async () => {
+        const requestHook = (span: Span, info: RestifyRequestInfo) => {
+          span.setAttribute(
+            SemanticAttributes.HTTP_METHOD,
+            info.request.method
+          );
+
+          throw Error('error thrown in requestHook');
+        };
+
+        plugin.setConfig({
+          ...plugin.getConfig(),
+          requestHook,
+        });
+
+        const rootSpan = tracer.startSpan('clientSpan');
+
+        await context.with(
+          trace.setSpan(context.active(), rootSpan),
+          async () => {
+            await httpRequest.get(`http://localhost:${port}/route/foo`);
+            rootSpan.end();
+            assert.strictEqual(memoryExporter.getFinishedSpans().length, 4);
+
+            {
+              // span from get
+              const span = memoryExporter.getFinishedSpans()[2];
+              assert.notStrictEqual(span, undefined);
+              assert.strictEqual(
+                span.attributes[SemanticAttributes.HTTP_METHOD],
+                'GET'
+              );
+            }
+          }
+        );
+      });
+    });
   });
 
   describe('Disabling restify instrumentation', () => {