From 7e6eaf4e562064f911fbedc31813e7aa4b569f70 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Wed, 17 Apr 2024 12:29:58 +0300 Subject: [PATCH 1/3] refactor(@opentelemetry/instrumentation): add patch and unpatch diag log messages --- .../src/instrumentation.ts | 6 +-- .../src/http.ts | 6 --- .../src/platform/node/instrumentation.ts | 54 +++++++++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts index 6a140694022..05b721cc747 100644 --- 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 index 1bc56cb4993..69fe8789994 100644 --- 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 => { - 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 => { 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 => { - 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 => { 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 index 2fe151a1fed..63e47348d06 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -183,6 +183,13 @@ 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, + version: module.moduleVersion, + } + ); return module.patch(exports); } } @@ -199,6 +206,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 +231,15 @@ export abstract class InstrumentationBase return supportedFileInstrumentations.reduce((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, + } + ); return file.patch(patchedExports, module.moduleVersion); } return patchedExports; @@ -232,10 +256,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); } } @@ -288,10 +327,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); } } From ed76808fc4c6c0e9f5083c55351bdc78d0eeb3a8 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Wed, 17 Apr 2024 13:08:20 +0300 Subject: [PATCH 2/3] chore: CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50e66e0532f..abeae427a31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ 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. + ### :rocket: (Enhancement) * feat(sdk-trace-base): log resource attributes in ConsoleSpanExporter [#4605](https://github.com/open-telemetry/opentelemetry-js/pull/4605) @pichlermarc From 38b9a0123427b2e4d0114a83da990852d408f9c7 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Wed, 17 Apr 2024 19:47:24 +0300 Subject: [PATCH 3/3] fix: exclude version in core packages --- .../src/platform/node/instrumentation.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index 63e47348d06..d724258382b 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -187,7 +187,6 @@ export abstract class InstrumentationBase 'Applying instrumentation patch for nodejs core module on require hook', { module: module.name, - version: module.moduleVersion, } ); return module.patch(exports);