Skip to content

Commit

Permalink
[FAB-13029] stringers avoid logging slice values
Browse files Browse the repository at this point in the history
Gossip currently logs peer identifiers and metadata at the info level
using the go fmt "value" format which writes byte slices as a comma
separated list of decimal values. These values would be easier to read
and parse if written in hex.

Change-Id: Id6703a7403c535e97cc4c25f17dfc320bbe5ec40
Signed-off-by: Matthew Sykes <[email protected]>
  • Loading branch information
sykesm committed Nov 28, 2018
1 parent 4cf2ff5 commit f8f5eac
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 4 deletions.
12 changes: 11 additions & 1 deletion gossip/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ SPDX-License-Identifier: Apache-2.0

package common

import "bytes"
import (
"bytes"
"encoding/hex"
)

func init() {
// This is just to satisfy the code coverage tool
Expand All @@ -20,6 +23,13 @@ func init() {
// which is the security identifier of a peer
type PKIidType []byte

func (p PKIidType) String() string {
if p == nil {
return "<nil>"
}
return hex.EncodeToString(p)
}

// IsNotSameFilter generate filter function which
// provides a predicate to identify whenever current id
// equals to another one.
Expand Down
14 changes: 14 additions & 0 deletions gossip/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,17 @@ func TestIsNotSame(t *testing.T) {
assert.False(t, id.IsNotSameFilter(PKIidType("1")))
assert.False(t, id.IsNotSameFilter(id))
}

func TestPKIidTypeStringer(t *testing.T) {
tests := []struct {
input PKIidType
expected string
}{
{nil, "<nil>"},
{PKIidType{}, ""},
{PKIidType{0, 1, 2, 3}, "00010203"},
}
for _, tt := range tests {
assert.Equal(t, tt.expected, tt.input.String())
}
}
4 changes: 2 additions & 2 deletions gossip/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ type NetworkMember struct {
}

// String returns a string representation of the NetworkMember
func (n *NetworkMember) String() string {
return fmt.Sprintf("Endpoint: %s, InternalEndpoint: %s, PKI-ID: %v, Metadata: %v", n.Endpoint, n.InternalEndpoint, n.PKIid, n.Metadata)
func (n NetworkMember) String() string {
return fmt.Sprintf("Endpoint: %s, InternalEndpoint: %s, PKI-ID: %s, Metadata: %x", n.Endpoint, n.InternalEndpoint, n.PKIid, n.Metadata)
}

// PreferredEndpoint computes the endpoint to connect to,
Expand Down
19 changes: 19 additions & 0 deletions gossip/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,25 @@ func TestToString(t *testing.T) {
assert.Equal(t, fmt.Sprintf("%d, %d", now.UnixNano(), 42), fmt.Sprint(ts))
}

func TestNetworkMemberString(t *testing.T) {
tests := []struct {
input NetworkMember
expected string
}{
{
input: NetworkMember{Endpoint: "endpoint", InternalEndpoint: "internal-endpoint", PKIid: common.PKIidType{0, 1, 2, 3}, Metadata: nil},
expected: "Endpoint: endpoint, InternalEndpoint: internal-endpoint, PKI-ID: 00010203, Metadata: ",
},
{
input: NetworkMember{Endpoint: "endpoint", InternalEndpoint: "internal-endpoint", PKIid: common.PKIidType{0, 1, 2, 3}, Metadata: []byte{4, 5, 6, 7}},
expected: "Endpoint: endpoint, InternalEndpoint: internal-endpoint, PKI-ID: 00010203, Metadata: 04050607",
},
}
for _, tt := range tests {
assert.Equal(t, tt.expected, tt.input.String())
}
}

func TestBadInput(t *testing.T) {
inst := createDiscoveryInstance(2048, fmt.Sprintf("d%d", 0), []string{})
inst.Discovery.(*gossipDiscoveryImpl).handleMsgFromComm(nil)
Expand Down
8 changes: 8 additions & 0 deletions gossip/election/election.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package election

import (
"bytes"
"encoding/hex"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -100,6 +101,13 @@ type LeaderElectionService interface {

type peerID []byte

func (p peerID) String() string {
if p == nil {
return "<nil>"
}
return hex.EncodeToString(p)
}

// Peer describes a remote peer
type Peer interface {
// ID returns the ID of the peer
Expand Down
13 changes: 13 additions & 0 deletions gossip/election/election_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,20 @@ func TestPartition(t *testing.T) {
waitForBoolFunc(t, p.isCallbackInvoked, true, "Leadership callback wasn't invoked for %s", p.id)
}
}
}

func Test_peerIDString(t *testing.T) {
tests := []struct {
input peerID
expected string
}{
{nil, "<nil>"},
{peerID{}, ""},
{peerID{0, 1, 2, 3}, "00010203"},
}
for _, tt := range tests {
assert.Equal(t, tt.expected, tt.input.String())
}
}

func TestConfigFromFile(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion gossip/gossip/gossip_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func NewGossipService(conf *Config, s *grpc.Server, sa api.SecurityAdvisor,
g.discAdapter = g.newDiscoveryAdapter()
g.disSecAdap = g.newDiscoverySecurityAdapter()
g.disc = discovery.NewDiscoveryService(g.selfNetworkMember(), g.discAdapter, g.disSecAdap, g.disclosurePolicy)
g.logger.Info("Creating gossip service with self membership of", g.selfNetworkMember())
g.logger.Infof("Creating gossip service with self membership of %s", g.selfNetworkMember())

g.certPuller = g.createCertStorePuller()
g.certStore = newCertStore(g.certPuller, g.idMapper, selfIdentity, mcs)
Expand Down

0 comments on commit f8f5eac

Please sign in to comment.