diff --git a/docs/validations.md b/docs/validations.md index 624da30..f6ac4fd 100644 --- a/docs/validations.md +++ b/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` diff --git a/examples/validation.yaml b/examples/validation.yaml index c0e4425..bf0e112 100644 --- a/examples/validation.yaml +++ b/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 diff --git a/pkg/validator/config.go b/pkg/validator/config.go index 3db803f..e423c01 100644 --- a/pkg/validator/config.go +++ b/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) { diff --git a/pkg/validator/expression.go b/pkg/validator/expression.go index 8d220f4..564df41 100644 --- a/pkg/validator/expression.go +++ b/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(¶ms); 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 +} diff --git a/pkg/validator/expression_helpers.go b/pkg/validator/expression_helpers.go index aa680a1..7a05b1c 100644 --- a/pkg/validator/expression_helpers.go +++ b/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 "" +} diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index 61f44c3..7c77d21 100644 --- a/pkg/validator/validator_test.go +++ b/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) {