Skip to content

Commit

Permalink
Merge 8905ad0 into 6ea4b41
Browse files Browse the repository at this point in the history
  • Loading branch information
mhassan1 authored Dec 5, 2022
2 parents 6ea4b41 + 8905ad0 commit bdf3a0d
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 12 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ class ModuleNameTrieNode {
children: Map<string, ModuleNameTrieNode> = 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)
*/
Expand Down Expand Up @@ -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 [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }), []);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Expand Down

0 comments on commit bdf3a0d

Please sign in to comment.