ref!: adopt log/slog + other prom configs, drop custom logging pkg

I would call this a breaking change, as there are updates to command
line flag interactions.

Changes included:
- convert `--debug` bool flag -> `--log.level` string flag. this aligns
  with prom conventions. This project uses urfave/cli for application
commands/flags rather than kingpin as other go packages do, so I
couldn't use the promslog/flag pkg directly. Rather, I use our upstream
promslog/kingpin configs in the configs we pass here for urfave/cli. We
can decide to move off urfave/cli and/or adopt kingpin in the future.
- removes `pkg/logging` in favor of using `prometheus/common/promslog`.
  A small utility function is included in main to simplify some
boilerplate with logger initialization, since we instantiate in main in
a few places. All functions have been converted to accept/use
*slog.Loggers rather than the custom logger interface.
- the removal of the custom logging pkg also removes the
  `IsDebugEnabled()` method, but this is fine because the logger is
leveled; debug logs will not print unless `--log.level debug` is passed
at the command line, there is no need to check if debug is enabled at
log time in the vast majority of cases.
- Updates tests as needed; updates nop loggers, test signatures that
  used `IsDebugEnabled()`, etc.
- enable sloglint in golangci-lint, remove go-kit/log configs

Signed-off-by: TJ Hoplock <[email protected]>

.golangci.yml

@@ -31,10 +31,8 @@ linters:
     - typecheck
     - unconvert
     - unused
+    - sloglint
 
 linters-settings:
-  errcheck:
-    exclude-functions:
-      - (github.com/go-kit/log.Logger).Log
   goimports:
     local-prefixes: "github.com/prometheus-community/yet-another-cloudwatch-exporter"

cmd/yace/main.go

@@ -15,11 +15,15 @@ package main
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"net/http/pprof"
 	"os"
+	"slices"
 	"strings"
 
+	"github.com/prometheus/common/promslog"
+	promslogflag "github.com/prometheus/common/promslog/flag"
 	"github.com/urfave/cli/v2"
 	"golang.org/x/sync/semaphore"
 
@@ -28,7 +32,6 @@ import (
 	v1 "github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/clients/v1"
 	v2 "github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/clients/v2"
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/config"
-	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/logging"
 )
 
 const (
@@ -56,7 +59,7 @@ const (
 var (
 	addr                  string
 	configFile            string
-	debug                 bool
+	logLevel              string
 	logFormat             string
 	fips                  bool
 	cloudwatchConcurrency cloudwatch.ConcurrencyConfig
@@ -66,17 +69,19 @@ var (
 	labelsSnakeCase       bool
 	profilingEnabled      bool
 
-	logger logging.Logger
+	logger *slog.Logger
 )
 
 func main() {
 	app := NewYACEApp()
 	if err := app.Run(os.Args); err != nil {
 		// if we exit very early we'll not have set up the logger yet
 		if logger == nil {
-			logger = logging.NewLogger(defaultLogFormat, debug, "version", version)
+			jsonFmt := &promslog.AllowedFormat{}
+			_ = jsonFmt.Set("json")
+			logger = promslog.New(&promslog.Config{Format: jsonFmt})
 		}
-		logger.Error(err, "Error running yace")
+		logger.Error("Error running yace", "err", err)
 		os.Exit(1)
 	}
 }
@@ -107,23 +112,25 @@ func NewYACEApp() *cli.App {
 			Destination: &configFile,
 			EnvVars:     []string{"config.file"},
 		},
-		&cli.BoolFlag{
-			Name:        "debug",
-			Value:       false,
-			Usage:       "Verbose logging",
-			Destination: &debug,
-			EnvVars:     []string{"debug"},
+		&cli.StringFlag{
+			Name:        "log.level",
+			Value:       "",
+			Usage:       promslogflag.LevelFlagHelp,
+			Destination: &logLevel,
+			Action: func(_ *cli.Context, s string) error {
+				if !slices.Contains(promslog.LevelFlagOptions, s) {
+					return fmt.Errorf("unrecognized log format %q", s)
+				}
+				return nil
+			},
 		},
 		&cli.StringFlag{
 			Name:        "log.format",
 			Value:       defaultLogFormat,
-			Usage:       "Output format of log messages. One of: [logfmt, json]. Default: [json].",
+			Usage:       promslogflag.FormatFlagHelp,
 			Destination: &logFormat,
 			Action: func(_ *cli.Context, s string) error {
-				switch s {
-				case "logfmt", "json":
-					break
-				default:
+				if !slices.Contains(promslog.FormatFlagOptions, s) {
 					return fmt.Errorf("unrecognized log format %q", s)
 				}
 				return nil
@@ -212,11 +219,11 @@ func NewYACEApp() *cli.App {
 				&cli.StringFlag{Name: "config.file", Value: "config.yml", Usage: "Path to configuration file.", Destination: &configFile},
 			},
 			Action: func(_ *cli.Context) error {
-				logger = logging.NewLogger(logFormat, debug, "version", version)
+				logger = newLogger(logFormat, logLevel).With("version", version)
 				logger.Info("Parsing config")
 				cfg := config.ScrapeConf{}
 				if _, err := cfg.Load(configFile, logger); err != nil {
-					logger.Error(err, "Couldn't read config file", "path", configFile)
+					logger.Error("Couldn't read config file", "err", err, "path", configFile)
 					os.Exit(1)
 				}
 				logger.Info("Config file is valid", "path", configFile)
@@ -242,7 +249,7 @@ func NewYACEApp() *cli.App {
 }
 
 func startScraper(c *cli.Context) error {
-	logger = logging.NewLogger(logFormat, debug, "version", version)
+	logger = newLogger(logFormat, logLevel).With("version", version)
 
 	// log warning if the two concurrency limiting methods are configured via CLI
 	if c.IsSet("cloudwatch-concurrency") && c.IsSet("cloudwatch-concurrency.per-api-limit-enabled") {
@@ -310,7 +317,7 @@ func startScraper(c *cli.Context) error {
 		newCfg := config.ScrapeConf{}
 		newJobsCfg, err := newCfg.Load(configFile, logger)
 		if err != nil {
-			logger.Error(err, "Couldn't read config file", "path", configFile)
+			logger.Error("Couldn't read config file", "err", err, "path", configFile)
 			return
 		}
 
@@ -323,7 +330,7 @@ func startScraper(c *cli.Context) error {
 				// Can't override cache while also creating err
 				cache, err = v2.NewFactory(logger, newJobsCfg, fips)
 				if err != nil {
-					logger.Error(err, "Failed to construct aws sdk v2 client cache", "path", configFile)
+					logger.Error("Failed to construct aws sdk v2 client cache", "err", err, "path", configFile)
 					return
 				}
 			}
@@ -339,3 +346,16 @@ func startScraper(c *cli.Context) error {
 	srv := &http.Server{Addr: addr, Handler: mux}
 	return srv.ListenAndServe()
 }
+
+func newLogger(format, level string) *slog.Logger {
+	// If flag parsing was successful, then we know that format and level
+	// are both valid options; no need to error check their returns, just
+	// set their values.
+	fmt := &promslog.AllowedFormat{}
+	_ = fmt.Set(format)
+
+	lvl := &promslog.AllowedLevel{}
+	_ = fmt.Set(level)
+
+	return promslog.New(&promslog.Config{Format: fmt, Level: lvl})
+}

cmd/yace/scraper.go

@@ -14,6 +14,7 @@ package main
 
 import (
 	"context"
+	"log/slog"
 	"net/http"
 	"sync/atomic"
 	"time"
@@ -23,7 +24,6 @@ import (
 
 	exporter "github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg"
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/clients"
-	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/logging"
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/model"
 )
 
@@ -56,7 +56,7 @@ func (s *scraper) makeHandler() func(http.ResponseWriter, *http.Request) {
 	}
 }
 
-func (s *scraper) decoupled(ctx context.Context, logger logging.Logger, jobsCfg model.JobsConfig, cache cachingFactory) {
+func (s *scraper) decoupled(ctx context.Context, logger *slog.Logger, jobsCfg model.JobsConfig, cache cachingFactory) {
 	logger.Debug("Starting scraping async")
 	s.scrape(ctx, logger, jobsCfg, cache)
 
@@ -75,7 +75,7 @@ func (s *scraper) decoupled(ctx context.Context, logger logging.Logger, jobsCfg
 	}
 }
 
-func (s *scraper) scrape(ctx context.Context, logger logging.Logger, jobsCfg model.JobsConfig, cache cachingFactory) {
+func (s *scraper) scrape(ctx context.Context, logger *slog.Logger, jobsCfg model.JobsConfig, cache cachingFactory) {
 	if !sem.TryAcquire(1) {
 		// This shouldn't happen under normal use, users should adjust their configuration when this occurs.
 		// Let them know by logging a warning.
@@ -120,7 +120,7 @@ func (s *scraper) scrape(ctx context.Context, logger logging.Logger, jobsCfg mod
 		options...,
 	)
 	if err != nil {
-		logger.Error(err, "error updating metrics")
+		logger.Error("error updating metrics", "err", err)
 	}
 
 	s.registry.Store(newRegistry)

go.mod

@@ -20,7 +20,6 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/storagegateway v1.34.6
 	github.com/aws/aws-sdk-go-v2/service/sts v1.33.1
 	github.com/aws/smithy-go v1.22.1
-	github.com/go-kit/log v0.2.1
 	github.com/grafana/regexp v0.0.0-20240607082908-2cb410fa05da
 	github.com/prometheus/client_golang v1.20.5
 	github.com/prometheus/client_model v0.6.1
@@ -34,6 +33,8 @@ require (
 )
 
 require (
+	github.com/alecthomas/kingpin/v2 v2.4.0 // indirect
+	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
 	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.20 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.24 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.24 // indirect
@@ -46,7 +47,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cpuguy83/go-md2man/v2 v2.0.5 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/go-logfmt/logfmt v0.5.1 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/klauspost/compress v1.17.9 // indirect
 	github.com/kr/text v0.2.0 // indirect
@@ -56,6 +56,7 @@ require (
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
+	github.com/xhit/go-str2duration/v2 v2.1.0 // indirect
 	github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect
 	golang.org/x/sys v0.25.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect

go.sum

@@ -1,3 +1,7 @@
+github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY=
+github.com/alecthomas/kingpin/v2 v2.4.0/go.mod h1:0gyi0zQnjuFk8xrkNKamJoyUo382HRL7ATRpFZCw6tE=
+github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAuRjVTiNNhvNRfY2Wxp9nhfyel4rklc=
+github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE=
 github.com/aws/aws-sdk-go v1.55.5 h1:KKUZBfBoyqy5d3swXyiC7Q76ic40rYcbqH7qjh59kzU=
 github.com/aws/aws-sdk-go v1.55.5/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU=
 github.com/aws/aws-sdk-go-v2 v1.32.5 h1:U8vdWJuY7ruAkzaOdD7guwJjD06YSKmnKCJs7s3IkIo=
@@ -58,10 +62,6 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU=
-github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
-github.com/go-logfmt/logfmt v0.5.1 h1:otpy5pqBCBZ1ng9RQ0dPu4PN7ba75Y/aA+UpowDyNVA=
-github.com/go-logfmt/logfmt v0.5.1/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/grafana/regexp v0.0.0-20240607082908-2cb410fa05da h1:BML5sNe+bw2uO8t8cQSwe5QhvoP04eHPF7bnaQma0Kw=
@@ -97,6 +97,7 @@ github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncj
 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
@@ -106,6 +107,8 @@ github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9
 github.com/vmihailenco/msgpack/v5 v5.3.5/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q/V5KbhBonMG9jc=
 github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
 github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
+github.com/xhit/go-str2duration/v2 v2.1.0 h1:lxklc02Drh6ynqX+DdPyp5pCKLUQpRT8bp8Ydu2Bstc=
+github.com/xhit/go-str2duration/v2 v2.1.0/go.mod h1:ohY8p+0f07DiV6Em5LKB0s2YpLtXVyJfNt1+BlmyAsU=
 github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 h1:gEOO8jv9F4OT7lGCjxCBTO/36wtF6j2nSip77qHd4x4=
 github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM=
 golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 h1:kx6Ds3MlpiUHKj7syVnbp57++8WpuKPcR5yjLBjvLEA=
@@ -119,6 +122,7 @@ google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWn
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
+gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=

pkg/clients/account/v1/client.go

@@ -15,24 +15,23 @@ package v1
 import (
 	"context"
 	"errors"
+	"log/slog"
 
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/clients/account"
 
 	"github.com/aws/aws-sdk-go/service/iam"
 	"github.com/aws/aws-sdk-go/service/iam/iamiface"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/aws/aws-sdk-go/service/sts/stsiface"
-
-	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/logging"
 )
 
 type client struct {
-	logger    logging.Logger
+	logger    *slog.Logger
 	stsClient stsiface.STSAPI
 	iamClient iamiface.IAMAPI
 }
 
-func NewClient(logger logging.Logger, stsClient stsiface.STSAPI, iamClient iamiface.IAMAPI) account.Client {
+func NewClient(logger *slog.Logger, stsClient stsiface.STSAPI, iamClient iamiface.IAMAPI) account.Client {
 	return &client{
 		logger:    logger,
 		stsClient: stsClient,

pkg/clients/account/v2/client.go

@@ -15,21 +15,21 @@ package v2
 import (
 	"context"
 	"errors"
+	"log/slog"
 
 	"github.com/aws/aws-sdk-go-v2/service/iam"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
 
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/clients/account"
-	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/logging"
 )
 
 type client struct {
-	logger    logging.Logger
+	logger    *slog.Logger
 	stsClient *sts.Client
 	iamClient *iam.Client
 }
 
-func NewClient(logger logging.Logger, stsClient *sts.Client, iamClient *iam.Client) account.Client {
+func NewClient(logger *slog.Logger, stsClient *sts.Client, iamClient *iam.Client) account.Client {
 	return &client{
 		logger:    logger,
 		stsClient: stsClient,

pkg/clients/cloudwatch/client.go

@@ -14,9 +14,9 @@ package cloudwatch
 
 import (
 	"context"
+	"log/slog"
 	"time"
 
-	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/logging"
 	"github.com/prometheus-community/yet-another-cloudwatch-exporter/pkg/model"
 )
 
@@ -37,7 +37,7 @@ type Client interface {
 	GetMetricData(ctx context.Context, getMetricData []*model.CloudwatchData, namespace string, startTime time.Time, endTime time.Time) []MetricDataResult
 
 	// GetMetricStatistics returns the output of the GetMetricStatistics CloudWatch API.
-	GetMetricStatistics(ctx context.Context, logger logging.Logger, dimensions []model.Dimension, namespace string, metric *model.MetricConfig) []*model.Datapoint
+	GetMetricStatistics(ctx context.Context, logger *slog.Logger, dimensions []model.Dimension, namespace string, metric *model.MetricConfig) []*model.Datapoint
 }
 
 // ConcurrencyLimiter limits the concurrency when calling AWS CloudWatch APIs. The functions implemented
@@ -73,7 +73,7 @@ func NewLimitedConcurrencyClient(client Client, limiter ConcurrencyLimiter) Clie
 	}
 }
 
-func (c limitedConcurrencyClient) GetMetricStatistics(ctx context.Context, logger logging.Logger, dimensions []model.Dimension, namespace string, metric *model.MetricConfig) []*model.Datapoint {
+func (c limitedConcurrencyClient) GetMetricStatistics(ctx context.Context, logger *slog.Logger, dimensions []model.Dimension, namespace string, metric *model.MetricConfig) []*model.Datapoint {
 	c.limiter.Acquire(getMetricStatisticsCall)
 	res := c.client.GetMetricStatistics(ctx, logger, dimensions, namespace, metric)
 	c.limiter.Release(getMetricStatisticsCall)