feat: add mechanism to load internal module files to patch (#52)

* feat: add mechanism to load internal module files to patch

* refactor: change Plugin Interface and BasePlugin to implement GoF Template Method

* refactor: change http plugin to new interface

* refactor: change https plugin to new interface

*  refactor: change http2 plugin to new interface

* refactor: change mongodb plugin to new interface

* refactor: improve load internal file test case

* refactor: rename methods of Plugin interface

Author: fabiogomessilva

packages/opencensus-core/src/trace/instrumentation/base-plugin.ts

@@ -13,11 +13,23 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import * as shimmer from 'shimmer';
+import * as path from 'path';
+import * as semver from 'semver';
+
+import {logger} from '../../common/console-logger';
+import {Logger} from '../../common/types';
 import * as modelTypes from '../model/types';
+
 import * as types from './types';
 
-// TODO: improve Jsdoc comments
+/**
+ * Maps a name (key) representing a internal file module and its exports
+ */
+export type ModuleExportsMapping = {
+  // tslint:disable:no-any
+  [key: string]: any;
+};
+
 
 /** This class represent the base to patch plugin. */
 export abstract class BasePlugin implements types.Plugin {
@@ -30,6 +42,14 @@ export abstract class BasePlugin implements types.Plugin {
   protected tracer: modelTypes.Tracer;
   /** The module version. */
   protected version: string;
+  /** a logger */
+  protected logger: Logger;
+  /** list of internal files that need patch and are not exported by default */
+  protected readonly internalFileList: types.PluginInternalFiles;
+  /**  internal files loaded */
+  protected internalFilesExports: ModuleExportsMapping;
+  /** module directory - used to load internal files */
+  protected basedir: string;
 
   /**
    * Constructs a new BasePlugin instance.
@@ -44,26 +64,109 @@ export abstract class BasePlugin implements types.Plugin {
    * @param moduleExports nodejs module exports to set as context
    * @param tracer tracer relating to context
    * @param version module version description
+   * @param basedir module absolute path
    */
   // tslint:disable:no-any
-  protected setPluginContext(
-      moduleExports: any, tracer: modelTypes.Tracer, version: string) {
+  private setPluginContext(
+      moduleExports: any, tracer: modelTypes.Tracer, version: string,
+      basedir?: string) {
     this.moduleExports = moduleExports;
     this.tracer = tracer;
     this.version = version;
+    this.basedir = basedir;
+    this.logger = tracer.logger;
+    this.internalFilesExports = this.loadInternalFiles();
   }
 
 
-  // TODO: review this implementation
-  // From the perspective of an instrumentation module author,
-  // that applyUnpatch is abstract makes it seem like patching is optional,
-  // while unpatching is not. It should be the other way around
+  /**
+   * Method that enables the instrumentation patch.
+   *
+   * This method implements the GoF Template Method Pattern
+   * 'enable' is the invariant part of the pattern and
+   * 'applyPatch' the variant.
+   *
+   * @param moduleExports nodejs module exports from the module to patch
+   * @param tracer a tracer instance
+   * @param version version of the current instaled module to patch
+   * @param basedir module absolute path
+   */
+  enable(
+      // tslint:disable:no-any
+      moduleExports: any, tracer: modelTypes.Tracer, version: string,
+      basedir: string) {
+    this.setPluginContext(moduleExports, tracer, version, basedir);
+    return this.applyPatch();
+  }
+
+  /** Method to disable the instrumentation  */
+  disable() {
+    this.applyUnpatch();
+  }
 
+  /**
+   * This method implements the GoF Template Method Pattern,
+   * 'applyPatch' is the variant part, each instrumentation should
+   * implement its own version, 'enable' method is the invariant.
+   * Wil be called when enable is called.
+   *
+   */
   // tslint:disable:no-any
-  applyPatch(moduleExports: any, tracer: modelTypes.Tracer, version: string):
-      any {
-    this.setPluginContext(moduleExports, tracer, version);
+  protected abstract applyPatch(): any;
+  protected abstract applyUnpatch(): void;
+
+
+  /**
+   * Load internal files according to version range
+   */
+  private loadInternalFiles(): ModuleExportsMapping {
+    let result: ModuleExportsMapping = null;
+    if (this.internalFileList) {
+      this.logger.debug('loadInternalFiles %o', this.internalFileList);
+      Object.keys(this.internalFileList).forEach(versionRange => {
+        if (semver.satisfies(this.version, versionRange)) {
+          if (result) {
+            this.logger.warn(
+                'Plugin for %s@%s, has overlap version range (%s) for internal files: %o',
+                this.moduleName, this.version, versionRange,
+                this.internalFileList);
+          }
+          result = this.loadInternalModuleFiles(
+              this.internalFileList[versionRange], this.basedir);
+        }
+      });
+      if (!result) {
+        this.logger.debug(
+            'No internal file could be loaded for %s@%s', this.moduleName,
+            this.version);
+      }
+    }
+
+    return result;
   }
 
-  abstract applyUnpatch(): void;
+
+  /**
+   * Load internal files from a module and  set internalFilesExports
+   */
+  private loadInternalModuleFiles(
+      extraModulesList: types.PluginNames,
+      basedir: string): ModuleExportsMapping {
+    const extraModules: ModuleExportsMapping = {};
+    if (extraModulesList) {
+      Object.keys(extraModulesList).forEach(moduleName => {
+        try {
+          this.logger.debug('loading File %s', extraModulesList[moduleName]);
+          extraModules[moduleName] =
+              require(path.join(basedir, extraModulesList[moduleName]));
+        } catch (e) {
+          this.logger.error(
+              'Could not load internal file %s of module %s. Error: %s',
+              path.join(basedir, extraModulesList[moduleName]), this.moduleName,
+              e.message);
+        }
+      });
+    }
+    return extraModules;
+  }
 }

packages/opencensus-core/src/trace/instrumentation/types.ts

@@ -19,15 +19,20 @@ import {Tracer} from '../model/types';
 /** Interface Plugin to apply patch. */
 export interface Plugin {
   /**
-   * Method to apply the instrumentation patch
+   * Method that enables the instrumentation patch.
+   *
    * @param moduleExports nodejs module exports from the module to patch
    * @param tracer a tracer instance
    * @param version version of the current instaled module to patch
+   * @param basedir module absolute path
    */
-  // tslint:disable:no-any
-  applyPatch(moduleExports: any, tracer: Tracer, version: string): any;
-  /** Method to unpatch the instrumentation  */
-  applyUnpatch(): void;
+  enable(
+      // tslint:disable-next-line:no-any
+      moduleExports: any, tracer: Tracer, version: string,
+      // tslint:disable-next-line:no-any
+      basedir?: string): any;
+  /** Method to disable the instrumentation  */
+  disable(): void;
 }
 
 
@@ -39,3 +44,11 @@ export interface Plugin {
 export type PluginNames = {
   [pluginName: string]: string;
 };
+
+/**
+ * Each key should be the name of the module to trace, and its value
+ * a mapping of a property name to a internal plugin file name.
+ */
+export type PluginInternalFiles = {
+  [versions: string]: PluginNames;
+};

packages/opencensus-instrumentation-http/src/http.ts

@@ -44,7 +44,6 @@ export class HttpPlugin extends classes.BasePlugin {
   static ATTRIBUTE_HTTP_ERROR_NAME = 'http.error_name';
   static ATTRIBUTE_HTTP_ERROR_MESSAGE = 'http.error_message';
 
-  logger: types.Logger;
 
   /** Constructs a new HttpPlugin instance. */
   constructor(moduleName: string) {
@@ -54,44 +53,37 @@ export class HttpPlugin extends classes.BasePlugin {
 
   /**
    * Patches HTTP incoming and outcoming request functions.
-   * @param moduleExports The http module exports
-   * @param tracer A tracer instance to create spans on.
-   * @param version The package version.
    */
-  // tslint:disable-next-line:no-any
-  applyPatch(moduleExports: HttpModule, tracer: types.Tracer, version: string) {
-    this.setPluginContext(moduleExports, tracer, version);
-    this.logger = tracer.logger || logger.logger('debug');
-
+  protected applyPatch() {
     this.logger.debug('applying pacth to %s@%s', this.moduleName, this.version);
 
     shimmer.wrap(
-        moduleExports, 'request', this.getPatchOutgoingRequestFunction());
+        this.moduleExports, 'request', this.getPatchOutgoingRequestFunction());
 
     // In Node 8, http.get calls a private request method, therefore we patch it
     // here too.
-    if (semver.satisfies(version, '>=8.0.0')) {
+    if (semver.satisfies(this.version, '>=8.0.0')) {
       shimmer.wrap(
-          moduleExports, 'get', this.getPatchOutgoingRequestFunction());
+          this.moduleExports, 'get', this.getPatchOutgoingRequestFunction());
     }
 
-    if (moduleExports && moduleExports.Server &&
-        moduleExports.Server.prototype) {
+    if (this.moduleExports && this.moduleExports.Server &&
+        this.moduleExports.Server.prototype) {
       shimmer.wrap(
-          moduleExports.Server.prototype, 'emit',
+          this.moduleExports.Server.prototype, 'emit',
           this.getPatchIncomingRequestFunction());
     } else {
       this.logger.error(
           'Could not apply patch to %s.emit. Interface is not as expected.',
           this.moduleName);
     }
 
-    return moduleExports;
+    return this.moduleExports;
   }
 
 
   /** Unpatches all HTTP patched function. */
-  applyUnpatch(): void {
+  protected applyUnpatch(): void {
     shimmer.unwrap(this.moduleExports, 'request');
     if (semver.satisfies(this.version, '>=8.0.0')) {
       shimmer.unwrap(this.moduleExports, 'get');

packages/opencensus-instrumentation-http/test/test-http.ts

@@ -100,7 +100,7 @@ describe('HttpPlugin', () => {
   });
 
   before(() => {
-    plugin.applyPatch(http, tracer, VERSION);
+    plugin.enable(http, tracer, VERSION, null);
     tracer.registerSpanEventListener(rootSpanVerifier);
     server = http.createServer((request, response) => {
       response.end('Test Server Response');
@@ -283,7 +283,7 @@ describe('HttpPlugin', () => {
   /** Should not intercept incoming and outgoing requests */
   describe('applyUnpatch()', () => {
     it('should not create a root span for incoming requests', async () => {
-      plugin.applyUnpatch();
+      plugin.disable();
       const testPath = '/incoming/unpatch/';
       nock.enableNetConnect();
 

packages/opencensus-instrumentation-http2/src/http2.ts

@@ -42,28 +42,22 @@ export class Http2Plugin extends HttpPlugin {
 
   /**
    * Patches HTTP2 incoming and outcoming request functions.
-   * @param moduleExporters The HTTP2 package.
-   * @param tracer A tracer instance to create spans on.
-   * @param version The package version.
    */
-  // tslint:disable-next-line:no-any
-  applyPatch(moduleExports: any, tracer: types.Tracer, version: string) {
-    this.setPluginContext(moduleExports, tracer, version);
-    this.logger = tracer.logger || logger.logger('debug');
-
+  protected applyPatch() {
     shimmer.wrap(
-        moduleExports, 'createServer', this.getPatchCreateServerFunction());
+        this.moduleExports, 'createServer',
+        this.getPatchCreateServerFunction());
     shimmer.wrap(
-        moduleExports, 'createSecureServer',
+        this.moduleExports, 'createSecureServer',
         this.getPatchCreateServerFunction());
 
-    shimmer.wrap(moduleExports, 'connect', this.getPatchConnectFunction());
+    shimmer.wrap(this.moduleExports, 'connect', this.getPatchConnectFunction());
 
-    return moduleExports;
+    return this.moduleExports;
   }
 
   /** Unpatches all HTTP2 patched function. */
-  applyUnpatch(): void {
+  protected applyUnpatch(): void {
     // Only Client and Server constructors will be unwrapped. Any existing
     // Client or Server instances will still trace
     shimmer.unwrap(this.moduleExports, 'createServer');

packages/opencensus-instrumentation-http2/test/test-http2.ts

@@ -100,7 +100,7 @@ describe('Http2Plugin', () => {
   before(() => {
     tracer.registerSpanEventListener(rootSpanVerifier);
 
-    plugin.applyPatch(http2, tracer, VERSION);
+    plugin.enable(http2, tracer, VERSION, null);
     server = http2.createServer();
     server.on('stream', (stream, requestHeaders) => {
       const statusCode = requestHeaders[':path'].length > 1 ?

packages/opencensus-instrumentation-https/src/https.ts

@@ -30,22 +30,15 @@ export class HttpsPlugin extends HttpPlugin {
 
   /**
    * Patches HTTPS incoming and outcoming request functions.
-   * @param moduleExports The HTTPS package.
-   * @param tracer A tracer instance to create spans on.
-   * @param version The package version.
    */
-  // tslint:disable:no-any
-  applyPatch(moduleExports: any, tracer: types.Tracer, version: string) {
-    this.setPluginContext(moduleExports, tracer, version);
-    this.logger = tracer.logger || logger.logger('debug');
-
+  protected applyPatch() {
     this.logger.debug('applying pacth to %s@%s', this.moduleName, this.version);
 
-    if (moduleExports && moduleExports.Server &&
-        moduleExports.Server.prototype) {
+    if (this.moduleExports && this.moduleExports.Server &&
+        this.moduleExports.Server.prototype) {
       shimmer.wrap(
-          moduleExports && moduleExports.Server &&
-              moduleExports.Server.prototype,
+          this.moduleExports && this.moduleExports.Server &&
+              this.moduleExports.Server.prototype,
           'emit', this.getPatchIncomingRequestFunction());
     } else {
       this.logger.error(
@@ -55,13 +48,14 @@ export class HttpsPlugin extends HttpPlugin {
 
     // TODO: review the need to patch 'request'
 
-    shimmer.wrap(moduleExports, 'get', this.getPatchOutgoingRequestFunction());
+    shimmer.wrap(
+        this.moduleExports, 'get', this.getPatchOutgoingRequestFunction());
 
-    return moduleExports;
+    return this.moduleExports;
   }
 
   /** Unpatches all HTTPS patched function. */
-  applyUnpatch(): void {
+  protected applyUnpatch(): void {
     if (this.moduleExports && this.moduleExports.Server &&
         this.moduleExports.Server.prototype) {
       shimmer.unwrap(

packages/opencensus-instrumentation-https/test/test-https.ts

@@ -108,7 +108,7 @@ describe('HttpsPlugin', () => {
   });
 
   before(() => {
-    plugin.applyPatch(https, tracer, VERSION);
+    plugin.enable(https, tracer, VERSION, null);
     tracer.registerSpanEventListener(rootSpanVerifier);
     server = https.createServer(httpsOptions, (request, response) => {
       response.end('Test Server Response');
@@ -289,7 +289,7 @@ describe('HttpsPlugin', () => {
   /** Should not intercept incoming and outgoing requests */
   describe('applyUnpatch()', () => {
     it('should not create a root span for incoming requests', async () => {
-      plugin.applyUnpatch();
+      plugin.disable();
       const testPath = '/incoming/unpatch/';
 
       const options = {host: 'localhost', path: testPath, port: serverPort};