From 9011c8aca30d353658ab5b778a464c5fde38057e Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Fri, 10 Feb 2017 13:53:03 -0800 Subject: [PATCH] Plugin API: Loading Improvements (#372) PR-URL: #372 --- src/trace-plugin-loader.js | 103 +++--------- src/util.js | 42 ++++- test/test-plugin-loader.js | 317 +++++++++++++++++++++++++++++++++++++ 3 files changed, 377 insertions(+), 85 deletions(-) create mode 100644 test/test-plugin-loader.js diff --git a/src/trace-plugin-loader.js b/src/trace-plugin-loader.js index 071da422f..e50147e0d 100644 --- a/src/trace-plugin-loader.js +++ b/src/trace-plugin-loader.js @@ -18,8 +18,8 @@ var Module = require('module'); var shimmer = require('shimmer'); var path = require('path'); -var fs = require('fs'); var semver = require('semver'); +var util = require('./util.js'); var PluginAPI = require('./trace-plugin-interface.js'); var plugins = Object.create(null); @@ -27,42 +27,6 @@ var activated = false; var logger; -/** - * Determines the path at which the requested module will be loaded given - * the provided parent module. - * - * @param {string} request The name of the module to be loaded. - * @param {object} parent The module into which the requested module will be loaded. - */ -function findModulePath(request, parent) { - var mainScriptDir = path.dirname(Module._resolveFilename(request, parent)); - var resolvedModule = Module._resolveLookupPaths(request, parent); - var paths = resolvedModule[1]; - for (var i = 0, PL = paths.length; i < PL; i++) { - if (mainScriptDir.indexOf(paths[i]) === 0) { - return path.join(paths[i], request.replace('/', path.sep)); - } - } - return null; -} - -/** - * Determines the version of the module located at `modulePath`. - * - * @param {?string} modulePath The absolute path to the root directory of the - * module being loaded. This may be null if we are loading an internal module - * such as http. - */ -function findModuleVersion(modulePath, load) { - if (modulePath) { - var pjson = path.join(modulePath, 'package.json'); - if (fs.existsSync(pjson)) { - return load(pjson).version; - } - } - return process.version; -} - function checkLoadedModules() { for (var moduleName in plugins) { // \\ is benign on unix and escapes \\ on windows @@ -116,16 +80,21 @@ function activate(agent) { if (!patchSet) { // Load the plugin object var plugin = originalModuleLoad(instrumentation.file, module, false); - patchSet = []; + patchSet = {}; plugin.forEach(function(patch) { - if (semver.satisfies(version, patch.versions)) { - patchSet[patch.file] = { - file: patch.file || '', + if (!patch.versions || semver.satisfies(version, patch.versions)) { + var file = patch.file || ''; + patchSet[file] = { + file: file, patch: patch.patch, intercept: patch.intercept }; } }); + if (Object.keys(patchSet).length === 0) { + logger.warn(moduleRoot + ': version ' + version + ' not supported ' + + 'by plugin.'); + } instrumentation.patches[moduleRoot][version] = patchSet; } Object.keys(patchSet).forEach(function(file) { @@ -140,9 +109,8 @@ function activate(agent) { if (patch.intercept) { patch.module = patch.intercept(patch.module, api); } - patch.active = true; }); - var rootPatch = patchSet.filter(function(patch) { return !patch.name; })[0]; + var rootPatch = patchSet['']; if (rootPatch && rootPatch.intercept) { return rootPatch.module; } else { @@ -150,39 +118,21 @@ function activate(agent) { } } - function moduleAlreadyPatched(instrumentation, moduleRoot) { - if (!instrumentation.patches[moduleRoot]) { - return false; - } - var modulePatch = instrumentation.patches[moduleRoot]; - return !!modulePatch && Object.keys(modulePatch).every(function(curr) { - return modulePatch[curr].active; - }, true); - } - - // If this is a reactivation, we may have a cached list of modules from last - // time that we need to go and patch pro-actively. - for (var moduleName in plugins) { - var instrumentation = plugins[moduleName]; - for (var moduleRoot in instrumentation.patches) { - var modulePatch = instrumentation.patches[moduleRoot]; - if (modulePatch) { - loadAndPatch(instrumentation, moduleRoot, null); - } - } + function moduleAlreadyPatched(instrumentation, moduleRoot, version) { + return instrumentation.patches[moduleRoot] && + instrumentation.patches[moduleRoot][version]; } // Future requires get patched as they get loaded. return function Module_load(request, parent, isMain) { var instrumentation = plugins[request]; - if (instrumentation && - agent.config().excludedHooks.indexOf(request) === -1) { - var moduleRoot = findModulePath(request, parent); - if (moduleAlreadyPatched(instrumentation, moduleRoot)) { + if (instrumentation) { + var moduleRoot = util.findModulePath(request, parent); + var moduleVersion = util.findModuleVersion(moduleRoot, originalModuleLoad); + if (moduleAlreadyPatched(instrumentation, moduleRoot, moduleVersion)) { return originalModuleLoad.apply(this, arguments); } - var moduleVersion = findModuleVersion(moduleRoot, originalModuleLoad); logger.info('Patching ' + request + ' at version ' + moduleVersion); var patchedRoot = loadAndPatch(instrumentation, moduleRoot, moduleVersion); @@ -197,19 +147,6 @@ function activate(agent) { } function deactivate() { - for (var moduleName in plugins) { - var instrumentation = plugins[moduleName]; - for (var moduleRoot in instrumentation.patches) { - var modulePatch = instrumentation.patches[moduleRoot]; - for (var patchedFile in modulePatch) { - var hook = modulePatch[patchedFile]; - logger.info('Attempting to unpatch ' + moduleName); - if (hook.unpatch !== undefined) { - hook.unpatch(hook.module); - } - } - } - } activated = false; // unhook module.load @@ -218,7 +155,5 @@ function deactivate() { module.exports = { activate: activate, - deactivate: deactivate, - findModulePath: findModulePath, - findModuleVersion: findModuleVersion + deactivate: deactivate }; diff --git a/src/util.js b/src/util.js index e255a67b5..d3a0aac92 100644 --- a/src/util.js +++ b/src/util.js @@ -16,6 +16,8 @@ 'use strict'; +var Module = require('module'); +var fs = require('fs'); var path = require('path'); /** @@ -89,7 +91,45 @@ function packageNameFromPath(path) { return matches && matches.length > 1 ? matches[1] : null; } +/** + * Determines the path at which the requested module will be loaded given + * the provided parent module. + * + * @param {string} request The name of the module to be loaded. + * @param {object} parent The module into which the requested module will be loaded. + */ +function findModulePath(request, parent) { + var mainScriptDir = path.dirname(Module._resolveFilename(request, parent)); + var resolvedModule = Module._resolveLookupPaths(request, parent); + var paths = resolvedModule[1]; + for (var i = 0, PL = paths.length; i < PL; i++) { + if (mainScriptDir.indexOf(paths[i]) === 0) { + return path.join(paths[i], request.replace('/', path.sep)); + } + } + return null; +} + +/** + * Determines the version of the module located at `modulePath`. + * + * @param {?string} modulePath The absolute path to the root directory of the + * module being loaded. This may be null if we are loading an internal module + * such as http. + */ +function findModuleVersion(modulePath, load) { + if (modulePath) { + var pjson = path.join(modulePath, 'package.json'); + if (fs.existsSync(pjson)) { + return load(pjson).version; + } + } + return process.version; +} + module.exports = { stringifyPrefix: stringifyPrefix, - packageNameFromPath: packageNameFromPath + packageNameFromPath: packageNameFromPath, + findModulePath: findModulePath, + findModuleVersion: findModuleVersion }; diff --git a/test/test-plugin-loader.js b/test/test-plugin-loader.js new file mode 100644 index 000000000..7477a3971 --- /dev/null +++ b/test/test-plugin-loader.js @@ -0,0 +1,317 @@ +/** + * Copyright 2017 Google Inc. All Rights Reserved. + * + * 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 + * + * http://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. + */ +'use strict'; + +var shimmer = require('shimmer'); +var Module = require('module'); +var assert = require('assert'); +var proxyquire = require('proxyquire'); +var path = require('path'); + +// Save logs because in some cases we want to verify that something was logged. +var logs = { + error: '', + warn: '', + info: '' +}; + +// Facilitates loading "fake" modules upon calling require(). +var fakeModules = {}; + +// Adds module moduleName to the set of fake modules, using mock as the object +// being "exported" by this module. In addition, providing version makes it +// accessible by calling findModuleVersion. +function addModuleMock(moduleName, version, mock) { + fakeModules[moduleName.replace('/', path.sep)] = { + exports: mock, + version: version + }; +} + +// This function creates an object with just enough properties to appear to the +// plugin loader as the trace agent. It accepts the list of plugins that the +// plugin loader reads. +function createFakeAgent(plugins) { + function writeToLog(log, data) { + logs[log] += data + '\n'; + } + return { + logger: { + error: writeToLog.bind(null, 'error'), + warn: writeToLog.bind(null, 'warn'), + info: writeToLog.bind(null, 'info') + }, + config: function() { + return { plugins: plugins }; + } + }; +} + +describe('Trace Plugin Loader', function() { + var pluginLoader; + + before(function() { + // Wrap Module._load so that it loads from our fake module set rather than the + // real thing + shimmer.wrap(Module, '_load', function(originalModuleLoad) { + return function wrappedModuleLoad(modulePath) { + if (fakeModules[modulePath.replace('/', path.sep)]) { + return fakeModules[modulePath.replace('/', path.sep)].exports; + } + return originalModuleLoad.apply(this, arguments); + }; + }); + + // proxyquire the plugin loader with stubbed module utility methods + pluginLoader = proxyquire('../src/trace-plugin-loader.js', { + './util.js': { + findModulePath: function(request) { + return request.replace('/', path.sep); + }, + findModuleVersion: function(modulePath) { + return fakeModules[modulePath].version; + } + } + }); + }); + + after(function() { + shimmer.unwrap(Module, '_load'); + }); + + afterEach(function() { + pluginLoader.deactivate(); + logs.error = ''; + logs.warn = ''; + logs.info = ''; + fakeModules = {}; + }); + + /** + * Loads two modules (one of them twice), and makes sure that plugins are + * applied correctly. + */ + it('loads plugins no more than once', function() { + var patched = []; + addModuleMock('module-a', '1.0.0', {}); + addModuleMock('module-b', '1.0.0', {}); + addModuleMock('module-a-plugin', '', [ + { patch: function() { patched.push('a'); } } + ]); + addModuleMock('module-b-plugin', '', [ + { file: '', patch: function() { patched.push('b'); } } + ]); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-a': 'module-a-plugin', + 'module-b': 'module-b-plugin' + })); + assert.deepEqual(patched, [], + 'No patches are initially loaded'); + require('module-a'); + assert.deepEqual(patched, ['a'], + 'Patches are applied when the relevant patch is loaded'); + assert(logs.info.indexOf('Patching module-a at version 1.0.0') !== -1, + 'Info log is emitted when a module if patched'); + require('module-a'); + assert.deepEqual(patched, ['a'], + 'Patches aren\'t applied twice'); + require('module-b'); + assert.deepEqual(patched, ['a', 'b'], + 'Multiple plugins can be loaded, and file can be set to an empty string'); + }); + + /** + * Loads two plugins that each monkeypatch modules, and checks that they are + * actually monkeypatched. + */ + it('applies patches', function() { + addModuleMock('module-c', '1.0.0', { + getStatus: function() { return 'not wrapped'; } + }); + addModuleMock('module-d', '1.0.0', { + getStatus: function() { return 'not wrapped'; } + }); + addModuleMock('module-c-plugin', '', [ + { + patch: function(originalModule, api) { + shimmer.wrap(originalModule, 'getStatus', function() { + return function() { return 'wrapped'; }; + }); + } + } + ]); + assert.strictEqual(require('module-c').getStatus(), 'not wrapped', + 'Plugin loader shouldn\'t affect module before plugin is loaded'); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-c': 'module-c-plugin' + })); + assert.strictEqual(require('module-c').getStatus(), 'wrapped', + 'Plugin patch() method is called the right arguments'); + assert.strictEqual(require('module-d').getStatus(), 'not wrapped', + 'Modules for which there aren\'t plugins won\'t be patched'); + }); + + /** + * Loads one module to check that plugin patches that aren't compatible don't + * get applied. Then, loads another module with no compatible patches to check + * that nothing gets patched at all. + */ + it('respects patch set semver conditions', function() { + var patched = []; + addModuleMock('module-e', '1.0.0', {}); + addModuleMock('module-f', '2.0.0', {}); + addModuleMock('module-e-plugin', '', [ + { versions: '1.x', patch: function() { patched.push('e-1.x'); } }, + { versions: '2.x', patch: function() { patched.push('e-2.x'); } } + ]); + addModuleMock('module-f-plugin', '', [ + { versions: '1.x', patch: function() { patched.push('f-1.x'); } } + ]); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-e': 'module-e-plugin', + 'module-f': 'module-f-plugin' + })); + assert.deepEqual(patched, []); + require('module-e'); + assert.deepEqual(patched, ['e-1.x'], + 'Only patches with a correct semver condition are loaded'); + require('module-f'); + assert.deepEqual(patched, ['e-1.x'], + 'No patches are loaded if the module version isn\'t supported at all'); + assert(logs.warn.indexOf('module-f: version 2.0.0 not supported') !== -1, + 'A warning is printed if the module version isn\'t supported at all'); + }); + + /** + * Loads a module with internal exports and patches them, and then makes sure + * that they are actually patched. + */ + it('patches internal files in modules', function() { + addModuleMock('module-g', '1.0.0', { + createSentence: function() { + return require('module-g/subject').get() + ' ' + + require('module-g/predicate').get() + '.'; + } + }); + addModuleMock('module-g/subject', '', { + get: function() { + return 'bad tests'; + } + }); + addModuleMock('module-g/predicate', '', { + get: function() { + return 'don\'t make sense'; + } + }); + addModuleMock('module-g-plugin', '', [ + { + file: 'subject', + patch: function(originalModule, api) { + shimmer.wrap(originalModule, 'get', function() { + return function() { + return 'good tests'; + }; + }); + } + }, + { + file: 'predicate', + patch: function(originalModule, api) { + shimmer.wrap(originalModule, 'get', function() { + return function() { + return 'make sense'; + }; + }); + } + } + ]); + assert.strictEqual(require('module-g').createSentence(), + 'bad tests don\'t make sense.'); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-g': 'module-g-plugin' + })); + assert.strictEqual(require('module-g').createSentence(), + 'good tests make sense.', + 'Files internal to a module are patched'); + }); + + /** + * Alternately loads two versions of the same module, and checks that each one + * is patched differently. + */ + it('can patch multiple different versions of the same module', function() { + var v1 = { getVersion: function() { return '1.0.0'; } }; + var v2 = { getVersion: function() { return '2.0.0'; } }; + addModuleMock('module-h', '1.0.0', v1); + addModuleMock('module-h-plugin', '', [ + { + versions: '1.x', + patch: function(originalModule, api) { + shimmer.wrap(originalModule, 'getVersion', function(origGetVersion) { + return function() { + return origGetVersion() + ' is ok'; + }; + }); + } + }, + { + versions: '2.x', + patch: function(originalModule, api) { + shimmer.wrap(originalModule, 'getVersion', function(origGetVersion) { + return function() { + return origGetVersion() + ' is better'; + }; + }); + } + } + ]); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-h': 'module-h-plugin' + })); + assert.strictEqual(require('module-h').getVersion(), '1.0.0 is ok', + 'Initial patch is correct'); + addModuleMock('module-h', '2.0.0', v2); + assert.strictEqual(require('module-h').getVersion(), '2.0.0 is better', + 'Second loaded version is also patched'); + addModuleMock('module-h', '1.0.0', v1); + assert.strictEqual(require('module-h').getVersion(), '1.0.0 is ok', + 'First loaded version doesn\'t get patched again'); + }); + + /** + * Uses module interception to + */ + it('can intercept modules', function() { + addModuleMock('module-i', '1.0.0', function() { return 1; }); + addModuleMock('module-i-plugin', '', [{ + intercept: function(originalModule, api) { + return function() { return originalModule() + 1; }; + } + }]); + assert.strictEqual(require('module-i')(), 1); + // Activate plugin loader + pluginLoader.activate(createFakeAgent({ + 'module-i': 'module-i-plugin' + })); + assert.strictEqual(require('module-i')(), 2, + 'Module can be intercepted'); + }); +});