feat!(instrumentation): remove moduleExports generic type from instru…

…mentation registration (#4598) * feat!(instrumentation): remove moudleExports generic type from instrumentation registration * fix: lint * chore: add changelog * fix: core instrumentations * docs: update README with the change * Update experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com> * Update experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com> * Update CHANGELOG.md Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com> * chore: lint * revert: sdk-logs in tsconfig * chore: lint markdown * Apply suggestions from code review Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com> * Update experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com> * Update experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleDefinition.ts Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com> * fix: remove unrelevant eslint ignore --------- Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com> Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com>
open-telemetry · Apr 19, 2024 · 99431df · 99431df
1 parent 73fddf9
commit 99431df
Show file tree

Hide file tree

Showing 13 changed files with 74 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,12 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :boom: Breaking Change
 
+* feat!(instrumentation): remove moduleExports generic type from instrumentation registration [#4598](https://github.com/open-telemetry/opentelemetry-js/pull/4598) @blumamir
+  * breaking for instrumentation authors that depend on
+    * `InstrumentationBase`
+    * `InstrumentationNodeModuleDefinition`
+    * `InstrumentationNodeModuleFile`
+
 ### :rocket: (Enhancement)
 
 * feat(sdk-trace-base): log resource attributes in ConsoleSpanExporter [#4605](https://github.com/open-telemetry/opentelemetry-js/pull/4605) @pichlermarc

diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
@@ -72,9 +72,7 @@ export interface FetchInstrumentationConfig extends InstrumentationConfig {
 /**
  * This class represents a fetch plugin for auto instrumentation
  */
-export class FetchInstrumentation extends InstrumentationBase<
-  Promise<Response>
-> {
+export class FetchInstrumentation extends InstrumentationBase {
   readonly component: string = 'fetch';
   readonly version: string = VERSION;
   moduleName = this.component;

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
@@ -98,7 +98,7 @@ export class GrpcInstrumentation extends InstrumentationBase {
 
   init() {
     return [
-      new InstrumentationNodeModuleDefinition<any>(
+      new InstrumentationNodeModuleDefinition(
         '@grpc/grpc-js',
         ['1.*'],
         (moduleExports, version) => {

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -63,7 +63,7 @@ import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
 /**
  * Http instrumentation instrumentation for Opentelemetry
  */
-export class HttpInstrumentation extends InstrumentationBase<Http> {
+export class HttpInstrumentation extends InstrumentationBase {
   /** keep track on spans not ended */
   private readonly _spanNotEnded: WeakSet<Span> = new WeakSet<Span>();
   private _headerCapture;
@@ -104,18 +104,18 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
   }
 
   init(): [
-    InstrumentationNodeModuleDefinition<Https>,
-    InstrumentationNodeModuleDefinition<Http>,
+    InstrumentationNodeModuleDefinition,
+    InstrumentationNodeModuleDefinition,
   ] {
     return [this._getHttpsInstrumentation(), this._getHttpInstrumentation()];
   }
 
   private _getHttpInstrumentation() {
     const version = process.versions.node;
-    return new InstrumentationNodeModuleDefinition<Http>(
+    return new InstrumentationNodeModuleDefinition(
       'http',
       ['*'],
-      moduleExports => {
+      (moduleExports: Http): Http => {
         this._diag.debug(`Applying patch for http@${version}`);
         if (isWrapped(moduleExports.request)) {
           this._unwrap(moduleExports, 'request');
@@ -143,7 +143,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         );
         return moduleExports;
       },
-      moduleExports => {
+      (moduleExports: Http) => {
         if (moduleExports === undefined) return;
         this._diag.debug(`Removing patch for http@${version}`);
 
@@ -156,10 +156,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
 
   private _getHttpsInstrumentation() {
     const version = process.versions.node;
-    return new InstrumentationNodeModuleDefinition<Https>(
+    return new InstrumentationNodeModuleDefinition(
       'https',
       ['*'],
-      moduleExports => {
+      (moduleExports: Https): Https => {
         this._diag.debug(`Applying patch for https@${version}`);
         if (isWrapped(moduleExports.request)) {
           this._unwrap(moduleExports, 'request');
@@ -187,7 +187,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         );
         return moduleExports;
       },
-      moduleExports => {
+      (moduleExports: Https) => {
         if (moduleExports === undefined) return;
         this._diag.debug(`Removing patch for https@${version}`);
 

diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts
@@ -81,7 +81,7 @@ export interface XMLHttpRequestInstrumentationConfig
 /**
  * This class represents a XMLHttpRequest plugin for auto instrumentation
  */
-export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRequest> {
+export class XMLHttpRequestInstrumentation extends InstrumentationBase {
   readonly component: string = 'xml-http-request';
   readonly version: string = VERSION;
   moduleName = this.component;

diff --git a/experimental/packages/opentelemetry-instrumentation/README.md b/experimental/packages/opentelemetry-instrumentation/README.md
@@ -36,7 +36,7 @@ export class MyInstrumentation extends InstrumentationBase {
    *   the plugin should patch multiple modules or versions.
    */
   protected init() {
-    const module = new InstrumentationNodeModuleDefinition<typeof module_name_to_be_patched>(
+    const module = new InstrumentationNodeModuleDefinition(
       'module_name_to_be_patched',
       ['1.*'],
        this._onPatchMain,
@@ -63,8 +63,8 @@ export class MyInstrumentation extends InstrumentationBase {
     this._unwrap(moduleExports, 'mainMethodName');
   }
 
-  private _addPatchingMethod(): InstrumentationNodeModuleFile<typeof module_name_to_be_patched> {
-    const file = new InstrumentationNodeModuleFile<typeof module_name_to_be_patched>(
+  private _addPatchingMethod(): InstrumentationNodeModuleFile {
+    const file = new InstrumentationNodeModuleFile(
       'module_name_to_be_patched/src/some_file.js',
       this._onPatchMethodName,
       this._onUnPatchMethodName,

diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts
@@ -35,9 +35,7 @@ import {
 /**
  * Base abstract internal class for instrumenting node and web plugins
  */
-export abstract class InstrumentationAbstract<T = any>
-  implements Instrumentation
-{
+export abstract class InstrumentationAbstract implements Instrumentation {
   protected _config: InstrumentationConfig;
 
   private _tracer: Tracer;
@@ -116,7 +114,7 @@ export abstract class InstrumentationAbstract<T = any>
    *
    * @returns an array of {@link InstrumentationModuleDefinition}
    */
-  public getModuleDefinitions(): InstrumentationModuleDefinition<T>[] {
+  public getModuleDefinitions(): InstrumentationModuleDefinition[] {
     const initResult = this.init() ?? [];
     if (!Array.isArray(initResult)) {
       return [initResult];
@@ -172,7 +170,7 @@ export abstract class InstrumentationAbstract<T = any>
    * methods.
    */
   protected abstract init():
-    | InstrumentationModuleDefinition<T>
-    | InstrumentationModuleDefinition<T>[]
+    | InstrumentationModuleDefinition
+    | InstrumentationModuleDefinition[]
     | void;
 }
diff --git a/...imental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleDefinition.ts b/...imental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleDefinition.ts
@@ -19,16 +19,18 @@ import {
   InstrumentationModuleFile,
 } from './types';
 
-export class InstrumentationNodeModuleDefinition<T>
-  implements InstrumentationModuleDefinition<T>
+export class InstrumentationNodeModuleDefinition
+  implements InstrumentationModuleDefinition
 {
-  files: InstrumentationModuleFile<T>[];
+  files: InstrumentationModuleFile[];
   constructor(
     public name: string,
     public supportedVersions: string[],
-    public patch?: (exports: T, moduleVersion?: string) => T,
-    public unpatch?: (exports: T, moduleVersion?: string) => void,
-    files?: InstrumentationModuleFile<any>[]
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    public patch?: (exports: any, moduleVersion?: string) => any,
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    public unpatch?: (exports: any, moduleVersion?: string) => void,
+    files?: InstrumentationModuleFile[]
   ) {
     this.files = files || [];
   }

diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts
@@ -17,15 +17,17 @@
 import { InstrumentationModuleFile } from './types';
 import { normalize } from './platform/index';
 
-export class InstrumentationNodeModuleFile<T>
-  implements InstrumentationModuleFile<T>
+export class InstrumentationNodeModuleFile
+  implements InstrumentationModuleFile
 {
   public name: string;
   constructor(
     name: string,
     public supportedVersions: string[],
-    public patch: (moduleExports: T, moduleVersion?: string) => T,
-    public unpatch: (moduleExports?: T, moduleVersion?: string) => void
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    public patch: (moduleExports: any, moduleVersion?: string) => any,
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    public unpatch: (moduleExports?: any, moduleVersion?: string) => void
   ) {
     this.name = normalize(name);
   }

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -35,11 +35,11 @@ import { readFileSync } from 'fs';
 /**
  * Base abstract class for instrumenting node plugins
  */
-export abstract class InstrumentationBase<T = any>
+export abstract class InstrumentationBase
   extends InstrumentationAbstract
   implements types.Instrumentation
 {
-  private _modules: InstrumentationModuleDefinition<T>[];
+  private _modules: InstrumentationModuleDefinition[];
   private _hooks: (Hooked | Hook)[] = [];
   private _requireInTheMiddleSingleton: RequireInTheMiddleSingleton =
     RequireInTheMiddleSingleton.getInstance();
@@ -58,7 +58,7 @@ export abstract class InstrumentationBase<T = any>
       modules = [modules];
     }
 
-    this._modules = (modules as InstrumentationModuleDefinition<T>[]) || [];
+    this._modules = (modules as InstrumentationModuleDefinition[]) || [];
 
     if (this._modules.length === 0) {
       diag.debug(
@@ -143,7 +143,7 @@ export abstract class InstrumentationBase<T = any>
   };
 
   private _warnOnPreloadedModules(): void {
-    this._modules.forEach((module: InstrumentationModuleDefinition<T>) => {
+    this._modules.forEach((module: InstrumentationModuleDefinition) => {
       const { name } = module;
       try {
         const resolvedModule = require.resolve(name);
@@ -174,7 +174,7 @@ export abstract class InstrumentationBase<T = any>
   }
 
   private _onRequire<T>(
-    module: InstrumentationModuleDefinition<T>,
+    module: InstrumentationModuleDefinition,
     exports: T,
     name: string,
     baseDir?: string | void
@@ -216,7 +216,8 @@ export abstract class InstrumentationBase<T = any>
     return supportedFileInstrumentations.reduce<T>((patchedExports, file) => {
       file.moduleExports = patchedExports;
       if (this._enabled) {
-        return file.patch(patchedExports, module.moduleVersion);
+        // patch signature is not typed, so we cast it assuming it's correct
+        return file.patch(patchedExports, module.moduleVersion) as T;
       }
       return patchedExports;
     }, exports);
@@ -246,20 +247,10 @@ export abstract class InstrumentationBase<T = any>
     this._warnOnPreloadedModules();
     for (const module of this._modules) {
       const hookFn: HookFn = (exports, name, baseDir) => {
-        return this._onRequire<typeof exports>(
-          module as unknown as InstrumentationModuleDefinition<typeof exports>,
-          exports,
-          name,
-          baseDir
-        );
+        return this._onRequire<typeof exports>(module, exports, name, baseDir);
       };
       const onRequire: OnRequireFn = (exports, name, baseDir) => {
-        return this._onRequire<typeof exports>(
-          module as unknown as InstrumentationModuleDefinition<typeof exports>,
-          exports,
-          name,
-          baseDir
-        );
+        return this._onRequire<typeof exports>(module, exports, name, baseDir);
       };
 
       // `RequireInTheMiddleSingleton` does not support absolute paths.

diff --git a/experimental/packages/opentelemetry-instrumentation/src/types.ts b/experimental/packages/opentelemetry-instrumentation/src/types.ts
@@ -82,29 +82,30 @@ export interface ShimWrapped extends Function {
   __original: Function;
 }
 
-export interface InstrumentationModuleFile<T> {
+export interface InstrumentationModuleFile {
   /** Name of file to be patched with relative path */
   name: string;
 
-  moduleExports?: T;
+  moduleExports?: unknown;
 
   /** Supported version this file */
   supportedVersions: string[];
 
   /** Method to patch the instrumentation  */
-  patch(moduleExports: T, moduleVersion?: string): T;
+  patch(moduleExports: unknown, moduleVersion?: string): unknown;
 
   /** Method to patch the instrumentation  */
 
   /** Method to unpatch the instrumentation  */
-  unpatch(moduleExports?: T, moduleVersion?: string): void;
+  unpatch(moduleExports?: unknown, moduleVersion?: string): void;
 }
 
-export interface InstrumentationModuleDefinition<T> {
+export interface InstrumentationModuleDefinition {
   /** Module name or path  */
   name: string;
 
-  moduleExports?: T;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  moduleExports?: any;
 
   /** Instrumented module version */
   moduleVersion?: string;
@@ -113,15 +114,16 @@ export interface InstrumentationModuleDefinition<T> {
   supportedVersions: string[];
 
   /** Module internal files to be patched  */
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  files: InstrumentationModuleFile<any>[];
+  files: InstrumentationModuleFile[];
 
   /** If set to true, the includePrerelease check will be included when calling semver.satisfies */
   includePrerelease?: boolean;
 
   /** Method to patch the instrumentation  */
-  patch?: (moduleExports: T, moduleVersion?: string) => T;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  patch?: (moduleExports: any, moduleVersion?: string) => any;
 
   /** Method to unpatch the instrumentation  */
-  unpatch?: (moduleExports: T, moduleVersion?: string) => void;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  unpatch?: (moduleExports: any, moduleVersion?: string) => void;
 }
diff --git a/experimental/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts b/experimental/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts
@@ -135,7 +135,7 @@ describe('BaseInstrumentation', () => {
   });
 
   describe('getModuleDefinitions', () => {
-    const moduleDefinition: InstrumentationModuleDefinition<unknown> = {
+    const moduleDefinition: InstrumentationModuleDefinition = {
       name: 'foo',
       patch: moduleExports => {},
       unpatch: moduleExports => {},