Skip to content

Commit

Permalink
Use log/slog instead of zerolog
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Oct 19, 2023
1 parent 0c259b8 commit a795ccc
Show file tree
Hide file tree
Showing 166 changed files with 1,900 additions and 1,692 deletions.
35 changes: 22 additions & 13 deletions cmd/pint/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"log/slog"
"os"
"regexp"
"strconv"
Expand All @@ -15,7 +16,6 @@ import (
"github.com/cloudflare/pint/internal/git"
"github.com/cloudflare/pint/internal/reporter"

"github.com/rs/zerolog/log"
"github.com/urfave/cli/v2"
)

Expand Down Expand Up @@ -82,9 +82,9 @@ func actionCI(c *cli.Context) error {
if err != nil {
return fmt.Errorf("failed to get the name of current branch")
}
log.Debug().Str("current", currentBranch).Str("base", baseBranch).Msg("Got branch information")
slog.Debug("Got branch information", slog.String("base", baseBranch), slog.String("current", currentBranch))
if currentBranch == strings.Split(baseBranch, "/")[len(strings.Split(baseBranch, "/"))-1] {
log.Info().Str("branch", currentBranch).Msg("Running from base branch, skipping checks")
slog.Info("Running from base branch, skipping checks", slog.String("branch", currentBranch))
return nil
}

Expand Down Expand Up @@ -176,15 +176,15 @@ func actionCI(c *cli.Context) error {
}

problemsFound := false
bySeverity := map[string]interface{}{} // interface{} is needed for log.Fields()
for s, c := range summary.CountBySeverity() {
bySeverity := summary.CountBySeverity()
for s := range bySeverity {
if s >= minSeverity {
problemsFound = true
break
}
bySeverity[s.String()] = c
}
if len(bySeverity) > 0 {
log.Info().Fields(bySeverity).Msg("Problems found")
slog.Info("Problems found", logSeverityCounters(bySeverity)...)
}

if err := submitReports(reps, summary); err != nil {
Expand All @@ -198,6 +198,15 @@ func actionCI(c *cli.Context) error {
return nil
}

func logSeverityCounters(src map[checks.Severity]int) (attrs []any) {
for _, s := range []checks.Severity{checks.Fatal, checks.Bug, checks.Warning, checks.Information} {
if c, ok := src[s]; ok {
attrs = append(attrs, slog.Attr{Key: s.String(), Value: slog.IntValue(c)})
}
}
return attrs
}

func detectCI(cfg *config.CI) *config.CI {
var isNil, isDirty bool

Expand All @@ -209,7 +218,7 @@ func detectCI(cfg *config.CI) *config.CI {
if bb := os.Getenv("GITHUB_BASE_REF"); bb != "" {
isDirty = true
cfg.BaseBranch = bb
log.Debug().Str("branch", bb).Msg("got base branch from GITHUB_BASE_REF env variable")
slog.Debug("got base branch from GITHUB_BASE_REF env variable", slog.String("branch", bb))
}

if isNil && !isDirty {
Expand Down Expand Up @@ -243,7 +252,7 @@ func detectGithubActions(gh *config.GitHub) *config.GitHub {
os.Getenv("GITHUB_REF") != "" {
parts := strings.Split(os.Getenv("GITHUB_REF"), "/")
if len(parts) >= 4 {
log.Info().Str("pr", parts[2]).Msg("Setting GITHUB_PULL_REQUEST_NUMBER from GITHUB_REF env variable")
slog.Info("Setting GITHUB_PULL_REQUEST_NUMBER from GITHUB_REF env variable", slog.String("pr", parts[2]))
os.Setenv("GITHUB_PULL_REQUEST_NUMBER", parts[2])
}
}
Expand All @@ -259,12 +268,12 @@ func detectGithubActions(gh *config.GitHub) *config.GitHub {
parts := strings.SplitN(repo, "/", 2)
if len(parts) == 2 {
if gh.Owner == "" {
log.Info().Str("owner", parts[0]).Msg("Setting repository owner from GITHUB_REPOSITORY env variable")
slog.Info("Setting repository owner from GITHUB_REPOSITORY env variable", slog.String("owner", parts[0]))
gh.Owner = parts[0]
isDirty = true
}
if gh.Repo == "" {
log.Info().Str("repo", parts[1]).Msg("Setting repository name from GITHUB_REPOSITORY env variable")
slog.Info("Setting repository name from GITHUB_REPOSITORY env variable", slog.String("repo", parts[1]))
gh.Repo = parts[1]
isDirty = true
}
Expand All @@ -273,11 +282,11 @@ func detectGithubActions(gh *config.GitHub) *config.GitHub {

if api := os.Getenv("GITHUB_API_URL"); api != "" {
if gh.BaseURI == "" {
log.Info().Str("baseuri", api).Msg("Setting repository base URI from GITHUB_API_URL env variable")
slog.Info("Setting repository base URI from GITHUB_API_URL env variable", slog.String("baseuri", api))
gh.BaseURI = api
}
if gh.UploadURI == "" {
log.Info().Str("uploaduri", api).Msg("Setting repository upload URI from GITHUB_API_URL env variable")
slog.Info("Setting repository upload URI from GITHUB_API_URL env variable", slog.String("uploaduri", api))
gh.UploadURI = api
}
}
Expand Down
11 changes: 5 additions & 6 deletions cmd/pint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"log/slog"
"os"
"regexp"

Expand All @@ -11,7 +12,6 @@ import (
"github.com/cloudflare/pint/internal/discovery"
"github.com/cloudflare/pint/internal/reporter"

"github.com/rs/zerolog/log"
"github.com/urfave/cli/v2"
)

Expand Down Expand Up @@ -87,25 +87,24 @@ func actionLint(c *cli.Context) error {
return err
}

bySeverity := map[string]interface{}{} // interface{} is needed for log.Fields()
bySeverity := summary.CountBySeverity()
var problems, hiddenProblems, failProblems int
for s, c := range summary.CountBySeverity() {
for s, c := range bySeverity {
if s >= failOn {
failProblems++
}
if s < minSeverity {
hiddenProblems++
}
bySeverity[s.String()] = c
if s >= checks.Bug {
problems += c
}
}
if len(bySeverity) > 0 {
log.Info().Fields(bySeverity).Msg("Problems found")
slog.Info("Problems found", logSeverityCounters(bySeverity)...)
}
if hiddenProblems > 0 {
log.Info().Msgf("%d problem(s) not visible because of --%s=%s flag", hiddenProblems, minSeverityFlag, c.String(minSeverityFlag))
slog.Info(fmt.Sprintf("%d problem(s) not visible because of --%s=%s flag", hiddenProblems, minSeverityFlag, c.String(minSeverityFlag)))
}

if failProblems > 0 {
Expand Down
34 changes: 9 additions & 25 deletions cmd/pint/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,21 @@ import (
"fmt"
"os"

"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"github.com/cloudflare/pint/internal/log"
)

func msgFormatter(msg interface{}) string {
return fmt.Sprintf("msg=%q", msg)
}

func lvlFormatter(level interface{}) string {
if level == nil {
return ""
}
return fmt.Sprintf("level=%s", level)
}

func initLogger(level string, noColor bool) error {
log.Logger = log.Logger.Output(zerolog.ConsoleWriter{
Out: os.Stderr,
NoColor: noColor,
FormatLevel: lvlFormatter,
FormatMessage: msgFormatter,
FormatTimestamp: func(interface{}) string {
return ""
},
})

l, err := zerolog.ParseLevel(level)
l, err := log.ParseLevel(level)
if err != nil {
return fmt.Errorf("'%s' is not a valid log level", level)
}
zerolog.SetGlobalLevel(l)

nc := os.Getenv("NO_COLOR")
if nc != "" && nc != "0" {
noColor = true
}

log.Setup(l, noColor)

return nil
}
9 changes: 4 additions & 5 deletions cmd/pint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ package main

import (
"fmt"
"log/slog"
"os"

"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"github.com/urfave/cli/v2"
"go.uber.org/automaxprocs/maxprocs"

Expand Down Expand Up @@ -45,7 +44,7 @@ func newApp() *cli.App {
&cli.StringFlag{
Name: logLevelFlag,
Aliases: []string{"l"},
Value: zerolog.InfoLevel.String(),
Value: slog.LevelInfo.String(),
Usage: "Log level",
},
&cli.BoolFlag{
Expand Down Expand Up @@ -98,7 +97,7 @@ func actionSetup(c *cli.Context) (meta actionMeta, err error) {
undo, err := maxprocs.Set()
defer undo()
if err != nil {
log.Error().Err(err).Msg("failed to set GOMAXPROCS")
slog.Error("failed to set GOMAXPROCS", slog.Any("err", err))
}

meta.workers = c.Int(workersFlag)
Expand All @@ -122,7 +121,7 @@ func main() {
app := newApp()
err := app.Run(os.Args)
if err != nil {
log.Fatal().Err(err).Msg("Execution completed with error(s)")
slog.Error("Execution completed with error(s)", slog.Any("err", err))
os.Exit(1)
}
}
7 changes: 3 additions & 4 deletions cmd/pint/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/pem"
"errors"
"fmt"
"log/slog"
"math/big"
"net"
"net/http"
Expand All @@ -20,16 +21,14 @@ import (
"time"

"github.com/rogpeppe/go-internal/testscript"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
)

// mock command that fails tests if error is returned
func mockMainShouldSucceed() int {
app := newApp()
err := app.Run(os.Args)
if err != nil {
log.WithLevel(zerolog.FatalLevel).Err(err).Msg("Fatal error")
slog.Error("Fatal error", slog.Any("err", err))
return 1
}
return 0
Expand All @@ -40,7 +39,7 @@ func mockMainShouldFail() int {
app := newApp()
err := app.Run(os.Args)
if err != nil {
log.WithLevel(zerolog.FatalLevel).Err(err).Msg("Fatal error")
slog.Error("Fatal error", slog.Any("err", err))
return 0
}
fmt.Fprintf(os.Stderr, "expected an error but none was returned\n")
Expand Down
30 changes: 15 additions & 15 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package main
import (
"context"
"errors"
"log/slog"
"regexp"
"strconv"
"sync"
"time"

"github.com/prometheus/prometheus/model/rulefmt"
"github.com/rs/zerolog/log"
"go.uber.org/atomic"

"github.com/cloudflare/pint/internal/checks"
Expand Down Expand Up @@ -93,19 +93,19 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d
case entry.PathError == nil && entry.Rule.Error.Err == nil:
if entry.Rule.RecordingRule != nil {
rulesParsedTotal.WithLabelValues(config.RecordingRuleType).Inc()
log.Debug().
Str("path", entry.SourcePath).
Str("record", entry.Rule.RecordingRule.Record.Value.Value).
Str("lines", output.FormatLineRangeString(entry.Rule.Lines())).
Msg("Found recording rule")
slog.Debug("Found recording rule",
slog.String("path", entry.SourcePath),
slog.String("record", entry.Rule.RecordingRule.Record.Value.Value),
slog.String("lines", output.FormatLineRangeString(entry.Rule.Lines())),
)
}
if entry.Rule.AlertingRule != nil {
rulesParsedTotal.WithLabelValues(config.AlertingRuleType).Inc()
log.Debug().
Str("path", entry.SourcePath).
Str("alert", entry.Rule.AlertingRule.Alert.Value.Value).
Str("lines", output.FormatLineRangeString(entry.Rule.Lines())).
Msg("Found alerting rule")
slog.Debug("Found alerting rule",
slog.String("path", entry.SourcePath),
slog.String("alert", entry.Rule.AlertingRule.Alert.Value.Value),
slog.String("lines", output.FormatLineRangeString(entry.Rule.Lines())),
)
}

checkList := cfg.GetChecksForRule(ctx, entry.SourcePath, entry.Rule, entry.DisabledChecks)
Expand All @@ -121,10 +121,10 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d
}
default:
if entry.Rule.Error.Err != nil {
log.Debug().
Str("path", entry.SourcePath).
Str("lines", output.FormatLineRangeString(entry.Rule.Lines())).
Msg("Found invalid rule")
slog.Debug("Found invalid rule",
slog.String("path", entry.SourcePath),
slog.String("lines", output.FormatLineRangeString(entry.Rule.Lines())),
)
rulesParsedTotal.WithLabelValues(config.InvalidRuleType).Inc()
}
jobs <- scanJob{entry: entry, allEntries: entries, check: nil}
Expand Down
6 changes: 3 additions & 3 deletions cmd/pint/tests/0001_match_path.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ pint.error --no-color lint rules
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Loading configuration file" path=.pint.hcl
rules/0002.yml:2 Bug: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
2 | expr: sum(foo) without(job)

level=info msg="Problems found" Bug=1
level=fatal msg="Fatal error" error="found 1 problem(s) with severity Bug or higher"
level=INFO msg="Problems found" Bug=1
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules/0001.yml --
- record: "colo:test1"
expr: sum(foo) without(job)
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0002_nothing_to_lint.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ pint.error --no-color lint rules
cmp stderr stderr.txt

-- stderr.txt --
level=fatal msg="Fatal error" error="no matching files"
level=ERROR msg="Fatal error" err="no matching files"
7 changes: 4 additions & 3 deletions cmd/pint/tests/0003_lint_workdir.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
env NO_COLOR=1
pint.error --no-color lint --min-severity=info rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Loading configuration file" path=.pint.hcl
rules/0001.yml:2 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
2 | expr: sum(rate(fl_cf_html_bytes_in[10m])) WITHOUT (colo_id, instance, node_type, region, node_status, job, colo_name)

Expand Down Expand Up @@ -60,8 +61,8 @@ rules/0003.yaml:58-61 Information: using the value of rate(errors[5m]) inside th
..
61 | summary: 'error rate: {{ $value }}'

level=info msg="Problems found" Bug=2 Fatal=1 Information=1 Warning=10
level=fatal msg="Fatal error" error="found 2 problem(s) with severity Bug or higher"
level=INFO msg="Problems found" Fatal=1 Bug=2 Warning=10 Information=1
level=ERROR msg="Fatal error" err="found 2 problem(s) with severity Bug or higher"
-- rules/0001.yml --
- record: colo_job:fl_cf_html_bytes_in:rate10m
expr: sum(rate(fl_cf_html_bytes_in[10m])) WITHOUT (colo_id, instance, node_type, region, node_status, job, colo_name)
Expand Down
Loading

0 comments on commit a795ccc

Please sign in to comment.