WIP / DRAFT : replacing shimmer with mpwrapper

packages/opentelemetry-core/src/common/types.ts

@@ -32,13 +32,3 @@ export interface TimeOriginLegacy {
     fetchStart: number;
   };
 }
-
-/**
- * This interface defines the params that are be added to the wrapped function
- * using the "shimmer.wrap"
- */
-export interface ShimWrapped {
-  __wrapped: boolean;
-  __unwrap: Function;
-  __original: Function;
-}

packages/opentelemetry-core/src/index.ts

@@ -34,4 +34,3 @@ export * from './trace/spancontext-utils';
 export * from './trace/TraceState';
 export * from './metrics/NoopMeter';
 export * from './utils/url';
-export * from './utils/wrap';

packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts

@@ -40,6 +40,8 @@ export abstract class BasePlugin<T> implements Plugin<T> {
   protected readonly _internalFilesList?: PluginInternalFiles; // required for internalFilesExports
   protected _config!: PluginConfig;
 
+  protected _unpatchArr: Function[] = [];
+
   constructor(
     protected readonly _tracerName: string,
     protected readonly _tracerVersion?: string
@@ -63,7 +65,7 @@ export abstract class BasePlugin<T> implements Plugin<T> {
   }
 
   disable(): void {
-    this.unpatch();
+    this._unpatch();
   }
 
   /**
@@ -146,6 +148,22 @@ export abstract class BasePlugin<T> implements Plugin<T> {
     });
   }
 
+  private _unpatch(): void {
+    this._logger.debug(
+      'removing patch to %s@%s',
+      this.moduleName,
+      this.version
+    );
+    this._unpatchArr.forEach(unpatch => {
+      if (typeof unpatch === 'function') {
+        unpatch();
+      }
+    });
+    this._unpatchArr = [];
+    this.unpatch();
+  }
+
+  protected unpatch(): void {}
+
   protected abstract patch(): T;
-  protected abstract unpatch(): void;
 }

packages/opentelemetry-core/test/trace/BasePlugin.test.ts

@@ -78,7 +78,6 @@ class TestPlugin extends BasePlugin<{ [key: string]: Function }> {
   protected patch(): { [key: string]: Function } {
     return this._moduleExports;
   }
-  protected unpatch(): void {}
 }
 
 const basedir = path.dirname(require.resolve('./fixtures/test-package'));

packages/opentelemetry-plugin-dns/package.json

@@ -46,7 +46,6 @@
     "@types/mocha": "^5.2.7",
     "@types/node": "^12.7.12",
     "@types/semver": "^6.2.0",
-    "@types/shimmer": "^1.0.1",
     "@types/sinon": "^7.5.0",
     "codecov": "^3.6.1",
     "gts": "^1.1.0",
@@ -63,7 +62,7 @@
   "dependencies": {
     "@opentelemetry/core": "^0.3.3",
     "@opentelemetry/types": "^0.3.3",
-    "semver": "^6.3.0",
-    "shimmer": "^1.2.1"
+    "mpwrapper": "^0.1.7",
+    "semver": "^6.3.0"
   }
 }

packages/opentelemetry-plugin-dns/src/dns.ts

@@ -18,14 +18,13 @@ import { BasePlugin } from '@opentelemetry/core';
 import { Span, SpanKind, SpanOptions } from '@opentelemetry/types';
 import { LookupAddress } from 'dns';
 import * as semver from 'semver';
-import * as shimmer from 'shimmer';
+import * as mpWrapper from 'mpwrapper';
 import { AddressFamily } from './enums/AddressFamily';
 import { AttributeNames } from './enums/AttributeNames';
 import {
   Dns,
   DnsPluginConfig,
   LookupCallbackSignature,
-  LookupFunction,
   LookupFunctionSignature,
   LookupPromiseSignature,
 } from './types';
@@ -54,21 +53,25 @@ export class DnsPlugin extends BasePlugin<Dns> {
       this.version
     );
 
-    shimmer.wrap<{ lookup: LookupFunction }, 'lookup'>(
-      this._moduleExports,
-      'lookup',
-      // tslint:disable-next-line:no-any
-      this._getLookup() as any
+    this._unpatchArr.push(
+      mpWrapper.wrap(
+        this._moduleExports,
+        'lookup',
+        // tslint:disable-next-line:no-any
+        this._getLookup() as any
+      ).unwrap
     );
 
     // new promise methods in node >= 10.6.0
     // https://nodejs.org/docs/latest/api/dns.html#dns_dnspromises_lookup_hostname_options
     if (semver.gte(this.version, '10.6.0')) {
-      shimmer.wrap(
-        this._moduleExports.promises,
-        'lookup',
-        // tslint:disable-next-line:no-any
-        this._getLookup() as any
+      this._unpatchArr.push(
+        mpWrapper.wrap(
+          this._moduleExports.promises,
+          'lookup',
+          // tslint:disable-next-line:no-any
+          this._getLookup() as any
+        ).unwrap
       );
     }
 
@@ -77,10 +80,10 @@ export class DnsPlugin extends BasePlugin<Dns> {
 
   /** Unpatches all DNS patched function. */
   protected unpatch(): void {
-    shimmer.unwrap(this._moduleExports, 'lookup');
-    if (semver.gte(this.version, '10.6.0')) {
-      shimmer.unwrap(this._moduleExports.promises, 'lookup');
-    }
+    // mpWrapper.unwrap(this._moduleExports, 'lookup');
+    // if (semver.gte(this.version, '10.6.0')) {
+    //   mpWrapper.unwrap(this._moduleExports.promises, 'lookup');
+    // }
   }
 
   /**

packages/opentelemetry-plugin-dns/test/functionals/dns-disable.test.ts

@@ -34,7 +34,7 @@ registry.addSpanProcessor(new SimpleSpanProcessor(memoryExporter));
 describe('DnsPlugin', () => {
   before(() => {
     plugin.enable(dns, registry, tracer.logger);
-    assert.strictEqual(dns.lookup.__wrapped, true);
+    assert.strictEqual(dns.lookup.__mpWrapped, true);
   });
 
   beforeEach(() => {
@@ -58,7 +58,7 @@ describe('DnsPlugin', () => {
         const spans = memoryExporter.getFinishedSpans();
         assert.strictEqual(spans.length, 0);
 
-        assert.strictEqual(dns.lookup.__wrapped, undefined);
+        assert.strictEqual(dns.lookup.__mpWrapped, undefined);
         assert.strictEqual((tracer.withSpan as sinon.SinonSpy).called, false);
         done();
       });

packages/opentelemetry-plugin-grpc/package.json

@@ -46,7 +46,6 @@
     "@types/mocha": "^5.2.7",
     "@types/node": "^12.6.9",
     "@types/semver": "^6.2.0",
-    "@types/shimmer": "^1.0.1",
     "@types/sinon": "^7.0.13",
     "codecov": "^3.6.1",
     "grpc": "^1.23.3",
@@ -65,6 +64,6 @@
   "dependencies": {
     "@opentelemetry/core": "^0.3.3",
     "@opentelemetry/types": "^0.3.3",
-    "shimmer": "^1.2.1"
+    "mpwrapper": "^0.1.7"
   }
 }

packages/opentelemetry-plugin-grpc/src/grpc.ts

@@ -26,7 +26,7 @@ import {
 import * as events from 'events';
 import * as grpcTypes from 'grpc';
 import * as path from 'path';
-import * as shimmer from 'shimmer';
+import * as mpWrapper from 'mpwrapper';
 import { AttributeNames } from './enums/AttributeNames';
 import {
   grpc,
@@ -74,19 +74,23 @@ export class GrpcPlugin extends BasePlugin<grpc> {
     );
 
     if (this._moduleExports.Server) {
-      shimmer.wrap(
-        this._moduleExports.Server.prototype,
-        'register',
-        // tslint:disable-next-line:no-any
-        this._patchServer() as any
+      this._unpatchArr.push(
+        mpWrapper.wrap(
+          this._moduleExports.Server.prototype,
+          'register',
+          // tslint:disable-next-line:no-any
+          this._patchServer() as any
+        ).unwrap
       );
     }
 
     // Wrap the externally exported client constructor
-    shimmer.wrap(
-      this._moduleExports,
-      'makeGenericClientConstructor',
-      this._patchClient()
+    this._unpatchArr.push(
+      mpWrapper.wrap(
+        this._moduleExports,
+        'makeGenericClientConstructor',
+        this._patchClient()
+      ).unwrap
     );
 
     if (this._internalFilesExports['client']) {
@@ -95,32 +99,17 @@ export class GrpcPlugin extends BasePlugin<grpc> {
       ] as GrpcInternalClientTypes;
 
       // Wrap the internally used client constructor
-      shimmer.wrap(
-        grpcClientModule,
-        'makeClientConstructor',
-        this._patchClient()
+      this._unpatchArr.push(
+        mpWrapper.wrap(
+          grpcClientModule,
+          'makeClientConstructor',
+          this._patchClient()
+        ).unwrap
       );
     }
 
     return this._moduleExports;
   }
-  protected unpatch(): void {
-    this._logger.debug(
-      'removing patch to %s@%s',
-      this.moduleName,
-      this.version
-    );
-
-    if (this._moduleExports.Server) {
-      shimmer.unwrap(this._moduleExports.Server.prototype, 'register');
-    }
-
-    shimmer.unwrap(this._moduleExports, 'makeGenericClientConstructor');
-
-    if (grpcClientModule) {
-      shimmer.unwrap(grpcClientModule, 'makeClientConstructor');
-    }
-  }
 
   private _getSpanContext(metadata: grpcTypes.Metadata): SpanContext | null {
     const metadataValue = metadata.getMap()[GRPC_TRACE_KEY] as Buffer;
@@ -160,7 +149,7 @@ export class GrpcPlugin extends BasePlugin<grpc> {
         const originalResult = originalRegister.apply(this, arguments as any);
         const handlerSet = this.handlers[name];
 
-        shimmer.wrap(
+        mpWrapper.wrap(
           handlerSet,
           'func',
           (originalFunc: grpcTypes.handleCall<RequestType, ResponseType>) => {
@@ -328,7 +317,7 @@ export class GrpcPlugin extends BasePlugin<grpc> {
       ) {
         // tslint:disable-next-line:no-any
         const client = original.apply(this, arguments as any);
-        shimmer.massWrap(
+        mpWrapper.massWrap(
           client.prototype as never,
           plugin._getMethodsToWrap(client, methods) as never[],
           // tslint:disable-next-line:no-any