Add flags to enable promql-at-modifier and negative offset #4026
Conversation
Signed-off-by: yeya24 <yb532204897@gmail.com>
Signed-off-by: yeya24 <yb532204897@gmail.com>
Signed-off-by: yeya24 <yb532204897@gmail.com>
| @@ -154,6 +154,10 @@ func registerQuery(app *extkingpin.App) { | |||
| storeResponseTimeout := extkingpin.ModelDuration(cmd.Flag("store.response-timeout", "If a Store doesn't send any data in this specified duration then a Store will be ignored and partial data will be returned if it's enabled. 0 disables timeout.").Default("0ms")) | |||
| reqLogConfig := extkingpin.RegisterRequestLoggingFlags(cmd) | |||
|
|
|||
| enableAtModifier := cmd.Flag("promql-at-modifier", "Experimental: Enable @ modifier in PromQL.").Default("false").Bool() | |||
There was a problem hiding this comment.
Wonder.... Do we want those feature flags? I would be happy to have those marked as experimental but literally enabled always. Why not? 🤗
cc @brancz @kakkoyun @GiedriusS @squat ?
There was a problem hiding this comment.
Essentially - more flags - more complexity and confusion, so it would be best to avoid them if we can
There was a problem hiding this comment.
I agree. I add them as flags in this pr in order to be consistent with Prometheus.
But I don't see the reason why we need to disable them. Happy to enable them by default in the engine and remove these flags.
There was a problem hiding this comment.
FWIW i'd prefer to not enable experimental Prometheus features by default. If an experimental feature was dropped at some point, it would be a breaking change for our end users.
There was a problem hiding this comment.
Yes, that's true but then you'd simply update to a newer version where they had been dropped and that's it. I would opt for fewer options here just like others have said.
There was a problem hiding this comment.
Yes, that's true but then you'd simply update to a newer version where they had been dropped and that's it.
Unfortunately, that's not so easy, I think @tiagomeireles is right. The problem is that users can depend on those experimental features - they will create rules, alerts with dashboards with negative offset, At modifiers etc. If they will be gone for some reason on Prometheus we would need to either keep the broken feature and maintain on our own or drop it which will break users with those dashboards, alerts etc.
My thinking is however that as Prometheus Maintainer, I am almost certain those features will not go away, but since they are experimental there could be bugs and some modifications of behavior, so yes = breaking changes.
Thanks for reminding us @tiagomeireles.
I would propose the following:
- Similar to https://prometheus.io/docs/prometheus/latest/disabled_features/#disabled-features add feature flags idea.
- Add to all components (for now Query)
--experimental-featureswhich will be https://github.com/thanos-io/thanos/blob/main/pkg/extflag/pathorcontent.go Reason why such flag and not--enable-featureas Prometheus: There might subsequent configuration for each experimental feature, so file or content in YAML format could give us future proof flexibility. WDYT? Also we could disable or enable inside this YAML that's why I would skipenableprefix. 🤔 - Allow specifying those feature, and add documentation page that explains the YAML (autogenerate even)
- I believe we cannot enable those features by default in this case.
The benefit here is that we don't have thousands of flags. Experiments are added and dropped so it's good to have common path. We could even at some point improve this flag to be dynamically reloaded / instrumented (to show in metric which thins was enabled and when etc)
WDYT? @brancz @GiedriusS @kakkoyun @yeya24
There was a problem hiding this comment.
I like to idea to have a dedicated file for experimental features and also a dedicated documentation page for it. Let's just do it.
We just need to be maybe backwards compatible and if we remove an option from the file we shouldn't fail, just let the user know that we don't have it as an experimental option anymore.
|
Let us know if you want to create this @yeya24 or don't have time 🤗 |
No sorry, I don't have time to implement the feature you mentioned. |
Signed-off-by: yeya24 yb532204897@gmail.com
Changes
Fixes #4022
Verification