feat: add new validator expressionIsWellFormatted (#60)

Signed-off-by: Martin Chodur <[email protected]>

CHANGELOG.md

@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Added new validator `expressionIsWellFormatted` to check if rules are well formatted as `promtool promql format` would do.
 
 ## [v2.8.1] - 2024-02-29
 - Fixed param validation of the `hasAllowedEvaluationInterval` validator, if the `maximum` was not set.

docs/validations.md

@@ -30,6 +30,7 @@
     - [`expressionUsesExistingLabels`](#expressionusesexistinglabels)
     - [`expressionSelectorsMatchesAnything`](#expressionselectorsmatchesanything)
     - [`expressionWithNoMetricName`](#expressionwithnometricname)
+    - [`expressionIsWellFormatted`](#expressioniswellformatted)
   - [Alerts](#alerts)
     - [`forIsNotLongerThan`](#forisnotlongerthan)
   - [Others](#others)
@@ -288,6 +289,18 @@ instance.
 
 Fails if an expression doesn't use an explicit metric name (also if used as `__name__` label) in all its selectors(eg `up{foo="bar"}`).
 
+### `expressionIsWellFormatted`
+
+Fails if the expression is not well formatted PromQL as would `promtool promql format` do.
+It does remove the comments from the expression before the validation, since the PromQL prettifier drops them, so this should avoid false positive diffs.
+But if you want to ignore the expressions with comments, you can set the `ignoreComments` to true.
+
+```yaml
+params:
+  showExpectedForm: true # Optional, will show how the query should be formatted
+  skipExpressionsWithComments: true # Optional, will skip the expressions with comments
+```
+
 ## Alerts
 
 ### `forIsNotLongerThan`

examples/human_readable.html

@@ -52,6 +52,11 @@ <h2><a href="#check-groups">check-groups</a></h2>
 		<li>Group has valid partial_response_strategy (one of <code>warn</code> or <code>abort</code>) if set</li>
 	  </ul>
 
+  <h2><a href="#check-formatting">check-formatting</a></h2>
+	  <ul>
+		<li>All rules expression is well formatted as would <code>promtool promql format</code> do or similar online tool such as https://o11y.tools/promqlparser/</li>
+	  </ul>
+
   <h2><a href="#another-checks">another-checks</a></h2>
 	  <ul>
 		<li>All rules labels does not have empty values</li>

examples/human_readable.md

@@ -36,6 +36,9 @@ Validation rules:
     - Group evaluation interval is between `20s` and `106751d23h47m16s854ms` if set
     - Group has valid partial_response_strategy (one of `warn` or `abort`) if set
 
+  check-formatting
+    - All rules expression is well formatted as would `promtool promql format` do or similar online tool such as https://o11y.tools/promqlparser/
+
   another-checks
     - All rules labels does not have empty values
 

examples/validation.yaml

@@ -95,3 +95,11 @@ validationRules:
           minimum: "20s"
           intervalMustBeSet: false
       - type: hasValidPartialStrategy
+
+  - name: check-formatting
+    scope: All rules
+    validations:
+      - type: expressionIsWellFormatted
+        params:
+          showExpectedForm: true
+          skipExpressionsWithComments: true

pkg/validator/config.go

@@ -39,6 +39,7 @@ var registeredRuleValidators = map[string]validatorCreator{
 	"expressionSelectorsMatchesAnything":   newExpressionSelectorsMatchesAnything,
 	"expressionWithNoMetricName":           newExpressionWithNoMetricName,
 	"hasSourceTenantsForMetrics":           newHasSourceTenantsForMetrics,
+	"expressionIsWellFormatted":            newExpressionIsWellFormatted,
 }
 
 var registeredGroupValidators = map[string]validatorCreator{

pkg/validator/expression.go

@@ -467,3 +467,49 @@ func (h expressionDoesNotUseMetrics) Validate(_ unmarshaler.RuleGroup, rule rule
 	}
 	return errs
 }
+
+func newExpressionIsWellFormatted(paramsConfig yaml.Node) (Validator, error) {
+	params := struct {
+		SkipExpressionsWithComments bool `yaml:"skipExpressionsWithComments"`
+		ShowFormatted               bool `yaml:"showExpectedForm"`
+	}{}
+	if err := paramsConfig.Decode(&params); err != nil {
+		return nil, err
+	}
+	return &expressionIsWellFormatted{showFormatted: params.ShowFormatted, skipExpressionsWithComments: params.SkipExpressionsWithComments}, nil
+}
+
+type expressionIsWellFormatted struct {
+	skipExpressionsWithComments bool
+	showFormatted               bool
+}
+
+func (h expressionIsWellFormatted) String() string {
+	text := "expression is well formatted as would `promtool promql format` do or similar online tool such as https://o11y.tools/promqlparser/"
+	return text
+}
+
+var commentRegexp = regexp.MustCompile(`\s*#.*`)
+
+func (h expressionIsWellFormatted) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
+	if h.skipExpressionsWithComments && commentRegexp.MatchString(rule.Expr) {
+		return nil
+	}
+	originalExpr := commentRegexp.ReplaceAllString(strings.TrimSpace(rule.Expr), "")
+	fmt.Println(rule.Expr)
+	fmt.Println(originalExpr)
+	expr, err := parser.ParseExpr(originalExpr)
+	if err != nil {
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
+	}
+	prettified := expr.Pretty(0)
+	if originalExpr == prettified {
+		return []error{}
+	}
+	errorText := "expression is not well formatted, use `promtool promql format`, Prometheus UI or some online tool such as https://o11y.tools/promqlparser/"
+	if h.showFormatted {
+		errorText += fmt.Sprintf(", the expected form is:\n%s", prettified)
+	}
+	return []error{fmt.Errorf(errorText)}
+
+}

pkg/validator/validator_test.go

@@ -223,6 +223,12 @@ var testCases = []struct {
 	{name: "invalidPartialStrategy", validator: hasValidPartialStrategy{}, group: unmarshaler.RuleGroup{PartialResponseStrategy: "foo"}, expectedErrors: 1},
 	{name: "unsetPartialStrategyAllowed", validator: hasValidPartialStrategy{mustBeSet: false}, group: unmarshaler.RuleGroup{}, expectedErrors: 0},
 	{name: "unsetPartialStrategyDisallowed", validator: hasValidPartialStrategy{mustBeSet: true}, group: unmarshaler.RuleGroup{}, expectedErrors: 1},
+
+	// expressionIsWellFormatted
+	{name: "validExpression", validator: expressionIsWellFormatted{showFormatted: true}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
+	{name: "invalidExpression", validator: expressionIsWellFormatted{showFormatted: true}, rule: rulefmt.Rule{Expr: `up     == 1`}, expectedErrors: 1},
+	{name: "validWithCommentThatShouldBeIgnored", validator: expressionIsWellFormatted{showFormatted: true}, rule: rulefmt.Rule{Expr: `up == 1 # fooo`}, expectedErrors: 0},
+	{name: "invalidButWithCommentAndShouldBeSkipped", validator: expressionIsWellFormatted{showFormatted: true, skipExpressionsWithComments: true}, rule: rulefmt.Rule{Expr: `up           == 1 # fooo`}, expectedErrors: 0},
 }
 
 func Test(t *testing.T) {