Skip to content

Commit

Permalink
fix(instrumentation): ensure .setConfig() results in config.enabled d…
Browse files Browse the repository at this point in the history
…efaulting to true (#4941)
  • Loading branch information
trentm authored Aug 22, 2024
1 parent 7aff06d commit 0ee398d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ All notable changes to experimental packages in this project will be documented

### :bug: (Bug Fix)

* fix(instrumentation): ensure .setConfig() results in config.enabled defaulting to true [#4941](https://github.com/open-telemetry/opentelemetry-js/pull/4941) @trentm
* fix(instrumentation-http): Ensure instrumentation of `http.get` and `https.get` work when used in ESM code [#4857](https://github.com/open-telemetry/opentelemetry-js/issues/4857) @trentm
* fix(api-logs): align AnyValue to spec [#4893](https://github.com/open-telemetry/opentelemetry-js/pull/4893) @blumamir
* fix(instrumentation): remove diag.debug() message for instrumentations that do not patch modules [#4925](https://github.com/open-telemetry/opentelemetry-js/pull/4925) @trentm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export abstract class InstrumentationAbstract<
ConfigType extends InstrumentationConfig = InstrumentationConfig,
> implements Instrumentation<ConfigType>
{
protected _config: ConfigType;
protected _config: ConfigType = {} as ConfigType;

private _tracer: Tracer;
private _meter: Meter;
Expand All @@ -53,12 +53,7 @@ export abstract class InstrumentationAbstract<
public readonly instrumentationVersion: string,
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,
};
this.setConfig(config);

this._diag = diag.createComponentLogger({
namespace: instrumentationName,
Expand Down Expand Up @@ -144,12 +139,15 @@ export abstract class InstrumentationAbstract<

/**
* Sets InstrumentationConfig to this plugin
* @param InstrumentationConfig
* @param 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 };
this._config = {
enabled: true,
...config,
};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ interface TestInstrumentationConfig extends InstrumentationConfig {
isActive?: boolean;
}

class TestInstrumentation extends InstrumentationBase {
constructor(config: TestInstrumentationConfig & InstrumentationConfig = {}) {
super('test', '1.0.0', Object.assign({}, config));
class TestInstrumentation extends InstrumentationBase<TestInstrumentationConfig> {
constructor(config = {}) {
super('test', '1.0.0', config);
}
override enable() {}
override disable() {}
Expand Down Expand Up @@ -117,14 +117,15 @@ describe('BaseInstrumentation', () => {
});

describe('getConfig', () => {
it('should return instrumentation config', () => {
it('should return instrumentation config, "enabled" should be true by default', () => {
const instrumentation: Instrumentation = new TestInstrumentation({
isActive: false,
});
const configuration =
instrumentation.getConfig() as TestInstrumentationConfig;
assert.notStrictEqual(configuration, null);
assert.strictEqual(configuration.isActive, false);
assert.strictEqual(configuration.enabled, true);
});
});

Expand All @@ -139,6 +140,18 @@ describe('BaseInstrumentation', () => {
instrumentation.getConfig() as TestInstrumentationConfig;
assert.strictEqual(configuration.isActive, true);
});

it('should ensure "enabled" defaults to true', () => {
const instrumentation: Instrumentation = new TestInstrumentation();
const config: TestInstrumentationConfig = {
isActive: true,
};
instrumentation.setConfig(config);
const configuration =
instrumentation.getConfig() as TestInstrumentationConfig;
assert.strictEqual(configuration.enabled, true);
assert.strictEqual(configuration.isActive, true);
});
});

describe('getModuleDefinitions', () => {
Expand Down

0 comments on commit 0ee398d

Please sign in to comment.