Skip to content

Commit ab98b76

Browse files
aryamohananaryamohanan
authored
fix(core): ensured env vars take precedence over in-code config (#2478)
BREAKING CHANGE: environment config now overrides in-code configuration. Enforces consistent precedence: env > in-code Previously inconsistent behavior is now fixed. This may impact setups relying on the old precedence. ref https://jsw.ibm.com/browse/INSTA-80965
1 parent f2bdbb7 commit ab98b76

8 files changed

Lines changed: 152 additions & 104 deletions

File tree

‎packages/core/src/config/configNormalizers/disable.js‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,56 @@ exports.init = function init(_config) {
1919
* Handles environment variables, and array inputs.
2020
*
2121
* Precedence order (highest to lowest):
22-
* 1. `tracing.disable`
23-
* 2. Environment variables (`INSTANA_TRACING_DISABLE*`)
22+
* 1. Environment variables (`INSTANA_TRACING_DISABLE*`)
23+
* 2. In-code tracing.disable
2424
*
2525
* @param {import('../../config').InstanaConfig} config
2626
*/
2727
exports.normalize = function normalize(config) {
2828
if (!config?.tracing) config.tracing = {};
2929
try {
30-
// Disable all tracing if explicitly set 'disable' to true
30+
const envDisableConfig = getDisableFromEnv();
31+
32+
if (envDisableConfig !== null) {
33+
if (envDisableConfig === true) {
34+
logger?.debug('[config] env:INSTANA_TRACING_DISABLE = true');
35+
return true;
36+
}
37+
38+
if (envDisableConfig === false) {
39+
logger?.debug('[config] env:INSTANA_TRACING_DISABLE = false (overrides in-code config)');
40+
return {};
41+
}
42+
43+
if (envDisableConfig.instrumentations?.length || envDisableConfig.groups?.length) {
44+
logger?.debug(`[config] env:INSTANA_TRACING_DISABLE* = ${JSON.stringify(envDisableConfig)}`);
45+
46+
if (envDisableConfig.instrumentations) {
47+
envDisableConfig.instrumentations = normalizeArray(envDisableConfig.instrumentations);
48+
}
49+
if (envDisableConfig.groups) {
50+
envDisableConfig.groups = normalizeArray(envDisableConfig.groups);
51+
}
52+
53+
return envDisableConfig;
54+
}
55+
}
56+
3157
if (config.tracing.disable === true) {
3258
logger?.debug('[config] incode:tracing.disable = true');
33-
3459
return true;
3560
}
61+
3662
const hasDisableConfig = isDisableConfigNonEmpty(config);
3763

3864
if (hasDisableConfig) {
3965
logger?.debug(`[config] incode:tracing.disable = ${JSON.stringify(config.tracing.disable)}`);
4066
}
4167

42-
// Fallback to environment variables if `disable` is not explicitly configured
43-
const disableConfig = isDisableConfigNonEmpty(config) ? config.tracing.disable : getDisableFromEnv();
68+
const disableConfig = isDisableConfigNonEmpty(config) ? config.tracing.disable : null;
4469

4570
if (!disableConfig) return {};
4671

47-
if (disableConfig === true) return true;
48-
4972
// Normalize instrumentations and groups
5073
if (disableConfig?.instrumentations) {
5174
disableConfig.instrumentations = normalizeArray(disableConfig.instrumentations);
@@ -90,7 +113,7 @@ exports.normalizeExternalConfig = function normalizeExternalConfig(config) {
90113
* 2. INSTANA_TRACING_DISABLE_INSTRUMENTATIONS / INSTANA_TRACING_DISABLE_GROUPS
91114
* 3. INSTANA_TRACING_DISABLE=list
92115
*
93-
* @returns {import('../../config/types').Disable}
116+
* @returns {import('../../config/types').Disable | boolean | null}
94117
*/
95118
function getDisableFromEnv() {
96119
const disable = {};
@@ -104,7 +127,12 @@ function getDisableFromEnv() {
104127
return true;
105128
}
106129

107-
if (envVarValue !== 'false' && envVarValue !== '') {
130+
if (envVarValue === 'false') {
131+
logger?.debug('[config] env:INSTANA_TRACING_DISABLE = false');
132+
return false;
133+
}
134+
135+
if (envVarValue !== '') {
108136
const categorized = categorizeDisableEntries(parseEnvVar(envVarValue));
109137
if (categorized?.instrumentations?.length) {
110138
disable.instrumentations = categorized.instrumentations;

‎packages/core/src/config/configNormalizers/stackTrace.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ exports.normalizeStackTraceMode = function (config) {
2525

2626
/**
2727
* Normalizes stack trace length configuration based on precedence.
28-
* Precedence: global config > config > env var > default
28+
* Precedence: env var > global config > config > default
2929
* @param {import('../../config').InstanaConfig} config
3030
* @returns {number} - Normalized value
3131
*/

‎packages/core/src/config/index.js‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,15 @@ function normalizeTracingHttp({ userConfig = {}, defaultConfig = {}, finalConfig
359359

360360
const userHeaders = userHttp?.extraHttpHeadersToCapture;
361361

362-
// 1. Check in-code configuration first
362+
// 1. Check environment variable
363+
if (process.env.INSTANA_EXTRA_HTTP_HEADERS) {
364+
const fromEnvVar = parseHeadersEnvVar(process.env.INSTANA_EXTRA_HTTP_HEADERS);
365+
finalConfig.tracing.http.extraHttpHeadersToCapture = fromEnvVar;
366+
logger.debug(`[config] env:INSTANA_EXTRA_HTTP_HEADERS = ${process.env.INSTANA_EXTRA_HTTP_HEADERS}`);
367+
return;
368+
}
369+
370+
// 2. Check in-code configuration
363371
if (userHeaders !== undefined) {
364372
if (!Array.isArray(userHeaders)) {
365373
logger.warn(
@@ -375,14 +383,6 @@ function normalizeTracingHttp({ userConfig = {}, defaultConfig = {}, finalConfig
375383
}
376384
}
377385

378-
// 2. Check environment variable
379-
if (process.env.INSTANA_EXTRA_HTTP_HEADERS) {
380-
const fromEnvVar = parseHeadersEnvVar(process.env.INSTANA_EXTRA_HTTP_HEADERS);
381-
finalConfig.tracing.http.extraHttpHeadersToCapture = fromEnvVar;
382-
logger.debug(`[config] env:INSTANA_EXTRA_HTTP_HEADERS = ${process.env.INSTANA_EXTRA_HTTP_HEADERS}`);
383-
return;
384-
}
385-
386386
// 3. Use default configuration
387387
finalConfig.tracing.http.extraHttpHeadersToCapture = defaultConfig.tracing.http.extraHttpHeadersToCapture;
388388
}
@@ -415,6 +415,7 @@ function normalizeTracingStackTrace({ userConfig = {}, defaultConfig = {}, final
415415
const envStackTrace = process.env.INSTANA_STACK_TRACE;
416416
const envStackTraceLength = process.env.INSTANA_STACK_TRACE_LENGTH;
417417

418+
// Priority 1: Environment variable
418419
if (envStackTrace !== undefined) {
419420
const result = validateStackTraceMode(envStackTrace);
420421

@@ -430,6 +431,7 @@ function normalizeTracingStackTrace({ userConfig = {}, defaultConfig = {}, final
430431
finalConfig.tracing.stackTrace = defaultConfig.tracing.stackTrace;
431432
}
432433
} else if (userGlobal?.stackTrace !== undefined) {
434+
// Priority 2: In-code configuration
433435
const result = validateStackTraceMode(userGlobal.stackTrace);
434436

435437
if (result.isValid) {
@@ -450,6 +452,7 @@ function normalizeTracingStackTrace({ userConfig = {}, defaultConfig = {}, final
450452
const isLegacyLengthDefined = userTracingConfig?.stackTraceLength !== undefined;
451453
const stackTraceConfigValue = userGlobal?.stackTraceLength || userTracingConfig?.stackTraceLength;
452454

455+
// Priority 1: Environment variable
453456
if (envStackTraceLength !== undefined) {
454457
const result = validateStackTraceLength(envStackTraceLength);
455458

@@ -465,6 +468,7 @@ function normalizeTracingStackTrace({ userConfig = {}, defaultConfig = {}, final
465468
finalConfig.tracing.stackTraceLength = defaultConfig.tracing.stackTraceLength;
466469
}
467470
} else if (stackTraceConfigValue !== undefined) {
471+
// Priority 2: In-code configuration
468472
if (isLegacyLengthDefined) {
469473
logger.warn(
470474
// eslint-disable-next-line max-len
@@ -685,14 +689,7 @@ function normalizeIgnoreEndpoints({ userConfig = {}, defaultConfig = {}, finalCo
685689
return;
686690
}
687691

688-
// Case 1: Use in-code configuration if available
689-
if (userIgnoreEndpoints && Object.keys(userIgnoreEndpoints).length) {
690-
finalConfig.tracing.ignoreEndpoints = configNormalizers.ignoreEndpoints.normalizeConfig(userIgnoreEndpoints);
691-
logger.debug('[config] incode:config.tracing.ignoreEndpoints');
692-
return;
693-
}
694-
695-
// Case 2: Load from a YAML file if `INSTANA_IGNORE_ENDPOINTS_PATH` is set
692+
// Priority 1: Load from a YAML file if `INSTANA_IGNORE_ENDPOINTS_PATH` is set
696693
// Introduced in Phase 2 for advanced filtering based on both methods and endpoints.
697694
// Also supports basic filtering for endpoints.
698695
if (process.env.INSTANA_IGNORE_ENDPOINTS_PATH) {
@@ -703,7 +700,7 @@ function normalizeIgnoreEndpoints({ userConfig = {}, defaultConfig = {}, finalCo
703700
return;
704701
}
705702

706-
// Case 3: Load from the `INSTANA_IGNORE_ENDPOINTS` environment variable
703+
// Priority 2: Load from the `INSTANA_IGNORE_ENDPOINTS` environment variable
707704
// Introduced in Phase 1 for basic filtering based only on operations (e.g., `redis.get`, `kafka.consume`).
708705
// Provides a simple way to configure ignored operations via environment variables.
709706
if (process.env.INSTANA_IGNORE_ENDPOINTS) {
@@ -714,6 +711,13 @@ function normalizeIgnoreEndpoints({ userConfig = {}, defaultConfig = {}, finalCo
714711
return;
715712
}
716713

714+
// Priority 3: Use in-code configuration if available
715+
if (userIgnoreEndpoints && Object.keys(userIgnoreEndpoints).length) {
716+
finalConfig.tracing.ignoreEndpoints = configNormalizers.ignoreEndpoints.normalizeConfig(userIgnoreEndpoints);
717+
logger.debug('[config] incode:config.tracing.ignoreEndpoints');
718+
return;
719+
}
720+
717721
finalConfig.tracing.ignoreEndpoints = defaultConfig.tracing.ignoreEndpoints;
718722
}
719723

‎packages/core/src/config/util.js‎

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,20 @@ function parseBooleanFromEnv(envValue) {
8585
* @returns {boolean}
8686
*/
8787
exports.resolveBooleanConfig = function resolveBooleanConfig({ envVar, configValue, defaultValue, configPath }) {
88+
// Priority 1: Environment variable
89+
const envValue = process.env[envVar];
90+
const envParsed = parseBooleanFromEnv(envValue);
91+
92+
if (envParsed !== undefined) {
93+
logger.debug(`[config] env:${envVar} = ${envParsed}`);
94+
return envParsed;
95+
}
96+
97+
if (envValue != null) {
98+
logger.warn(`Invalid boolean value for ${envValue}: "${envValue}".`);
99+
}
100+
101+
// Priority 2: In-code configuration
88102
if (typeof configValue === 'boolean') {
89103
logger.debug(`[config] incode:${configPath} = ${configValue}`);
90104
return configValue;
@@ -98,18 +112,7 @@ exports.resolveBooleanConfig = function resolveBooleanConfig({ envVar, configVal
98112
);
99113
}
100114

101-
const envValue = process.env[envVar];
102-
const envParsed = parseBooleanFromEnv(envValue);
103-
104-
if (envParsed !== undefined) {
105-
logger.debug(`[config] env:${envVar} = ${envParsed}`);
106-
return envParsed;
107-
}
108-
109-
if (envValue != null) {
110-
logger.warn(`Invalid boolean value for ${envValue}: "${envValue}".`);
111-
}
112-
115+
// Priority 3: Default value
113116
return defaultValue;
114117
};
115118

@@ -130,16 +133,24 @@ exports.resolveBooleanConfigWithInvertedEnv = function resolveBooleanConfigWithI
130133
defaultValue,
131134
configPath
132135
}) {
133-
if (typeof configValue === 'boolean') {
134-
logger.debug(`[config] incode:${configPath} = ${configValue}`);
136+
// Priority 1: Environment variable
137+
const envValue = process.env[envVar];
138+
const envParsed = parseBooleanFromEnv(envValue);
135139

136-
return configValue;
140+
if (envParsed !== undefined) {
141+
const invertedValue = !envParsed;
142+
logger.debug(`[config] env:${envVar} = ${envParsed} (inverted to ${invertedValue})`);
143+
return invertedValue;
137144
}
138145

139-
const envValue = process.env[envVar];
140-
if (envValue === 'true') {
141-
logger.debug(`[config] env:${envVar} = true (inverted to false)`);
142-
return false;
146+
if (envValue != null) {
147+
logger.warn(`Invalid boolean value for ${envVar}: "${envValue}". Checking in-code config.`);
148+
}
149+
150+
// Priority 2: In-code configuration
151+
if (typeof configValue === 'boolean') {
152+
logger.debug(`[config] incode:${configPath} = ${configValue}`);
153+
return configValue;
143154
}
144155

145156
if (configValue != null && configPath) {
@@ -150,6 +161,7 @@ exports.resolveBooleanConfigWithInvertedEnv = function resolveBooleanConfigWithI
150161
);
151162
}
152163

164+
// Priority 3: Default value
153165
return defaultValue;
154166
};
155167

@@ -170,17 +182,20 @@ exports.resolveBooleanConfigWithTruthyEnv = function resolveBooleanConfigWithTru
170182
defaultValue,
171183
configPath
172184
}) {
173-
if (typeof configValue === 'boolean') {
174-
logger.debug(`[config] incode:${configPath} = ${configValue}`);
175-
return configValue;
176-
}
177-
185+
// Priority 1: Environment variable
178186
const envValue = process.env[envVar];
179187
if (envValue) {
180188
logger.debug(`[config] env:${envVar} = ${envValue}`);
181189
return true;
182190
}
183191

192+
// Priority 2: In-code configuration
193+
if (typeof configValue === 'boolean') {
194+
logger.debug(`[config] incode:${configPath} = ${configValue}`);
195+
return configValue;
196+
}
197+
198+
// Priority 3: Default value
184199
return defaultValue;
185200
};
186201

@@ -192,6 +207,14 @@ exports.resolveBooleanConfigWithTruthyEnv = function resolveBooleanConfigWithTru
192207
* @param {string} [params.configPath]
193208
*/
194209
exports.resolveStringConfig = function resolveStringConfig({ envVar, configValue, defaultValue, configPath }) {
210+
// Priority 1: Environment variable
211+
const envValue = process.env[envVar];
212+
if (envValue != null) {
213+
logger.debug(`[config] env:${envVar} = ${envValue}`);
214+
return envValue;
215+
}
216+
217+
// Priority 2: In-code configuration
195218
if (configValue != null) {
196219
if (typeof configValue !== 'string') {
197220
logger.warn(
@@ -204,12 +227,5 @@ exports.resolveStringConfig = function resolveStringConfig({ envVar, configValue
204227
logger.debug(`[config] incode:${configPath} = ${configValue}`);
205228
return configValue;
206229
}
207-
208-
const envValue = process.env[envVar];
209-
if (envValue != null) {
210-
logger.debug(`[config] env:${envVar} = ${envValue}`);
211-
return envValue;
212-
}
213-
214230
return defaultValue;
215231
};

‎packages/core/test/config/configNormalizers/disable_test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ describe('util.configNormalizers.disable', () => {
288288
expect(result).to.deep.equal({});
289289
});
290290

291-
it.skip('should give precedence to INSTANA_TRACING_DISABLE=false over config.tracing.disable=true', () => {
291+
it('should give precedence to INSTANA_TRACING_DISABLE=false over config.tracing.disable=true', () => {
292292
process.env.INSTANA_TRACING_DISABLE = 'false';
293293

294294
const config = {
@@ -301,7 +301,7 @@ describe('util.configNormalizers.disable', () => {
301301
expect(result).to.deep.equal({});
302302
});
303303

304-
it.skip('should give precedence to INSTANA_TRACING_DISABLE=false over config with instrumentations', () => {
304+
it('should give precedence to INSTANA_TRACING_DISABLE=false over config with instrumentations', () => {
305305
process.env.INSTANA_TRACING_DISABLE = 'false';
306306

307307
const config = {

0 commit comments

Comments
 (0)