Skip to content

Commit

Permalink
Merge pull request #1001 from cloudflare/printf
Browse files Browse the repository at this point in the history
Don't suggest humanize on printf
  • Loading branch information
prymitive committed Jun 11, 2024
2 parents 7f42c89 + 0670cb7 commit a8bb763
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 4 deletions.
1 change: 1 addition & 0 deletions .github/spellcheck/wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ matcher
matchers
md
nav
printf
prometheus
promql
PromQL
Expand Down
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.61.0

### Fixed

- Don't suggest using `humanize` when alert template is already using printf on format the `$value`.

## v0.60.0

### Fixed
Expand Down
5 changes: 1 addition & 4 deletions internal/checks/alerts_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,13 +470,10 @@ func hasHumanize(name, text string) bool {
continue
}
if n, ok := node.(*parse.ActionNode); ok {
if len(n.Pipe.Cmds) <= 1 {
continue
}
for _, cmd := range n.Pipe.Cmds {
for _, arg := range cmd.Args {
if m, ok := arg.(*parse.IdentifierNode); ok {
for _, f := range []string{"humanize", "humanize1024", "humanizePercentage", "humanizeDuration"} {
for _, f := range []string{"humanize", "humanize1024", "humanizePercentage", "humanizeDuration", "printf"} {
for _, a := range aliases.varAliases(f) {
if m.Ident == a {
return true
Expand Down
48 changes: 48 additions & 0 deletions internal/checks/alerts_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,54 @@ func TestTemplateCheck(t *testing.T) {
prometheus: noProm,
problems: noProblems,
},
{
description: "humanize not needed on wjen using printf %.2f",
content: `
- alert: Foo
expr: rate(errors_total[5m]) > 0
annotations:
summary: Seeing {{ printf "%.2f" $value }} instances with errors
`,
checker: newTemplateCheck,
prometheus: noProm,
problems: noProblems,
},
{
description: "humanize not needed on wjen using printf %f",
content: `
- alert: Foo
expr: rate(errors_total[5m]) > 0
annotations:
summary: Seeing {{ printf "%f" $value }} instances with errors
`,
checker: newTemplateCheck,
prometheus: noProm,
problems: noProblems,
},
{
description: "humanize still needed for printf on another value",
content: `
- alert: Foo
expr: rate(errors_total[5m]) > 0
annotations:
summary: Seeing {{ printf "%f" 2 }}{{ $value }} instances with errors
`,
checker: newTemplateCheck,
prometheus: noProm,
problems: func(_ string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 5,
Last: 5,
},
Reporter: checks.TemplateCheckName,
Text: humanizeText("rate(errors_total[5m])"),
Severity: checks.Information,
},
}
},
},
{
description: "toTime",
content: `
Expand Down

0 comments on commit a8bb763

Please sign in to comment.