Add new expressionWithNoMetricName validator (#36)

* WIP: adding metric name validation

* WIP: update config + tests

* check for __name__ in labels

* white space and formatting

* get metric name from LabelMatchers only

* removed prometheusClient from expressionWithNoMetricName Validate

* return vector selector from expressionWithNoMetricName

* added expressionWithNoMetricName to docs

* removed prometheus client from expressionWithNoMetricName tests

docs/validations.md

@@ -29,7 +29,8 @@
     - [expressionCanBeEvaluated](./validations.md#expressioncanbeevaluated)
     - [expressionUsesExistingLabels](./validations.md#expressionusesexistinglabels)
     - [expressionSelectorsMatchesAnything](./validations.md#expressionselectorsmatchesanything)
-    -
+    - [expressionWithNoMetricName]
+    - 
 - [Alert](#alert)
     - [forIsNotLongerThan](./validations.md#forisnotlongerthan)
 
@@ -264,6 +265,12 @@ Fails if any used label is not present in the configured Prometheus instance.
 Verifies if any of the selectors in the expression (eg `up{foo="bar"}`) matches actual data in the configured Prometheus
 instance.
 
+### `expressionWithNoMetricName`
+
+> Fails if an expression doesn't use an explicit metric name.
+
+Verifies that any 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.
+
 ## Alert
 
 ### `forIsNotLongerThan`

examples/validation.yaml

@@ -69,3 +69,8 @@ validationRules:
       - type: expressionDoesNotUseLabels
         params:
           labels: [ "cluster", "locality", "prometheus-type", "replica" ]
+
+  - name: check-metric-name
+    scope: Alert
+    validations:
+      - type: expressionWithNoMetricName

pkg/validator/config.go

@@ -35,6 +35,7 @@ var registeredValidators = map[string]validatorCreator{
 	"expressionCanBeEvaluated":             newExpressionCanBeEvaluated,
 	"expressionUsesExistingLabels":         newExpressionUsesExistingLabels,
 	"expressionSelectorsMatchesAnything":   newExpressionSelectorsMatchesAnything,
+	"expressionWithNoMetricName":           newExpressionWithNoMetricName,
 }
 
 func NewFromConfig(config config.ValidatorConfig) (Validator, error) {

pkg/validator/expression.go

@@ -386,3 +386,31 @@ func (h expressionSelectorsMatchesAnything) Validate(rule rulefmt.Rule, promethe
 	}
 	return errs
 }
+
+func newExpressionWithNoMetricName(paramsConfig yaml.Node) (Validator, error) {
+	params := struct{}{}
+	if err := paramsConfig.Decode(&params); err != nil {
+		return nil, err
+	}
+	return &expressionWithNoMetricName{}, nil
+}
+
+type expressionWithNoMetricName struct{}
+
+func (e expressionWithNoMetricName) String() string {
+	return "expression with no metric name"
+}
+
+func (e expressionWithNoMetricName) Validate(rule rulefmt.Rule, _ *prometheus.Client) []error {
+	var errs []error
+	vectorsWithNames, err := getExpressionMetricsNames(rule.Expr)
+	if err != nil {
+		return []error{err}
+	}
+	for _, v := range vectorsWithNames {
+		if v.MetricName == "" {
+			errs = append(errs, fmt.Errorf("missing metric name for vector `%s`", v.Vector.String()))
+		}
+	}
+	return errs
+}

pkg/validator/expression_helpers.go

@@ -2,6 +2,7 @@ package validator
 
 import (
 	"fmt"
+	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/promql/parser"
 )
 
@@ -46,3 +47,34 @@ func getExpressionSelectors(expr string) ([]string, error) {
 	})
 	return selectors, nil
 }
+
+type VectorSelectorWithMetricName struct {
+	Vector     *parser.VectorSelector
+	MetricName string
+}
+
+func getExpressionMetricsNames(expr string) ([]VectorSelectorWithMetricName, error) {
+	promQl, err := parser.ParseExpr(expr)
+	if err != nil {
+		return []VectorSelectorWithMetricName{}, fmt.Errorf("failed to parse expression `%s`: %s", expr, err)
+	}
+	var vectors []VectorSelectorWithMetricName
+	parser.Inspect(promQl, func(n parser.Node, ns []parser.Node) error {
+		switch v := n.(type) {
+		case *parser.VectorSelector:
+			metricName := getMetricNameFromLabels(v.LabelMatchers)
+			vectors = append(vectors, VectorSelectorWithMetricName{Vector: v, MetricName: metricName})
+		}
+		return nil
+	})
+	return vectors, nil
+}
+
+func getMetricNameFromLabels(labels []*labels.Matcher) string {
+	for _, l := range labels {
+		if l.Name == "__name__" {
+			return l.Value
+		}
+	}
+	return ""
+}

pkg/validator/validator_test.go

@@ -2,15 +2,14 @@ package validator
 
 import (
 	"fmt"
-	"reflect"
-	"regexp"
-	"testing"
-	"time"
-
 	"github.com/fusakla/promruval/v2/pkg/prometheus"
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/model/rulefmt"
 	"gotest.tools/assert"
+	"reflect"
+	"regexp"
+	"testing"
+	"time"
 )
 
 var testCases = []struct {
@@ -157,6 +156,31 @@ var testCases = []struct {
 	// expressionUsesExistingLabels
 	{name: "labelsExists", validator: expressionUsesExistingLabels{}, promClient: prometheus.NewClientMock([]string{"__name__", "foo"}, 0, false, false), rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
 	{name: "labelsDoesNotExist", validator: expressionUsesExistingLabels{}, promClient: prometheus.NewClientMock([]string{"__name__"}, 0, false, false), rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 1},
+
+	{name: "withName", validator: expressionWithNoMetricName{}, promClient: nil, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
+	{name: "withNameInLabel", validator: expressionWithNoMetricName{}, promClient: nil, rule: rulefmt.Rule{Expr: `{__name__="up", foo="bar"}`}, expectedErrors: 0},
+	{name: "noName", validator: expressionWithNoMetricName{}, promClient: nil, rule: rulefmt.Rule{Expr: `{foo="bar"}`}, expectedErrors: 1},
+	{name: "complexExpressionsWithName", validator: expressionWithNoMetricName{}, promClient: nil, rule: rulefmt.Rule{Expr: `(
+	 sum(rate(http_requests_total{code=~"5..", job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	/
+	 sum(rate(http_requests_total{job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	) * 100 > 10
+	and
+	sum(rate(http_requests_total{job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler) > 2`}, expectedErrors: 0},
+	{name: "complexExpressionsNoName", validator: expressionWithNoMetricName{}, promClient: prometheus.NewClientMock(prometheus.NewSeriesResponseMock(2), 0, false, false), rule: rulefmt.Rule{Expr: `(
+	  sum(rate(http_requests_total{code=~"5..", job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	/
+	  sum(rate( {job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	) * 100 > 10
+	and
+	sum(rate(http_requests_total{job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler) > 2`}, expectedErrors: 1},
+	{name: "complexExpressionsMultipleNoName", validator: expressionWithNoMetricName{}, promClient: prometheus.NewClientMock(prometheus.NewSeriesResponseMock(2), 0, false, false), rule: rulefmt.Rule{Expr: `(
+	   sum(rate(http_requests_total{code=~"5..", job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	 /
+	   sum(rate( {job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler)
+	 ) * 100 > 10
+	 and
+	 sum(rate( {job=~"thanos-query",handler!="exemplars"}[5m])) by (role,handler) > 2`}, expectedErrors: 2},
 }
 
 func Test(t *testing.T) {