From 4098e6a3d4257e5da9b8cece430bde7d70319cf3 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 13 Dec 2022 09:33:06 +0100 Subject: [PATCH] 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 * 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: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/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 --- .../README.md | 21 +++++ .../src/index.ts | 1 + .../src/instrumentation.ts | 41 ++++++++- .../src/internal-types.ts | 6 +- .../src/types.ts | 45 ++++++++++ .../test/restify.test.ts | 83 ++++++++++++++++++- 6 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 plugins/node/opentelemetry-instrumentation-restify/src/types.ts diff --git a/plugins/node/opentelemetry-instrumentation-restify/README.md b/plugins/node/opentelemetry-instrumentation-restify/README.md index 9ef35ea4da..719a042ab7 100644 --- a/plugins/node/opentelemetry-instrumentation-restify/README.md +++ b/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: diff --git a/plugins/node/opentelemetry-instrumentation-restify/src/index.ts b/plugins/node/opentelemetry-instrumentation-restify/src/index.ts index dae24360d1..331bf99930 100644 --- a/plugins/node/opentelemetry-instrumentation-restify/src/index.ts +++ b/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'; diff --git a/plugins/node/opentelemetry-instrumentation-restify/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-restify/src/instrumentation.ts index 76fb7b95f9..a856359d96 100644 --- a/plugins/node/opentelemetry-instrumentation-restify/src/instrumentation.ts +++ b/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 { - 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( constants.MODULE_NAME, @@ -185,6 +198,26 @@ export class RestifyInstrumentation extends InstrumentationBase { }, 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); diff --git a/plugins/node/opentelemetry-instrumentation-restify/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-restify/src/internal-types.ts index d6280a6bda..deb0ea3e98 100644 --- a/plugins/node/opentelemetry-instrumentation-restify/src/internal-types.ts +++ b/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 }; diff --git a/plugins/node/opentelemetry-instrumentation-restify/src/types.ts b/plugins/node/opentelemetry-instrumentation-restify/src/types.ts new file mode 100644 index 0000000000..238f247a81 --- /dev/null +++ b/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; +} diff --git a/plugins/node/opentelemetry-instrumentation-restify/test/restify.test.ts b/plugins/node/opentelemetry-instrumentation-restify/test/restify.test.ts index 33ce317188..0e381bf969 100644 --- a/plugins/node/opentelemetry-instrumentation-restify/test/restify.test.ts +++ b/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', () => {