Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation): implement require-in-the-middle singleton #3161

Merged
merged 17 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ All notable changes to experimental packages in this project will be documented

* Add `resourceDetectors` option to `NodeSDK` [#3210](https://github.com/open-telemetry/opentelemetry-js/issues/3210)
* feat: add Logs API @mkuba [#3117](https://github.com/open-telemetry/opentelemetry-js/pull/3117)
* feat(instrumentation): implement `require-in-the-middle` singleton [#3161](https://github.com/open-telemetry/opentelemetry-js/pull/3161) @mhassan1
mhassan1 marked this conversation as resolved.
Show resolved Hide resolved

### :bug: (Bug Fix)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@

import * as types from '../../types';
import * as path from 'path';
import * as RequireInTheMiddle from 'require-in-the-middle';
import { satisfies } from 'semver';
import { InstrumentationAbstract } from '../../instrumentation';
import { requireInTheMiddleSingleton, Hooked } from './requireInTheMiddleSingleton';
import { InstrumentationModuleDefinition } from './types';
import { diag } from '@opentelemetry/api';

Expand All @@ -29,7 +29,7 @@ export abstract class InstrumentationBase<T = any>
extends InstrumentationAbstract
implements types.Instrumentation {
private _modules: InstrumentationModuleDefinition<T>[];
private _hooks: RequireInTheMiddle.Hooked[] = [];
mhassan1 marked this conversation as resolved.
Show resolved Hide resolved
private _hooks: Hooked[] = [];
private _enabled = false;

constructor(
Expand Down Expand Up @@ -159,9 +159,8 @@ export abstract class InstrumentationBase<T = any>
this._warnOnPreloadedModules();
for (const module of this._modules) {
this._hooks.push(
RequireInTheMiddle(
[module.name],
{ internals: true },
requireInTheMiddleSingleton.register(
module.name,
(exports, name, baseDir) => {
return this._onRequire<typeof exports>(
(module as unknown) as InstrumentationModuleDefinition<
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as RequireInTheMiddle from 'require-in-the-middle';
import * as path from 'path';

export type Hooked = {
moduleName: string
onRequire: RequireInTheMiddle.OnRequireFn
};

// The version number at the end of this symbol should be incremented whenever there are
// changes (even non-breaking) to the `RequireInTheMiddleSingleton` class's public interface
const RITM_SINGLETON_SYM = Symbol.for('OpenTelemetry.js.sdk.require-in-the-middle.v1');

/**
* Singleton class for `require-in-the-middle`
* Allows instrumentation plugins to patch modules with only a single `require` patch
* WARNING: Because this class will be used to create a process-global singleton,
* any change to the public interface of the class (even a non-breaking change like adding a method or argument)
* could break the integration with different versions of `InstrumentationBase`.
* When a change to the public interface of the class is made,
* we should increment the version number at the end of the `RITM_SINGLETON_SYM` symbol.
*/
class RequireInTheMiddleSingleton {
private _modulesToHook: Hooked[] = [];

constructor() {
mhassan1 marked this conversation as resolved.
Show resolved Hide resolved
this._initialize();
}

private _initialize() {
RequireInTheMiddle(
// Intercept all `require` calls; we will filter the matching ones below
null,
{ internals: true },
(exports, name, basedir) => {
// For internal files on Windows, `name` will use backslash as the path separator
const normalizedModuleName = normalizePathSeparators(name);
const matches = this._modulesToHook.filter(({ moduleName: hookedModuleName }) => {
mhassan1 marked this conversation as resolved.
Show resolved Hide resolved
return shouldHook(hookedModuleName, normalizedModuleName);
});

for (const { onRequire } of matches) {
exports = onRequire(exports, name, basedir);
}

return exports;
}
);
}

register(moduleName: string, onRequire: RequireInTheMiddle.OnRequireFn): Hooked {
const hooked = { moduleName, onRequire };
this._modulesToHook.push(hooked);
return hooked;
}

static getGlobalInstance(): RequireInTheMiddleSingleton {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (global as any)[RITM_SINGLETON_SYM] = (global as any)[RITM_SINGLETON_SYM] ?? new RequireInTheMiddleSingleton();
}
}

/**
* Determine whether a `require`d module should be hooked
*
* @param {string} hookedModuleName Hooked module name
* @param {string} requiredModuleName Required module name
* @returns {boolean} Whether to hook the required module
* @private
*/
export function shouldHook(hookedModuleName: string, requiredModuleName: string): boolean {
return requiredModuleName === hookedModuleName || requiredModuleName.startsWith(hookedModuleName + '/');
}

/**
* Normalize the path separators to forward slash in a module name or path
*
* @param {string} moduleNameOrPath Module name or path
* @returns {string} Normalized module name or path
*/
function normalizePathSeparators(moduleNameOrPath: string): string {
return path.sep !== '/'
? moduleNameOrPath.split(path.sep).join('/')
: moduleNameOrPath;
}

export const requireInTheMiddleSingleton = RequireInTheMiddleSingleton.getGlobalInstance();
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as assert from 'assert';
import * as sinon from 'sinon';
import * as path from 'path';
import * as RequireInTheMiddle from 'require-in-the-middle';
import { requireInTheMiddleSingleton, shouldHook } from '../../src/platform/node/requireInTheMiddleSingleton';

type AugmentedExports = {
__ritmOnRequires?: string[]
};

const makeOnRequiresStub = (label: string): sinon.SinonStub => sinon.stub().callsFake(((exports: AugmentedExports) => {
exports.__ritmOnRequires ??= [];
exports.__ritmOnRequires.push(label);
return exports;
}) as RequireInTheMiddle.OnRequireFn);

describe('requireInTheMiddleSingleton', () => {
describe('register', () => {
const onRequireFsStub = makeOnRequiresStub('fs');
const onRequireFsPromisesStub = makeOnRequiresStub('fs-promises');
const onRequireCodecovStub = makeOnRequiresStub('codecov');
const onRequireCodecovLibStub = makeOnRequiresStub('codecov-lib');
const onRequireCpxStub = makeOnRequiresStub('cpx');
const onRequireCpxLibStub = makeOnRequiresStub('cpx-lib');

before(() => {
requireInTheMiddleSingleton.register('fs', onRequireFsStub);
requireInTheMiddleSingleton.register('fs/promises', onRequireFsPromisesStub);
requireInTheMiddleSingleton.register('codecov', onRequireCodecovStub);
requireInTheMiddleSingleton.register('codecov/lib/codecov.js', onRequireCodecovLibStub);
requireInTheMiddleSingleton.register('cpx', onRequireCpxStub);
requireInTheMiddleSingleton.register('cpx/lib/copy-sync.js', onRequireCpxLibStub);
});

beforeEach(() => {
onRequireFsStub.resetHistory();
onRequireFsPromisesStub.resetHistory();
onRequireCodecovStub.resetHistory();
onRequireCodecovLibStub.resetHistory();
onRequireCpxStub.resetHistory();
onRequireCpxLibStub.resetHistory();
});

it('should return a hooked object', () => {
const moduleName = 'm';
const onRequire = makeOnRequiresStub('m');
const hooked = requireInTheMiddleSingleton.register(moduleName, onRequire);
assert.deepStrictEqual(hooked, { moduleName, onRequire });
});

describe('core module', () => {
describe('AND module name matches', () => {
it('should call `onRequire`', () => {
const exports = require('fs');
assert.deepStrictEqual(exports.__ritmOnRequires, ['fs']);
sinon.assert.calledOnceWithExactly(onRequireFsStub, exports, 'fs', undefined);
sinon.assert.notCalled(onRequireFsPromisesStub);
});
});
describe('AND module name does not match', () => {
it('should not call `onRequire`', () => {
const exports = require('crypto');
assert.equal(exports.__ritmOnRequires, undefined);
sinon.assert.notCalled(onRequireFsStub);
});
});
});

describe('core module with sub-path', () => {
describe('AND module name matches', () => {
it('should call `onRequire`', () => {
const exports = require('fs/promises');
assert.deepStrictEqual(exports.__ritmOnRequires, ['fs', 'fs-promises']);
sinon.assert.calledOnceWithExactly(onRequireFsPromisesStub, exports, 'fs/promises', undefined);
sinon.assert.calledOnceWithMatch(onRequireFsStub, { __ritmOnRequires: ['fs', 'fs-promises'] }, 'fs/promises', undefined);
});
});
});

describe('non-core module', () => {
describe('AND module name matches', () => {
const baseDir = path.dirname(require.resolve('codecov'));
const modulePath = path.join('codecov', 'lib', 'codecov.js');
it('should call `onRequire`', () => {
const exports = require('codecov');
assert.deepStrictEqual(exports.__ritmOnRequires, ['codecov']);
sinon.assert.calledWithExactly(onRequireCodecovStub, exports, 'codecov', baseDir);
sinon.assert.calledWithMatch(onRequireCodecovStub, { __ritmOnRequires: ['codecov', 'codecov-lib'] }, modulePath, baseDir);
sinon.assert.calledWithMatch(onRequireCodecovLibStub, { __ritmOnRequires: ['codecov', 'codecov-lib'] }, modulePath, baseDir);
});
});
});

describe('non-core module with sub-path', () => {
describe('AND module name matches', () => {
const baseDir = path.resolve(path.dirname(require.resolve('cpx')), '..');
const modulePath = path.join('cpx', 'lib', 'copy-sync.js');
it('should call `onRequire`', () => {
const exports = require('cpx/lib/copy-sync');
assert.deepStrictEqual(exports.__ritmOnRequires, ['cpx', 'cpx-lib']);
sinon.assert.calledWithMatch(onRequireCpxStub, { __ritmOnRequires: ['cpx', 'cpx-lib'] }, modulePath, baseDir);
sinon.assert.calledWithExactly(onRequireCpxStub, exports, modulePath, baseDir);
sinon.assert.calledWithExactly(onRequireCpxLibStub, exports, modulePath, baseDir);
});
});
});
});

describe('shouldHook', () => {
describe('module that matches', () => {
it('should be hooked', () => {
assert.equal(shouldHook('c', 'c'), true);
assert.equal(shouldHook('c', 'c/d'), true);
assert.equal(shouldHook('c.js', 'c.js'), true);
});
});
describe('module that does not match', () => {
it('should not be hooked', () => {
assert.equal(shouldHook('c', 'c.js'), false);
assert.equal(shouldHook('c', 'e'), false);
assert.equal(shouldHook('c.js', 'c'), false);
});
});
});
});