chore: replace std log with zap

With use zap in most of our projects already, so there is no reason of doing things differently here. Signed-off-by: Dmitriy Matrenichev <[email protected]>
siderolabs · May 22, 2023 · b23a173 · b23a173
1 parent 1a2f374
commit b23a173
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 29 deletions.
diff --git a/controlplane/controlplane.go b/controlplane/controlplane.go
@@ -7,12 +7,12 @@ package controlplane
 
 import (
 	"fmt"
-	"io"
-	"log"
 	"net"
 	"strconv"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/siderolabs/go-loadbalancer/loadbalancer"
 	"github.com/siderolabs/go-loadbalancer/upstream"
 )
@@ -61,7 +61,7 @@ func WithHealthCheckOptions(options ...upstream.ListOption) LoadBalancerOption {
 // NewLoadBalancer initializes the load balancer.
 //
 // If bindPort is zero, load balancer will bind to a random available port.
-func NewLoadBalancer(bindAddress string, bindPort int, logWriter io.Writer, options ...LoadBalancerOption) (*LoadBalancer, error) {
+func NewLoadBalancer(bindAddress string, bindPort int, logger *zap.Logger, options ...LoadBalancerOption) (*LoadBalancer, error) {
 	if bindPort == 0 {
 		var err error
 
@@ -84,7 +84,11 @@ func NewLoadBalancer(bindAddress string, bindPort int, logWriter io.Writer, opti
 		option(lb)
 	}
 
-	lb.lb.Logger = log.New(logWriter, lb.endpoint+" ", log.Default().Flags())
+	if logger == nil {
+		logger = zap.Must(zap.NewProduction())
+	}
+
+	lb.lb.Logger = logger.Named("controlplane-lb").With(zap.String("endpoint", lb.endpoint))
 
 	// create a route without any upstreams yet
 	if err := lb.lb.AddRoute(lb.endpoint, nil, lb.healthCheckOptions...); err != nil {
@@ -114,7 +118,10 @@ func (lb *LoadBalancer) Start(upstreamCh <-chan []string) error {
 			select {
 			case upstreams := <-upstreamCh:
 				if err := lb.lb.ReconcileRoute(lb.endpoint, upstreams); err != nil {
-					lb.lb.Logger.Printf("failed reconciling list of upstreams: %s", err)
+					lb.lb.Logger.Info("failed reconciling list of upstreams",
+						zap.Strings("upstreams", upstreams),
+						zap.Error(err),
+					)
 				}
 			case <-lb.done:
 				return

diff --git a/controlplane/controlplane_test.go b/controlplane/controlplane_test.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"io"
 	"net"
-	"os"
 	"strconv"
 	"testing"
 	"time"
@@ -17,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
+	"go.uber.org/zap/zaptest"
 
 	"github.com/siderolabs/go-loadbalancer/controlplane"
 )
@@ -78,7 +78,7 @@ func TestLoadBalancer(t *testing.T) {
 		upstreamAddrs[i] = upstreams[i].addr
 	}
 
-	lb, err := controlplane.NewLoadBalancer("localhost", 0, os.Stderr)
+	lb, err := controlplane.NewLoadBalancer("localhost", 0, zaptest.NewLogger(t))
 	require.NoError(t, err)
 
 	upstreamCh := make(chan []string)

diff --git a/go.mod b/go.mod
@@ -6,13 +6,17 @@ require (
 	github.com/siderolabs/gen v0.4.5
 	github.com/siderolabs/go-retry v0.3.2
 	github.com/siderolabs/tcpproxy v0.1.0
-	github.com/stretchr/testify v1.8.2
+	github.com/stretchr/testify v1.8.3
 	go.uber.org/goleak v1.2.1
+	go.uber.org/zap v1.24.0
 )
 
 require (
+	github.com/benbjohnson/clock v1.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	go.uber.org/atomic v1.7.0 // indirect
+	go.uber.org/multierr v1.6.0 // indirect
 	golang.org/x/sys v0.8.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

diff --git a/go.sum b/go.sum
@@ -1,4 +1,6 @@
 github.com/armon/go-proxyproto v0.0.0-20210323213023-7e956b284f0a/go.mod h1:QmP9hvJ91BbJmGVGSbutW19IC0Q9phDCLGaomwTJbgU=
+github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
+github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 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=
@@ -7,6 +9,7 @@ github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfn
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/siderolabs/gen v0.4.5 h1:rwXUVJlL7hYza1LrSVXfT905ZC9Rgei37jMKKs/+eP0=
@@ -16,19 +19,21 @@ github.com/siderolabs/go-retry v0.3.2/go.mod h1:Ac8HIh0nAYDQm04FGZHNofVAXteyd4xR
 github.com/siderolabs/tcpproxy v0.1.0 h1:IbkS9vRhjMOscc1US3M5P1RnsGKFgB6U5IzUk+4WkKA=
 github.com/siderolabs/tcpproxy v0.1.0/go.mod h1:onn6CPPj/w1UNqQ0U97oRPF0CqbrgEApYCw4P9IiCW8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
-github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
-github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
-github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
-github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
+github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY=
+github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
+go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A=
 go.uber.org/goleak v1.2.1/go.mod h1:qlT2yGI9QafXHhZZLxlSuNsMw3FFLxBr+tBRlmO1xH4=
+go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
+go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
+go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60=
+go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg=
 golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 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.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/loadbalancer/loadbalancer.go b/loadbalancer/loadbalancer.go
@@ -8,11 +8,11 @@ package loadbalancer
 import (
 	"errors"
 	"fmt"
-	"log"
 	"time"
 
 	"github.com/siderolabs/gen/slices"
 	"github.com/siderolabs/tcpproxy"
+	"go.uber.org/zap"
 
 	"github.com/siderolabs/go-loadbalancer/upstream"
 )
@@ -28,7 +28,7 @@ import (
 type TCP struct {
 	tcpproxy.Proxy
 
-	Logger *log.Logger
+	Logger *zap.Logger
 
 	routes map[string]*upstream.List[node]
 
@@ -64,7 +64,7 @@ func (t *TCP) IsRouteHealthy(ipPort string) (bool, error) {
 // AddRoute should be called before Start().
 func (t *TCP) AddRoute(ipPort string, upstreamAddrs []string, options ...upstream.ListOption) error {
 	if t.Logger == nil {
-		t.Logger = log.New(log.Writer(), "", log.Flags())
+		t.Logger = zap.Must(zap.NewProduction())
 	}
 
 	if t.routes == nil {

diff --git a/loadbalancer/loadbalancer_test.go b/loadbalancer/loadbalancer_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/stretchr/testify/suite"
 	"go.uber.org/goleak"
+	"go.uber.org/zap/zaptest"
 
 	"github.com/siderolabs/go-loadbalancer/loadbalancer"
 	"github.com/siderolabs/go-loadbalancer/upstream"
@@ -93,7 +94,7 @@ func (suite *TCPSuite) TestReconcile() {
 	listenAddr, err := findListenAddress()
 	suite.Require().NoError(err)
 
-	lb := &loadbalancer.TCP{}
+	lb := &loadbalancer.TCP{Logger: zaptest.NewLogger(suite.T())}
 	suite.Require().NoError(lb.AddRoute(
 		listenAddr,
 		upstreamAddrs[:pivot],
@@ -192,7 +193,7 @@ func (suite *TCPSuite) TestBalancer() {
 	listenAddr, err := findListenAddress()
 	suite.Require().NoError(err)
 
-	lb := &loadbalancer.TCP{}
+	lb := &loadbalancer.TCP{Logger: zaptest.NewLogger(suite.T())}
 	suite.Require().NoError(lb.AddRoute(
 		listenAddr,
 		upstreamAddrs,

diff --git a/loadbalancer/node.go b/loadbalancer/node.go
@@ -6,14 +6,15 @@ package loadbalancer
 
 import (
 	"context"
-	"log"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/siderolabs/go-loadbalancer/upstream"
 )
 
 type node struct {
-	logger  *log.Logger
+	logger  *zap.Logger
 	address string // host:port
 }
 
@@ -30,7 +31,7 @@ func (upstream node) healthCheck(ctx context.Context) error {
 
 	c, err := d.DialContext(ctx, "tcp", upstream.address)
 	if err != nil {
-		upstream.logger.Printf("healthcheck failed for %q: %s", upstream.address, err)
+		upstream.logger.Info("healthcheck failed", zap.String("address", upstream.address), zap.Error(err))
 
 		return err
 	}

diff --git a/loadbalancer/target.go b/loadbalancer/target.go
@@ -5,18 +5,18 @@
 package loadbalancer
 
 import (
-	"log"
 	"net"
 	"time"
 
 	"github.com/siderolabs/tcpproxy"
+	"go.uber.org/zap"
 
 	"github.com/siderolabs/go-loadbalancer/upstream"
 )
 
 type lbTarget struct {
 	list            *upstream.List[node]
-	logger          *log.Logger
+	logger          *zap.Logger
 	dialTimeout     time.Duration
 	keepAlivePeriod time.Duration
 	tcpUserTimeout  time.Duration
@@ -25,13 +25,20 @@ type lbTarget struct {
 func (target *lbTarget) HandleConn(conn net.Conn) {
 	upstreamBackend, err := target.list.Pick()
 	if err != nil {
-		target.logger.Printf("no upstreams available, closing connection from %s", conn.RemoteAddr())
+		target.logger.Info(
+			"no upstreams available, closing connection",
+			zap.String("remote_addr", conn.RemoteAddr().String()),
+		)
 		conn.Close() //nolint: errcheck
 
 		return
 	}
 
-	target.logger.Printf("proxying connection %s -> %s", conn.RemoteAddr(), upstreamBackend.address)
+	target.logger.Debug(
+		"proxying connection",
+		zap.String("remote_addr", conn.RemoteAddr().String()),
+		zap.String("upstream_addr", upstreamBackend.address),
+	)
 
 	upstreamTarget := tcpproxy.To(upstreamBackend.address)
 	upstreamTarget.DialTimeout = target.dialTimeout
@@ -40,12 +47,20 @@ func (target *lbTarget) HandleConn(conn net.Conn) {
 	upstreamTarget.OnDialError = func(src net.Conn, dstDialErr error) {
 		src.Close() //nolint: errcheck
 
-		target.logger.Printf("error dialing upstream %s: %s", upstreamBackend.address, dstDialErr)
+		target.logger.Info(
+			"error dialing upstream",
+			zap.String("upstream_addr", upstreamBackend.address),
+			zap.Error(dstDialErr),
+		)
 
 		target.list.Down(upstreamBackend)
 	}
 
 	upstreamTarget.HandleConn(conn)
 
-	target.logger.Printf("closing connection %s -> %s", conn.RemoteAddr(), upstreamBackend.address)
+	target.logger.Debug(
+		"closing connection",
+		zap.String("remote_addr", conn.RemoteAddr().String()),
+		zap.String("upstream_addr", upstreamBackend.address),
+	)
 }