Skip to content

Commit

Permalink
Merge pull request #1060 from cloudflare/ignoreLabelsValue
Browse files Browse the repository at this point in the history
Validate ignoreLabelsValue
  • Loading branch information
prymitive authored Aug 9, 2024
2 parents c095cda + 5b8c8c2 commit c802c41
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 0 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.1

### Fixed

- Validate `ignoreLabelsValue` option values in the pint config.

## v0.64.0

### Added
Expand Down
29 changes: 29 additions & 0 deletions docs/checks/promql/series.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ check "promql/series" {
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.
The value of this option is a map where the key is a metric selector to match on and the value
is the list of label names.

Example:

Expand Down Expand Up @@ -193,6 +195,33 @@ check "promql/series" {
}
```

You can use any metric selectors as keys in `ignoreLabelsValue` if you want apply it only
to metric selectors in queries that match the selector in `ignoreLabelsValue`.
For example if you have a rule that uses the same metric with two different selectors:

```yaml
- alerts: ...
expr: |
rate(http_requests_total{env="prod", code="401"}[5m]) > 0
or
rate(http_requests_total{env="dev", code="401"}[5m]) > 0
```

And you want to disable pint warnings only for the second selector (`http_requests_total{env="dev", code="401"}`)
but not the first one (`http_requests_total{env="prod", code="401"}`) you can do that by adding any label matcher
used in the query:

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

You can only use label matchers that would match the selector from the query itself, not from the time series
the query would return. This whole logic applies only to the query, not to the results of it.

### min-age

But default this check will report a problem if a metric was present
Expand Down
6 changes: 6 additions & 0 deletions internal/checks/promql_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ func (c *PromqlSeriesSettings) Validate() error {
c.lookbackStepDuration = time.Duration(dur)
}

for selector := range c.IgnoreLabelsValue {
if _, err := promParser.ParseMetricSelector(selector); err != nil {
return fmt.Errorf("%q is not a valid PromQL metric selector: %w", selector, err)
}
}

return nil
}

Expand Down
16 changes: 16 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2148,6 +2148,22 @@ func TestConfigErrors(t *testing.T) {
config: `check "promql/series" { ignoreMetrics = [".+++"] }`,
err: "error parsing regexp: invalid nested repetition operator: `++`",
},
{
config: `check "promql/series" {
ignoreLabelsValue = {
"foo bar" = [ "abc" ]
}
}`,
err: `"foo bar" is not a valid PromQL metric selector: 1:5: parse error: unexpected identifier "bar"`,
},
{
config: `check "promql/series" {
ignoreLabelsValue = {
"foo{" = [ "abc" ]
}
}`,
err: `"foo{" is not a valid PromQL metric selector: 1:5: parse error: unexpected end of input inside braces`,
},
{
config: `rule {
link ".+++" {}
Expand Down

0 comments on commit c802c41

Please sign in to comment.