Runtime config endpoint now supports diff parameter #3700
Conversation
|
@pstibrany Don't know where I should ping to ask for review so given that you gently did the last review... :D |
There was a problem hiding this comment.
Thank you for your PR! Very useful, especially for overrides!
As it is now, "diff" doesn't show diff for entire "runtime config", only for overrides.
I'd also suggest defining the handler in the pkg/cortex/runtime_config.go file, and passing only fully constructed http.Handler to the API registration. This may require moving some methods (eg. yamlMarshalUnmarshal) to util package. There should be no need for api package to depend on validation.
CHANGELOG.md
Outdated
| * [ENHANCEMENT] API: Add a `mode` query parameter for the runtime config endpoint: #3645 | ||
| * `/runtime_config?mode=diff`: Shows the YAML runtime configuration with all values that differ from the defaults. |
There was a problem hiding this comment.
| * [ENHANCEMENT] API: Add a `mode` query parameter for the runtime config endpoint: #3645 | |
| * `/runtime_config?mode=diff`: Shows the YAML runtime configuration with all values that differ from the defaults. | |
| * [ENHANCEMENT] API: Add a `mode` query parameter for the runtime config endpoint. `/runtime_config?mode=diff` now shows the YAML runtime configuration with all values that differ from the defaults. #3645 |
I see it's copied from another entry, but I'd suggest using "Runtime Config:" prefix instead of API:.
CHANGELOG.md
Outdated
| @@ -36,6 +36,8 @@ | |||
| * [ENHANCEMENT] Disabled in-memory shuffle-sharding subring cache in the store-gateway, ruler and compactor. This should reduce the memory utilisation in these services when shuffle-sharding is enabled, without introducing a significantly increase CPU utilisation. #3601 | |||
| * [ENHANCEMENT] Shuffle sharding: optimised subring generation used by shuffle sharding. #3601 | |||
| * [ENHANCEMENT] New /runtime_config endpoint that returns the defined runtime configuration in YAML format. The returned configuration includes overrides. #3639 | |||
| * [ENHANCEMENT] API: Add a `mode` query parameter for the runtime config endpoint: #3645 | |||
There was a problem hiding this comment.
We're used to link the PR number, not the issue number. Please change #3645 > #3700. Thanks!
There was a problem hiding this comment.
We've just released 1.7.0-rc.0. Could you rebase master and move the CHANGELOG entry under the master / unreleased section, please?
|
@pstibrany I'll tackle the changes today, thank you for the review. |
ethervoid
force-pushed
the
issue_3672_diff_for_runtime_config_endpoint
branch
from
d1c304c to
555e4a3
Compare
ethervoid
force-pushed
the
issue_3672_diff_for_runtime_config_endpoint
branch
2 times, most recently
from
f3d9f70 to
c786408
Compare
|
@pstibrany Changes made. I've made it possible to do a diff in the kv.Multi object but right now I can't see clear default values for that struct, like the ones you can see in the overrides. |
There was a problem hiding this comment.
Thank you for your continuous work on this PR!
I took a liberty to modify and simplify runtimeConfigHandler, to avoid comparing individual fields of runtimeConfigValues but work with default value which is simply empty struct. (See suggestion)
The rest of the PR looks good.
ethervoid
force-pushed
the
issue_3672_diff_for_runtime_config_endpoint
branch
from
d28ae87 to
cf6cc00
Compare
The runtime config endpoint now supports the diff parameter meaning that using /runtime_config?mode=diff will show only the non-default values for the configuration Signed-off-by: Mario de Frutos <mario@defrutos.org>
Signed-off-by: Mario de Frutos <mario@defrutos.org>
To avoid having a dependency of the validation package in the api one we have move the runtime configuration handle to the cortex package. This change also led to move some function of the handle to the util package Signed-off-by: Mario de Frutos <mario@defrutos.org>
Now we're doing diff with the kv.Multi component for the runtime config Signed-off-by: Mario de Frutos <mario@defrutos.org>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com> Signed-off-by: Mario de Frutos <mario@defrutos.org>
ethervoid
force-pushed
the
issue_3672_diff_for_runtime_config_endpoint
branch
from
cf6cc00 to
2d4280e
Compare
What this PR does:
This PR introduces a new parameter for the
/runtime_configendpoint that will allow the user to filter the outcome and just get the non-default values of the runtime configuration.I've tried to add tests for the
runtimeConfigurationHandlerbut given that the handler depends onruntimeConfigManagerthe tests become very complex unless we're able to mock theruntimeConfigManagerWhich issue(s) this PR fixes:
Fixes #3672
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]