From 82379b86927ec4a84d4d0f98b2baf5ebc4f0e3e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serkan=20=C3=96ZAL?= Date: Mon, 28 Oct 2024 19:15:18 +0300 Subject: [PATCH 1/2] Fix wrapping ESM files with absolute path --- .../src/platform/node/instrumentation.ts | 5 ++ .../test/node/EsmInstrumentation.test.mjs | 65 +++++++++++++++++++ .../test/node/esm/test.mjs | 3 + 3 files changed, 73 insertions(+) create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/esm/test.mjs diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index b69e91dfebf..c99d52ea2c7 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -287,6 +287,11 @@ export abstract class InstrumentationBase< this._warnOnPreloadedModules(); for (const module of this._modules) { const hookFn: HookFn = (exports, name, baseDir) => { + if (!baseDir && path.isAbsolute(name)) { + const parsedPath = path.parse(name); + name = parsedPath.name; + baseDir = parsedPath.dir; + } return this._onRequire(module, exports, name, baseDir); }; const onRequire: OnRequireFn = (exports, name, baseDir) => { diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs b/experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs index a46fdc0fd3e..3cea21cc698 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs +++ b/experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs @@ -18,8 +18,15 @@ import * as assert from 'assert'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, } from '../../build/src/index.js'; import * as exported from 'test-esm-module'; +import * as exportedAbsolute from './esm/test.mjs'; + +import path from 'path'; +import url from 'url'; + +const TEST_DIR_NAME = path.dirname(url.fileURLToPath(import.meta.url)); class TestInstrumentationWrapFn extends InstrumentationBase { constructor(config) { @@ -95,6 +102,45 @@ class TestInstrumentationSimple extends InstrumentationBase { ); } } + +class TestAbsoluteFileInstrumentationPatchFn extends InstrumentationBase { + constructor(config) { + super('test-esm-instrumentation', '0.0.1', config); + } + init() { + return new InstrumentationNodeModuleDefinition( + path.join(TEST_DIR_NAME, '/esm/test.mjs'), + ['*'], + undefined, + undefined, + [ + new InstrumentationNodeModuleFile( + 'test', + ['*'], + moduleExports => { + const wrapRetval = this._wrap(moduleExports, 'testFunction', () => { + return function wrappedTestFunction() { + return 'patched'; + }; + }); + assert.strictEqual(typeof wrapRetval, 'function'); + assert.strictEqual( + wrapRetval.name, + 'wrappedTestFunction', + '_wrap(..., "testFunction", ...) return value is the wrapped function' + ); + return moduleExports; + }, + moduleExports => { + this._unwrap(moduleExports, 'testFunction'); + return moduleExports; + } + ) + ] + ) + } +} + describe('when loading esm module', () => { const instrumentationWrap = new TestInstrumentationWrapFn({ enabled: false, @@ -140,4 +186,23 @@ describe('when loading esm module', () => { assert.deepEqual(exported.testFunction(), 'original'); assert.deepEqual(exported.secondTestFunction(), 'original'); }); + + it('should patch function from a file with absolute path', async () => { + const instrumentation = new TestAbsoluteFileInstrumentationPatchFn({ + enabled: false, + }); + instrumentation.enable(); + assert.deepEqual(exportedAbsolute.testFunction(), 'patched'); + }); + + it('should unwrap a patched function from a file with absolute path', async () => { + const instrumentation = new TestAbsoluteFileInstrumentationPatchFn({ + enabled: false, + }); + + instrumentation.enable(); + // disable to trigger unwrap + instrumentation.disable(); + assert.deepEqual(exported.testFunction(), 'original'); + }); }); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/esm/test.mjs b/experimental/packages/opentelemetry-instrumentation/test/node/esm/test.mjs new file mode 100644 index 00000000000..56e1af29552 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/esm/test.mjs @@ -0,0 +1,3 @@ +export const testFunction = () => { + return 'original'; +}; From af9f47ec96dadb0135899a9702067791d4b1f70f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serkan=20=C3=96ZAL?= Date: Wed, 6 Nov 2024 19:27:34 +0300 Subject: [PATCH 2/2] Add changelog entry --- experimental/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index aad560d8fca..9d6d0e7b3a2 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(instrumentation): Fix wrapping ESM files with absolute path [#5094](https://github.com/open-telemetry/opentelemetry-js/pull/5094) @serkan-ozal + ### :books: (Refine Doc) ### :house: (Internal)