config: Remove validation GetScrapeConfigs; require using config.Load.#15637
Merged
config: Remove validation GetScrapeConfigs; require using config.Load.#15637
Conversation
We should never modify (or even shallow copy) Config after config.Load; added comments and modified GetScrapeConfigs to do so. For GetScrapeConfigs the validation (even repeated) was likely doing writes (because global fields was 0). We GetScrapeConfigs concurrently in tests and ApplyConfig causing test races. In prod there were races but likelyt only to replace 0 with 0, so not too severe. I removed validation since I don't see anyone using our config.Config without Load. I had to refactor one test that was doing it, all others use yaml config. Fixes #15538 Previous attempt: #15634 Signed-off-by: bwplotka <bwplotka@gmail.com>
ArthurSens
reviewed
Dec 11, 2024
Member
ArthurSens
left a comment
There was a problem hiding this comment.
overall LGTM, just some questions/comments
machine424
reviewed
Dec 11, 2024
Member
machine424
left a comment
There was a problem hiding this comment.
lgtm as well, thanks for this.
(I'll look tomorrow at the tests changes when I have some time)
Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com> Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Member
|
This was merged on Dec 12, but I just got the same error again on Dec 14 unfortunately. |
sky333999
added a commit
to aws/amazon-cloudwatch-agent
that referenced
this pull request
Feb 12, 2026
Prometheus v0.308.1 added an internal 'loaded' boolean to promconfig.Config. GetScrapeConfigs() now returns an error if the config was not loaded through promconfig.Load() or Reload(). Our translator manually copies GlobalConfig, ScrapeConfigs, and TracingConfig from a parsed PromConfig, bypassing Load(), so the loaded flag was never set. Added cfg.PrometheusConfig.Reload() after the manual field copy. Reload() runs the config through Load() internally, setting the loaded flag and populating new default fields like ScrapeFallbackProtocol. Upstream PR: prometheus/prometheus#15637
sky333999
added a commit
to aws/amazon-cloudwatch-agent
that referenced
this pull request
Feb 15, 2026
Prometheus v0.308.1 added an internal 'loaded' boolean to promconfig.Config. GetScrapeConfigs() now returns an error if the config was not loaded through promconfig.Load() or Reload(). Our translator manually copies GlobalConfig, ScrapeConfigs, and TracingConfig from a parsed PromConfig, bypassing Load(), so the loaded flag was never set. Added cfg.PrometheusConfig.Reload() after the manual field copy. Reload() runs the config through Load() internally, setting the loaded flag and populating new default fields like ScrapeFallbackProtocol. Upstream PR: prometheus/prometheus#15637
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We should never modify (or even shallow copy) Config after config.Load; added comments and modified GetScrapeConfigs to do so. For GetScrapeConfigs the validation (even repeated) was likely doing writes (because global fields was 0). We GetScrapeConfigs concurrently in tests and ApplyConfig causing test races. In prod there were races but likelyt only to replace 0 with 0, so not too severe.
I removed validation since I don't see anyone using our config.Config without Load. I had to refactor one test that was doing it, all others use yaml config.
Fixes #15538
Previous attempt: #15634