feat!(instrumentation): add patch and unpatch diag log messages (#4641)

* refactor(@opentelemetry/instrumentation): add patch and unpatch diag log messages * chore: CHANGELOG * fix: exclude version in core packages --------- Co-authored-by: Marc Pichler <[email protected]>
open-telemetry · Apr 19, 2024 · 31eb60d · 31eb60d
1 parent 99431df
commit 31eb60d
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :boom: Breaking Change
 
+* feat(instrumentation): add patch and unpatch diag log messages [#4641](https://github.com/open-telemetry/opentelemetry-js/pull/4641)
+  * Instrumentations should not log patch and unpatch messages to diag channel.
 * feat!(instrumentation): remove moduleExports generic type from instrumentation registration [#4598](https://github.com/open-telemetry/opentelemetry-js/pull/4598) @blumamir
   * breaking for instrumentation authors that depend on
     * `InstrumentationBase`

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
@@ -101,8 +101,7 @@ export class GrpcInstrumentation extends InstrumentationBase {
       new InstrumentationNodeModuleDefinition(
         '@grpc/grpc-js',
         ['1.*'],
-        (moduleExports, version) => {
-          this._diag.debug(`Applying patch for @grpc/grpc-js@${version}`);
+        moduleExports => {
           if (isWrapped(moduleExports.Server.prototype.register)) {
             this._unwrap(moduleExports.Server.prototype, 'register');
           }
@@ -174,9 +173,8 @@ export class GrpcInstrumentation extends InstrumentationBase {
           );
           return moduleExports;
         },
-        (moduleExports, version) => {
+        moduleExports => {
           if (moduleExports === undefined) return;
-          this._diag.debug(`Removing patch for @grpc/grpc-js@${version}`);
 
           this._unwrap(moduleExports.Server.prototype, 'register');
           this._unwrap(moduleExports, 'makeClientConstructor');

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -111,12 +111,10 @@ export class HttpInstrumentation extends InstrumentationBase {
   }
 
   private _getHttpInstrumentation() {
-    const version = process.versions.node;
     return new InstrumentationNodeModuleDefinition(
       'http',
       ['*'],
       (moduleExports: Http): Http => {
-        this._diag.debug(`Applying patch for http@${version}`);
         if (isWrapped(moduleExports.request)) {
           this._unwrap(moduleExports, 'request');
         }
@@ -145,7 +143,6 @@ export class HttpInstrumentation extends InstrumentationBase {
       },
       (moduleExports: Http) => {
         if (moduleExports === undefined) return;
-        this._diag.debug(`Removing patch for http@${version}`);
 
         this._unwrap(moduleExports, 'request');
         this._unwrap(moduleExports, 'get');
@@ -155,12 +152,10 @@ export class HttpInstrumentation extends InstrumentationBase {
   }
 
   private _getHttpsInstrumentation() {
-    const version = process.versions.node;
     return new InstrumentationNodeModuleDefinition(
       'https',
       ['*'],
       (moduleExports: Https): Https => {
-        this._diag.debug(`Applying patch for https@${version}`);
         if (isWrapped(moduleExports.request)) {
           this._unwrap(moduleExports, 'request');
         }
@@ -189,7 +184,6 @@ export class HttpInstrumentation extends InstrumentationBase {
       },
       (moduleExports: Https) => {
         if (moduleExports === undefined) return;
-        this._diag.debug(`Removing patch for https@${version}`);
 
         this._unwrap(moduleExports, 'request');
         this._unwrap(moduleExports, 'get');

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -183,6 +183,12 @@ export abstract class InstrumentationBase
       if (typeof module.patch === 'function') {
         module.moduleExports = exports;
         if (this._enabled) {
+          this._diag.debug(
+            'Applying instrumentation patch for nodejs core module on require hook',
+            {
+              module: module.name,
+            }
+          );
           return module.patch(exports);
         }
       }
@@ -199,6 +205,14 @@ export abstract class InstrumentationBase
         if (typeof module.patch === 'function') {
           module.moduleExports = exports;
           if (this._enabled) {
+            this._diag.debug(
+              'Applying instrumentation patch for module on require hook',
+              {
+                module: module.name,
+                version: module.moduleVersion,
+                baseDir,
+              }
+            );
             return module.patch(exports, module.moduleVersion);
           }
         }
@@ -216,6 +230,16 @@ export abstract class InstrumentationBase
     return supportedFileInstrumentations.reduce<T>((patchedExports, file) => {
       file.moduleExports = patchedExports;
       if (this._enabled) {
+        this._diag.debug(
+          'Applying instrumentation patch for nodejs module file on require hook',
+          {
+            module: module.name,
+            version: module.moduleVersion,
+            fileName: file.name,
+            baseDir,
+          }
+        );
+
         // patch signature is not typed, so we cast it assuming it's correct
         return file.patch(patchedExports, module.moduleVersion) as T;
       }
@@ -233,10 +257,25 @@ export abstract class InstrumentationBase
     if (this._hooks.length > 0) {
       for (const module of this._modules) {
         if (typeof module.patch === 'function' && module.moduleExports) {
+          this._diag.debug(
+            'Applying instrumentation patch for nodejs module on instrumentation enabled',
+            {
+              module: module.name,
+              version: module.moduleVersion,
+            }
+          );
           module.patch(module.moduleExports, module.moduleVersion);
         }
         for (const file of module.files) {
           if (file.moduleExports) {
+            this._diag.debug(
+              'Applying instrumentation patch for nodejs module file on instrumentation enabled',
+              {
+                module: module.name,
+                version: module.moduleVersion,
+                fileName: file.name,
+              }
+            );
             file.patch(file.moduleExports, module.moduleVersion);
           }
         }
@@ -279,10 +318,25 @@ export abstract class InstrumentationBase
 
     for (const module of this._modules) {
       if (typeof module.unpatch === 'function' && module.moduleExports) {
+        this._diag.debug(
+          'Removing instrumentation patch for nodejs module on instrumentation disabled',
+          {
+            module: module.name,
+            version: module.moduleVersion,
+          }
+        );
         module.unpatch(module.moduleExports, module.moduleVersion);
       }
       for (const file of module.files) {
         if (file.moduleExports) {
+          this._diag.debug(
+            'Removing instrumentation patch for nodejs module file on instrumentation disabled',
+            {
+              module: module.name,
+              version: module.moduleVersion,
+              fileName: file.name,
+            }
+          );
           file.unpatch(file.moduleExports, module.moduleVersion);
         }
       }