feat(instrumentation): apply unwrap before wrap in base class (open-t…

…elemetry#4692) * feat(instrumentation): apply unwrap before wrap in base class * chore: CHANGELOG
Zirak · Sep 14, 2024 · ba59680 · ba59680
1 parent d9b235c
commit ba59680
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 47 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -20,6 +20,7 @@ All notable changes to experimental packages in this project will be documented
 
 ### :rocket: (Enhancement)
 
+* feat(instrumentation): apply unwrap before wrap in base class [#4692](https://github.com/open-telemetry/opentelemetry-js/pull/4692)
 * feat(instrumentation): add util to execute span customization hook in base class [#4663](https://github.com/open-telemetry/opentelemetry-js/pull/4663) @blumamir
 * feat(instrumentation): generic config type in instrumentation base [#4659](https://github.com/open-telemetry/opentelemetry-js/pull/4659) @blumamir
 * feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
@@ -53,7 +53,6 @@ import {
 } from '@opentelemetry/api';
 import {
   InstrumentationNodeModuleDefinition,
-  isWrapped,
   InstrumentationBase,
 } from '@opentelemetry/instrumentation';
 import {
@@ -102,55 +101,28 @@ export class GrpcInstrumentation extends InstrumentationBase<GrpcInstrumentation
         '@grpc/grpc-js',
         ['1.*'],
         moduleExports => {
-          if (isWrapped(moduleExports.Server.prototype.register)) {
-            this._unwrap(moduleExports.Server.prototype, 'register');
-          }
           // Patch Server methods
           this._wrap(
             moduleExports.Server.prototype,
             'register',
             this._patchServer()
           );
           // Patch Client methods
-          if (isWrapped(moduleExports.makeGenericClientConstructor)) {
-            this._unwrap(moduleExports, 'makeGenericClientConstructor');
-          }
           this._wrap(
             moduleExports,
             'makeGenericClientConstructor',
             this._patchClient(moduleExports)
           );
-          if (isWrapped(moduleExports.makeClientConstructor)) {
-            this._unwrap(moduleExports, 'makeClientConstructor');
-          }
           this._wrap(
             moduleExports,
             'makeClientConstructor',
             this._patchClient(moduleExports)
           );
-          if (isWrapped(moduleExports.loadPackageDefinition)) {
-            this._unwrap(moduleExports, 'loadPackageDefinition');
-          }
           this._wrap(
             moduleExports,
             'loadPackageDefinition',
             this._patchLoadPackageDefinition(moduleExports)
           );
-          if (isWrapped(moduleExports.Client.prototype)) {
-            this._unwrap(moduleExports.Client.prototype, 'makeUnaryRequest');
-            this._unwrap(
-              moduleExports.Client.prototype,
-              'makeClientStreamRequest'
-            );
-            this._unwrap(
-              moduleExports.Client.prototype,
-              'makeServerStreamRequest'
-            );
-            this._unwrap(
-              moduleExports.Client.prototype,
-              'makeBidiStreamRequest'
-            );
-          }
           this._wrap(
             moduleExports.Client.prototype,
             'makeUnaryRequest',

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -53,7 +53,6 @@ import { VERSION } from './version';
 import {
   InstrumentationBase,
   InstrumentationNodeModuleDefinition,
-  isWrapped,
   safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
 import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
@@ -111,25 +110,16 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'http',
       ['*'],
       (moduleExports: Http): Http => {
-        if (isWrapped(moduleExports.request)) {
-          this._unwrap(moduleExports, 'request');
-        }
         this._wrap(
           moduleExports,
           'request',
           this._getPatchOutgoingRequestFunction('http')
         );
-        if (isWrapped(moduleExports.get)) {
-          this._unwrap(moduleExports, 'get');
-        }
         this._wrap(
           moduleExports,
           'get',
           this._getPatchOutgoingGetFunction(moduleExports.request)
         );
-        if (isWrapped(moduleExports.Server.prototype.emit)) {
-          this._unwrap(moduleExports.Server.prototype, 'emit');
-        }
         this._wrap(
           moduleExports.Server.prototype,
           'emit',
@@ -152,25 +142,16 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'https',
       ['*'],
       (moduleExports: Https): Https => {
-        if (isWrapped(moduleExports.request)) {
-          this._unwrap(moduleExports, 'request');
-        }
         this._wrap(
           moduleExports,
           'request',
           this._getPatchHttpsOutgoingRequestFunction('https')
         );
-        if (isWrapped(moduleExports.get)) {
-          this._unwrap(moduleExports, 'get');
-        }
         this._wrap(
           moduleExports,
           'get',
           this._getPatchHttpsOutgoingGetFunction(moduleExports.request)
         );
-        if (isWrapped(moduleExports.Server.prototype.emit)) {
-          this._unwrap(moduleExports.Server.prototype, 'emit');
-        }
         this._wrap(
           moduleExports.Server.prototype,
           'emit',

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -34,6 +34,7 @@ import { diag } from '@opentelemetry/api';
 import type { OnRequireFn } from 'require-in-the-middle';
 import { Hook } from 'require-in-the-middle';
 import { readFileSync } from 'fs';
+import { isWrapped } from '../../utils';
 
 /**
  * Base abstract class for instrumenting node plugins
@@ -79,6 +80,9 @@ export abstract class InstrumentationBase<
   }
 
   protected override _wrap: typeof wrap = (moduleExports, name, wrapper) => {
+    if (isWrapped(moduleExports[name])) {
+      this._unwrap(moduleExports, name);
+    }
     if (!utilTypes.isProxy(moduleExports)) {
       return wrap(moduleExports, name, wrapper);
     } else {