-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Promtool: Add feature flags to unit tests #8958
Conversation
Signed-off-by: Levi Harrison <git@leviharrison.dev>
Signed-off-by: Levi Harrison <git@leviharrison.dev>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have left a few comments.
cmd/promtool/main.go
Outdated
@@ -39,6 +39,7 @@ import ( | |||
"github.com/prometheus/common/model" | |||
"github.com/prometheus/common/version" | |||
"github.com/prometheus/exporter-toolkit/web" | |||
"github.com/prometheus/prometheus/promql" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import is misplaced.
cmd/promtool/main.go
Outdated
@@ -162,6 +163,8 @@ func main() { | |||
"A list of one or more files containing recording rules to be backfilled. All recording rules listed in the files will be backfilled. Alerting rules are not evaluated.", | |||
).Required().ExistingFiles() | |||
|
|||
featureList := app.Flag("enable-feature", "Comma separated feature names to enable (only PromQL related). Valid options: promql-at-modifier, promql-negative-offset. See https://prometheus.io/docs/prometheus/latest/disabled_features/ for more details.").Default("").Strings() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could we keep that in sync with cmd/prometheus? Maybe we should just link to the web page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that sounds good
Signed-off-by: Levi Harrison <git@leviharrison.dev>
This PR adds feature flag support to Promtool unit tests.
(Partially) Fixes #8607 - I've left out feature flags for rule linting because they're only parsed, not executed, which is where expressions are checked for disabled features. Discussions are still ongoing though.