config: set gogc default value when config body is empty #16052
Merged
Conversation
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
Signed-off-by: Danial Eskandari <doneskandari@gmail.com>
machine424
reviewed
Feb 21, 2025
There was a problem hiding this comment.
Thanks for this.
This should address the empty config filecase raised in the issue, but empty config file with GOGC env var set still needs a fix. That will require moving some parts, we can take care of that in a another PR if you want.
(we can also add more tests to this gogc part)
…nfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com>
ab90765 to
fe55698
Compare
…plicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com>
machine424
approved these changes
Feb 25, 2025
There was a problem hiding this comment.
Thanks again for this.
lgtm.
ok, let's have the other change in a follow-up and let's have more tests as well. Again you can fix/adjust and make use of the tests I have here #15238 if needed.
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Feb 28, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com>
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Feb 28, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com> Signed-off-by: machine424 <ayoubmrini424@gmail.com>
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Mar 5, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com> Signed-off-by: machine424 <ayoubmrini424@gmail.com>
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Mar 5, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com> Signed-off-by: machine424 <ayoubmrini424@gmail.com>
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Mar 5, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com> Signed-off-by: machine424 <ayoubmrini424@gmail.com>
machine424
pushed a commit
to machine424/prometheus
that referenced
this pull request
Mar 5, 2025
…16052) * fix: set gogc default value when config body is empty Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: explicitly check value 75 in `TestGoGCDefaultValueOnEmptyConfigBody` add GoGC assertion in `TestEmptyConfig`, also removed the no longer needed runtime config assignment in `TestEmptyGlobalBlock` Signed-off-by: Danial Eskandari <doneskandari@gmail.com> * refactor: remove `TestGoGCDefaultValueOnEmptyConfigBody` to reduce duplicate assertions Signed-off-by: Danial Eskandari <doneskandari@gmail.com> --------- Signed-off-by: Danial Eskandari <doneskandari@gmail.com> Signed-off-by: machine424 <ayoubmrini424@gmail.com>
krajorama
added a commit
that referenced
this pull request
Mar 11, 2025
backport[2.53]: config: set gogc default value when config body is empty (#16052)
mrnonz
added a commit
to mrnonz/prometheus
that referenced
this pull request
Mar 18, 2025
…16052) Signed-off-by: Nontawat Numor <nontawat39@gmail.com>
mrnonz
added a commit
to mrnonz/prometheus
that referenced
this pull request
Mar 18, 2025
…16052) Signed-off-by: Nontawat Numor <nontawat39@gmail.com>
wbh1
added a commit
to wbh1/prometheus
that referenced
this pull request
May 5, 2025
…exists Fixes: prometheus#16334 Related to: - prometheus#15238 - prometheus#16052 Currently, when the GOGC environment variable is set -- and no `runtime` block is set in the Prometheus config file -- it is ignored and the default value of 75% is always used. However, if there is an empty runtime block (e.g. `runtime: {}`), _then_ the GOGC environment variable is checked. This PR changes this behavior to consistently check and use the GOGC environment variable when it is set (unless the `gogc` field is set in the `runtime` block of the loaded config file, in which case it still gives that precedence). Co-authored-by: Adam Rambo <arambo@protonmail.com> Signed-off-by: Will Hegedus <whegedus@akamai.com>
wbh1
added a commit
to wbh1/prometheus
that referenced
this pull request
May 12, 2025
…exists Fixes: prometheus#16334 Related to: - prometheus#15238 - prometheus#16052 Currently, when the GOGC environment variable is set -- and no `runtime` block is set in the Prometheus config file -- it is ignored and the default value of 75% is always used. However, if there is an empty runtime block (e.g. `runtime: {}`), _then_ the GOGC environment variable is checked. This PR changes this behavior to consistently check and use the GOGC environment variable when it is set (unless the `gogc` field is set in the `runtime` block of the loaded config file, in which case it still gives that precedence). Co-authored-by: Adam Rambo <arambo@protonmail.com> Signed-off-by: Will Hegedus <whegedus@akamai.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR resolves the issue where running Prometheus with an empty config body results in
GoGCbeing set to0. The fix ensures that the default value ofGoGCis properly set inDefaultConfig.Fixes #16029