Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation): remove default value for config in base instrumentation constructor #4695

Merged
merged 12 commits into from
May 17, 2024
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 };
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down