Merge 5069c86 into f129138

packages/opentelemetry-instrumentation-fetch/src/fetch.ts

@@ -70,9 +70,6 @@ export interface FetchInstrumentationConfig extends InstrumentationConfig {
 export class FetchInstrumentation extends InstrumentationBase<
   Promise<Response>
 > {
-  readonly component: string = 'fetch';
-  readonly version: string = VERSION;
-  moduleName = this.component;
   private _usedResources = new WeakSet<PerformanceResourceTiming>();
   private _tasksCount = 0;
 
@@ -82,10 +79,9 @@ export class FetchInstrumentation extends InstrumentationBase<
       VERSION,
       Object.assign({}, config)
     );
+    this.loadInstrumentation();
   }
 
-  init() {}
-
   private _getConfig(): FetchInstrumentationConfig {
     return this._config;
   }
@@ -196,7 +192,7 @@ export class FetchInstrumentation extends InstrumentationBase<
     return this.tracer.startSpan(spanName, {
       kind: api.SpanKind.CLIENT,
       attributes: {
-        [AttributeNames.COMPONENT]: this.moduleName,
+        [AttributeNames.COMPONENT]: this.instrumentationName,
         [SemanticAttributes.HTTP_METHOD]: method,
         [SemanticAttributes.HTTP_URL]: url,
       },

packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts

@@ -61,6 +61,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
     version: string
   ) {
     super(name, version, _config);
+    this.loadInstrumentation(this.getInstrumentationsModules());
   }
 
   public override setConfig(
@@ -69,7 +70,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
     this._config = Object.assign({}, config);
   }
 
-  init() {
+  getInstrumentationsModules() {
     return [
       new InstrumentationNodeModuleDefinition<typeof grpcJs>(
         '@grpc/grpc-js',

packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts

@@ -60,6 +60,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
     version: string
   ) {
     super(name, version, _config);
+    this.loadInstrumentation(this.getInstrumentationsModules());
   }
 
   public override setConfig(
@@ -68,7 +69,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
     this._config = Object.assign({}, config);
   }
 
-  init() {
+  getInstrumentationsModules() {
     return [
       new InstrumentationNodeModuleDefinition<typeof grpcTypes>(
         'grpc',

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

@@ -65,11 +65,6 @@ export class GrpcInstrumentation {
     return this._config;
   }
 
-  init() {
-    // sub instrumentations will already be init when constructing them
-    return;
-  }
-
   enable() {
     this._grpcJsInstrumentation.enable();
     this._grpcNativeInstrumentation.enable();

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

@@ -65,6 +65,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       VERSION,
       Object.assign({}, config)
     );
+    this.loadInstrumentation(this.getInstrumentationsModules());
   }
 
   private _getConfig(): HttpInstrumentationConfig {
@@ -75,7 +76,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
     this._config = Object.assign({}, config);
   }
 
-  init() {
+  getInstrumentationsModules() {
     return [this._getHttpsInstrumentation(), this._getHttpInstrumentation()];
   }
 

packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts

@@ -79,10 +79,6 @@ export interface XMLHttpRequestInstrumentationConfig
  * This class represents a XMLHttpRequest plugin for auto instrumentation
  */
 export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRequest> {
-  readonly component: string = 'xml-http-request';
-  readonly version: string = VERSION;
-  moduleName = this.component;
-
   private _tasksCount = 0;
   private _xhrMem = new WeakMap<XMLHttpRequest, XhrMem>();
   private _usedResources = new WeakSet<PerformanceResourceTiming>();
@@ -95,10 +91,9 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
       VERSION,
       Object.assign({}, config)
     );
+    this.loadInstrumentation();
   }
 
-  init() {}
-
   private _getConfig(): XMLHttpRequestInstrumentationConfig {
     return this._config;
   }
@@ -515,7 +510,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
    * implements enable function
    */
   override enable() {
-    this._diag.debug('applying patch to', this.moduleName, this.version);
+    this._diag.debug('applying patch to', this.instrumentationName, this.instrumentationVersion);
 
     if (isWrapped(XMLHttpRequest.prototype.open)) {
       this._unwrap(XMLHttpRequest.prototype, 'open');
@@ -535,7 +530,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
    * implements disable function
    */
   override disable() {
-    this._diag.debug('removing patch from', this.moduleName, this.version);
+    this._diag.debug('removing patch from', this.instrumentationName, this.instrumentationVersion);
 
     this._unwrap(XMLHttpRequest.prototype, 'open');
     this._unwrap(XMLHttpRequest.prototype, 'send');

packages/opentelemetry-instrumentation/src/instrumentation.ts

@@ -23,13 +23,12 @@ import {
 } from '@opentelemetry/api';
 import { Meter, MeterProvider, metrics } from '@opentelemetry/api-metrics';
 import * as shimmer from 'shimmer';
-import { InstrumentationModuleDefinition } from './platform/node';
 import * as types from './types';
 
 /**
  * Base abstract internal class for instrumenting node and web plugins
  */
-export abstract class InstrumentationAbstract<T = any>
+export abstract class InstrumentationAbstract
   implements types.Instrumentation {
   protected _config: types.InstrumentationConfig;
 
@@ -116,12 +115,4 @@ export abstract class InstrumentationAbstract<T = any>
   /* Enable plugin */
   public abstract disable(): void;
 
-  /**
-   * Init method in which plugin should define _modules and patches for
-   * methods
-   */
-  protected abstract init():
-    | InstrumentationModuleDefinition<T>
-    | InstrumentationModuleDefinition<T>[]
-    | void;
 }

packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts

@@ -16,20 +16,29 @@
 
 import { InstrumentationAbstract } from '../../instrumentation';
 import * as types from '../../types';
+import { InstrumentationBaseBrowser } from './types';
 
 /**
  * Base abstract class for instrumenting web plugins
  */
 export abstract class InstrumentationBase
   extends InstrumentationAbstract
-  implements types.Instrumentation {
+  implements InstrumentationBaseBrowser {
+  private _timer?: number;
+
   constructor(
     instrumentationName: string,
     instrumentationVersion: string,
     config: types.InstrumentationConfig = {}
   ) {
     super(instrumentationName, instrumentationVersion, config);
+    this._timer = window?.setTimeout(() => {
+      throw ('You forgot to call loadInstrumentation in constructor');
+    });
+  }
 
+  loadInstrumentation() {
+    clearTimeout(this._timer);
     if (this._config.enabled) {
       this.enable();
     }

packages/opentelemetry-instrumentation/src/platform/browser/types.ts

@@ -0,0 +1,21 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * 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
+ *
+ *      https://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.
+ */
+
+import { Instrumentation } from '../../types';
+
+export interface InstrumentationBaseBrowser extends Instrumentation {
+  loadInstrumentation(): void;
+}

packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts

@@ -19,41 +19,57 @@ import * as path from 'path';
 import * as RequireInTheMiddle from 'require-in-the-middle';
 import { satisfies } from 'semver';
 import { InstrumentationAbstract } from '../../instrumentation';
-import { InstrumentationModuleDefinition } from './types';
+import {
+  InstrumentationBaseNode,
+  InstrumentationModuleDefinition
+} from './types';
 import { diag } from '@opentelemetry/api';
 
 /**
  * Base abstract class for instrumenting node plugins
  */
 export abstract class InstrumentationBase<T = any>
   extends InstrumentationAbstract
-  implements types.Instrumentation {
-  private _modules: InstrumentationModuleDefinition<T>[];
+  implements InstrumentationBaseNode<T> {
+  private _modules?: InstrumentationModuleDefinition<any>[];
   private _hooks: RequireInTheMiddle.Hooked[] = [];
   private _enabled = false;
+  private _timer?: NodeJS.Timeout;
 
   constructor(
     instrumentationName: string,
     instrumentationVersion: string,
     config: types.InstrumentationConfig = {}
   ) {
     super(instrumentationName, instrumentationVersion, config);
+    this._timer = setTimeout(() => {
+      throw new Error('You forgot to call loadInstrumentation in constructor');
+    });
+  }
+
+  loadInstrumentation(instrumentationModuleDefinitions:
+                        InstrumentationModuleDefinition<any>
+                        | InstrumentationModuleDefinition<any>[]
+                        | void
+  ) {
+    if (typeof this._timer !== 'undefined'){
+      clearTimeout(this._timer);
+    }
 
-    let modules = this.init();
+    let modules = instrumentationModuleDefinitions;
 
     if (modules && !Array.isArray(modules)) {
       modules = [modules];
     }
 
-    this._modules = (modules as InstrumentationModuleDefinition<T>[]) || [];
+    this._modules = modules || [];
 
     if (this._modules.length === 0) {
       diag.warn(
         'No modules instrumentation has been defined,' +
         ' nothing will be patched'
       );
     }
-
     if (this._config.enabled) {
       this.enable();
     }
@@ -109,6 +125,10 @@ export abstract class InstrumentationBase<T = any>
     }
     this._enabled = true;
 
+    if (!this._modules) {
+      throw new Error('Modules not loaded, please call "loadInstrumentation" in' +
+        ' constructor with InstrumentationModuleDefinition as param');
+    }
     // already hooked, just call patch again
     if (this._hooks.length > 0) {
       for (const module of this._modules) {
@@ -149,7 +169,10 @@ export abstract class InstrumentationBase<T = any>
       return;
     }
     this._enabled = false;
-
+    if (!this._modules) {
+      throw new Error('Modules not loaded, please call "loadInstrumentation" in' +
+        ' constructor with InstrumentationModuleDefinition as param');
+    }
     for (const module of this._modules) {
       if (typeof module.unpatch === 'function' && module.moduleExports) {
         module.unpatch(module.moduleExports, module.moduleVersion);

packages/opentelemetry-instrumentation/src/platform/node/types.ts

@@ -14,6 +14,15 @@
  * limitations under the License.
  */
 
+import { Instrumentation } from '../../types';
+
+export interface InstrumentationBaseNode<T = any> extends Instrumentation {
+  loadInstrumentation(instrumentationModuleDefinitions: InstrumentationModuleDefinition<T>
+    | InstrumentationModuleDefinition<T>[]
+    | void
+  ): void;
+}
+
 export interface InstrumentationModuleFile<T> {
   /** Name of file to be patched with relative path */
   name: string;

packages/opentelemetry-instrumentation/src/types.ts

@@ -49,14 +49,6 @@ export interface Instrumentation {
 
   /** Method to get instrumentation config  */
   getConfig(): InstrumentationConfig;
-
-  /**
-   * Contains all supported versions.
-   * All versions must be compatible with [semver](https://semver.org/spec/v2.0.0.html) format.
-   * If the version is not supported, we won't apply instrumentation patch (see `enable` method).
-   * If omitted, all versions of the module will be patched.
-   */
-  supportedVersions?: string[];
 }
 
 export interface InstrumentationConfig {

packages/opentelemetry-instrumentation/test/browser/Instrumentation.test.ts

@@ -0,0 +1,49 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * 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
+ *
+ *      https://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.
+ */
+
+import * as assert from 'assert';
+import { InstrumentationConfig } from '../../src';
+import {
+  InstrumentationBase,
+} from '../../src/platform/browser';
+
+interface TestInstrumentationConfig extends InstrumentationConfig {
+  isActive?: boolean;
+}
+
+class TestInstrumentationWithMissingLoad extends InstrumentationBase {
+  constructor(config: TestInstrumentationConfig & InstrumentationConfig = {}) {
+    super('test', '1.0.0', Object.assign({}, config));
+  }
+
+  override enable() {}
+
+  override disable() {}
+}
+
+describe('BaseInstrumentation', () => {
+  describe('constructor', () => {
+    describe('when instrumentation does NOT call loadInstrumentation in  constructor', () => {
+      it('should raise an error', done => {
+        window.onerror = error => {
+          assert.strictEqual(error, 'Uncaught You forgot to call loadInstrumentation in constructor');
+          done();
+        };
+        new TestInstrumentationWithMissingLoad();
+      });
+    });
+  });
+});

packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts

@@ -28,10 +28,12 @@ interface TestInstrumentationConfig extends InstrumentationConfig {
 class TestInstrumentation extends InstrumentationBase {
   constructor(config: TestInstrumentationConfig & InstrumentationConfig = {}) {
     super('test', '1.0.0', Object.assign({}, config));
+    this.loadInstrumentation();
   }
+
   override enable() {}
+
   override disable() {}
-  init() {}
 }
 
 describe('BaseInstrumentation', () => {