diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index c8db9ca78a6..06491883f5c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to experimental packages in this project will be documented * fix(instrumentation-xhr): http.url attribute should be absolute [#3200](https://github.com/open-telemetry/opentelemetry-js/pull/3200) @t2t2 * fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir +* fix(instrumentation): only call `onRequire` for full matches on core modules with sub-paths [#3451](https://github.com/open-telemetry/opentelemetry-js/pull/3451) @mhassan1 * fix(instrumentation): add back support for absolute paths via `require-in-the-middle` [#3457](https://github.com/open-telemetry/opentelemetry-js/pull/3457) @mhassan1 ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts index 19b3e0912ae..e971421cda0 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts @@ -26,6 +26,17 @@ class ModuleNameTrieNode { children: Map = new Map(); } +type ModuleNameTrieSearchOptions = { + /** + * Whether to return the results in insertion order + */ + maintainInsertionOrder?: boolean; + /** + * Whether to return only full matches + */ + fullOnly?: boolean; +}; + /** * Trie containing nodes that represent a part of a module name (i.e. the parts separated by forward slash) */ @@ -57,24 +68,33 @@ export class ModuleNameTrie { * * @param {string} moduleName Module name * @param {boolean} maintainInsertionOrder Whether to return the results in insertion order + * @param {boolean} fullOnly Whether to return only full matches * @returns {Hooked[]} Matching hooks */ search( moduleName: string, - { maintainInsertionOrder }: { maintainInsertionOrder?: boolean } = {} + { maintainInsertionOrder, fullOnly }: ModuleNameTrieSearchOptions = {} ): Hooked[] { let trieNode = this._trie; const results: ModuleNameTrieNode['hooks'] = []; + let foundFull = true; for (const moduleNamePart of moduleName.split(ModuleNameSeparator)) { const nextNode = trieNode.children.get(moduleNamePart); if (!nextNode) { + foundFull = false; break; } - results.push(...nextNode.hooks); + if (!fullOnly) { + results.push(...nextNode.hooks); + } trieNode = nextNode; } + if (fullOnly && foundFull) { + results.push(...trieNode.hooks); + } + if (results.length === 0) { return []; } diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts index 76089a142fc..0bcea53ea8b 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts @@ -69,6 +69,10 @@ export class RequireInTheMiddleSingleton { const matches = this._moduleNameTrie.search(normalizedModuleName, { maintainInsertionOrder: true, + // For core modules (e.g. `fs`), do not match on sub-paths (e.g. `fs/promises'). + // This matches the behavior of `require-in-the-middle`. + // `basedir` is always `undefined` for core modules. + fullOnly: basedir === undefined, }); for (const { onRequire } of matches) { diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts index 7418722e26d..304aae996c5 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts @@ -61,5 +61,27 @@ describe('ModuleNameTrie', () => { ]); }); }); + + describe('fullOnly = false', () => { + it('should return a list of matches for prefixes', () => { + assert.deepEqual(trie.search('a/b'), [ + inserts[0], + inserts[2], + inserts[1], + ]); + }); + }); + + describe('fullOnly = true', () => { + it('should return a list of matches for full values only', () => { + assert.deepEqual(trie.search('a', { fullOnly: true }), [ + inserts[0], + inserts[2], + ]); + assert.deepEqual(trie.search('a/b', { fullOnly: true }), [inserts[1]]); + assert.deepEqual(trie.search('e', { fullOnly: true }), []); + assert.deepEqual(trie.search('a/b/e', { fullOnly: true }), []); + }); + }); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts index 8fddad45984..df7059987cb 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts @@ -106,22 +106,14 @@ describe('RequireInTheMiddleSingleton', () => { describe('AND module name matches', () => { it('should call `onRequire`', () => { const exports = require('fs/promises'); - assert.deepStrictEqual(exports.__ritmOnRequires, [ - 'fs', - 'fs-promises', - ]); + assert.deepStrictEqual(exports.__ritmOnRequires, ['fs-promises']); sinon.assert.calledOnceWithExactly( onRequireFsPromisesStub, exports, 'fs/promises', undefined ); - sinon.assert.calledOnceWithMatch( - onRequireFsStub, - { __ritmOnRequires: ['fs', 'fs-promises'] }, - 'fs/promises', - undefined - ); + sinon.assert.notCalled(onRequireFsStub); }); }); });