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

fix(instrumentation): only patch core modules if enabled #2993

Merged
merged 3 commits into from
May 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -38,6 +38,7 @@ All notable changes to experimental packages in this project will be documented
* fix(opentelemetry-instrumentation-http): use correct origin when port is `null` #2948 @danielgblanco
* fix(otlp-exporter-base): include esm and esnext in package files #2952 @dyladan
* fix(otlp-http-exporter): update endpoint to match spec #2895 @svetlanabrennan
* fix(instrumentation): only patch core modules if enabled #2993 @santigimeno
* fix(otlp-transformer): include esm and esnext in package files and update README #2992 @pichlermarc
* fix(metrics): specification compliant default metric unit #2983 @andyfleming

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ export abstract class InstrumentationBase<T = any>
if (!baseDir) {
if (typeof module.patch === 'function') {
module.moduleExports = exports;
return module.patch(exports);
if (this._enabled) {
return module.patch(exports);
}
}
return exports;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const MODULE_FILE_NAME = 'test-module-file';
const MODULE_VERSION = '0.1.0';
const WILDCARD_VERSION = '*';
const MODULE_DIR = '/random/dir';
const CORE_MODULE = 'random_core';

class TestInstrumentation extends InstrumentationBase {
constructor() {
Expand All @@ -33,6 +34,61 @@ class TestInstrumentation extends InstrumentationBase {
}

describe('InstrumentationBase', () => {
describe('_onRequire - core module', () => {
let instrumentation: TestInstrumentation;
let modulePatchSpy: sinon.SinonSpy;
beforeEach(() => {
instrumentation = new TestInstrumentation();
modulePatchSpy = sinon.spy();
});

describe('AND module is not enabled', () => {
it('should not patch the module', () => {
// @ts-expect-error access internal property for testing
instrumentation._enabled = false;
const moduleExports = {};
const instrumentationModule = {
name: CORE_MODULE,
patch: modulePatchSpy as unknown,
} as InstrumentationModuleDefinition<unknown>;

// @ts-expect-error access internal property for testing
instrumentation._onRequire<unknown>(
instrumentationModule,
moduleExports,
CORE_MODULE,
undefined
);

assert.strictEqual(instrumentationModule.moduleExports, moduleExports);
sinon.assert.notCalled(modulePatchSpy);
});
});

describe('AND module is enabled', () => {
it('should patch the module', () => {
// @ts-expect-error access internal property for testing
instrumentation._enabled = true;
const moduleExports = {};
const instrumentationModule = {
name: CORE_MODULE,
patch: modulePatchSpy as unknown,
} as InstrumentationModuleDefinition<unknown>;

// @ts-expect-error access internal property for testing
instrumentation._onRequire<unknown>(
instrumentationModule,
moduleExports,
CORE_MODULE,
undefined
);

assert.strictEqual(instrumentationModule.moduleExports, moduleExports);
sinon.assert.calledOnceWithExactly(modulePatchSpy, moduleExports);
});
});
});

describe('_onRequire - module version is not available', () => {
// For all of these cases, there is no indication of the actual module version,
// so we require there to be a wildcard supported version.
Expand Down