Merge pull request #183 from prometheus-community/superq/slog

chore!: adopt log/slog, drop go-kit/log

.circleci/config.yml

@@ -5,14 +5,14 @@ orbs:
 executors:
   golang:
     docker:
-      - image: cimg/go:1.22
+      - image: cimg/go:1.23
       - image: quay.io/prometheus/node-exporter:latest
 
 jobs:
   test:
     executor: golang
     environment:
-      prom_ver: 2.51.0
+      prom_ver: 2.54.1
     steps:
       - prometheus/setup_environment
       - run: wget https://github.com/prometheus/prometheus/releases/download/v${prom_ver}/prometheus-${prom_ver}.linux-amd64.tar.gz

.golangci.yml

@@ -3,6 +3,7 @@ linters:
   enable:
     - misspell
     - revive
+    - sloglint
 
 linters-settings:
   errcheck:
@@ -11,8 +12,6 @@ linters-settings:
       - io.Copy
       # Used in HTTP handlers, any error is handled by the server itself.
       - (net/http.ResponseWriter).Write
-      # Never check for logger errors.
-      - (github.com/go-kit/log.Logger).Log
   revive:
     rules:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter

.promu.yml

@@ -1,6 +1,6 @@
 go:
     # This must match .circle/config.yml.
-    version: 1.22
+    version: 1.23
 repository:
     path: github.com/prometheus-community/pushprox
 build:

cmd/client/main.go

@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"net"
 	"net/http"
 	"net/url"
@@ -32,13 +33,11 @@ import (
 	"github.com/Showmax/go-fqdn"
 	"github.com/alecthomas/kingpin/v2"
 	"github.com/cenkalti/backoff/v4"
-	"github.com/go-kit/log"
-	"github.com/go-kit/log/level"
 	"github.com/prometheus-community/pushprox/util"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
-	"github.com/prometheus/common/promlog"
-	"github.com/prometheus/common/promlog/flag"
+	"github.com/prometheus/common/promslog"
+	"github.com/prometheus/common/promslog/flag"
 )
 
 var (
@@ -89,11 +88,11 @@ func newBackOffFromFlags() backoff.BackOff {
 
 // Coordinator for scrape requests and responses
 type Coordinator struct {
-	logger log.Logger
+	logger *slog.Logger
 }
 
 func (c *Coordinator) handleErr(request *http.Request, client *http.Client, err error) {
-	level.Error(c.logger).Log("err", err)
+	c.logger.Error("Coordinator error", "error", err)
 	scrapeErrorCounter.Inc()
 	resp := &http.Response{
 		StatusCode: http.StatusInternalServerError,
@@ -102,14 +101,14 @@ func (c *Coordinator) handleErr(request *http.Request, client *http.Client, err
 	}
 	if err = c.doPush(resp, request, client); err != nil {
 		pushErrorCounter.Inc()
-		level.Warn(c.logger).Log("msg", "Failed to push failed scrape response:", "err", err)
+		c.logger.Warn("Failed to push failed scrape response:", "err", err)
 		return
 	}
-	level.Info(c.logger).Log("msg", "Pushed failed scrape response")
+	c.logger.Info("Pushed failed scrape response")
 }
 
 func (c *Coordinator) doScrape(request *http.Request, client *http.Client) {
-	logger := log.With(c.logger, "scrape_id", request.Header.Get("id"))
+	logger := c.logger.With("scrape_id", request.Header.Get("id"))
 	timeout, err := util.GetHeaderTimeout(request.Header)
 	if err != nil {
 		c.handleErr(request, client, err)
@@ -137,13 +136,13 @@ func (c *Coordinator) doScrape(request *http.Request, client *http.Client) {
 		c.handleErr(request, client, fmt.Errorf("failed to scrape %s: %w", request.URL.String(), err))
 		return
 	}
-	level.Info(logger).Log("msg", "Retrieved scrape response")
+	logger.Info("Retrieved scrape response")
 	if err = c.doPush(scrapeResp, request, client); err != nil {
 		pushErrorCounter.Inc()
-		level.Warn(logger).Log("msg", "Failed to push scrape response:", "err", err)
+		logger.Warn("Failed to push scrape response:", "err", err)
 		return
 	}
-	level.Info(logger).Log("msg", "Pushed scrape result")
+	logger.Info("Pushed scrape result")
 }
 
 // Report the result of the scrape back up to the proxy.
@@ -182,28 +181,28 @@ func (c *Coordinator) doPush(resp *http.Response, origRequest *http.Request, cli
 func (c *Coordinator) doPoll(client *http.Client) error {
 	base, err := url.Parse(*proxyURL)
 	if err != nil {
-		level.Error(c.logger).Log("msg", "Error parsing url:", "err", err)
+		c.logger.Error("Error parsing url:", "err", err)
 		return fmt.Errorf("error parsing url: %w", err)
 	}
 	u, err := url.Parse("poll")
 	if err != nil {
-		level.Error(c.logger).Log("msg", "Error parsing url:", "err", err)
+		c.logger.Error("Error parsing url:", "err", err)
 		return fmt.Errorf("error parsing url poll: %w", err)
 	}
 	url := base.ResolveReference(u)
 	resp, err := client.Post(url.String(), "", strings.NewReader(*myFqdn))
 	if err != nil {
-		level.Error(c.logger).Log("msg", "Error polling:", "err", err)
+		c.logger.Error("Error polling:", "err", err)
 		return fmt.Errorf("error polling: %w", err)
 	}
 	defer resp.Body.Close()
 
 	request, err := http.ReadRequest(bufio.NewReader(resp.Body))
 	if err != nil {
-		level.Error(c.logger).Log("msg", "Error reading request:", "err", err)
+		c.logger.Error("Error reading request:", "err", err)
 		return fmt.Errorf("error reading request: %w", err)
 	}
-	level.Info(c.logger).Log("msg", "Got scrape request", "scrape_id", request.Header.Get("id"), "url", request.URL)
+	c.logger.Info("Got scrape request", "scrape_id", request.Header.Get("id"), "url", request.URL)
 
 	request.RequestURI = ""
 
@@ -221,32 +220,32 @@ func (c *Coordinator) loop(bo backoff.BackOff, client *http.Client) {
 		if err := backoff.RetryNotify(op, bo, func(err error, _ time.Duration) {
 			pollErrorCounter.Inc()
 		}); err != nil {
-			level.Error(c.logger).Log("err", err)
+			c.logger.Error("backoff returned error", "error", err)
 		}
 	}
 }
 
 func main() {
-	promlogConfig := promlog.Config{}
-	flag.AddFlags(kingpin.CommandLine, &promlogConfig)
+	promslogConfig := promslog.Config{}
+	flag.AddFlags(kingpin.CommandLine, &promslogConfig)
 	kingpin.HelpFlag.Short('h')
 	kingpin.Parse()
-	logger := promlog.New(&promlogConfig)
+	logger := promslog.New(&promslogConfig)
 	coordinator := Coordinator{logger: logger}
 
 	if *proxyURL == "" {
-		level.Error(coordinator.logger).Log("msg", "--proxy-url flag must be specified.")
+		coordinator.logger.Error("--proxy-url flag must be specified.")
 		os.Exit(1)
 	}
 	// Make sure proxyURL ends with a single '/'
 	*proxyURL = strings.TrimRight(*proxyURL, "/") + "/"
-	level.Info(coordinator.logger).Log("msg", "URL and FQDN info", "proxy_url", *proxyURL, "fqdn", *myFqdn)
+	coordinator.logger.Info("URL and FQDN info", "proxy_url", *proxyURL, "fqdn", *myFqdn)
 
 	tlsConfig := &tls.Config{}
 	if *tlsCert != "" {
 		cert, err := tls.LoadX509KeyPair(*tlsCert, *tlsKey)
 		if err != nil {
-			level.Error(coordinator.logger).Log("msg", "Certificate or Key is invalid", "err", err)
+			coordinator.logger.Error("Certificate or Key is invalid", "err", err)
 			os.Exit(1)
 		}
 
@@ -257,12 +256,12 @@ func main() {
 	if *caCertFile != "" {
 		caCert, err := os.ReadFile(*caCertFile)
 		if err != nil {
-			level.Error(coordinator.logger).Log("msg", "Not able to read cacert file", "err", err)
+			coordinator.logger.Error("Not able to read cacert file", "err", err)
 			os.Exit(1)
 		}
 		caCertPool := x509.NewCertPool()
 		if ok := caCertPool.AppendCertsFromPEM(caCert); !ok {
-			level.Error(coordinator.logger).Log("msg", "Failed to use cacert file as ca certificate")
+			coordinator.logger.Error("Failed to use cacert file as ca certificate")
 			os.Exit(1)
 		}
 
@@ -272,7 +271,7 @@ func main() {
 	if *metricsAddr != "" {
 		go func() {
 			if err := http.ListenAndServe(*metricsAddr, promhttp.Handler()); err != nil {
-				level.Warn(coordinator.logger).Log("msg", "ListenAndServe", "err", err)
+				coordinator.logger.Warn("ListenAndServe", "err", err)
 			}
 		}()
 	}

cmd/client/main_test.go

@@ -19,21 +19,16 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
-)
-
-type TestLogger struct{}
 
-func (tl *TestLogger) Log(vars ...interface{}) error {
-	fmt.Printf("%+v\n", vars)
-	return nil
-}
+	"github.com/prometheus/common/promslog"
+)
 
 func prepareTest() (*httptest.Server, Coordinator) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		fmt.Fprintln(w, "GET /index.html HTTP/1.0\n\nOK")
 	}))
-	c := Coordinator{logger: &TestLogger{}}
+	c := Coordinator{logger: promslog.NewNopLogger()}
 	*proxyURL = ts.URL
 	return ts, c
 }

cmd/proxy/coordinator.go

@@ -16,13 +16,12 @@ package main
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"sync"
 	"time"
 
 	"github.com/alecthomas/kingpin/v2"
-	"github.com/go-kit/log"
-	"github.com/go-kit/log/level"
 	"github.com/google/uuid"
 	"github.com/prometheus-community/pushprox/util"
 	"github.com/prometheus/client_golang/prometheus"
@@ -55,11 +54,11 @@ type Coordinator struct {
 	// Clients we know about and when they last contacted us.
 	known map[string]time.Time
 
-	logger log.Logger
+	logger *slog.Logger
 }
 
 // NewCoordinator initiates the coordinator and starts the client cleanup routine
-func NewCoordinator(logger log.Logger) (*Coordinator, error) {
+func NewCoordinator(logger *slog.Logger) (*Coordinator, error) {
 	c := &Coordinator{
 		waiting:   map[string]chan *http.Request{},
 		responses: map[string]chan *http.Response{},
@@ -112,7 +111,7 @@ func (c *Coordinator) DoScrape(ctx context.Context, r *http.Request) (*http.Resp
 	if err != nil {
 		return nil, err
 	}
-	level.Info(c.logger).Log("msg", "DoScrape", "scrape_id", id, "url", r.URL.String())
+	c.logger.Info("DoScrape", "scrape_id", id, "url", r.URL.String())
 	r.Header.Add("Id", id)
 	select {
 	case <-ctx.Done():
@@ -133,7 +132,7 @@ func (c *Coordinator) DoScrape(ctx context.Context, r *http.Request) (*http.Resp
 
 // WaitForScrapeInstruction registers a client waiting for a scrape result
 func (c *Coordinator) WaitForScrapeInstruction(fqdn string) (*http.Request, error) {
-	level.Info(c.logger).Log("msg", "WaitForScrapeInstruction", "fqdn", fqdn)
+	c.logger.Info("WaitForScrapeInstruction", "fqdn", fqdn)
 
 	c.addKnownClient(fqdn)
 	// TODO: What if the client times out?
@@ -165,7 +164,7 @@ func (c *Coordinator) WaitForScrapeInstruction(fqdn string) (*http.Request, erro
 // ScrapeResult send by client
 func (c *Coordinator) ScrapeResult(r *http.Response) error {
 	id := r.Header.Get("Id")
-	level.Info(c.logger).Log("msg", "ScrapeResult", "scrape_id", id)
+	c.logger.Info("ScrapeResult", "scrape_id", id)
 	ctx, cancel := context.WithTimeout(context.Background(), util.GetScrapeTimeout(maxScrapeTimeout, defaultScrapeTimeout, r.Header))
 	defer cancel()
 	// Don't expose internal headers.
@@ -217,7 +216,7 @@ func (c *Coordinator) gc() {
 					deleted++
 				}
 			}
-			level.Info(c.logger).Log("msg", "GC of clients completed", "deleted", deleted, "remaining", len(c.known))
+			c.logger.Info("GC of clients completed", "deleted", deleted, "remaining", len(c.known))
 			knownClients.Set(float64(len(c.known)))
 		}()
 	}