Skip to content

Commit a06247d

Browse files
trentmZirak
authored andcommitted
fix(instrumentation): ensure .setConfig() results in config.enabled defaulting to true (open-telemetry#4941)
1 parent 1a13b1c commit a06247d

File tree

3 files changed

+25
-13
lines changed

3 files changed

+25
-13
lines changed

experimental/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ All notable changes to experimental packages in this project will be documented
2727

2828
### :bug: (Bug Fix)
2929

30+
* fix(instrumentation): ensure .setConfig() results in config.enabled defaulting to true [#4941](https://github.com/open-telemetry/opentelemetry-js/pull/4941) @trentm
3031
* 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
3132
* fix(api-logs): align AnyValue to spec [#4893](https://github.com/open-telemetry/opentelemetry-js/pull/4893) @blumamir
3233
* fix(instrumentation): remove diag.debug() message for instrumentations that do not patch modules [#4925](https://github.com/open-telemetry/opentelemetry-js/pull/4925) @trentm

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export abstract class InstrumentationAbstract<
4141
ConfigType extends InstrumentationConfig = InstrumentationConfig,
4242
> implements Instrumentation<ConfigType>
4343
{
44-
protected _config: ConfigType;
44+
protected _config: ConfigType = {} as ConfigType;
4545

4646
private _tracer: Tracer;
4747
private _meter: Meter;
@@ -53,12 +53,7 @@ export abstract class InstrumentationAbstract<
5353
public readonly instrumentationVersion: string,
5454
config: ConfigType
5555
) {
56-
// copy config first level properties to ensure they are immutable.
57-
// nested properties are not copied, thus are mutable from the outside.
58-
this._config = {
59-
enabled: true,
60-
...config,
61-
};
56+
this.setConfig(config);
6257

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

145140
/**
146141
* Sets InstrumentationConfig to this plugin
147-
* @param InstrumentationConfig
142+
* @param config
148143
*/
149144
public setConfig(config: ConfigType): void {
150145
// copy config first level properties to ensure they are immutable.
151146
// nested properties are not copied, thus are mutable from the outside.
152-
this._config = { ...config };
147+
this._config = {
148+
enabled: true,
149+
...config,
150+
};
153151
}
154152

155153
/**

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ interface TestInstrumentationConfig extends InstrumentationConfig {
3030
isActive?: boolean;
3131
}
3232

33-
class TestInstrumentation extends InstrumentationBase {
34-
constructor(config: TestInstrumentationConfig & InstrumentationConfig = {}) {
35-
super('test', '1.0.0', Object.assign({}, config));
33+
class TestInstrumentation extends InstrumentationBase<TestInstrumentationConfig> {
34+
constructor(config = {}) {
35+
super('test', '1.0.0', config);
3636
}
3737
override enable() {}
3838
override disable() {}
@@ -117,14 +117,15 @@ describe('BaseInstrumentation', () => {
117117
});
118118

119119
describe('getConfig', () => {
120-
it('should return instrumentation config', () => {
120+
it('should return instrumentation config, "enabled" should be true by default', () => {
121121
const instrumentation: Instrumentation = new TestInstrumentation({
122122
isActive: false,
123123
});
124124
const configuration =
125125
instrumentation.getConfig() as TestInstrumentationConfig;
126126
assert.notStrictEqual(configuration, null);
127127
assert.strictEqual(configuration.isActive, false);
128+
assert.strictEqual(configuration.enabled, true);
128129
});
129130
});
130131

@@ -139,6 +140,18 @@ describe('BaseInstrumentation', () => {
139140
instrumentation.getConfig() as TestInstrumentationConfig;
140141
assert.strictEqual(configuration.isActive, true);
141142
});
143+
144+
it('should ensure "enabled" defaults to true', () => {
145+
const instrumentation: Instrumentation = new TestInstrumentation();
146+
const config: TestInstrumentationConfig = {
147+
isActive: true,
148+
};
149+
instrumentation.setConfig(config);
150+
const configuration =
151+
instrumentation.getConfig() as TestInstrumentationConfig;
152+
assert.strictEqual(configuration.enabled, true);
153+
assert.strictEqual(configuration.isActive, true);
154+
});
142155
});
143156

144157
describe('getModuleDefinitions', () => {

0 commit comments

Comments
 (0)