Skip to content

refactor: build config from user config instead of mutating pre-filled object#2448

Draft
aryamohanan wants to merge 2 commits intofix-config-precedencefrom
fix-config-refact
Draft

refactor: build config from user config instead of mutating pre-filled object#2448
aryamohanan wants to merge 2 commits intofix-config-precedencefrom
fix-config-refact

Conversation

@aryamohanan
Copy link
Copy Markdown
Contributor

Normalization functions previously operated on a pre-filled config, making it unclear whether values were resolved or already present. This change separates input and output, making value resolution explicit and easier to reason about.

@aryamohanan aryamohanan added the v6 label Mar 30, 2026
@aryamohanan aryamohanan changed the title refactor: build config from userConfig instead of mutating pre-filled object refactor: build config from user config instead of mutating pre-filled object Mar 30, 2026
Comment on lines +247 to +275
function normalizeTracingConfig(userConfig, defaultConfig, finalConfig) {
finalConfig.tracing = {};

normalizeTracingEnabled(userConfig, defaultConfig, finalConfig);
normalizeUseOpentelemetry(userConfig, defaultConfig, finalConfig);
normalizeDisableTracing(userConfig, defaultConfig, finalConfig);
normalizeAutomaticTracingEnabled(userConfig, defaultConfig, finalConfig);
normalizeActivateImmediately(userConfig, defaultConfig, finalConfig);
normalizeTracingTransmission(userConfig, defaultConfig, finalConfig);
normalizeTracingHttp(userConfig, defaultConfig, finalConfig);
normalizeTracingStackTrace(userConfig, defaultConfig, finalConfig);
normalizeSpanBatchingEnabled(userConfig, defaultConfig, finalConfig);
normalizeDisableW3cTraceCorrelation(userConfig, defaultConfig, finalConfig);
normalizeTracingKafka(userConfig, defaultConfig, finalConfig);
normalizeAllowRootExitSpan(userConfig, defaultConfig, finalConfig);
normalizeIgnoreEndpoints(userConfig, defaultConfig, finalConfig);
normalizeIgnoreEndpointsDisableSuppression(userConfig, defaultConfig, finalConfig);
normalizeDisableEOLEvents(userConfig, defaultConfig, finalConfig);
}

/**
*
* @param {InstanaConfig} config
* @param {InstanaConfig|null} userConfig
* @param {InstanaConfig} defaultConfig
* @param {InstanaConfig} finalConfig
*/
function normalizeTracingEnabled(config) {
config.tracing.enabled = util.resolveBooleanConfigWithInvertedEnv({
function normalizeTracingEnabled(userConfig, defaultConfig, finalConfig) {
finalConfig.tracing.enabled = util.resolveBooleanConfigWithInvertedEnv({
envVar: 'INSTANA_TRACING_DISABLE',
configValue: config.tracing.enabled,
defaultValue: defaults.tracing.enabled,
configValue: userConfig?.tracing?.enabled,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need these extra optional chainings? I see them a lot in this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional chaining ensures safe access when userConfig is null or undefined, or when the tracing property is missing, thereby preventing runtime errors. I believe it will be particularly useful when dealing with user-provided configuration input.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya but we overuse it.

userConfig?.tracing?

There is IMO not reason to always to optional chaining for ^
We do it 17 times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants