fix: add states to plugin loader mechanism (#72)

* fix: add states to plugin loader mechanism

* fix: address comments

* chore(instrumentation-grpc): pin gRPC to 1.12

Author: kjin

packages/opencensus-instrumentation-grpc/package.json

@@ -60,7 +60,7 @@
   },
   "dependencies": {
     "@opencensus/core": "^0.0.2",
-    "grpc": "^1.12.2",
+    "grpc": "~1.12.2",
     "lodash": "^4.17.10",
     "semver": "^5.5.0",
     "shimmer": "^1.2.0"

packages/opencensus-nodejs/src/trace/instrumentation/plugin-loader.ts

@@ -21,6 +21,12 @@ import * as hook from 'require-in-the-middle';
 
 import {Constants} from '../constants';
 
+enum HookState {
+  UNINITIALIZED,
+  ENABLED,
+  DISABLED
+}
+
 /**
  * The PluginLoader class can load instrumentation plugins that
  * use a patch mechanism to enable automatic tracing for
@@ -33,6 +39,11 @@ export class PluginLoader {
   private logger: Logger;
   /** A list of loaded plugins. */
   plugins: Plugin[] = [];
+  /**
+   * A field that tracks whether the r-i-t-m hook has been loaded for the
+   * first time, as well as whether the hook body is enabled or not.
+   */
+  private hookState = HookState.UNINITIALIZED;
 
   /**
    * Constructs a new PluginLoader instance.
@@ -102,30 +113,33 @@ export class PluginLoader {
    * @param pluginList A list of plugins.
    */
   loadPlugins(pluginList: PluginNames) {
-    // tslint:disable:no-any
-    hook(Object.keys(pluginList), (exports, name, basedir) => {
-      const version = this.getPackageVersion(name, basedir as string);
-      this.logger.info('trying loading %s.%s', name, version);
-      let moduleExports = exports;
-      if (!version) {
-        return moduleExports;
-      } else {
+    if (this.hookState === HookState.UNINITIALIZED) {
+      hook(Object.keys(pluginList), (exports, name, basedir) => {
+        if (this.hookState !== HookState.ENABLED) {
+          return exports;
+        }
+        const version = this.getPackageVersion(name, basedir as string);
+        this.logger.info('trying loading %s.%s', name, version);
+        if (!version) {
+          return exports;
+        }
         this.logger.debug('applying patch to %s@%s module', name, version);
         this.logger.debug(
             'using package %s to patch %s', pluginList[name], name);
         // Expecting a plugin from module;
         try {
           const plugin: Plugin = require(pluginList[name]).plugin;
           this.plugins.push(plugin);
-          moduleExports = plugin.enable(exports, this.tracer, version, basedir);
+          return plugin.enable(exports, this.tracer, version, basedir);
         } catch (e) {
           this.logger.error(
               'could not load plugin %s of module %s. Error: %s',
               pluginList[name], name, e.message);
+          return exports;
         }
-        return moduleExports;
-      }
-    });
+      });
+    }
+    this.hookState = HookState.ENABLED;
   }
 
 
@@ -135,6 +149,7 @@ export class PluginLoader {
       plugin.disable();
     }
     this.plugins = [];
+    this.hookState = HookState.DISABLED;
   }
 
   /**

packages/opencensus-nodejs/test/test-plugin-loader.ts

@@ -102,6 +102,7 @@ describe('Plugin Loader', () => {
         assert.strictEqual(pluginLoader.plugins.length, 1);
         assert.strictEqual(simpleModule.name(), 'patched-' + TEST_MODULES[0]);
         assert.strictEqual(simpleModule.value(), 101);
+        pluginLoader.unloadPlugins();
       });
 
       it('should load and patch extra plugin file', () => {
@@ -119,6 +120,7 @@ describe('Plugin Loader', () => {
         assert.strictEqual(
             loadInternalFileModule.extraName(), 'patched-' + extraModuleName);
         assert.strictEqual(loadInternalFileModule.extraValue(), 121);
+        pluginLoader.unloadPlugins();
       });
 
 
@@ -133,6 +135,7 @@ describe('Plugin Loader', () => {
           assert.ok(txt.indexOf('error') >= 0);
         })();
         assert.strictEqual(pluginLoader.plugins.length, 0);
+        pluginLoader.unloadPlugins();
       });
     });
 
@@ -154,7 +157,7 @@ describe('Plugin Loader', () => {
     });
 
     // Should load/unload end-user (non-default named) plugin.
-    describe('load/unload end-user pluging', () => {
+    describe('load/unload end-user plugin', () => {
       it('should load/unload patch/unpatch end-user plugins', () => {
         const pluginLoader = new PluginLoader(log, tracer);
         assert.strictEqual(pluginLoader.plugins.length, 0);