feat(instrumentation): remove default value for config in base instru…

…mentation constructor (#4695) * fix(instrumentation)make config object required in base instrumentation * chore: CHANGELOG * fix: constructor pattern for instrumentations * chore: lint fix * Update experimental/CHANGELOG.md * Update experimental/CHANGELOG.md Co-authored-by: Daniel Dyla <[email protected]> * Update CHANGELOG.md --------- Co-authored-by: Daniel Dyla <[email protected]> Co-authored-by: Marc Pichler <[email protected]>
open-telemetry · May 17, 2024 · 4c01b33 · 4c01b33
1 parent 806fa97
commit 4c01b33
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 11 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -17,6 +17,7 @@ All notable changes to experimental packages in this project will be documented
     * (internal) OTLPExporterBrowserBase: `RequestType` has been replaced by a `ResponseType` type-argument
     * (internal) OTLPExporterNodeBase: `ServiceRequest` has been replaced by a `ServiceResponse` type-argument
     * (internal) the `@opentelemetry/otlp-exporter-proto-base` package has been removed, and will from now on be deprecated in `npm`
+* feat(instrumentation): remove default value for config in base instrumentation constructor [#4695](https://github.com/open-telemetry/opentelemetry-js/pull/4695): @blumamir
 * fix(instrumentation)!: remove unused supportedVersions from Instrumentation interface [#4694](https://github.com/open-telemetry/opentelemetry-js/pull/4694) @blumamir
 
 ### :rocket: (Enhancement)

diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
@@ -79,7 +79,7 @@ export class FetchInstrumentation extends InstrumentationBase<FetchInstrumentati
   private _usedResources = new WeakSet<PerformanceResourceTiming>();
   private _tasksCount = 0;
 
-  constructor(config?: FetchInstrumentationConfig) {
+  constructor(config: FetchInstrumentationConfig = {}) {
     super('@opentelemetry/instrumentation-fetch', VERSION, config);
   }
 

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
@@ -90,7 +90,7 @@ import { VERSION } from './version';
 export class GrpcInstrumentation extends InstrumentationBase<GrpcInstrumentationConfig> {
   private _metadataCapture: metadataCaptureType;
 
-  constructor(config?: GrpcInstrumentationConfig) {
+  constructor(config: GrpcInstrumentationConfig = {}) {
     super('@opentelemetry/instrumentation-grpc', VERSION, config);
     this._metadataCapture = this._createMetadataCapture();
   }

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -69,7 +69,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
   private _httpServerDurationHistogram!: Histogram;
   private _httpClientDurationHistogram!: Histogram;
 
-  constructor(config?: HttpInstrumentationConfig) {
+  constructor(config: HttpInstrumentationConfig = {}) {
     super('@opentelemetry/instrumentation-http', VERSION, config);
     this._headerCapture = this._createHeaderCapture();
   }

diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts
@@ -90,7 +90,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
   private _xhrMem = new WeakMap<XMLHttpRequest, XhrMem>();
   private _usedResources = new WeakSet<PerformanceResourceTiming>();
 
-  constructor(config?: XMLHttpRequestInstrumentationConfig) {
+  constructor(config: XMLHttpRequestInstrumentationConfig = {}) {
     super('@opentelemetry/instrumentation-xml-http-request', VERSION, config);
   }
 

diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts
@@ -51,8 +51,10 @@ export abstract class InstrumentationAbstract<
   constructor(
     public readonly instrumentationName: string,
     public readonly instrumentationVersion: string,
-    config: ConfigType = {} as ConfigType // assuming ConfigType is an object with optional fields only
+    config: ConfigType
   ) {
+    // copy config first level properties to ensure they are immutable.
+    // nested properties are not copied, thus are mutable from the outside.
     this._config = {
       enabled: true,
       ...config,
@@ -144,10 +146,10 @@ export abstract class InstrumentationAbstract<
    * Sets InstrumentationConfig to this plugin
    * @param InstrumentationConfig
    */
-  public setConfig(config: ConfigType = {} as ConfigType): void {
-    // the assertion that {} is compatible with ConfigType may not be correct,
-    // ConfigType should contain only optional fields, but there is no enforcement in place for that
-    this._config = Object.assign({}, config);
+  public setConfig(config: ConfigType): void {
+    // copy config first level properties to ensure they are immutable.
+    // nested properties are not copied, thus are mutable from the outside.
+    this._config = { ...config };
   }
 
   /**

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts
@@ -30,7 +30,7 @@ export abstract class InstrumentationBase<
   constructor(
     instrumentationName: string,
     instrumentationVersion: string,
-    config: ConfigType = {} as ConfigType // The cast here may be wrong as ConfigType may contain required fields
+    config: ConfigType
   ) {
     super(instrumentationName, instrumentationVersion, config);
 

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -54,7 +54,7 @@ export abstract class InstrumentationBase<
   constructor(
     instrumentationName: string,
     instrumentationVersion: string,
-    config: ConfigType = {} as ConfigType // The cast here may be wrong as ConfigType may contain required fields
+    config: ConfigType
   ) {
     super(instrumentationName, instrumentationVersion, config);