From d9a13667fc26df826d3874184e3509b71cb734a0 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Tue, 18 Dec 2018 17:13:32 +0200 Subject: [PATCH] Use fixed logger for Kademlia and make not being able to connect in discovery debug statement. (#899) --- internal/testplanet/node.go | 2 +- pkg/kademlia/config.go | 8 +++++--- pkg/kademlia/kademlia.go | 16 +++++++++------- pkg/kademlia/kademlia_test.go | 13 +++++++------ pkg/kademlia/peer_discovery.go | 12 +++++------- 5 files changed, 27 insertions(+), 24 deletions(-) diff --git a/internal/testplanet/node.go b/internal/testplanet/node.go index e72006300..de7c0f851 100644 --- a/internal/testplanet/node.go +++ b/internal/testplanet/node.go @@ -171,7 +171,7 @@ func (node *Node) initOverlay(planet *Planet) error { return err } - kad, err := kademlia.NewKademliaWithRoutingTable(node.Info, planet.nodeInfos, node.Identity, 5, routing) + kad, err := kademlia.NewKademliaWithRoutingTable(node.Log.Named("kademlia"), node.Info, planet.nodeInfos, node.Identity, 5, routing) if err != nil { return utils.CombineErrors(err, routing.Close()) } diff --git a/pkg/kademlia/config.go b/pkg/kademlia/config.go index 8d33b55ca..33940d4a8 100644 --- a/pkg/kademlia/config.go +++ b/pkg/kademlia/config.go @@ -75,17 +75,19 @@ func (c Config) Run(ctx context.Context, server *provider.Provider) ( addr = c.ExternalAddress } - kad, err := NewKademlia(server.Identity().ID, nodeType, []pb.Node{*in}, addr, metadata, server.Identity(), c.DBPath, c.Alpha) + logger := zap.L() + + kad, err := NewKademlia(logger, server.Identity().ID, nodeType, []pb.Node{*in}, addr, metadata, server.Identity(), c.DBPath, c.Alpha) if err != nil { return err } defer func() { err = utils.CombineErrors(err, kad.Disconnect()) }() - pb.RegisterNodesServer(server.GRPC(), node.NewServer(zap.L(), kad)) + pb.RegisterNodesServer(server.GRPC(), node.NewServer(logger, kad)) go func() { if err = kad.Bootstrap(ctx); err != nil { - zap.L().Error("Failed to bootstrap Kademlia", zap.String("ID", server.Identity().ID.String())) + logger.Error("Failed to bootstrap Kademlia", zap.Any("ID", server.Identity().ID)) } }() diff --git a/pkg/kademlia/kademlia.go b/pkg/kademlia/kademlia.go index 908cdaba8..ababaefaf 100644 --- a/pkg/kademlia/kademlia.go +++ b/pkg/kademlia/kademlia.go @@ -48,6 +48,7 @@ type discoveryOptions struct { // Kademlia is an implementation of kademlia adhering to the DHT interface. type Kademlia struct { + log *zap.Logger alpha int // alpha is a system wide concurrency parameter routingTable *RoutingTable bootstrapNodes []pb.Node @@ -58,7 +59,7 @@ type Kademlia struct { } // NewKademlia returns a newly configured Kademlia instance -func NewKademlia(id storj.NodeID, nodeType pb.NodeType, bootstrapNodes []pb.Node, address string, metadata *pb.NodeMetadata, identity *provider.FullIdentity, path string, alpha int) (*Kademlia, error) { +func NewKademlia(log *zap.Logger, id storj.NodeID, nodeType pb.NodeType, bootstrapNodes []pb.Node, address string, metadata *pb.NodeMetadata, identity *provider.FullIdentity, path string, alpha int) (*Kademlia, error) { self := pb.Node{ Id: id, Type: nodeType, @@ -86,12 +87,13 @@ func NewKademlia(id storj.NodeID, nodeType pb.NodeType, bootstrapNodes []pb.Node return nil, BootstrapErr.Wrap(err) } - return NewKademliaWithRoutingTable(self, bootstrapNodes, identity, alpha, rt) + return NewKademliaWithRoutingTable(log, self, bootstrapNodes, identity, alpha, rt) } // NewKademliaWithRoutingTable returns a newly configured Kademlia instance -func NewKademliaWithRoutingTable(self pb.Node, bootstrapNodes []pb.Node, identity *provider.FullIdentity, alpha int, rt *RoutingTable) (*Kademlia, error) { +func NewKademliaWithRoutingTable(log *zap.Logger, self pb.Node, bootstrapNodes []pb.Node, identity *provider.FullIdentity, alpha int, rt *RoutingTable) (*Kademlia, error) { k := &Kademlia{ + log: log, alpha: alpha, routingTable: rt, bootstrapNodes: bootstrapNodes, @@ -203,11 +205,11 @@ func (k *Kademlia) lookup(ctx context.Context, target storj.NodeID, opts discove } } - lookup := newPeerDiscovery(nodes, k.nodeClient, target, opts) + lookup := newPeerDiscovery(k.log, nodes, k.nodeClient, target, opts) _, err = lookup.Run(ctx) if err != nil { - zap.L().Warn("lookup failed", zap.Error(err)) + k.log.Warn("lookup failed", zap.Error(err)) return err } @@ -237,7 +239,7 @@ func (k *Kademlia) FindNode(ctx context.Context, ID storj.NodeID) (pb.Node, erro return pb.Node{}, err } - lookup := newPeerDiscovery(nodes, k.nodeClient, ID, discoveryOptions{ + lookup := newPeerDiscovery(k.log, nodes, k.nodeClient, ID, discoveryOptions{ concurrency: k.alpha, retries: defaultRetries, bootstrap: false, bootstrapNodes: k.bootstrapNodes, }) @@ -259,7 +261,7 @@ func (k *Kademlia) ListenAndServe() error { } grpcServer := grpc.NewServer(identOpt) - mn := node.NewServer(zap.L(), k) + mn := node.NewServer(k.log, k) pb.RegisterNodesServer(grpcServer, mn) lis, err := net.Listen("tcp", k.address) diff --git a/pkg/kademlia/kademlia_test.go b/pkg/kademlia/kademlia_test.go index c0300c375..0e4f178ab 100644 --- a/pkg/kademlia/kademlia_test.go +++ b/pkg/kademlia/kademlia_test.go @@ -16,7 +16,7 @@ import ( "testing" "github.com/stretchr/testify/assert" - "go.uber.org/zap" + "go.uber.org/zap/zaptest" "google.golang.org/grpc" testidentity "storj.io/storj/internal/identity" @@ -68,7 +68,7 @@ func TestNewKademlia(t *testing.T) { identity, err := ca.NewIdentity() assert.NoError(t, err) - kad, err := NewKademlia(v.id, pb.NodeType_STORAGE, v.bn, v.addr, nil, identity, dir, defaultAlpha) + kad, err := NewKademlia(zaptest.NewLogger(t), v.id, pb.NodeType_STORAGE, v.bn, v.addr, nil, identity, dir, defaultAlpha) assert.NoError(t, err) assert.Equal(t, v.expectedErr, err) assert.Equal(t, kad.bootstrapNodes, v.bn) @@ -95,7 +95,7 @@ func TestPeerDiscovery(t *testing.T) { Email: "foo@bar.com", Wallet: "FarmerWallet", } - k, err := NewKademlia(testID.ID, pb.NodeType_STORAGE, bootstrapNodes, testAddress, metadata, testID, dir, defaultAlpha) + k, err := NewKademlia(zaptest.NewLogger(t), testID.ID, pb.NodeType_STORAGE, bootstrapNodes, testAddress, metadata, testID, dir, defaultAlpha) assert.NoError(t, err) rt, err := k.GetRoutingTable(context.Background()) assert.NoError(t, err) @@ -169,9 +169,10 @@ func testNode(t *testing.T, bn []pb.Node) (*Kademlia, *grpc.Server, func()) { // new kademlia dir, cleanup := mktempdir(t, "kademlia") - k, err := NewKademlia(fid.ID, pb.NodeType_STORAGE, bn, lis.Addr().String(), nil, fid, dir, defaultAlpha) + logger := zaptest.NewLogger(t) + k, err := NewKademlia(logger, fid.ID, pb.NodeType_STORAGE, bn, lis.Addr().String(), nil, fid, dir, defaultAlpha) assert.NoError(t, err) - s := node.NewServer(zap.L(), k) + s := node.NewServer(logger, k) // new ident opts identOpt, err := fid.ServerOption() assert.NoError(t, err) @@ -215,7 +216,7 @@ func TestGetNodes(t *testing.T) { assert.NotEqual(t, fid.ID, fid2.ID) dir, cleanup := mktempdir(t, "kademlia") defer cleanup() - k, err := NewKademlia(fid.ID, pb.NodeType_STORAGE, []pb.Node{{Id: fid2.ID, Address: &pb.NodeAddress{Address: lis.Addr().String()}}}, lis.Addr().String(), nil, fid, dir, defaultAlpha) + k, err := NewKademlia(zaptest.NewLogger(t), fid.ID, pb.NodeType_STORAGE, []pb.Node{{Id: fid2.ID, Address: &pb.NodeAddress{Address: lis.Addr().String()}}}, lis.Addr().String(), nil, fid, dir, defaultAlpha) assert.NoError(t, err) defer func() { assert.NoError(t, k.Disconnect()) diff --git a/pkg/kademlia/peer_discovery.go b/pkg/kademlia/peer_discovery.go index f4ae75f09..044bdae9b 100644 --- a/pkg/kademlia/peer_discovery.go +++ b/pkg/kademlia/peer_discovery.go @@ -17,6 +17,8 @@ import ( ) type peerDiscovery struct { + log *zap.Logger + client node.Client target storj.NodeID opts discoveryOptions @@ -28,8 +30,9 @@ type peerDiscovery struct { // ErrMaxRetries is used when a lookup has been retried the max number of times var ErrMaxRetries = errs.Class("max retries exceeded for id:") -func newPeerDiscovery(nodes []*pb.Node, client node.Client, target storj.NodeID, opts discoveryOptions) *peerDiscovery { +func newPeerDiscovery(log *zap.Logger, nodes []*pb.Node, client node.Client, target storj.NodeID, opts discoveryOptions) *peerDiscovery { discovery := &peerDiscovery{ + log: log, client: client, target: target, opts: opts, @@ -94,12 +97,7 @@ func (lookup *peerDiscovery) Run(ctx context.Context) (target *pb.Node, err erro // ok := lookup.queue.Reinsert(lookup.target, next, lookup.opts.retries) ok := false if !ok { - zap.S().Errorf( - "Error occurred during lookup of %s :: %s :: error = %s", - lookup.target, - ErrMaxRetries.New("%s", next.Id), - err.Error(), - ) + lookup.log.Debug("connecting to node failed", zap.Any("target", lookup.target), zap.Any("dial", next.Id), zap.Error(err)) } }