Skip to content

fix: corrected the config precedence for agent config#2492

Merged
aryamohanan merged 36 commits intofix-config-precedencefrom
fix-config-source
Apr 20, 2026
Merged

fix: corrected the config precedence for agent config#2492
aryamohanan merged 36 commits intofix-config-precedencefrom
fix-config-source

Conversation

@aryamohanan
Copy link
Copy Markdown
Contributor

@aryamohanan aryamohanan commented Apr 14, 2026

TODO later

  • Fix Disable config
  • Fix Tests
  • Add util.js test
  • Add 100% test coverage
  • Fix collector tests
  • fix logging

@aryamohanan aryamohanan changed the title refactor(core): record config source fix: correct the config precedence for agent config Apr 16, 2026
@aryamohanan aryamohanan changed the title fix: correct the config precedence for agent config fix: corrected the config precedence for agent config Apr 16, 2026
Comment thread packages/core/src/tracing/instrumentation/messaging/kafkaJs.js
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js
}

exports.activate = function activate(extraConfig = {}) {
exports.activate = function activate(_config = config) {
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.

Suggested change
exports.activate = function activate(_config = config) {
exports.activate = function activate(_config) {

Comment thread packages/collector/src/util/normalizeConfig.js
normalizedUserConfig = {};
}
// Clear previous sources if any
configStore.clear();
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.

Wondering if we have to call this only on init?
Instead of // Clear previous sources if any, could you please describe why we call this here?

Copy link
Copy Markdown
Contributor Author

@aryamohanan aryamohanan Apr 20, 2026

Choose a reason for hiding this comment

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

We don't clear configStore on init because init() is called only once during the application lifecycle, whereas normalize() can be called multiple times (e.g., when re-normalizing configuration). The configStore.clear() call in normalize() ensures we start fresh for each normalization.

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.

normalize -> resets configStore
This is not clean, but we can revisit later

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.

Lets add a clear comment in the codebase here 👍

Comment thread packages/core/src/config/index.js
Comment thread packages/core/src/config/index.js
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js Outdated
Comment thread packages/core/src/config/index.js Outdated
aryamohanan and others added 3 commits April 20, 2026 13:50
Co-authored-by: kirrg001 <katharina.irrgang@ibm.com>
Co-authored-by: kirrg001 <katharina.irrgang@ibm.com>
Co-authored-by: kirrg001 <katharina.irrgang@ibm.com>
Comment thread packages/core/src/config/index.js Outdated
Co-authored-by: kirrg001 <katharina.irrgang@ibm.com>
@aryamohanan aryamohanan marked this pull request as ready for review April 20, 2026 09:52
@aryamohanan aryamohanan requested a review from a team as a code owner April 20, 2026 09:52
@aryamohanan aryamohanan added the v6 label Apr 20, 2026
@aryamohanan aryamohanan merged commit 954dd59 into fix-config-precedence Apr 20, 2026
1 check failed
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