Skip to content

Commit

Permalink
Merge pull request #1058 from cloudflare/ignoreLabelValue
Browse files Browse the repository at this point in the history
Ignore label value
  • Loading branch information
prymitive authored Aug 8, 2024
2 parents 7e8ee31 + e8bf8e7 commit d1ca76c
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 10 deletions.
6 changes: 6 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## v0.64.0

### Added

- Added `ignoreLabelsValue` to [promql/series](checks/promql/series.md) check settings.

## v0.63.0

### Added
Expand Down
25 changes: 23 additions & 2 deletions docs/checks/promql/series.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ Syntax:

```js
check "promql/series" {
ignoreMetrics = [ "(.*)", ... ]
ignoreMetrics = [ "(.*)", ... ]
ignoreLabelsValue = { "...": [ "...", ... ] }
}
```

Expand All @@ -159,6 +160,10 @@ check "promql/series" {
- `ignoreMetrics` - list of regexp matchers, if a metric is missing from Prometheus
but the name matches any of provided regexp matchers then pint will only report a
warning, instead of a bug level report.
- `ignoreLabelsValue` - allows to configure a global list of label **names** for which pint
should **NOT** report problems if there's a query that uses a **value** that does not exist.
This can be also set per rule using `# pint rule/set promql/series ignore/label-value $labelName`
comments, see below.

Example:

Expand All @@ -175,6 +180,19 @@ check "promql/series" {
}
```

If you might have a query with `http_requests_total{code="401"}` selector but `http_requests_total`
will only have time series with `code="401"` label if there were requests that resulted in responses
with HTTP code `401`, which would result in a pint reports. This example would disable all such pint
reports for all Prometheus rules:

```js
check "promql/series" {
ignoreLabelsValue = {
"http_requests_total" = [ "code" ]
}
}
```

### min-age

But default this check will report a problem if a metric was present
Expand Down Expand Up @@ -232,10 +250,13 @@ there's at least one HTTP response that generated this code.
You can relax pint checks so it doesn't validate if label values for specific
labels are present on any time series.

This can be also set globally for all rules using `ignoreLabelsValue` config option,
see above.

Syntax:

```yaml
# pint rule/set promql/series ignore/label-value $labelName`
# pint rule/set promql/series ignore/label-value $labelName
```

Example:
Expand Down
2 changes: 1 addition & 1 deletion go.ver
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.22.5
1.22.6
23 changes: 17 additions & 6 deletions internal/checks/promql_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import (
)

type PromqlSeriesSettings struct {
LookbackRange string `hcl:"lookbackRange,optional" json:"lookbackRange,omitempty"`
LookbackStep string `hcl:"lookbackStep,optional" json:"lookbackStep,omitempty"`
IgnoreMetrics []string `hcl:"ignoreMetrics,optional" json:"ignoreMetrics,omitempty"`
IgnoreLabelsValue map[string][]string `hcl:"ignoreLabelsValue,optional" json:"ignoreLabelsValue,omitempty"`
LookbackRange string `hcl:"lookbackRange,optional" json:"lookbackRange,omitempty"`
LookbackStep string `hcl:"lookbackStep,optional" json:"lookbackStep,omitempty"`
IgnoreMetrics []string `hcl:"ignoreMetrics,optional" json:"ignoreMetrics,omitempty"`
ignoreMetricsRe []*regexp.Regexp
lookbackRangeDuration time.Duration
lookbackStepDuration time.Duration
Expand Down Expand Up @@ -389,8 +390,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru
if lm.Type != labels.MatchEqual && lm.Type != labels.MatchRegexp {
continue
}
if c.isLabelValueIgnored(rule, selector, lm.Name) {
slog.Debug("Label check disabled by comment", slog.String("selector", (&selector).String()), slog.String("label", lm.Name))
if c.isLabelValueIgnored(settings, rule, selector, lm.Name) {
continue
}
labelSelector := promParser.VectorSelector{
Expand Down Expand Up @@ -674,7 +674,17 @@ func (c SeriesCheck) getMinAge(rule parser.Rule, selector promParser.VectorSelec
return minAge, problems
}

func (c SeriesCheck) isLabelValueIgnored(rule parser.Rule, selector promParser.VectorSelector, labelName string) bool {
func (c SeriesCheck) isLabelValueIgnored(settings *PromqlSeriesSettings, rule parser.Rule, selector promParser.VectorSelector, labelName string) bool {
for matcher, names := range settings.IgnoreLabelsValue {
isMatch, _ := matchSelectorToMetric(selector, matcher)
if !isMatch {
continue
}
if slices.Contains(names, labelName) {
slog.Debug("Label check disabled globally via config", slog.String("label", labelName))
return true
}
}
for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) {
matcher, key, value := parseRuleSet(ruleSet.Value)
if key != "ignore/label-value" {
Expand All @@ -687,6 +697,7 @@ func (c SeriesCheck) isLabelValueIgnored(rule parser.Rule, selector promParser.V
}
}
if labelName == value {
slog.Debug("Label check disabled by comment", slog.String("selector", (&selector).String()), slog.String("label", labelName))
return true
}
}
Expand Down
178 changes: 177 additions & 1 deletion internal/checks/promql_series_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2294,7 +2294,7 @@ func TestSeriesCheck(t *testing.T) {
},
},
{
description: "#5 ignored label value",
description: "#5 ignored label value via comment",
content: `
- record: foo
# pint rule/set promql/series ignore/label-value error
Expand Down Expand Up @@ -2334,6 +2334,182 @@ func TestSeriesCheck(t *testing.T) {
},
},
},
{
description: "#5 ignored label value globally / match name",
content: `
- record: foo
expr: sum(foo{error="notfound"})
`,
checker: newSeriesCheck,
prometheus: newSimpleProm,
ctx: func(_ string) context.Context {
s := checks.PromqlSeriesSettings{
IgnoreLabelsValue: map[string][]string{
"foo": {"error"},
},
}
if err := s.Validate(); err != nil {
t.Error(err)
t.FailNow()
}
return context.WithValue(context.Background(), checks.SettingsKey(checks.SeriesCheckName), &s)
},
problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo{error="notfound"})`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo)`},
},
resp: respondWithSingleRangeVector1W(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `absent(foo{error=~".+"})`},
},
resp: respondWithEmptyMatrix(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(up)"},
},
resp: respondWithSingleRangeVector1W(),
},
},
},
{
description: "#5 ignored label value globally / match selector",
content: `
- record: foo
expr: sum(foo{error="notfound"})
`,
checker: newSeriesCheck,
prometheus: newSimpleProm,
ctx: func(_ string) context.Context {
s := checks.PromqlSeriesSettings{
IgnoreLabelsValue: map[string][]string{
"foo{}": {"error"},
},
}
if err := s.Validate(); err != nil {
t.Error(err)
t.FailNow()
}
return context.WithValue(context.Background(), checks.SettingsKey(checks.SeriesCheckName), &s)
},
problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo{error="notfound"})`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo)`},
},
resp: respondWithSingleRangeVector1W(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `absent(foo{error=~".+"})`},
},
resp: respondWithEmptyMatrix(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(up)"},
},
resp: respondWithSingleRangeVector1W(),
},
},
},
{
description: "#5 ignored label value globally / no match",
content: `
- record: foo
expr: sum(foo{error="notfound"})
`,
checker: newSeriesCheck,
prometheus: newSimpleProm,
ctx: func(_ string) context.Context {
s := checks.PromqlSeriesSettings{
IgnoreLabelsValue: map[string][]string{
"foo{cluster=\"dev\"}": {"error"},
},
}
if err := s.Validate(); err != nil {
t.Error(err)
t.FailNow()
}
return context.WithValue(context.Background(), checks.SettingsKey(checks.SeriesCheckName), &s)
},
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 3,
Last: 3,
},
Reporter: checks.SeriesCheckName,
Text: noFilterMatchText("prom", uri, "foo", "error", `{error="notfound"}`, "1w"),
Details: checks.SeriesCheckCommonProblemDetails,
Severity: checks.Bug,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo{error="notfound"})`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo{error="notfound"})`},
},
resp: respondWithEmptyMatrix(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo)`},
},
resp: respondWithSingleRangeVector1W(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `absent(foo{error=~".+"})`},
},
resp: respondWithEmptyMatrix(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(up)"},
},
resp: respondWithSingleRangeVector1W(),
},
},
},
{
description: "#5 ignored label value / selector match",
content: `
Expand Down

0 comments on commit d1ca76c

Please sign in to comment.