Skip to content
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

Add metric name validation #36

Merged
merged 9 commits into from
Dec 6, 2023
5 changes: 5 additions & 0 deletions examples/validation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,8 @@ validationRules:
- type: expressionDoesNotUseLabels
params:
labels: [ "cluster", "locality", "prometheus-type", "replica" ]

- name: check-metric-name
scope: Alert
validations:
- type: expressionWithNoMetricName
1 change: 1 addition & 0 deletions pkg/validator/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var registeredValidators = map[string]validatorCreator{
"expressionCanBeEvaluated": newExpressionCanBeEvaluated,
"expressionUsesExistingLabels": newExpressionUsesExistingLabels,
"expressionSelectorsMatchesAnything": newExpressionSelectorsMatchesAnything,
"expressionWithNoMetricName": newExpressionWithNoMetricName,
}

func NewFromConfig(config config.ValidatorConfig) (Validator, error) {
Expand Down
32 changes: 32 additions & 0 deletions pkg/validator/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,35 @@ 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, prometheusClient *prometheus.Client) []error {
if prometheusClient == nil {
FUSAKLA marked this conversation as resolved.
Show resolved Hide resolved
log.Error("missing the `prometheus` section of configuration for querying prometheus, skipping check that requires it...")
return nil
}
var errs []error
names, err := getExpressionMetricsNames(rule.Expr)
if err != nil {
return []error{err}
}
for _, s := range names {
if s == "" {
errs = append(errs, fmt.Errorf("missing metric name for expression `%s`", rule.Expr))
FUSAKLA marked this conversation as resolved.
Show resolved Hide resolved
}
}
return errs
}
35 changes: 35 additions & 0 deletions pkg/validator/expression_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validator

import (
"fmt"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/promql/parser"
)

Expand Down Expand Up @@ -46,3 +47,37 @@ func getExpressionSelectors(expr string) ([]string, error) {
})
return selectors, nil
}

func getExpressionMetricsNames(expr string) ([]string, error) {
promQl, err := parser.ParseExpr(expr)
if err != nil {
return []string{}, fmt.Errorf("failed to parse expression `%s`: %s", expr, err)
}
var names []string
parser.Inspect(promQl, func(n parser.Node, ns []parser.Node) error {
switch v := n.(type) {
case *parser.VectorSelector:
names = append(names, getMetricNameFromVectorSelector(v))
}
return nil
})
return names, nil
}

func getMetricNameFromVectorSelector(v *parser.VectorSelector) string {
FUSAKLA marked this conversation as resolved.
Show resolved Hide resolved
s := &parser.VectorSelector{Name: v.Name}
name := s.String()
if name == "" {
name = getMetricNameFromLabels(v.LabelMatchers)
}
return name
}

func getMetricNameFromLabels(labels []*labels.Matcher) string {
for _, l := range labels {
if l.Name == "__name__" {
return l.Value
}
}
return ""
}
27 changes: 22 additions & 5 deletions pkg/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -157,6 +156,24 @@ 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: prometheus.NewClientMock(prometheus.NewSeriesResponseMock(2), 0, false, false), rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
FUSAKLA marked this conversation as resolved.
Show resolved Hide resolved
{name: "withNameInLabel", validator: expressionWithNoMetricName{}, promClient: prometheus.NewClientMock(prometheus.NewSeriesResponseMock(2), 0, false, false), rule: rulefmt.Rule{Expr: `{__name__="up", foo="bar"}`}, expectedErrors: 0},
{name: "noName", validator: expressionWithNoMetricName{}, promClient: prometheus.NewClientMock(prometheus.NewSeriesResponseMock(2), 0, false, false), rule: rulefmt.Rule{Expr: `{foo="bar"}`}, expectedErrors: 1},
{name: "complexExpressionsWithName", 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(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},
}

func Test(t *testing.T) {
Expand Down
Loading