pkg/netutil: use structured logging for TCP resolve

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

pkg/netutil/netutil.go

@@ -25,16 +25,13 @@ import (
 	"time"
 
 	"github.com/coreos/etcd/pkg/types"
-	"github.com/coreos/pkg/capnslog"
-)
-
-var (
-	plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "pkg/netutil")
 
-	// indirection for testing
-	resolveTCPAddr = resolveTCPAddrDefault
+	"go.uber.org/zap"
 )
 
+// indirection for testing
+var resolveTCPAddr = resolveTCPAddrDefault
+
 const retryInterval = time.Second
 
 // taken from go's ResolveTCP code but uses configurable ctx
@@ -67,7 +64,7 @@ func resolveTCPAddrDefault(ctx context.Context, addr string) (*net.TCPAddr, erro
 // resolveTCPAddrs is a convenience wrapper for net.ResolveTCPAddr.
 // resolveTCPAddrs return a new set of url.URLs, in which all DNS hostnames
 // are resolved.
-func resolveTCPAddrs(ctx context.Context, urls [][]url.URL) ([][]url.URL, error) {
+func resolveTCPAddrs(ctx context.Context, lg *zap.Logger, urls [][]url.URL) ([][]url.URL, error) {
 	newurls := make([][]url.URL, 0)
 	for _, us := range urls {
 		nus := make([]url.URL, len(us))
@@ -79,7 +76,7 @@ func resolveTCPAddrs(ctx context.Context, urls [][]url.URL) ([][]url.URL, error)
 			nus[i] = *nu
 		}
 		for i, u := range nus {
-			h, err := resolveURL(ctx, u)
+			h, err := resolveURL(ctx, lg, u)
 			if err != nil {
 				return nil, fmt.Errorf("failed to resolve %q (%v)", u.String(), err)
 			}
@@ -92,14 +89,19 @@ func resolveTCPAddrs(ctx context.Context, urls [][]url.URL) ([][]url.URL, error)
 	return newurls, nil
 }
 
-func resolveURL(ctx context.Context, u url.URL) (string, error) {
+func resolveURL(ctx context.Context, lg *zap.Logger, u url.URL) (string, error) {
 	if u.Scheme == "unix" || u.Scheme == "unixs" {
 		// unix sockets don't resolve over TCP
 		return "", nil
 	}
 	host, _, err := net.SplitHostPort(u.Host)
 	if err != nil {
-		plog.Errorf("could not parse url %s during tcp resolving", u.Host)
+		lg.Warn(
+			"failed to parse URL Host while resolving URL",
+			zap.String("url", u.String()),
+			zap.String("host", u.Host),
+			zap.Error(err),
+		)
 		return "", err
 	}
 	if host == "localhost" || net.ParseIP(host) != nil {
@@ -108,13 +110,32 @@ func resolveURL(ctx context.Context, u url.URL) (string, error) {
 	for ctx.Err() == nil {
 		tcpAddr, err := resolveTCPAddr(ctx, u.Host)
 		if err == nil {
-			plog.Infof("resolving %s to %s", u.Host, tcpAddr.String())
+			lg.Info(
+				"resolved URL Host",
+				zap.String("url", u.String()),
+				zap.String("host", u.Host),
+				zap.String("resolved-addr", tcpAddr.String()),
+			)
 			return tcpAddr.String(), nil
 		}
-		plog.Warningf("failed resolving host %s (%v); retrying in %v", u.Host, err, retryInterval)
+
+		lg.Warn(
+			"failed to resolve URL Host",
+			zap.String("url", u.String()),
+			zap.String("host", u.Host),
+			zap.Duration("retry-interval", retryInterval),
+			zap.Error(err),
+		)
+
 		select {
 		case <-ctx.Done():
-			plog.Errorf("could not resolve host %s", u.Host)
+			lg.Warn(
+				"failed to resolve URL Host; returning",
+				zap.String("url", u.String()),
+				zap.String("host", u.Host),
+				zap.Duration("retry-interval", retryInterval),
+				zap.Error(err),
+			)
 			return "", err
 		case <-time.After(retryInterval):
 		}
@@ -124,11 +145,11 @@ func resolveURL(ctx context.Context, u url.URL) (string, error) {
 
 // urlsEqual checks equality of url.URLS between two arrays.
 // This check pass even if an URL is in hostname and opposite is in IP address.
-func urlsEqual(ctx context.Context, a []url.URL, b []url.URL) (bool, error) {
+func urlsEqual(ctx context.Context, lg *zap.Logger, a []url.URL, b []url.URL) (bool, error) {
 	if len(a) != len(b) {
 		return false, fmt.Errorf("len(%q) != len(%q)", urlsToStrings(a), urlsToStrings(b))
 	}
-	urls, err := resolveTCPAddrs(ctx, [][]url.URL{a, b})
+	urls, err := resolveTCPAddrs(ctx, lg, [][]url.URL{a, b})
 	if err != nil {
 		return false, err
 	}
@@ -150,7 +171,7 @@ func urlsEqual(ctx context.Context, a []url.URL, b []url.URL) (bool, error) {
 // URLStringsEqual returns "true" if given URLs are valid
 // and resolved to same IP addresses. Otherwise, return "false"
 // and error, if any.
-func URLStringsEqual(ctx context.Context, a []string, b []string) (bool, error) {
+func URLStringsEqual(ctx context.Context, lg *zap.Logger, a []string, b []string) (bool, error) {
 	if len(a) != len(b) {
 		return false, fmt.Errorf("len(%q) != len(%q)", a, b)
 	}
@@ -170,7 +191,13 @@ func URLStringsEqual(ctx context.Context, a []string, b []string) (bool, error)
 		}
 		urlsB = append(urlsB, *u)
 	}
-	return urlsEqual(ctx, urlsA, urlsB)
+	if lg == nil {
+		lg, _ = zap.NewProduction()
+		if lg == nil {
+			lg = zap.NewExample()
+		}
+	}
+	return urlsEqual(ctx, lg, urlsA, urlsB)
 }
 
 func urlsToStrings(us []url.URL) []string {

pkg/netutil/netutil_test.go

@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"testing"
 	"time"
+
+	"go.uber.org/zap"
 )
 
 func TestResolveTCPAddrs(t *testing.T) {
@@ -118,7 +120,7 @@ func TestResolveTCPAddrs(t *testing.T) {
 				return nil, err
 			}
 			if tt.hostMap[host] == "" {
-				return nil, errors.New("cannot resolve host.")
+				return nil, errors.New("cannot resolve host")
 			}
 			i, err := strconv.Atoi(port)
 			if err != nil {
@@ -127,7 +129,7 @@ func TestResolveTCPAddrs(t *testing.T) {
 			return &net.TCPAddr{IP: net.ParseIP(tt.hostMap[host]), Port: i, Zone: ""}, nil
 		}
 		ctx, cancel := context.WithTimeout(context.TODO(), time.Second)
-		urls, err := resolveTCPAddrs(ctx, tt.urls)
+		urls, err := resolveTCPAddrs(ctx, zap.NewExample(), tt.urls)
 		cancel()
 		if tt.hasError {
 			if err == nil {
@@ -278,7 +280,7 @@ func TestURLsEqual(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		result, err := urlsEqual(context.TODO(), test.a, test.b)
+		result, err := urlsEqual(context.TODO(), zap.NewExample(), test.a, test.b)
 		if result != test.expect {
 			t.Errorf("#%d: a:%v b:%v, expected %v but %v", i, test.a, test.b, test.expect, result)
 		}
@@ -290,7 +292,7 @@ func TestURLsEqual(t *testing.T) {
 	}
 }
 func TestURLStringsEqual(t *testing.T) {
-	result, err := URLStringsEqual(context.TODO(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"})
+	result, err := URLStringsEqual(context.TODO(), zap.NewExample(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"})
 	if !result {
 		t.Errorf("unexpected result %v", result)
 	}