Added new validator expressionDoesNotUseMetrics (#35)

* feat: add new expressionDoesNotUseMetrics validator Signed-off-by: Martin Chodur <[email protected]> * chore: changelog entry Signed-off-by: Martin Chodur <[email protected]> * chore: changelog entry Signed-off-by: Martin Chodur <[email protected]> * chore: fix changelog Signed-off-by: Martin Chodur <[email protected]> --------- Signed-off-by: Martin Chodur <[email protected]>
FUSAKLA · Dec 6, 2023 · 2da1bdd · 2da1bdd
1 parent 45214f3
commit 2da1bdd
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- Added new validator `expressionDoesNotUseMetrics`, see its [docs](docs/validations.md#expressiondoesnotusemetrics).
+
 ## [v2.6.0] - 2023-12-06
 - Added new validator `expressionWithNoMetricName`, see its [docs](docs/validations.md#expressionwithnometricname). Thanks @tizki !
 - Upgrade to go 1.21

diff --git a/docs/validations.md b/docs/validations.md
@@ -19,6 +19,7 @@
     - [`annotationIsValidPromQL`](#annotationisvalidpromql)
     - [`validateAnnotationTemplates`](#validateannotationtemplates)
   - [PromQL expression](#promql-expression)
+    - [`expressionDoesNotUseMetrics`](#expressiondoesnotusemetrics)
     - [`expressionDoesNotUseLabels`](#expressiondoesnotuselabels)
     - [`expressionDoesNotUseOlderDataThan`](#expressiondoesnotuseolderdatathan)
     - [`expressionDoesNotUseRangeShorterThan`](#expressiondoesnotuserangeshorterthan)
@@ -168,7 +169,7 @@ params:
 
 Fails if annotation value is not a valid URL. If `resolveURL` is enabled, tries to make an HTTP request to the specified
 URL and fails if the request does not succeed or returns 404 HTTP status code.
-> It's common practise to link a playbook with guide how to solve the alert in the alert itself.
+> It's common practice to link a playbook with guide how to solve the alert in the alert itself.
 > This way you can verify it's a working URL and possibly if it really exists.
 
 ```yaml
@@ -192,6 +193,16 @@ Fails if the annotation contains invalid Go template.
 
 ## PromQL expression
 
+### `expressionDoesNotUseMetrics`
+
+Fails if the rule expression uses metrics matching any of the metric name fully anchored(will be surrounded by `^...$`) regexps.
+> If you want to avoid using some metrics in the rules, you can use this validation to make sure it won't happen.
+
+```yaml
+params:
+  metricNameRegexps: [ "foo_bar.*", "foo_baz" ]
+```
+
 ### `expressionDoesNotUseLabels`
 
 Fails if the rule uses any of specified labels in its `expr` label matchers, aggregations or joins.
@@ -205,7 +216,7 @@ params:
 
 ### `expressionDoesNotUseOlderDataThan`
 
-Fails if the rule `expr` uses older data than specified limit in Prometheus duration syntax. Checks even in subqueries
+Fails if the rule `expr` uses older data than specified limit in Prometheus duration syntax. Checks even in sub-queries
 and offsets.
 > Useful to avoid writing queries which expects longer data retention than the Prometheus actually has.
 
@@ -242,7 +253,7 @@ Fails if aggregation function is used before the `rate` or `increase` functions.
 > Queries live prometheus instance, requires the `prometheus` config to be set.
 
 This validation runs the expression against the actual Prometheus instance and checks if it ends with error.
-Possibly you can set maximum allowed query execution time and maximum number of resulting timeseries.
+Possibly you can set maximum allowed query execution time and maximum number of resulting time series.
 
 ```yaml
 params:
@@ -265,9 +276,7 @@ instance.
 
 ### `expressionWithNoMetricName`
 
-> Fails if an expression doesn't use an explicit metric name.
-
-Verifies that all of the selectors in the expression (eg `up{foo="bar"}`) has a metric name. The metric name can appear before the curly braces or in `__name__` label. Fails if an expressions doesn't have a metric name.
+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"}`).
 
 ## Alert
 

diff --git a/go.mod b/go.mod
@@ -8,7 +8,6 @@ require (
 	github.com/creasty/defaults v1.7.0
 	github.com/prometheus/client_golang v1.17.0
 	github.com/prometheus/common v0.45.0
-	github.com/prometheus/prometheus v0.48.0
 	github.com/sirupsen/logrus v1.9.3
 	gopkg.in/alecthomas/kingpin.v2 v2.2.6
 	gopkg.in/yaml.v3 v3.0.1
@@ -55,6 +54,7 @@ require (
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common/sigv4 v0.1.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
+	github.com/prometheus/prometheus v0.48.0
 	github.com/stretchr/testify v1.8.4 // indirect
 	go.opentelemetry.io/otel v1.21.0 // indirect
 	go.opentelemetry.io/otel/metric v1.21.0 // indirect

diff --git a/pkg/validator/config.go b/pkg/validator/config.go
@@ -24,6 +24,7 @@ var registeredValidators = map[string]validatorCreator{
 	"expressionDoesNotUseLabels":           newExpressionDoesNotUseLabels,
 	"expressionDoesNotUseOlderDataThan":    newExpressionDoesNotUseOlderDataThan,
 	"expressionDoesNotUseRangeShorterThan": newExpressionDoesNotUseRangeShorterThan,
+	"expressionDoesNotUseMetrics":          newExpressionDoesNotUseMetrics,
 	"annotationIsValidPromQL":              newAnnotationIsValidPromQL,
 	"validateAnnotationTemplates":          newValidateAnnotationTemplates,
 	"forIsNotLongerThan":                   newForIsNotLongerThan,

diff --git a/pkg/validator/expression.go b/pkg/validator/expression.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/fusakla/promruval/v2/pkg/prometheus"
 	"github.com/prometheus/common/model"
+	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/model/rulefmt"
 	"github.com/prometheus/prometheus/promql/parser"
 	log "github.com/sirupsen/logrus"
@@ -414,3 +415,63 @@ func (e expressionWithNoMetricName) Validate(rule rulefmt.Rule, _ *prometheus.Cl
 	}
 	return errs
 }
+
+func newExpressionDoesNotUseMetrics(paramsConfig yaml.Node) (Validator, error) {
+	params := struct {
+		MetricNameRegexps []string `yaml:"metricNameRegexps"`
+	}{}
+	if err := paramsConfig.Decode(&params); err != nil {
+		return nil, err
+	}
+	v := expressionDoesNotUseMetrics{}
+	for _, r := range params.MetricNameRegexps {
+		compiled, err := regexp.Compile("^" + r + "$")
+		if err != nil {
+			return nil, err
+		}
+		v.metricNameRegexps = append(v.metricNameRegexps, compiled)
+	}
+	return &v, nil
+}
+
+type expressionDoesNotUseMetrics struct {
+	metricNameRegexps []*regexp.Regexp
+}
+
+func (h expressionDoesNotUseMetrics) String() string {
+	return "expression does not use any of these metrics regexps: " + strings.Join(func() []string {
+		var res []string
+		for _, r := range h.metricNameRegexps {
+			res = append(res, r.String())
+		}
+		return res
+	}(), ",")
+}
+
+func (h expressionDoesNotUseMetrics) Validate(rule rulefmt.Rule, _ *prometheus.Client) []error {
+	expr, err := parser.ParseExpr(rule.Expr)
+	if err != nil {
+		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+	}
+	var errs []error
+	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
+		switch v := n.(type) {
+		case *parser.VectorSelector:
+			// Do not check the VectorSelector.Name since it is automatically parsed into the __name__ LabelMatcher
+			for _, l := range v.LabelMatchers {
+				// We care just for the special __name__ label containing name of the metric
+				// and since we cannot match regexp on regexp.. lets just check for equality
+				if l.Name != "__name__" || l.Type != labels.MatchEqual {
+					continue
+				}
+				for _, r := range h.metricNameRegexps {
+					if r.MatchString(l.Value) {
+						errs = append(errs, fmt.Errorf("expression uses metric `%s` which is forbidden", l.Value))
+					}
+				}
+			}
+		}
+		return nil
+	})
+	return errs
+}
diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go
@@ -181,6 +181,14 @@ var testCases = []struct {
 	 ) * 100 > 10
 	 and
 	 sum(rate( {job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler) > 2`}, expectedErrors: 2},
+
+	// expressionDoesNotUseMetrics
+	{name: "emptyList", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
+	{name: "doesNotUseForbiddenMetric", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`foo_bar`)}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
+	{name: "usesForbiddenMetric", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`foo_bar`)}}, rule: rulefmt.Rule{Expr: `foo_bar{foo="bar"}`}, expectedErrors: 1},
+	{name: "usesTwoOfThreeForbiddenMetrics", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`foo_bar`), regexp.MustCompile(`foo_baz`), regexp.MustCompile(`^foo$`)}}, rule: rulefmt.Rule{Expr: `foo_baz{foo="bar"} and foo_bar`}, expectedErrors: 2},
+	{name: "usesMetricMatchingRegexp", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`foo_bar.*`)}}, rule: rulefmt.Rule{Expr: `foo_baz_baz{foo="bar"} and foo_bar`}, expectedErrors: 1},
+	{name: "regexpIsFullyAnchored", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`^foo_bar$`)}}, rule: rulefmt.Rule{Expr: `foo_baz_baz{foo="bar"} and foo_bar`}, expectedErrors: 1},
 }
 
 func Test(t *testing.T) {