From e8ba8feaedd9022c040966d34562a539b5d5301d Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Fri, 27 Apr 2018 11:40:13 -0700 Subject: [PATCH] pkg/netutil: use structured logging for TCP resolve Signed-off-by: Gyuho Lee --- pkg/netutil/netutil.go | 63 ++++++++++++++++++++++++++----------- pkg/netutil/netutil_test.go | 10 +++--- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/pkg/netutil/netutil.go b/pkg/netutil/netutil.go index e3db8c50a..c2da933e0 100644 --- a/pkg/netutil/netutil.go +++ b/pkg/netutil/netutil.go @@ -25,15 +25,12 @@ import ( "time" "github.com/coreos/etcd/pkg/types" - "github.com/coreos/pkg/capnslog" + + "go.uber.org/zap" ) -var ( - plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "pkg/netutil") - - // indirection for testing - resolveTCPAddr = resolveTCPAddrDefault -) +// indirection for testing +var resolveTCPAddr = resolveTCPAddrDefault const retryInterval = time.Second @@ -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 { diff --git a/pkg/netutil/netutil_test.go b/pkg/netutil/netutil_test.go index c1f9c5e50..42b05ca29 100644 --- a/pkg/netutil/netutil_test.go +++ b/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) }