Skip to content

Commit

Permalink
Merge pull request #923 from cloudflare/query/cost
Browse files Browse the repository at this point in the history 
Fix query/cost reporting
  • Loading branch information
@prymitive
prymitive authored Mar 22, 2024
2 parents 4662448 + ee1fe2e commit d14c85f
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 86 deletions.
7 changes: 7 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## v0.56.2

### Fixed

- [query/cost](checks/query/cost.md) will now only create reports if a query is more
expensive than any of the configured limits.

## v0.56.1

### Fixed
Expand Down
35 changes: 21 additions & 14 deletions internal/checks/query_cost.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,17 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di
}
}

var above string
severity := Information
if c.maxSeries > 0 && series > c.maxSeries {
severity = c.severity
above = fmt.Sprintf(", maximum allowed series is %d.", c.maxSeries)
} else {
estimate += "."
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("%s returned %d result(s)%s, maximum allowed series is %d.", promText(c.prom.Name(), qr.URI), series, estimate, c.maxSeries),
Details: maybeComment(c.comment),
Severity: c.severity,
})
return problems
}

problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("%s returned %d result(s)%s%s", promText(c.prom.Name(), qr.URI), series, estimate, above),
Details: maybeComment(c.comment),
Severity: severity,
})

if c.maxTotalSamples > 0 && qr.Stats.Samples.TotalQueryableSamples > c.maxTotalSamples {
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Expand All @@ -123,6 +117,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di
Details: maybeComment(c.comment),
Severity: c.severity,
})
return problems
}

if c.maxPeakSamples > 0 && qr.Stats.Samples.PeakSamples > c.maxPeakSamples {
Expand All @@ -133,6 +128,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di
Details: maybeComment(c.comment),
Severity: c.severity,
})
return problems
}

evalDur := time.Duration(qr.Stats.Timings.EvalTotalTime * float64(time.Second))
Expand All @@ -144,6 +140,17 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di
Details: maybeComment(c.comment),
Severity: c.severity,
})
return problems
}

if series > 0 && c.maxSeries == 0 && c.maxTotalSamples == 0 && c.maxPeakSamples == 0 && c.maxEvaluationDuration == 0 {
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("%s returned %d result(s)%s.", promText(c.prom.Name(), qr.URI), series, estimate),
Details: maybeComment(c.comment),
Severity: Information,
})
}

return problems
Expand Down
116 changes: 44 additions & 72 deletions internal/checks/query_cost_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,7 @@ func TestCostCheck(t *testing.T) {
return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug)
},
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "query/cost",
Text: costText("prom", uri, 0) + ".",
Severity: checks.Information,
},
}
},
problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{
Expand Down Expand Up @@ -513,19 +501,7 @@ func TestCostCheck(t *testing.T) {
return checks.NewCostCheck(prom, 0, 0, 0, time.Second*5, "", checks.Bug)
},
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "query/cost",
Text: costText("prom", uri, 1) + memUsageText("4.0KiB") + ".",
Severity: checks.Information,
},
}
},
problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{
Expand Down Expand Up @@ -563,15 +539,6 @@ func TestCostCheck(t *testing.T) {
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "query/cost",
Text: costText("prom", uri, 1) + memUsageText("4.0KiB") + ".",
Severity: checks.Information,
},
{
Lines: parser.LineRange{
First: 2,
Expand All @@ -581,24 +548,6 @@ func TestCostCheck(t *testing.T) {
Text: totalSamplesText("prom", uri, 200, 100),
Severity: checks.Bug,
},
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "query/cost",
Text: peakSamplesText("prom", uri, 20, 10),
Severity: checks.Bug,
},
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "query/cost",
Text: evalDurText("prom", uri, "5s100ms", "5s"),
Severity: checks.Bug,
},
}
},
mocks: []*prometheusMock{
Expand Down Expand Up @@ -634,10 +583,10 @@ func TestCostCheck(t *testing.T) {
},
},
{
description: "stats - info",
description: "stats - peak samples",
content: content,
checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewCostCheck(prom, 0, 100, 10, time.Second*5, "some text", checks.Information)
return checks.NewCostCheck(prom, 0, 300, 10, time.Second*5, "some text", checks.Information)
},
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
Expand All @@ -648,30 +597,53 @@ func TestCostCheck(t *testing.T) {
Last: 2,
},
Reporter: "query/cost",
Text: costText("prom", uri, 1) + memUsageText("4.0KiB") + ".",
Text: peakSamplesText("prom", uri, 20, 10),
Details: "Rule comment: some text",
Severity: checks.Information,
},
{
Lines: parser.LineRange{
First: 2,
Last: 2,
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(sum(foo))`},
},
resp: vectorResponse{
samples: []*model.Sample{generateSample(map[string]string{})},
stats: promapi.QueryStats{
Timings: promapi.QueryTimings{
EvalTotalTime: 5.1,
},
Samples: promapi.QuerySamples{
TotalQueryableSamples: 200,
PeakSamples: 20,
},
},
Reporter: "query/cost",
Text: totalSamplesText("prom", uri, 200, 100),
Details: "Rule comment: some text",
Severity: checks.Information,
},
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: checks.BytesPerSampleQuery},
},
resp: vectorResponse{
samples: []*model.Sample{
generateSampleWithValue(map[string]string{}, 4096),
},
Reporter: "query/cost",
Text: peakSamplesText("prom", uri, 20, 10),
Details: "Rule comment: some text",
Severity: checks.Information,
},
},
},
},
{
description: "stats - duration",
content: content,
checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewCostCheck(prom, 0, 300, 30, time.Second*5, "some text", checks.Information)
},
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Expand Down

0 comments on commit d14c85f

Please sign in to comment.