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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion docs/validations.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
- [expressionCanBeEvaluated](./validations.md#expressioncanbeevaluated)
- [expressionUsesExistingLabels](./validations.md#expressionusesexistinglabels)
- [expressionSelectorsMatchesAnything](./validations.md#expressionselectorsmatchesanything)
-
- [expressionWithNoMetricName]
-
- [Alert](#alert)
- [forIsNotLongerThan](./validations.md#forisnotlongerthan)

Expand Down Expand Up @@ -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`
Expand Down
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
28 changes: 28 additions & 0 deletions pkg/validator/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
32 changes: 32 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,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 ""
}
34 changes: 29 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,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) {
Expand Down
Loading