Skip to content

Commit

Permalink
*: clean up/fix server structured logs
Browse files Browse the repository at this point in the history 
Signed-off-by: Gyuho Lee <[email protected]>
  • Loading branch information
@gyuho
gyuho committed Apr 18, 2018
1 parent 229a0c9 commit d0847f4
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 62 deletions.
28 changes: 14 additions & 14 deletions etcdserver/cluster_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func getVersions(lg *zap.Logger, cl *membership.RaftCluster, local types.ID, rt
ver, err := getVersion(lg, m, rt)
if err != nil {
if lg != nil {
lg.Warn("failed to get version", zap.String("remote-member-id", m.ID.String()), zap.Error(err))
lg.Warn("failed to get version", zap.String("remote-peer-id", m.ID.String()), zap.Error(err))
} else {
plog.Warningf("cannot get the version of member %s (%v)", m.ID, err)
}
Expand Down Expand Up @@ -191,8 +191,8 @@ func decideClusterVersion(lg *zap.Logger, vers map[string]*version.Versions) *se
if lg != nil {
lg.Warn(
"failed to parse server version of remote member",
zap.String("remote-member-id", mid),
zap.String("remote-member-version", ver.Server),
zap.String("remote-peer-id", mid),
zap.String("remote-peer-version", ver.Server),
zap.Error(err),
)
} else {
Expand All @@ -205,8 +205,8 @@ func decideClusterVersion(lg *zap.Logger, vers map[string]*version.Versions) *se
lg.Warn(
"local etcd version is not up-to-date",
zap.String("local-member-version", lv.String()),
zap.String("remote-member-id", mid),
zap.String("remote-member-version", ver.Server),
zap.String("remote-peer-id", mid),
zap.String("remote-peer-version", ver.Server),
)
} else {
plog.Warningf("the local etcd version %s is not up-to-date", lv.String())
Expand Down Expand Up @@ -254,8 +254,8 @@ func isCompatibleWithVers(lg *zap.Logger, vers map[string]*version.Versions, loc
if lg != nil {
lg.Warn(
"failed to parse cluster version of remote member",
zap.String("remote-member-id", id),
zap.String("remote-member-cluster-version", v.Cluster),
zap.String("remote-peer-id", id),
zap.String("remote-peer-cluster-version", v.Cluster),
zap.Error(err),
)
} else {
Expand All @@ -267,8 +267,8 @@ func isCompatibleWithVers(lg *zap.Logger, vers map[string]*version.Versions, loc
if lg != nil {
lg.Warn(
"cluster version of remote member is not compatible; too low",
zap.String("remote-member-id", id),
zap.String("remote-member-cluster-version", clusterv.String()),
zap.String("remote-peer-id", id),
zap.String("remote-peer-cluster-version", clusterv.String()),
zap.String("minimum-cluster-version-supported", minV.String()),
)
} else {
Expand All @@ -280,8 +280,8 @@ func isCompatibleWithVers(lg *zap.Logger, vers map[string]*version.Versions, loc
if lg != nil {
lg.Warn(
"cluster version of remote member is not compatible; too high",
zap.String("remote-member-id", id),
zap.String("remote-member-cluster-version", clusterv.String()),
zap.String("remote-peer-id", id),
zap.String("remote-peer-cluster-version", clusterv.String()),
zap.String("minimum-cluster-version-supported", minV.String()),
)
} else {
Expand Down Expand Up @@ -313,7 +313,7 @@ func getVersion(lg *zap.Logger, m *membership.Member, rt http.RoundTripper) (*ve
lg.Warn(
"failed to reach the peer URL",
zap.String("address", addr),
zap.String("remote-member-id", m.ID.String()),
zap.String("remote-peer-id", m.ID.String()),
zap.Error(err),
)
} else {
Expand All @@ -329,7 +329,7 @@ func getVersion(lg *zap.Logger, m *membership.Member, rt http.RoundTripper) (*ve
lg.Warn(
"failed to read body of response",
zap.String("address", addr),
zap.String("remote-member-id", m.ID.String()),
zap.String("remote-peer-id", m.ID.String()),
zap.Error(err),
)
} else {
Expand All @@ -343,7 +343,7 @@ func getVersion(lg *zap.Logger, m *membership.Member, rt http.RoundTripper) (*ve
lg.Warn(
"failed to unmarshal response",
zap.String("address", addr),
zap.String("remote-member-id", m.ID.String()),
zap.String("remote-peer-id", m.ID.String()),
zap.Error(err),
)
} else {
Expand Down
24 changes: 12 additions & 12 deletions etcdserver/corrupt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ func (s *EtcdServer) CheckInitialHashKV() error {
zap.Int64("local-member-revision", rev),
zap.Int64("local-member-compact-revision", crev),
zap.Uint32("local-member-hash", h),
zap.String("remote-member-id", peerID.String()),
zap.Strings("remote-member-endpoints", p.eps),
zap.Int64("remote-member-revision", p.resp.Header.Revision),
zap.Int64("remote-member-compact-revision", p.resp.CompactRevision),
zap.Uint32("remote-member-hash", p.resp.Hash),
zap.String("remote-peer-id", peerID.String()),
zap.Strings("remote-peer-endpoints", p.eps),
zap.Int64("remote-peer-revision", p.resp.Header.Revision),
zap.Int64("remote-peer-compact-revision", p.resp.CompactRevision),
zap.Uint32("remote-peer-hash", p.resp.Hash),
}

if h != p.resp.Hash {
Expand Down Expand Up @@ -100,8 +100,8 @@ func (s *EtcdServer) CheckInitialHashKV() error {
zap.Int64("local-member-revision", rev),
zap.Int64("local-member-compact-revision", crev),
zap.Uint32("local-member-hash", h),
zap.String("remote-member-id", p.id.String()),
zap.Strings("remote-member-endpoints", p.eps),
zap.String("remote-peer-id", p.id.String()),
zap.Strings("remote-peer-endpoints", p.eps),
zap.Error(err),
)
} else {
Expand All @@ -115,8 +115,8 @@ func (s *EtcdServer) CheckInitialHashKV() error {
zap.Int64("local-member-revision", rev),
zap.Int64("local-member-compact-revision", crev),
zap.Uint32("local-member-hash", h),
zap.String("remote-member-id", p.id.String()),
zap.Strings("remote-member-endpoints", p.eps),
zap.String("remote-peer-id", p.id.String()),
zap.Strings("remote-peer-endpoints", p.eps),
zap.Error(err),
)
} else {
Expand Down Expand Up @@ -336,8 +336,8 @@ func (s *EtcdServer) getPeerHashKVs(rev int64) (resps []*peerHashKVResp) {
lg.Warn(
"failed to create client to peer URL",
zap.String("local-member-id", s.ID().String()),
zap.String("remote-member-id", p.id.String()),
zap.Strings("remote-member-endpoints", p.eps),
zap.String("remote-peer-id", p.id.String()),
zap.Strings("remote-peer-endpoints", p.eps),
zap.Error(cerr),
)
} else {
Expand All @@ -361,7 +361,7 @@ func (s *EtcdServer) getPeerHashKVs(rev int64) (resps []*peerHashKVResp) {
"failed hash kv request",
zap.String("local-member-id", s.ID().String()),
zap.Int64("requested-revision", rev),
zap.String("remote-member-endpoint", c),
zap.String("remote-peer-endpoint", c),
zap.Error(cerr),
)
} else {
Expand Down
95 changes: 67 additions & 28 deletions etcdserver/membership/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ import (
type RaftCluster struct {
lg *zap.Logger

id types.ID
token string
localID types.ID
cid types.ID
token string

v2store v2store.Store
be backend.Backend
Expand Down Expand Up @@ -75,7 +76,7 @@ func NewClusterFromURLsMap(lg *zap.Logger, token string, urlsmap types.URLsMap)

func NewClusterFromMembers(lg *zap.Logger, token string, id types.ID, membs []*Member) *RaftCluster {
c := NewCluster(lg, token)
c.id = id
c.cid = id
for _, m := range membs {
c.members[m.ID] = m
}
Expand All @@ -91,7 +92,7 @@ func NewCluster(lg *zap.Logger, token string) *RaftCluster {
}
}

func (c *RaftCluster) ID() types.ID { return c.id }
func (c *RaftCluster) ID() types.ID { return c.cid }

func (c *RaftCluster) Members() []*Member {
c.Lock()
Expand Down Expand Up @@ -178,7 +179,7 @@ func (c *RaftCluster) String() string {
c.Lock()
defer c.Unlock()
b := &bytes.Buffer{}
fmt.Fprintf(b, "{ClusterID:%s ", c.id)
fmt.Fprintf(b, "{ClusterID:%s ", c.cid)
var ms []string
for _, m := range c.members {
ms = append(ms, fmt.Sprintf("%+v", m))
Expand All @@ -199,10 +200,13 @@ func (c *RaftCluster) genID() {
binary.BigEndian.PutUint64(b[8*i:], uint64(id))
}
hash := sha1.Sum(b)
c.id = types.ID(binary.BigEndian.Uint64(hash[:8]))
c.cid = types.ID(binary.BigEndian.Uint64(hash[:8]))
}

func (c *RaftCluster) SetID(id types.ID) { c.id = id }
func (c *RaftCluster) SetID(localID, cid types.ID) {
c.localID = localID
c.cid = cid
}

func (c *RaftCluster) SetStore(st v2store.Store) { c.v2store = st }

Expand All @@ -223,13 +227,14 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {
for _, m := range c.members {
if c.lg != nil {
c.lg.Info(
"added member from store",
zap.String("cluster-id", c.id.String()),
zap.String("member-id", m.ID.String()),
zap.Strings("member-peer-urls", m.PeerURLs),
"recovered/added member from store",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("recovered-remote-peer-id", m.ID.String()),
zap.Strings("recovered-remote-peer-urls", m.PeerURLs),
)
} else {
plog.Infof("added member %s %v to cluster %s from store", m.ID, m.PeerURLs, c.id)
plog.Infof("added member %s %v to cluster %s from store", m.ID, m.PeerURLs, c.cid)
}
}
if c.version != nil {
Expand Down Expand Up @@ -337,12 +342,13 @@ func (c *RaftCluster) AddMember(m *Member) {
if c.lg != nil {
c.lg.Info(
"added member",
zap.String("member-id", m.ID.String()),
zap.Strings("member-peer-urls", m.PeerURLs),
zap.String("cluster-id", c.id.String()),
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("removed-remote-peer-id", m.ID.String()),
zap.Strings("removed-remote-peer-peer-urls", m.PeerURLs),
)
} else {
plog.Infof("added member %s %v to cluster %s", m.ID, m.PeerURLs, c.id)
plog.Infof("added member %s %v to cluster %s", m.ID, m.PeerURLs, c.cid)
}
}

Expand All @@ -358,23 +364,36 @@ func (c *RaftCluster) RemoveMember(id types.ID) {
mustDeleteMemberFromBackend(c.be, id)
}

m, ok := c.members[id]
delete(c.members, id)
c.removed[id] = true

if c.lg != nil {
c.lg.Info(
"removed member",
zap.String("member-id", id.String()),
zap.String("cluster-id", c.id.String()),
)
if ok {
c.lg.Info(
"removed member",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("removed-remote-peer-id", id.String()),
zap.Strings("removed-remote-peer-urls", m.PeerURLs),
)
} else {
c.lg.Warn(
"skipped removing already removed member",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("removed-remote-peer-id", id.String()),
)
}
} else {
plog.Infof("removed member %s from cluster %s", id, c.id)
plog.Infof("removed member %s from cluster %s", id, c.cid)
}
}

func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) {
c.Lock()
defer c.Unlock()

if m, ok := c.members[id]; ok {
m.Attributes = attr
if c.v2store != nil {
Expand All @@ -385,17 +404,28 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) {
}
return
}

_, ok := c.removed[id]
if !ok {
if c.lg != nil {
c.lg.Panic("failed to update; member unknown", zap.String("member-id", id.String()))
c.lg.Panic(
"failed to update; member unknown",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("unknown-remote-peer-id", id.String()),
)
} else {
plog.Panicf("error updating attributes of unknown member %s", id)
}
}

if c.lg != nil {
c.lg.Warn("skipped attributes update of removed member", zap.String("member-id", id.String()))
c.lg.Warn(
"skipped attributes update of removed member",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("removed-remote-peer-id", id.String()),
)
} else {
plog.Warningf("skipped updating attributes of removed member %s", id)
}
Expand All @@ -416,12 +446,13 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes)
if c.lg != nil {
c.lg.Info(
"updated member",
zap.String("member-id", id.String()),
zap.Strings("member-peer-urls", raftAttr.PeerURLs),
zap.String("cluster-id", c.id.String()),
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("updated-remote-peer-id", id.String()),
zap.Strings("updated-remote-peer-urls", raftAttr.PeerURLs),
)
} else {
plog.Noticef("updated member %s %v in cluster %s", id, raftAttr.PeerURLs, c.id)
plog.Noticef("updated member %s %v in cluster %s", id, raftAttr.PeerURLs, c.cid)
}
}

Expand All @@ -441,6 +472,8 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s
if c.lg != nil {
c.lg.Info(
"updated cluster version",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("from", version.Cluster(c.version.String())),
zap.String("from", version.Cluster(ver.String())),
)
Expand All @@ -451,6 +484,8 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s
if c.lg != nil {
c.lg.Info(
"set initial cluster version",
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
zap.String("cluster-version", version.Cluster(ver.String())),
)
} else {
Expand Down Expand Up @@ -497,6 +532,8 @@ func (c *RaftCluster) IsReadyToAddNewMember() bool {
"rejecting member add; started member will be less than quorum",
zap.Int("number-of-started-member", nstarted),
zap.Int("quorum", nquorum),
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
)
} else {
plog.Warningf("Reject add member request: the number of started member (%d) will be less than the quorum number of the cluster (%d)", nstarted, nquorum)
Expand Down Expand Up @@ -529,6 +566,8 @@ func (c *RaftCluster) IsReadyToRemoveMember(id uint64) bool {
"rejecting member remove; started member will be less than quorum",
zap.Int("number-of-started-member", nstarted),
zap.Int("quorum", nquorum),
zap.String("cluster-id", c.cid.String()),
zap.String("local-member-id", c.localID.String()),
)
} else {
plog.Warningf("Reject remove member request: the number of started member (%d) will be less than the quorum number of the cluster (%d)", nstarted, nquorum)
Expand Down
Loading

0 comments on commit d0847f4

Please sign in to comment.