Skip to content

Commit f2bdbb7

Browse files
aryamohananaryamohanan
authored
refactor(collector): moved resolve string config to core utility (#2479)
1 parent b8c0e30 commit f2bdbb7

5 files changed

Lines changed: 164 additions & 58 deletions

File tree

‎packages/collector/src/types/collector.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export interface CollectorConfig {
2929
stackTraceLength?: number;
3030
[key: string]: any;
3131
};
32-
autoProfile?: boolean | string;
32+
autoProfile?: boolean;
3333
reportUnhandledPromiseRejections?: boolean;
3434
logger?: GenericLogger;
3535
level?: string | number;

‎packages/collector/src/util/normalizeConfig.js‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ module.exports = function normalizeConfig(userConfig = {}) {
4242
* @returns {string}
4343
*/
4444
function normalizeAgentHost(userConfig, defaultConfig) {
45-
return resolveConfig(process.env.INSTANA_AGENT_HOST, userConfig.agentHost, defaultConfig.agentHost);
45+
return util.resolveStringConfig({
46+
envVar: 'INSTANA_AGENT_HOST',
47+
configValue: userConfig.agentHost,
48+
defaultValue: defaultConfig.agentHost,
49+
configPath: 'config.agentHost'
50+
});
4651
}
4752

4853
/**
@@ -75,11 +80,16 @@ function normalizeAgentRequestTimeout(userConfig, defaultConfig) {
7580

7681
/**
7782
* @param {import('../types/collector').CollectorConfig} userConfig
78-
* @param {{ autoProfile: string | boolean }} defaultConfig
79-
* @returns {string | boolean}
83+
* @param {{ autoProfile: boolean }} defaultConfig
84+
* @returns {boolean}
8085
*/
8186
function normalizeAutoProfile(userConfig, defaultConfig) {
82-
return resolveConfig(process.env.INSTANA_AUTO_PROFILE, userConfig.autoProfile, defaultConfig.autoProfile);
87+
return util.resolveBooleanConfig({
88+
envVar: 'INSTANA_AUTO_PROFILE',
89+
configValue: userConfig.autoProfile,
90+
defaultValue: defaultConfig.autoProfile,
91+
configPath: 'config.autoProfile'
92+
});
8393
}
8494

8595
/**
@@ -89,22 +99,3 @@ function normalizeAutoProfile(userConfig, defaultConfig) {
8999
function normalizeUnhandledRejections(userConfig) {
90100
return userConfig.reportUnhandledPromiseRejections ?? false;
91101
}
92-
93-
/**
94-
* @template T
95-
* @param {T | undefined} envValue
96-
* @param {T | undefined} configValue
97-
* @param {T} defaultValue
98-
* @returns {T}
99-
*/
100-
function resolveConfig(envValue, configValue, defaultValue) {
101-
if (configValue != null) {
102-
return configValue;
103-
}
104-
105-
if (envValue != null) {
106-
return envValue;
107-
}
108-
109-
return defaultValue;
110-
}

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

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -183,46 +183,24 @@ module.exports.normalize = ({ userConfig = {}, finalConfigBase = {}, defaultsOve
183183
* @param {{ userConfig?: InstanaConfig|null, defaultConfig?: InstanaConfig, finalConfig?: InstanaConfig }} [options]
184184
*/
185185
function normalizeServiceName({ userConfig = {}, defaultConfig = {}, finalConfig = {} } = {}) {
186-
const userValue = userConfig.serviceName;
187-
188-
if (userValue != null) {
189-
if (typeof userValue === 'string') {
190-
finalConfig.serviceName = userValue;
191-
logger.debug(`[config] incode:config.serviceName = ${finalConfig.serviceName}`);
192-
} else {
193-
logger.warn(`Invalid configuration: config.serviceName is not a string, the value will be ignored: ${userValue}`);
194-
finalConfig.serviceName = defaultConfig.serviceName;
195-
}
196-
} else if (process.env.INSTANA_SERVICE_NAME) {
197-
finalConfig.serviceName = process.env.INSTANA_SERVICE_NAME;
198-
logger.debug(`[config] env:INSTANA_SERVICE_NAME = ${process.env.INSTANA_SERVICE_NAME}`);
199-
} else {
200-
finalConfig.serviceName = defaultConfig.serviceName;
201-
}
186+
finalConfig.serviceName = util.resolveStringConfig({
187+
envVar: 'INSTANA_SERVICE_NAME',
188+
configValue: userConfig.serviceName,
189+
defaultValue: defaultConfig.serviceName,
190+
configPath: 'config.serviceName'
191+
});
202192
}
203193

204194
/**
205195
* @param {{ userConfig?: InstanaConfig|null, defaultConfig?: InstanaConfig, finalConfig?: InstanaConfig }} [options]
206196
*/
207197
function normalizePackageJsonPath({ userConfig = {}, defaultConfig = {}, finalConfig = {} } = {}) {
208-
const userValue = userConfig.packageJsonPath;
209-
210-
if (userValue != null) {
211-
if (typeof userValue === 'string') {
212-
finalConfig.packageJsonPath = userValue;
213-
logger.debug(`[config] incode:config.packageJsonPath = ${finalConfig.packageJsonPath}`);
214-
} else {
215-
logger.warn(
216-
`Invalid configuration: config.packageJsonPath is not a string, the value will be ignored: ${userValue}`
217-
);
218-
finalConfig.packageJsonPath = defaultConfig.packageJsonPath;
219-
}
220-
} else if (process.env.INSTANA_PACKAGE_JSON_PATH) {
221-
finalConfig.packageJsonPath = process.env.INSTANA_PACKAGE_JSON_PATH;
222-
logger.debug(`[config] env:INSTANA_PACKAGE_JSON_PATH = ${process.env.INSTANA_PACKAGE_JSON_PATH}`);
223-
} else {
224-
finalConfig.packageJsonPath = defaultConfig.packageJsonPath;
225-
}
198+
finalConfig.packageJsonPath = util.resolveStringConfig({
199+
envVar: 'INSTANA_PACKAGE_JSON_PATH',
200+
configValue: userConfig.packageJsonPath,
201+
defaultValue: defaultConfig.packageJsonPath,
202+
configPath: 'config.packageJsonPath'
203+
});
226204
}
227205

228206
/**

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,33 @@ exports.resolveBooleanConfigWithTruthyEnv = function resolveBooleanConfigWithTru
183183

184184
return defaultValue;
185185
};
186+
187+
/**
188+
* @param {Object} params
189+
* @param {any} params.envVar
190+
* @param {any} params.configValue
191+
* @param {any} params.defaultValue
192+
* @param {string} [params.configPath]
193+
*/
194+
exports.resolveStringConfig = function resolveStringConfig({ envVar, configValue, defaultValue, configPath }) {
195+
if (configValue != null) {
196+
if (typeof configValue !== 'string') {
197+
logger.warn(
198+
`Invalid configuration: ${configPath} is not a string value, will be ignored: ${JSON.stringify(
199+
configValue
200+
)}. Falling back to default: ${defaultValue}.`
201+
);
202+
return defaultValue;
203+
}
204+
logger.debug(`[config] incode:${configPath} = ${configValue}`);
205+
return configValue;
206+
}
207+
208+
const envValue = process.env[envVar];
209+
if (envValue != null) {
210+
logger.debug(`[config] env:${envVar} = ${envValue}`);
211+
return envValue;
212+
}
213+
214+
return defaultValue;
215+
};

‎packages/core/test/config/util_test.js‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,4 +581,111 @@ describe('config.util', () => {
581581
expect(result).to.equal(false);
582582
});
583583
});
584+
585+
describe('resolveStringConfig', () => {
586+
beforeEach(() => {
587+
delete process.env.TEST_STRING_VAR;
588+
});
589+
590+
afterEach(() => {
591+
delete process.env.TEST_STRING_VAR;
592+
});
593+
594+
it('should return the default value when no env var or config value is provided', () => {
595+
const result = util.resolveStringConfig({
596+
envVar: 'TEST_STRING_VAR',
597+
configValue: undefined,
598+
defaultValue: 'default-value',
599+
configPath: 'config.test.string'
600+
});
601+
602+
expect(result).to.equal('default-value');
603+
});
604+
605+
it.skip('should prioritize env var over config value', () => {
606+
process.env.TEST_STRING_VAR = 'env-value';
607+
608+
const result = util.resolveStringConfig({
609+
envVar: 'TEST_STRING_VAR',
610+
configValue: 'config-value',
611+
defaultValue: 'default-value',
612+
configPath: 'config.test.string'
613+
});
614+
615+
expect(result).to.equal('env-value');
616+
});
617+
618+
it('should use env var when config value is not set', () => {
619+
process.env.TEST_STRING_VAR = 'env-value';
620+
621+
const result = util.resolveStringConfig({
622+
envVar: 'TEST_STRING_VAR',
623+
configValue: undefined,
624+
defaultValue: 'default-value',
625+
configPath: 'config.test.string'
626+
});
627+
628+
expect(result).to.equal('env-value');
629+
});
630+
631+
it('should use config value when env var is not set', () => {
632+
const result = util.resolveStringConfig({
633+
envVar: 'TEST_STRING_VAR',
634+
configValue: 'config-value',
635+
defaultValue: 'default-value',
636+
configPath: 'config.test.string'
637+
});
638+
639+
expect(result).to.equal('config-value');
640+
});
641+
642+
it('should handle empty string as a valid config value', () => {
643+
const result = util.resolveStringConfig({
644+
envVar: 'TEST_STRING_VAR',
645+
configValue: '',
646+
defaultValue: 'default-value',
647+
configPath: 'config.test.string'
648+
});
649+
650+
expect(result).to.equal('');
651+
});
652+
653+
it('should handle empty string as a valid env var value', () => {
654+
process.env.TEST_STRING_VAR = '';
655+
656+
const result = util.resolveStringConfig({
657+
envVar: 'TEST_STRING_VAR',
658+
configValue: undefined,
659+
defaultValue: 'default-value',
660+
configPath: 'config.test.string'
661+
});
662+
663+
expect(result).to.equal('');
664+
});
665+
666+
it('should handle undefined config value as not set', () => {
667+
process.env.TEST_STRING_VAR = 'env-value';
668+
669+
const result = util.resolveStringConfig({
670+
envVar: 'TEST_STRING_VAR',
671+
configValue: undefined,
672+
defaultValue: 'default-value',
673+
configPath: 'config.test.string'
674+
});
675+
676+
expect(result).to.equal('env-value');
677+
});
678+
679+
it('should handle multiline string values', () => {
680+
const multilineValue = 'line1\nline2\nline3';
681+
const result = util.resolveStringConfig({
682+
envVar: undefined,
683+
configValue: multilineValue,
684+
defaultValue: 'default-value',
685+
configPath: 'config.test.string'
686+
});
687+
688+
expect(result).to.equal(multilineValue);
689+
});
690+
});
584691
});

0 commit comments

Comments
 (0)