Skip to content

Commit

Permalink
Merge pull request #1033 from cloudflare/vm
Browse files Browse the repository at this point in the history 
Print more details in promql/vector_matching reports
  • Loading branch information
@prymitive
prymitive committed Jul 17, 2024
2 parents ed582ef + e32e77f commit ab538fd
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 30 deletions.
6 changes: 6 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## v0.63.0

### Changed

- [promql/vector_matching](checks/promql/vector_matching.md) will now report more details, including which Prometheus server reports problems and which part of the query is the issue.

## v0.62.2

### Fixed
Expand Down
26 changes: 16 additions & 10 deletions internal/checks/promql_vector_matching.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,24 +167,28 @@ func (c VectorMatchingCheck) checkNode(ctx context.Context, node *parser.PromQLN
for _, name := range n.VectorMatching.MatchingLabels {
if !leftLabels.hasName(name) && rightLabels.hasName(name) {
problems = append(problems, exprProblem{
expr: node.Expr.String(),
text: fmt.Sprintf("Using `on(%s)` won't produce any results because the left hand side of the query doesn't have this label: `%s`.", name, node.Expr.(*promParser.BinaryExpr).LHS),
expr: node.Expr.String(),
text: fmt.Sprintf(
"Using `on(%s)` won't produce any results on %s because results from the left hand side of the query don't have this label: `%s`.",
name, promText(c.prom.Name(), qr.URI), node.Expr.(*promParser.BinaryExpr).LHS),
details: VectorMatchingCheckDetails,
severity: Bug,
})
}
if leftLabels.hasName(name) && !rightLabels.hasName(name) {
problems = append(problems, exprProblem{
expr: node.Expr.String(),
text: fmt.Sprintf("Using `on(%s)` won't produce any results because the right hand side of the query doesn't have this label: `%s`.", name, node.Expr.(*promParser.BinaryExpr).RHS),
expr: node.Expr.String(),
text: fmt.Sprintf("Using `on(%s)` won't produce any results on %s because results from the right hand side of the query don't have this label: `%s`.",
name, promText(c.prom.Name(), qr.URI), node.Expr.(*promParser.BinaryExpr).RHS),
details: VectorMatchingCheckDetails,
severity: Bug,
})
}
if !leftLabels.hasName(name) && !rightLabels.hasName(name) {
problems = append(problems, exprProblem{
expr: node.Expr.String(),
text: fmt.Sprintf("Using `on(%s)` won't produce any results because both sides of the query don't have this label.", name),
expr: node.Expr.String(),
text: fmt.Sprintf("Using `on(%s)` won't produce any results on %s because results from both sides of the query don't have this label: `%s`.",
name, promText(c.prom.Name(), qr.URI), node.Expr),
details: VectorMatchingCheckDetails,
severity: Bug,
})
Expand All @@ -194,15 +198,17 @@ func (c VectorMatchingCheck) checkNode(ctx context.Context, node *parser.PromQLN
l, r := leftLabels.getFirstNonOverlap(rightLabels)
if len(n.VectorMatching.MatchingLabels) == 0 {
problems = append(problems, exprProblem{
expr: node.Expr.String(),
text: fmt.Sprintf("This query will never return anything because the right and the left hand side have different labels: `%s` != `%s`.", l, r),
expr: node.Expr.String(),
text: fmt.Sprintf("This query will never return anything on %s because results from the right and the left hand side have different labels: `%s` != `%s`. Failing query: `%s`.",
promText(c.prom.Name(), qr.URI), l, r, node.Expr),
details: VectorMatchingCheckDetails,
severity: Bug,
})
} else {
problems = append(problems, exprProblem{
expr: node.Expr.String(),
text: fmt.Sprintf("Using `ignoring(%s)` won't produce any results because both sides of the query have different labels: `%s` != `%s`.", strings.Join(n.VectorMatching.MatchingLabels, ","), l, r),
expr: node.Expr.String(),
text: fmt.Sprintf("Using `ignoring(%s)` won't produce any results on %s because results from both sides of the query have different labels: `%s` != `%s`. Failing query: `%s`.",
strings.Join(n.VectorMatching.MatchingLabels, ","), promText(c.prom.Name(), qr.URI), l, r, node.Expr),
details: VectorMatchingCheckDetails,
severity: Bug,
})
Expand Down
48 changes: 28 additions & 20 deletions internal/checks/promql_vector_matching_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,20 @@ func newVectorMatchingCheck(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewVectorMatchingCheck(prom)
}

func differentLabelsText(l, r string) string {
return fmt.Sprintf("This query will never return anything because the right and the left hand side have different labels: `[%s]` != `[%s]`.", l, r)
func differentLabelsText(name, uri, q, l, r string) string {
return fmt.Sprintf("This query will never return anything on `%s` Prometheus server at %s because results from the right and the left hand side have different labels: `[%s]` != `[%s]`. Failing query: `%s`.", name, uri, l, r, q)
}

func usingMismatchText(f, l, r string) string {
return fmt.Sprintf("Using `%s` won't produce any results because both sides of the query have different labels: `[%s]` != `[%s]`.", f, l, r)
func usingMismatchText(name, uri, q, f, l, r string) string {
return fmt.Sprintf("Using `%s` won't produce any results on `%s` Prometheus server at %s because results from both sides of the query have different labels: `[%s]` != `[%s]`. Failing query: `%s`.", f, name, uri, l, r, q)
}

func usingBothMissing(name, uri, q, f string) string {
return fmt.Sprintf("Using `%s` won't produce any results on `%s` Prometheus server at %s because results from both sides of the query don't have this label: `%s`.", f, name, uri, q)
}

func usingOneMissing(name, uri, q, side, f string) string {
return fmt.Sprintf("Using `%s` won't produce any results on `%s` Prometheus server at %s because results from the %s hand side of the query don't have this label: `%s`.", f, name, uri, side, q)
}

func differentFilters(k, lv, rv string) string {
Expand Down Expand Up @@ -49,15 +57,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- record: foo\n expr: foo_with_notfound / bar\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: differentLabelsText("instance, job, notfound", "instance, job"),
Text: differentLabelsText("prom", uri, "foo_with_notfound / bar", "instance, job, notfound", "instance, job"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -194,15 +202,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- record: foo\n expr: foo / ignoring(xxx) app_registry\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: usingMismatchText(`ignoring(xxx)`, "instance, job", "app_name"),
Text: usingMismatchText("prom", uri, "foo / ignoring (xxx) app_registry", "ignoring(xxx)", "instance, job", "app_name"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -254,15 +262,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- record: foo\n expr: foo / on(notfound) bar\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "Using `on(notfound)` won't produce any results because both sides of the query don't have this label.",
Text: usingBothMissing("prom", uri, "foo / on (notfound) bar", "on(notfound)"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -503,15 +511,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- record: foo\n expr: foo / on(notfound) bar_with_notfound\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "Using `on(notfound)` won't produce any results because the left hand side of the query doesn't have this label: `foo`.",
Text: usingOneMissing("prom", uri, "foo", "left", "on(notfound)"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -561,15 +569,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- record: foo\n expr: foo_with_notfound / on(notfound) bar\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "Using `on(notfound)` won't produce any results because the right hand side of the query doesn't have this label: `bar`.",
Text: usingOneMissing("prom", uri, "bar", "right", "on(notfound)"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -619,15 +627,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- alert: foo\n expr: (memory_bytes / ignoring(job) (memory_limit > 0)) * on(app_name) group_left(a,b,c) app_registry\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "Using `on(app_name)` won't produce any results because the left hand side of the query doesn't have this label: `(memory_bytes / ignoring (job) (memory_limit > 0))`.",
Text: usingOneMissing("prom", uri, "(memory_bytes / ignoring (job) (memory_limit > 0))", "left", "on(app_name)"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -810,15 +818,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- alert: foo\n expr: min_over_time((foo_with_notfound > 0)[30m:1m]) / bar\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "This query will never return anything because the right and the left hand side have different labels: `[instance, job, notfound]` != `[instance, job]`.",
Text: differentLabelsText("prom", uri, "min_over_time((foo_with_notfound > 0)[30m:1m]) / bar", "instance, job, notfound", "instance, job"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down Expand Up @@ -941,15 +949,15 @@ func TestVectorMatchingCheck(t *testing.T) {
content: "- alert: foo\n expr: (foo / ignoring(notfound) foo_with_notfound) / (memory_bytes / ignoring(job) memory_limit)\n",
checker: newVectorMatchingCheck,
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.VectorMatchingCheckName,
Text: "This query will never return anything because the right and the left hand side have different labels: `[instance, job]` != `[dev, instance, job]`.",
Text: differentLabelsText("prom", uri, "(foo / ignoring (notfound) foo_with_notfound) / (memory_bytes / ignoring (job) memory_limit)", "instance, job", "dev, instance, job"),
Details: checks.VectorMatchingCheckDetails,
Severity: checks.Bug,
},
Expand Down

0 comments on commit ab538fd

Please sign in to comment.