From 61f279454e54d06b09da24e54c66c972803623e2 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Tue, 11 Feb 2020 13:51:25 -0800 Subject: [PATCH] etcdserver/api: remove capnslog (#11606) * etcdserver/api/rafthttp: remove capnslog * etcdserver/api/membership: remove capnslog * etcdserver/api/v2auth: remove capnslog * etcdserver/api/v2discovery: remove capnslog * etdserver/api/v2stats: remove capnslog * etcdserver/api/v2http: remove capnslog * etcdserver/api/v3rpc: remove capnslog * etcdserver/api: remove capnslog Remove capnslog from etcdserver/api. Note that capnslog was already removed in some packages under etcdserver/api in previous commits. --- contrib/raftexample/raft.go | 2 +- etcdserver/api/capability.go | 5 - etcdserver/api/etcdhttp/base.go | 46 +-- etcdserver/api/etcdhttp/peer.go | 29 +- etcdserver/api/membership/cluster.go | 363 +++++++----------- etcdserver/api/membership/cluster_test.go | 20 +- etcdserver/api/membership/member.go | 5 - etcdserver/api/membership/store.go | 65 +++- etcdserver/api/rafthttp/functional_test.go | 10 +- etcdserver/api/rafthttp/http.go | 310 +++++++-------- etcdserver/api/rafthttp/peer.go | 22 +- etcdserver/api/rafthttp/peer_status.go | 16 +- etcdserver/api/rafthttp/pipeline.go | 8 +- etcdserver/api/rafthttp/probing_status.go | 8 +- etcdserver/api/rafthttp/remote.go | 4 - etcdserver/api/rafthttp/snapshot_sender.go | 12 +- etcdserver/api/rafthttp/stream.go | 55 +-- etcdserver/api/rafthttp/stream_test.go | 2 +- etcdserver/api/rafthttp/transport.go | 20 - .../api/rafthttp/transport_bench_test.go | 6 +- etcdserver/api/rafthttp/transport_test.go | 7 +- etcdserver/api/rafthttp/util.go | 26 +- etcdserver/api/snap/db.go | 16 +- etcdserver/api/snap/snapshotter.go | 32 +- etcdserver/api/v2auth/auth.go | 144 ++----- etcdserver/api/v2auth/auth_requests.go | 36 +- etcdserver/api/v2discovery/discovery.go | 142 +++---- etcdserver/api/v2discovery/discovery_test.go | 49 ++- etcdserver/api/v2http/capability.go | 3 +- etcdserver/api/v2http/client.go | 79 ++-- etcdserver/api/v2http/client_auth.go | 188 +++------ etcdserver/api/v2http/client_test.go | 7 +- etcdserver/api/v2http/http.go | 11 - etcdserver/api/v2http/httptypes/errors.go | 9 +- etcdserver/api/v2stats/leader.go | 11 +- etcdserver/api/v2stats/stats.go | 4 - etcdserver/api/v3alarm/alarms.go | 15 +- etcdserver/api/v3compactor/compactor.go | 8 +- etcdserver/api/v3compactor/periodic.go | 45 +-- etcdserver/api/v3compactor/revision.go | 47 +-- etcdserver/api/v3rpc/header.go | 2 +- etcdserver/api/v3rpc/interceptor.go | 40 +- etcdserver/api/v3rpc/key.go | 6 - etcdserver/api/v3rpc/lease.go | 30 +- etcdserver/api/v3rpc/maintenance.go | 27 +- etcdserver/api/v3rpc/watch.go | 54 +-- etcdserver/apply_v2.go | 2 +- etcdserver/server.go | 4 +- 48 files changed, 734 insertions(+), 1318 deletions(-) diff --git a/contrib/raftexample/raft.go b/contrib/raftexample/raft.go index ca290388bcd..19831fe7139 100644 --- a/contrib/raftexample/raft.go +++ b/contrib/raftexample/raft.go @@ -299,7 +299,7 @@ func (rc *raftNode) startRaft() { ClusterID: 0x1000, Raft: rc, ServerStats: stats.NewServerStats("", ""), - LeaderStats: stats.NewLeaderStats(strconv.Itoa(rc.id)), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), strconv.Itoa(rc.id)), ErrorC: make(chan error), } diff --git a/etcdserver/api/capability.go b/etcdserver/api/capability.go index 09b754d1343..1e0ed196c32 100644 --- a/etcdserver/api/capability.go +++ b/etcdserver/api/capability.go @@ -21,7 +21,6 @@ import ( "go.uber.org/zap" "github.com/coreos/go-semver/semver" - "github.com/coreos/pkg/capnslog" ) type Capability string @@ -32,8 +31,6 @@ const ( ) var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "etcdserver/api") - // capabilityMaps is a static map of version to capability map. capabilityMaps = map[string]map[Capability]bool{ "3.0.0": {AuthCapability: true, V3rpcCapability: true}, @@ -78,8 +75,6 @@ func UpdateCapability(lg *zap.Logger, v *semver.Version) { "enabled capabilities for version", zap.String("cluster-version", version.Cluster(v.String())), ) - } else { - plog.Infof("enabled capabilities for version %s", version.Cluster(v.String())) } } diff --git a/etcdserver/api/etcdhttp/base.go b/etcdserver/api/etcdhttp/base.go index c9df62ea8e6..e520f81b518 100644 --- a/etcdserver/api/etcdhttp/base.go +++ b/etcdserver/api/etcdhttp/base.go @@ -19,24 +19,16 @@ import ( "expvar" "fmt" "net/http" - "strings" "go.etcd.io/etcd/etcdserver" "go.etcd.io/etcd/etcdserver/api" "go.etcd.io/etcd/etcdserver/api/v2error" "go.etcd.io/etcd/etcdserver/api/v2http/httptypes" - "go.etcd.io/etcd/pkg/logutil" "go.etcd.io/etcd/version" - "github.com/coreos/pkg/capnslog" "go.uber.org/zap" ) -var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "etcdserver/api/etcdhttp") - mlog = logutil.NewMergeLogger(plog) -) - const ( configPath = "/config" varsPath = "/debug/vars" @@ -48,9 +40,6 @@ const ( func HandleBasic(mux *http.ServeMux, server etcdserver.ServerPeer) { mux.HandleFunc(varsPath, serveVars) - // TODO: deprecate '/config/local/log' in v3.5 - mux.HandleFunc(configPath+"/local/log", logHandleFunc) - HandleMetricsHealth(mux, server) mux.HandleFunc(versionPath, versionHandler(server.Cluster(), serveVersion)) } @@ -78,36 +67,11 @@ func serveVersion(w http.ResponseWriter, r *http.Request, clusterV string) { w.Header().Set("Content-Type", "application/json") b, err := json.Marshal(&vs) if err != nil { - plog.Panicf("cannot marshal versions to json (%v)", err) + panic(fmt.Sprintf("cannot marshal versions to json (%v)", err)) } w.Write(b) } -// TODO: deprecate '/config/local/log' in v3.5 -func logHandleFunc(w http.ResponseWriter, r *http.Request) { - if !allowMethod(w, r, "PUT") { - return - } - - in := struct{ Level string }{} - - d := json.NewDecoder(r.Body) - if err := d.Decode(&in); err != nil { - WriteError(nil, w, r, httptypes.NewHTTPError(http.StatusBadRequest, "Invalid json body")) - return - } - - logl, err := capnslog.ParseLevel(strings.ToUpper(in.Level)) - if err != nil { - WriteError(nil, w, r, httptypes.NewHTTPError(http.StatusBadRequest, "Invalid log level "+in.Level)) - return - } - - plog.Noticef("globalLogLevel set to %q", logl.String()) - capnslog.SetGlobalLogLevel(logl) - w.WriteHeader(http.StatusNoContent) -} - func serveVars(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r, "GET") { return @@ -155,8 +119,6 @@ func WriteError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro zap.String("internal-server-error", e.Error()), zap.Error(et), ) - } else { - plog.Debugf("error writing HTTPError (%v) to %s", et, r.RemoteAddr) } } @@ -170,8 +132,6 @@ func WriteError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro zap.String("remote-addr", r.RemoteAddr), zap.String("internal-server-error", err.Error()), ) - } else { - mlog.MergeError(err) } default: @@ -181,8 +141,6 @@ func WriteError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro zap.String("remote-addr", r.RemoteAddr), zap.String("internal-server-error", err.Error()), ) - } else { - mlog.MergeErrorf("got unexpected response error (%v)", err) } } @@ -195,8 +153,6 @@ func WriteError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro zap.String("internal-server-error", err.Error()), zap.Error(et), ) - } else { - plog.Debugf("error writing HTTPError (%v) to %s", et, r.RemoteAddr) } } } diff --git a/etcdserver/api/etcdhttp/peer.go b/etcdserver/api/etcdhttp/peer.go index 6c61bf5d510..1fb7ccb7b96 100644 --- a/etcdserver/api/etcdhttp/peer.go +++ b/etcdserver/api/etcdhttp/peer.go @@ -42,6 +42,9 @@ func NewPeerHandler(lg *zap.Logger, s etcdserver.ServerPeer) http.Handler { } func newPeerHandler(lg *zap.Logger, s etcdserver.Server, raftHandler http.Handler, leaseHandler http.Handler) http.Handler { + if lg == nil { + lg = zap.NewNop() + } peerMembersHandler := newPeerMembersHandler(lg, s.Cluster()) peerMemberPromoteHandler := newPeerMemberPromoteHandler(lg, s) @@ -98,11 +101,7 @@ func (h *peerMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ms := h.cluster.Members() w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(ms); err != nil { - if h.lg != nil { - h.lg.Warn("failed to encode membership members", zap.Error(err)) - } else { - plog.Warningf("failed to encode members response (%v)", err) - } + h.lg.Warn("failed to encode membership members", zap.Error(err)) } } @@ -135,25 +134,17 @@ func (h *peerMemberPromoteHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ default: WriteError(h.lg, w, r, err) } - if h.lg != nil { - h.lg.Warn( - "failed to promote a member", - zap.String("member-id", types.ID(id).String()), - zap.Error(err), - ) - } else { - plog.Errorf("error promoting member %s (%v)", types.ID(id).String(), err) - } + h.lg.Warn( + "failed to promote a member", + zap.String("member-id", types.ID(id).String()), + zap.Error(err), + ) return } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) if err := json.NewEncoder(w).Encode(resp); err != nil { - if h.lg != nil { - h.lg.Warn("failed to encode members response", zap.Error(err)) - } else { - plog.Warningf("failed to encode members response (%v)", err) - } + h.lg.Warn("failed to encode members response", zap.Error(err)) } } diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go index 0938c97221d..bac67d8b415 100644 --- a/etcdserver/api/membership/cluster.go +++ b/etcdserver/api/membership/cluster.go @@ -98,6 +98,9 @@ func NewClusterFromMembers(lg *zap.Logger, token string, id types.ID, membs []*M } func NewCluster(lg *zap.Logger, token string) *RaftCluster { + if lg == nil { + lg = zap.NewNop() + } return &RaftCluster{ lg: lg, token: token, @@ -147,11 +150,7 @@ func (c *RaftCluster) MemberByName(name string) *Member { for _, m := range c.members { if m.Name == name { if memb != nil { - if c.lg != nil { - c.lg.Panic("two member with same name found", zap.String("name", name)) - } else { - plog.Panicf("two members with the given name %q exist", name) - } + c.lg.Panic("two member with same name found", zap.String("name", name)) } memb = m } @@ -257,27 +256,19 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { onSet(c.lg, c.version) for _, m := range c.members { - if c.lg != nil { - c.lg.Info( - "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.cid) - } + c.lg.Info( + "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), + ) } if c.version != nil { - if c.lg != nil { - c.lg.Info( - "set cluster version from store", - zap.String("cluster-version", version.Cluster(c.version.String())), - ) - } else { - plog.Infof("set the cluster version to %v from store", version.Cluster(c.version.String())) - } + c.lg.Info( + "set cluster version from store", + zap.String("cluster-version", version.Cluster(c.version.String())), + ) } } @@ -293,11 +284,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode: confChangeContext := new(ConfigChangeContext) if err := json.Unmarshal(cc.Context, confChangeContext); err != nil { - if c.lg != nil { - c.lg.Panic("failed to unmarshal confChangeContext", zap.Error(err)) - } else { - plog.Panicf("unmarshal confChangeContext should never fail: %v", err) - } + c.lg.Panic("failed to unmarshal confChangeContext", zap.Error(err)) } if confChangeContext.IsPromote { // promoting a learner member to voting member @@ -356,11 +343,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { } m := new(Member) if err := json.Unmarshal(cc.Context, m); err != nil { - if c.lg != nil { - c.lg.Panic("failed to unmarshal member", zap.Error(err)) - } else { - plog.Panicf("unmarshal member should never fail: %v", err) - } + c.lg.Panic("failed to unmarshal member", zap.Error(err)) } for _, u := range m.PeerURLs { if urls[u] { @@ -369,11 +352,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { } default: - if c.lg != nil { - c.lg.Panic("unknown ConfChange type", zap.String("type", cc.Type.String())) - } else { - plog.Panicf("ConfChange type should be either AddNode, RemoveNode or UpdateNode") - } + c.lg.Panic("unknown ConfChange type", zap.String("type", cc.Type.String())) } return nil } @@ -385,25 +364,21 @@ func (c *RaftCluster) AddMember(m *Member) { c.Lock() defer c.Unlock() if c.v2store != nil { - mustSaveMemberToStore(c.v2store, m) + mustSaveMemberToStore(c.lg, c.v2store, m) } if c.be != nil { - mustSaveMemberToBackend(c.be, m) + mustSaveMemberToBackend(c.lg, c.be, m) } c.members[m.ID] = m - if c.lg != nil { - c.lg.Info( - "added member", - zap.String("cluster-id", c.cid.String()), - zap.String("local-member-id", c.localID.String()), - zap.String("added-peer-id", m.ID.String()), - zap.Strings("added-peer-peer-urls", m.PeerURLs), - ) - } else { - plog.Infof("added member %s %v to cluster %s", m.ID, m.PeerURLs, c.cid) - } + c.lg.Info( + "added member", + zap.String("cluster-id", c.cid.String()), + zap.String("local-member-id", c.localID.String()), + zap.String("added-peer-id", m.ID.String()), + zap.Strings("added-peer-peer-urls", m.PeerURLs), + ) } // RemoveMember removes a member from the store. @@ -412,7 +387,7 @@ func (c *RaftCluster) RemoveMember(id types.ID) { c.Lock() defer c.Unlock() if c.v2store != nil { - mustDeleteMemberFromStore(c.v2store, id) + mustDeleteMemberFromStore(c.lg, c.v2store, id) } if c.be != nil { mustDeleteMemberFromBackend(c.be, id) @@ -422,25 +397,21 @@ func (c *RaftCluster) RemoveMember(id types.ID) { delete(c.members, id) c.removed[id] = true - if c.lg != nil { - 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()), - ) - } + 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 { - plog.Infof("removed member %s from cluster %s", id, c.cid) + 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()), + ) } } @@ -451,38 +422,30 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) { if m, ok := c.members[id]; ok { m.Attributes = attr if c.v2store != nil { - mustUpdateMemberAttrInStore(c.v2store, m) + mustUpdateMemberAttrInStore(c.lg, c.v2store, m) } if c.be != nil { - mustSaveMemberToBackend(c.be, m) + mustSaveMemberToBackend(c.lg, c.be, m) } return } _, ok := c.removed[id] if !ok { - if c.lg != nil { - 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", + 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("updated-peer-id", id.String()), + zap.String("unknown-remote-peer-id", id.String()), ) - } else { - plog.Warningf("skipped updating attributes of removed member %s", id) } + + 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("updated-peer-id", id.String()), + ) } // PromoteMember marks the member's IsLearner RaftAttributes to false. @@ -492,21 +455,17 @@ func (c *RaftCluster) PromoteMember(id types.ID) { c.members[id].RaftAttributes.IsLearner = false if c.v2store != nil { - mustUpdateMemberInStore(c.v2store, c.members[id]) + mustUpdateMemberInStore(c.lg, c.v2store, c.members[id]) } if c.be != nil { - mustSaveMemberToBackend(c.be, c.members[id]) + mustSaveMemberToBackend(c.lg, c.be, c.members[id]) } - if c.lg != nil { - c.lg.Info( - "promote member", - zap.String("cluster-id", c.cid.String()), - zap.String("local-member-id", c.localID.String()), - ) - } else { - plog.Noticef("promote member %s in cluster %s", id, c.cid) - } + c.lg.Info( + "promote member", + zap.String("cluster-id", c.cid.String()), + zap.String("local-member-id", c.localID.String()), + ) } func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes) { @@ -515,23 +474,19 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes) c.members[id].RaftAttributes = raftAttr if c.v2store != nil { - mustUpdateMemberInStore(c.v2store, c.members[id]) + mustUpdateMemberInStore(c.lg, c.v2store, c.members[id]) } if c.be != nil { - mustSaveMemberToBackend(c.be, c.members[id]) + mustSaveMemberToBackend(c.lg, c.be, c.members[id]) } - if c.lg != nil { - c.lg.Info( - "updated member", - 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.cid) - } + c.lg.Info( + "updated member", + 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), + ) } func (c *RaftCluster) Version() *semver.Version { @@ -547,34 +502,26 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s c.Lock() defer c.Unlock() if c.version != nil { - 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())), - ) - } else { - plog.Noticef("updated the cluster version from %v to %v", version.Cluster(c.version.String()), version.Cluster(ver.String())) - } + 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())), + ) } else { - 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 { - plog.Noticef("set the initial cluster version to %v", version.Cluster(ver.String())) - } + 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())), + ) } oldVer := c.version c.version = ver mustDetectDowngrade(c.lg, c.version) if c.v2store != nil { - mustSaveClusterVersionToStore(c.v2store, ver) + mustSaveClusterVersionToStore(c.lg, c.v2store, ver) } if c.be != nil { mustSaveClusterVersionToBackend(c.be, ver) @@ -600,27 +547,19 @@ func (c *RaftCluster) IsReadyToAddVotingMember() bool { if nstarted == 1 && nmembers == 2 { // a case of adding a new node to 1-member cluster for restoring cluster data // https://github.com/etcd-io/etcd/blob/master/Documentation/v2/admin_guide.md#restoring-the-cluster - if c.lg != nil { - c.lg.Debug("number of started member is 1; can accept add member request") - } else { - plog.Debugf("The number of started member is 1. This cluster can accept add member request.") - } + c.lg.Debug("number of started member is 1; can accept add member request") return true } nquorum := nmembers/2 + 1 if nstarted < nquorum { - if c.lg != nil { - c.lg.Warn( - "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) - } + c.lg.Warn( + "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()), + ) return false } @@ -644,17 +583,13 @@ func (c *RaftCluster) IsReadyToRemoveVotingMember(id uint64) bool { nquorum := nmembers/2 + 1 if nstarted < nquorum { - if c.lg != nil { - c.lg.Warn( - "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) - } + c.lg.Warn( + "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()), + ) return false } @@ -674,17 +609,13 @@ func (c *RaftCluster) IsReadyToPromoteMember(id uint64) bool { nquorum := nmembers/2 + 1 if nstarted < nquorum { - if c.lg != nil { - c.lg.Warn( - "rejecting member promote; 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 promote member request: the number of started member (%d) will be less than the quorum number of the cluster (%d)", nstarted, nquorum) - } + c.lg.Warn( + "rejecting member promote; 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()), + ) return false } @@ -699,21 +630,13 @@ func membersFromStore(lg *zap.Logger, st v2store.Store) (map[types.ID]*Member, m if isKeyNotFound(err) { return members, removed } - if lg != nil { - lg.Panic("failed to get members from store", zap.String("path", StoreMembersPrefix), zap.Error(err)) - } else { - plog.Panicf("get storeMembers should never fail: %v", err) - } + lg.Panic("failed to get members from store", zap.String("path", StoreMembersPrefix), zap.Error(err)) } for _, n := range e.Node.Nodes { var m *Member - m, err = nodeToMember(n) + m, err = nodeToMember(lg, n) if err != nil { - if lg != nil { - lg.Panic("failed to nodeToMember", zap.Error(err)) - } else { - plog.Panicf("nodeToMember should never fail: %v", err) - } + lg.Panic("failed to nodeToMember", zap.Error(err)) } members[m.ID] = m } @@ -723,18 +646,14 @@ func membersFromStore(lg *zap.Logger, st v2store.Store) (map[types.ID]*Member, m if isKeyNotFound(err) { return members, removed } - if lg != nil { - lg.Panic( - "failed to get removed members from store", - zap.String("path", storeRemovedMembersPrefix), - zap.Error(err), - ) - } else { - plog.Panicf("get storeRemovedMembers should never fail: %v", err) - } + lg.Panic( + "failed to get removed members from store", + zap.String("path", storeRemovedMembersPrefix), + zap.Error(err), + ) } for _, n := range e.Node.Nodes { - removed[MustParseMemberIDFromKey(n.Key)] = true + removed[MustParseMemberIDFromKey(lg, n.Key)] = true } return members, removed } @@ -745,15 +664,11 @@ func clusterVersionFromStore(lg *zap.Logger, st v2store.Store) *semver.Version { if isKeyNotFound(err) { return nil } - if lg != nil { - lg.Panic( - "failed to get cluster version from store", - zap.String("path", path.Join(storePrefix, "version")), - zap.Error(err), - ) - } else { - plog.Panicf("unexpected error (%v) when getting cluster version from store", err) - } + lg.Panic( + "failed to get cluster version from store", + zap.String("path", path.Join(storePrefix, "version")), + zap.Error(err), + ) } return semver.Must(semver.NewVersion(*e.Node.Value)) } @@ -768,12 +683,10 @@ func clusterVersionFromBackend(lg *zap.Logger, be backend.Backend) *semver.Versi return nil } if len(keys) != 1 { - if lg != nil { - lg.Panic( - "unexpected number of keys when getting cluster version from backend", - zap.Int("number-of-key", len(keys)), - ) - } + lg.Panic( + "unexpected number of keys when getting cluster version from backend", + zap.Int("number-of-key", len(keys)), + ) } return semver.Must(semver.NewVersion(string(vals[0]))) } @@ -816,15 +729,11 @@ func mustDetectDowngrade(lg *zap.Logger, cv *semver.Version) { // only keep major.minor version for comparison against cluster version lv = &semver.Version{Major: lv.Major, Minor: lv.Minor} if cv != nil && lv.LessThan(*cv) { - if lg != nil { - lg.Fatal( - "invalid downgrade; server version is lower than determined cluster version", - zap.String("current-server-version", version.Version), - zap.String("determined-cluster-version", version.Cluster(cv.String())), - ) - } else { - plog.Fatalf("cluster cannot be downgraded (current version: %s is lower than determined cluster version: %s).", version.Version, version.Cluster(cv.String())) - } + lg.Fatal( + "invalid downgrade; server version is lower than determined cluster version", + zap.String("current-server-version", version.Version), + zap.String("determined-cluster-version", version.Cluster(cv.String())), + ) } } @@ -834,15 +743,11 @@ func (c *RaftCluster) IsLocalMemberLearner() bool { defer c.Unlock() localMember, ok := c.members[c.localID] if !ok { - if c.lg != nil { - c.lg.Panic( - "failed to find local ID in cluster members", - zap.String("cluster-id", c.cid.String()), - zap.String("local-member-id", c.localID.String()), - ) - } else { - plog.Panicf("failed to find local ID %s in cluster %s", c.localID.String(), c.cid.String()) - } + c.lg.Panic( + "failed to find local ID in cluster members", + zap.String("cluster-id", c.cid.String()), + zap.String("local-member-id", c.localID.String()), + ) } return localMember.IsLearner } diff --git a/etcdserver/api/membership/cluster_test.go b/etcdserver/api/membership/cluster_test.go index 55b72ee08c2..9fd15ef3a31 100644 --- a/etcdserver/api/membership/cluster_test.go +++ b/etcdserver/api/membership/cluster_test.go @@ -476,7 +476,7 @@ func TestNodeToMemberBad(t *testing.T) { }}, } for i, tt := range tests { - if _, err := nodeToMember(tt); err == nil { + if _, err := nodeToMember(zap.NewExample(), tt); err == nil { t.Errorf("#%d: unexpected nil error", i) } } @@ -529,15 +529,13 @@ func TestClusterAddMemberAsLearner(t *testing.T) { } func TestClusterMembers(t *testing.T) { - cls := &RaftCluster{ - members: map[types.ID]*Member{ - 1: {ID: 1}, - 20: {ID: 20}, - 100: {ID: 100}, - 5: {ID: 5}, - 50: {ID: 50}, - }, - } + cls := newTestCluster([]*Member{ + {ID: 1}, + {ID: 20}, + {ID: 100}, + {ID: 5}, + {ID: 50}, + }) w := []*Member{ {ID: 1}, {ID: 5}, @@ -607,7 +605,7 @@ func TestNodeToMember(t *testing.T) { {Key: "/1234/raftAttributes", Value: stringp(`{"peerURLs":null}`)}, }} wm := &Member{ID: 0x1234, RaftAttributes: RaftAttributes{}, Attributes: Attributes{Name: "node1"}} - m, err := nodeToMember(n) + m, err := nodeToMember(zap.NewExample(), n) if err != nil { t.Fatalf("unexpected nodeToMember error: %v", err) } diff --git a/etcdserver/api/membership/member.go b/etcdserver/api/membership/member.go index 896cb36aa45..5ab43950e42 100644 --- a/etcdserver/api/membership/member.go +++ b/etcdserver/api/membership/member.go @@ -22,14 +22,9 @@ import ( "sort" "time" - "github.com/coreos/pkg/capnslog" "go.etcd.io/etcd/pkg/types" ) -var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd/v3", "etcdserver/membership") -) - // RaftAttributes represents the raft related attributes of an etcd member. type RaftAttributes struct { // PeerURLs is the list of peers in the raft cluster. diff --git a/etcdserver/api/membership/store.go b/etcdserver/api/membership/store.go index 14ab1190ed9..c3a638ade65 100644 --- a/etcdserver/api/membership/store.go +++ b/etcdserver/api/membership/store.go @@ -24,6 +24,7 @@ import ( "go.etcd.io/etcd/pkg/types" "github.com/coreos/go-semver/semver" + "go.uber.org/zap" ) const ( @@ -43,11 +44,11 @@ var ( storeRemovedMembersPrefix = path.Join(storePrefix, "removed_members") ) -func mustSaveMemberToBackend(be backend.Backend, m *Member) { +func mustSaveMemberToBackend(lg *zap.Logger, be backend.Backend, m *Member) { mkey := backendMemberKey(m.ID) mvalue, err := json.Marshal(m) if err != nil { - plog.Panicf("marshal raftAttributes should never fail: %v", err) + lg.Panic("failed to marshal member", zap.Error(err)) } tx := be.BatchTx() @@ -75,58 +76,82 @@ func mustSaveClusterVersionToBackend(be backend.Backend, ver *semver.Version) { tx.UnsafePut(clusterBucketName, ckey, []byte(ver.String())) } -func mustSaveMemberToStore(s v2store.Store, m *Member) { +func mustSaveMemberToStore(lg *zap.Logger, s v2store.Store, m *Member) { b, err := json.Marshal(m.RaftAttributes) if err != nil { - plog.Panicf("marshal raftAttributes should never fail: %v", err) + lg.Panic("failed to marshal raftAttributes", zap.Error(err)) } p := path.Join(MemberStoreKey(m.ID), raftAttributesSuffix) if _, err := s.Create(p, false, string(b), false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - plog.Panicf("create raftAttributes should never fail: %v", err) + lg.Panic( + "failed to save member to store", + zap.String("path", p), + zap.Error(err), + ) } } -func mustDeleteMemberFromStore(s v2store.Store, id types.ID) { +func mustDeleteMemberFromStore(lg *zap.Logger, s v2store.Store, id types.ID) { if _, err := s.Delete(MemberStoreKey(id), true, true); err != nil { - plog.Panicf("delete member should never fail: %v", err) + lg.Panic( + "failed to delete member from store", + zap.String("path", MemberStoreKey(id)), + zap.Error(err), + ) } if _, err := s.Create(RemovedMemberStoreKey(id), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - plog.Panicf("create removedMember should never fail: %v", err) + lg.Panic( + "failed to create removedMember", + zap.String("path", RemovedMemberStoreKey(id)), + zap.Error(err), + ) } } -func mustUpdateMemberInStore(s v2store.Store, m *Member) { +func mustUpdateMemberInStore(lg *zap.Logger, s v2store.Store, m *Member) { b, err := json.Marshal(m.RaftAttributes) if err != nil { - plog.Panicf("marshal raftAttributes should never fail: %v", err) + lg.Panic("failed to marshal raftAttributes", zap.Error(err)) } p := path.Join(MemberStoreKey(m.ID), raftAttributesSuffix) if _, err := s.Update(p, string(b), v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - plog.Panicf("update raftAttributes should never fail: %v", err) + lg.Panic( + "failed to update raftAttributes", + zap.String("path", p), + zap.Error(err), + ) } } -func mustUpdateMemberAttrInStore(s v2store.Store, m *Member) { +func mustUpdateMemberAttrInStore(lg *zap.Logger, s v2store.Store, m *Member) { b, err := json.Marshal(m.Attributes) if err != nil { - plog.Panicf("marshal raftAttributes should never fail: %v", err) + lg.Panic("failed to marshal attributes", zap.Error(err)) } p := path.Join(MemberStoreKey(m.ID), attributesSuffix) if _, err := s.Set(p, false, string(b), v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - plog.Panicf("update raftAttributes should never fail: %v", err) + lg.Panic( + "failed to update attributes", + zap.String("path", p), + zap.Error(err), + ) } } -func mustSaveClusterVersionToStore(s v2store.Store, ver *semver.Version) { +func mustSaveClusterVersionToStore(lg *zap.Logger, s v2store.Store, ver *semver.Version) { if _, err := s.Set(StoreClusterVersionKey(), false, ver.String(), v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - plog.Panicf("save cluster version should never fail: %v", err) + lg.Panic( + "failed to save cluster version to store", + zap.String("path", StoreClusterVersionKey()), + zap.Error(err), + ) } } // nodeToMember builds member from a key value node. // the child nodes of the given node MUST be sorted by key. -func nodeToMember(n *v2store.NodeExtern) (*Member, error) { - m := &Member{ID: MustParseMemberIDFromKey(n.Key)} +func nodeToMember(lg *zap.Logger, n *v2store.NodeExtern) (*Member, error) { + m := &Member{ID: MustParseMemberIDFromKey(lg, n.Key)} attrs := make(map[string][]byte) raftAttrKey := path.Join(n.Key, raftAttributesSuffix) attrKey := path.Join(n.Key, attributesSuffix) @@ -180,10 +205,10 @@ func MemberAttributesStorePath(id types.ID) string { return path.Join(MemberStoreKey(id), attributesSuffix) } -func MustParseMemberIDFromKey(key string) types.ID { +func MustParseMemberIDFromKey(lg *zap.Logger, key string) types.ID { id, err := types.IDFromString(path.Base(key)) if err != nil { - plog.Panicf("unexpected parse member id error: %v", err) + lg.Panic("failed to parse memver id from key", zap.Error(err)) } return id } diff --git a/etcdserver/api/rafthttp/functional_test.go b/etcdserver/api/rafthttp/functional_test.go index 83de44478c6..ed6e97574a2 100644 --- a/etcdserver/api/rafthttp/functional_test.go +++ b/etcdserver/api/rafthttp/functional_test.go @@ -25,6 +25,8 @@ import ( "go.etcd.io/etcd/pkg/types" "go.etcd.io/etcd/raft" "go.etcd.io/etcd/raft/raftpb" + + "go.uber.org/zap" ) func TestSendMessage(t *testing.T) { @@ -34,7 +36,7 @@ func TestSendMessage(t *testing.T) { ClusterID: types.ID(1), Raft: &fakeRaft{}, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("1"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "1"), } tr.Start() srv := httptest.NewServer(tr.Handler()) @@ -48,7 +50,7 @@ func TestSendMessage(t *testing.T) { ClusterID: types.ID(1), Raft: p, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("2"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "2"), } tr2.Start() srv2 := httptest.NewServer(tr2.Handler()) @@ -92,7 +94,7 @@ func TestSendMessageWhenStreamIsBroken(t *testing.T) { ClusterID: types.ID(1), Raft: &fakeRaft{}, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("1"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "1"), } tr.Start() srv := httptest.NewServer(tr.Handler()) @@ -106,7 +108,7 @@ func TestSendMessageWhenStreamIsBroken(t *testing.T) { ClusterID: types.ID(1), Raft: p, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("2"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "2"), } tr2.Start() srv2 := httptest.NewServer(tr2.Handler()) diff --git a/etcdserver/api/rafthttp/http.go b/etcdserver/api/rafthttp/http.go index d0e0c81e209..30a329f96aa 100644 --- a/etcdserver/api/rafthttp/http.go +++ b/etcdserver/api/rafthttp/http.go @@ -76,13 +76,17 @@ type pipelineHandler struct { // The handler reads out the raft message from request body, // and forwards it to the given raft state machine for processing. func newPipelineHandler(t *Transport, r Raft, cid types.ID) http.Handler { - return &pipelineHandler{ + h := &pipelineHandler{ lg: t.Logger, localID: t.ID, tr: t, r: r, cid: cid, } + if h.lg == nil { + h.lg = zap.NewNop() + } + return h } func (h *pipelineHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -106,15 +110,11 @@ func (h *pipelineHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { limitedr := pioutil.NewLimitedBufferReader(r.Body, connReadLimitByte) b, err := ioutil.ReadAll(limitedr) if err != nil { - if h.lg != nil { - h.lg.Warn( - "failed to read Raft message", - zap.String("local-member-id", h.localID.String()), - zap.Error(err), - ) - } else { - plog.Errorf("failed to read raft message (%v)", err) - } + h.lg.Warn( + "failed to read Raft message", + zap.String("local-member-id", h.localID.String()), + zap.Error(err), + ) http.Error(w, "error reading raft message", http.StatusBadRequest) recvFailures.WithLabelValues(r.RemoteAddr).Inc() return @@ -122,15 +122,11 @@ func (h *pipelineHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var m raftpb.Message if err := m.Unmarshal(b); err != nil { - if h.lg != nil { - h.lg.Warn( - "failed to unmarshal Raft message", - zap.String("local-member-id", h.localID.String()), - zap.Error(err), - ) - } else { - plog.Errorf("failed to unmarshal raft message (%v)", err) - } + h.lg.Warn( + "failed to unmarshal Raft message", + zap.String("local-member-id", h.localID.String()), + zap.Error(err), + ) http.Error(w, "error unmarshalling raft message", http.StatusBadRequest) recvFailures.WithLabelValues(r.RemoteAddr).Inc() return @@ -143,15 +139,11 @@ func (h *pipelineHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { case writerToResponse: v.WriteTo(w) default: - if h.lg != nil { - h.lg.Warn( - "failed to process Raft message", - zap.String("local-member-id", h.localID.String()), - zap.Error(err), - ) - } else { - plog.Warningf("failed to process raft message (%v)", err) - } + h.lg.Warn( + "failed to process Raft message", + zap.String("local-member-id", h.localID.String()), + zap.Error(err), + ) http.Error(w, "error processing raft message", http.StatusInternalServerError) w.(http.Flusher).Flush() // disconnect the http stream @@ -176,7 +168,7 @@ type snapshotHandler struct { } func newSnapshotHandler(t *Transport, r Raft, snapshotter *snap.Snapshotter, cid types.ID) http.Handler { - return &snapshotHandler{ + h := &snapshotHandler{ lg: t.Logger, tr: t, r: r, @@ -184,6 +176,10 @@ func newSnapshotHandler(t *Transport, r Raft, snapshotter *snap.Snapshotter, cid localID: t.ID, cid: cid, } + if h.lg == nil { + h.lg = zap.NewNop() + } + return h } const unknownSnapshotSender = "UNKNOWN_SNAPSHOT_SENDER" @@ -223,16 +219,12 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { from := types.ID(m.From).String() if err != nil { msg := fmt.Sprintf("failed to decode raft message (%v)", err) - if h.lg != nil { - h.lg.Warn( - "failed to decode Raft message", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.Error(err), - ) - } else { - plog.Error(msg) - } + h.lg.Warn( + "failed to decode Raft message", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.Error(err), + ) http.Error(w, msg, http.StatusBadRequest) recvFailures.WithLabelValues(r.RemoteAddr).Inc() snapshotReceiveFailures.WithLabelValues(from).Inc() @@ -243,16 +235,12 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { receivedBytes.WithLabelValues(from).Add(float64(msgSize)) if m.Type != raftpb.MsgSnap { - if h.lg != nil { - h.lg.Warn( - "unexpected Raft message type", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.String("message-type", m.Type.String()), - ) - } else { - plog.Errorf("unexpected raft message type %s on snapshot path", m.Type) - } + h.lg.Warn( + "unexpected Raft message type", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.String("message-type", m.Type.String()), + ) http.Error(w, "wrong raft message type", http.StatusBadRequest) snapshotReceiveFailures.WithLabelValues(from).Inc() return @@ -263,34 +251,26 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { snapshotReceiveInflights.WithLabelValues(from).Dec() }() - if h.lg != nil { - h.lg.Info( - "receiving database snapshot", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), - zap.Int("incoming-snapshot-message-size-bytes", msgSize), - zap.String("incoming-snapshot-message-size", humanize.Bytes(uint64(msgSize))), - ) - } else { - plog.Infof("receiving database snapshot [index:%d, from %s] ...", m.Snapshot.Metadata.Index, types.ID(m.From)) - } + h.lg.Info( + "receiving database snapshot", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), + zap.Int("incoming-snapshot-message-size-bytes", msgSize), + zap.String("incoming-snapshot-message-size", humanize.Bytes(uint64(msgSize))), + ) // save incoming database snapshot. n, err := h.snapshotter.SaveDBFrom(r.Body, m.Snapshot.Metadata.Index) if err != nil { msg := fmt.Sprintf("failed to save KV snapshot (%v)", err) - if h.lg != nil { - h.lg.Warn( - "failed to save incoming database snapshot", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), - zap.Error(err), - ) - } else { - plog.Error(msg) - } + h.lg.Warn( + "failed to save incoming database snapshot", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), + zap.Error(err), + ) http.Error(w, msg, http.StatusInternalServerError) snapshotReceiveFailures.WithLabelValues(from).Inc() return @@ -298,18 +278,14 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { receivedBytes.WithLabelValues(from).Add(float64(n)) - if h.lg != nil { - h.lg.Info( - "received and saved database snapshot", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), - zap.Int64("incoming-snapshot-size-bytes", n), - zap.String("incoming-snapshot-size", humanize.Bytes(uint64(n))), - ) - } else { - plog.Infof("received and saved database snapshot [index: %d, from: %s] successfully", m.Snapshot.Metadata.Index, types.ID(m.From)) - } + h.lg.Info( + "received and saved database snapshot", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.Uint64("incoming-snapshot-index", m.Snapshot.Metadata.Index), + zap.Int64("incoming-snapshot-size-bytes", n), + zap.String("incoming-snapshot-size", humanize.Bytes(uint64(n))), + ) if err := h.r.Process(context.TODO(), m); err != nil { switch v := err.(type) { @@ -319,16 +295,12 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { v.WriteTo(w) default: msg := fmt.Sprintf("failed to process raft message (%v)", err) - if h.lg != nil { - h.lg.Warn( - "failed to process Raft message", - zap.String("local-member-id", h.localID.String()), - zap.String("remote-snapshot-sender-id", from), - zap.Error(err), - ) - } else { - plog.Error(msg) - } + h.lg.Warn( + "failed to process Raft message", + zap.String("local-member-id", h.localID.String()), + zap.String("remote-snapshot-sender-id", from), + zap.Error(err), + ) http.Error(w, msg, http.StatusInternalServerError) snapshotReceiveFailures.WithLabelValues(from).Inc() } @@ -353,7 +325,7 @@ type streamHandler struct { } func newStreamHandler(t *Transport, pg peerGetter, r Raft, id, cid types.ID) http.Handler { - return &streamHandler{ + h := &streamHandler{ lg: t.Logger, tr: t, peerGetter: pg, @@ -361,6 +333,10 @@ func newStreamHandler(t *Transport, pg peerGetter, r Raft, id, cid types.ID) htt id: id, cid: cid, } + if h.lg == nil { + h.lg = zap.NewNop() + } + return h } func (h *streamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -380,21 +356,17 @@ func (h *streamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var t streamType switch path.Dir(r.URL.Path) { - case streamTypeMsgAppV2.endpoint(): + case streamTypeMsgAppV2.endpoint(h.lg): t = streamTypeMsgAppV2 - case streamTypeMessage.endpoint(): + case streamTypeMessage.endpoint(h.lg): t = streamTypeMessage default: - if h.lg != nil { - h.lg.Debug( - "ignored unexpected streaming request path", - zap.String("local-member-id", h.tr.ID.String()), - zap.String("remote-peer-id-stream-handler", h.id.String()), - zap.String("path", r.URL.Path), - ) - } else { - plog.Debugf("ignored unexpected streaming request path %s", r.URL.Path) - } + h.lg.Debug( + "ignored unexpected streaming request path", + zap.String("local-member-id", h.tr.ID.String()), + zap.String("remote-peer-id-stream-handler", h.id.String()), + zap.String("path", r.URL.Path), + ) http.Error(w, "invalid path", http.StatusNotFound) return } @@ -402,31 +374,23 @@ func (h *streamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { fromStr := path.Base(r.URL.Path) from, err := types.IDFromString(fromStr) if err != nil { - if h.lg != nil { - h.lg.Warn( - "failed to parse path into ID", - zap.String("local-member-id", h.tr.ID.String()), - zap.String("remote-peer-id-stream-handler", h.id.String()), - zap.String("path", fromStr), - zap.Error(err), - ) - } else { - plog.Errorf("failed to parse from %s into ID (%v)", fromStr, err) - } + h.lg.Warn( + "failed to parse path into ID", + zap.String("local-member-id", h.tr.ID.String()), + zap.String("remote-peer-id-stream-handler", h.id.String()), + zap.String("path", fromStr), + zap.Error(err), + ) http.Error(w, "invalid from", http.StatusNotFound) return } if h.r.IsIDRemoved(uint64(from)) { - if h.lg != nil { - h.lg.Warn( - "rejected stream from remote peer because it was removed", - zap.String("local-member-id", h.tr.ID.String()), - zap.String("remote-peer-id-stream-handler", h.id.String()), - zap.String("remote-peer-id-from", from.String()), - ) - } else { - plog.Warningf("rejected the stream from peer %s since it was removed", from) - } + h.lg.Warn( + "rejected stream from remote peer because it was removed", + zap.String("local-member-id", h.tr.ID.String()), + zap.String("remote-peer-id-stream-handler", h.id.String()), + zap.String("remote-peer-id-from", from.String()), + ) http.Error(w, "removed member", http.StatusGone) return } @@ -440,35 +404,27 @@ func (h *streamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if urls := r.Header.Get("X-PeerURLs"); urls != "" { h.tr.AddRemote(from, strings.Split(urls, ",")) } - if h.lg != nil { - h.lg.Warn( - "failed to find remote peer in cluster", - zap.String("local-member-id", h.tr.ID.String()), - zap.String("remote-peer-id-stream-handler", h.id.String()), - zap.String("remote-peer-id-from", from.String()), - zap.String("cluster-id", h.cid.String()), - ) - } else { - plog.Errorf("failed to find member %s in cluster %s", from, h.cid) - } + h.lg.Warn( + "failed to find remote peer in cluster", + zap.String("local-member-id", h.tr.ID.String()), + zap.String("remote-peer-id-stream-handler", h.id.String()), + zap.String("remote-peer-id-from", from.String()), + zap.String("cluster-id", h.cid.String()), + ) http.Error(w, "error sender not found", http.StatusNotFound) return } wto := h.id.String() if gto := r.Header.Get("X-Raft-To"); gto != wto { - if h.lg != nil { - h.lg.Warn( - "ignored streaming request; ID mismatch", - zap.String("local-member-id", h.tr.ID.String()), - zap.String("remote-peer-id-stream-handler", h.id.String()), - zap.String("remote-peer-id-header", gto), - zap.String("remote-peer-id-from", from.String()), - zap.String("cluster-id", h.cid.String()), - ) - } else { - plog.Errorf("streaming request ignored (ID mismatch got %s want %s)", gto, wto) - } + h.lg.Warn( + "ignored streaming request; ID mismatch", + zap.String("local-member-id", h.tr.ID.String()), + zap.String("remote-peer-id-stream-handler", h.id.String()), + zap.String("remote-peer-id-header", gto), + zap.String("remote-peer-id-from", from.String()), + zap.String("cluster-id", h.cid.String()), + ) http.Error(w, "to field mismatch", http.StatusPreconditionFailed) return } @@ -521,39 +477,31 @@ func checkClusterCompatibilityFromHeader(lg *zap.Logger, localID types.ID, heade } if err != nil { - if lg != nil { - lg.Warn( - "failed to check version compatibility", - zap.String("local-member-id", localID.String()), - zap.String("local-member-cluster-id", cid.String()), - zap.String("local-member-server-version", localVs), - zap.String("local-member-server-minimum-cluster-version", localMinClusterVs), - zap.String("remote-peer-server-name", remoteName), - zap.String("remote-peer-server-version", remoteVs), - zap.String("remote-peer-server-minimum-cluster-version", remoteMinClusterVs), - zap.Error(err), - ) - } else { - plog.Errorf("request version incompatibility (%v)", err) - } + lg.Warn( + "failed to check version compatibility", + zap.String("local-member-id", localID.String()), + zap.String("local-member-cluster-id", cid.String()), + zap.String("local-member-server-version", localVs), + zap.String("local-member-server-minimum-cluster-version", localMinClusterVs), + zap.String("remote-peer-server-name", remoteName), + zap.String("remote-peer-server-version", remoteVs), + zap.String("remote-peer-server-minimum-cluster-version", remoteMinClusterVs), + zap.Error(err), + ) return errIncompatibleVersion } if gcid := header.Get("X-Etcd-Cluster-ID"); gcid != cid.String() { - if lg != nil { - lg.Warn( - "request cluster ID mismatch", - zap.String("local-member-id", localID.String()), - zap.String("local-member-cluster-id", cid.String()), - zap.String("local-member-server-version", localVs), - zap.String("local-member-server-minimum-cluster-version", localMinClusterVs), - zap.String("remote-peer-server-name", remoteName), - zap.String("remote-peer-server-version", remoteVs), - zap.String("remote-peer-server-minimum-cluster-version", remoteMinClusterVs), - zap.String("remote-peer-cluster-id", gcid), - ) - } else { - plog.Errorf("request cluster ID mismatch (got %s want %s)", gcid, cid) - } + lg.Warn( + "request cluster ID mismatch", + zap.String("local-member-id", localID.String()), + zap.String("local-member-cluster-id", cid.String()), + zap.String("local-member-server-version", localVs), + zap.String("local-member-server-minimum-cluster-version", localMinClusterVs), + zap.String("remote-peer-server-name", remoteName), + zap.String("remote-peer-server-version", remoteVs), + zap.String("remote-peer-server-minimum-cluster-version", remoteMinClusterVs), + zap.String("remote-peer-cluster-id", gcid), + ) return errClusterIDMismatch } return nil diff --git a/etcdserver/api/rafthttp/peer.go b/etcdserver/api/rafthttp/peer.go index 8130c4a96b2..d6783c5d6e4 100644 --- a/etcdserver/api/rafthttp/peer.go +++ b/etcdserver/api/rafthttp/peer.go @@ -126,14 +126,10 @@ type peer struct { func startPeer(t *Transport, urls types.URLs, peerID types.ID, fs *stats.FollowerStats) *peer { if t.Logger != nil { t.Logger.Info("starting remote peer", zap.String("remote-peer-id", peerID.String())) - } else { - plog.Infof("starting peer %s...", peerID) } defer func() { if t.Logger != nil { t.Logger.Info("started remote peer", zap.String("remote-peer-id", peerID.String())) - } else { - plog.Infof("started peer %s", peerID) } }() @@ -177,8 +173,6 @@ func startPeer(t *Transport, urls types.URLs, peerID types.ID, fs *stats.Followe if err := r.Process(ctx, mm); err != nil { if t.Logger != nil { t.Logger.Warn("failed to process Raft message", zap.Error(err)) - } else { - plog.Warningf("failed to process raft message (%v)", err) } } case <-p.stopc: @@ -195,7 +189,9 @@ func startPeer(t *Transport, urls types.URLs, peerID types.ID, fs *stats.Followe select { case mm := <-p.propc: if err := r.Process(ctx, mm); err != nil { - plog.Warningf("failed to process raft message (%v)", err) + if t.Logger != nil { + t.Logger.Warn("failed to process Raft message", zap.Error(err)) + } } case <-p.stopc: return @@ -257,10 +253,9 @@ func (p *peer) send(m raftpb.Message) { zap.String("local-member-id", p.localID.String()), zap.String("from", types.ID(m.From).String()), zap.String("remote-peer-id", p.id.String()), + zap.String("remote-peer-name", name), zap.Bool("remote-peer-active", p.status.isActive()), ) - } else { - plog.MergeWarningf("dropped internal raft message to %s since %s's sending buffer is full (bad/overloaded network)", p.id, name) } } else { if p.lg != nil { @@ -270,10 +265,9 @@ func (p *peer) send(m raftpb.Message) { zap.String("local-member-id", p.localID.String()), zap.String("from", types.ID(m.From).String()), zap.String("remote-peer-id", p.id.String()), + zap.String("remote-peer-name", name), zap.Bool("remote-peer-active", p.status.isActive()), ) - } else { - plog.Debugf("dropped %s to %s since %s's sending buffer is full", m.Type, p.id, name) } } sentFailures.WithLabelValues(types.ID(m.To).String()).Inc() @@ -298,8 +292,6 @@ func (p *peer) attachOutgoingConn(conn *outgoingConn) { default: if p.lg != nil { p.lg.Panic("unknown stream type", zap.String("type", conn.t.String())) - } else { - plog.Panicf("unhandled stream type %s", conn.t) } } if !ok { @@ -331,15 +323,11 @@ func (p *peer) Resume() { func (p *peer) stop() { if p.lg != nil { p.lg.Info("stopping remote peer", zap.String("remote-peer-id", p.id.String())) - } else { - plog.Infof("stopping peer %s...", p.id) } defer func() { if p.lg != nil { p.lg.Info("stopped remote peer", zap.String("remote-peer-id", p.id.String())) - } else { - plog.Infof("stopped peer %s", p.id) } }() diff --git a/etcdserver/api/rafthttp/peer_status.go b/etcdserver/api/rafthttp/peer_status.go index 66149ff67d2..78e59ac7691 100644 --- a/etcdserver/api/rafthttp/peer_status.go +++ b/etcdserver/api/rafthttp/peer_status.go @@ -40,6 +40,9 @@ type peerStatus struct { } func newPeerStatus(lg *zap.Logger, local, id types.ID) *peerStatus { + if lg == nil { + lg = zap.NewNop() + } return &peerStatus{lg: lg, local: local, id: id} } @@ -47,11 +50,7 @@ func (s *peerStatus) activate() { s.mu.Lock() defer s.mu.Unlock() if !s.active { - if s.lg != nil { - s.lg.Info("peer became active", zap.String("peer-id", s.id.String())) - } else { - plog.Infof("peer %s became active", s.id) - } + s.lg.Info("peer became active", zap.String("peer-id", s.id.String())) s.active = true s.since = time.Now() @@ -64,12 +63,7 @@ func (s *peerStatus) deactivate(failure failureType, reason string) { defer s.mu.Unlock() msg := fmt.Sprintf("failed to %s %s on %s (%s)", failure.action, s.id, failure.source, reason) if s.active { - if s.lg != nil { - s.lg.Warn("peer became inactive (message send to peer failed)", zap.String("peer-id", s.id.String()), zap.Error(errors.New(msg))) - } else { - plog.Errorf(msg) - plog.Infof("peer %s became inactive (message send to peer failed)", s.id) - } + s.lg.Warn("peer became inactive (message send to peer failed)", zap.String("peer-id", s.id.String()), zap.Error(errors.New(msg))) s.active = false s.since = time.Time{} diff --git a/etcdserver/api/rafthttp/pipeline.go b/etcdserver/api/rafthttp/pipeline.go index 70f92575d13..05d8c67256d 100644 --- a/etcdserver/api/rafthttp/pipeline.go +++ b/etcdserver/api/rafthttp/pipeline.go @@ -73,8 +73,6 @@ func (p *pipeline) start() { zap.String("local-member-id", p.tr.ID.String()), zap.String("remote-peer-id", p.peerID.String()), ) - } else { - plog.Infof("started HTTP pipelining with peer %s", p.peerID) } } @@ -88,8 +86,6 @@ func (p *pipeline) stop() { zap.String("local-member-id", p.tr.ID.String()), zap.String("remote-peer-id", p.peerID.String()), ) - } else { - plog.Infof("stopped HTTP pipelining with peer %s", p.peerID) } } @@ -135,7 +131,7 @@ func (p *pipeline) handle() { // error on any failure. func (p *pipeline) post(data []byte) (err error) { u := p.picker.pick() - req := createPostRequest(u, RaftPrefix, bytes.NewBuffer(data), "application/protobuf", p.tr.URLs, p.tr.ID, p.tr.ClusterID) + req := createPostRequest(p.tr.Logger, u, RaftPrefix, bytes.NewBuffer(data), "application/protobuf", p.tr.URLs, p.tr.ID, p.tr.ClusterID) done := make(chan struct{}, 1) ctx, cancel := context.WithCancel(context.Background()) @@ -162,7 +158,7 @@ func (p *pipeline) post(data []byte) (err error) { return err } - err = checkPostResponse(resp, b, req, p.peerID) + err = checkPostResponse(p.tr.Logger, resp, b, req, p.peerID) if err != nil { p.picker.unreachable(u) // errMemberRemoved is a critical error since a removed member should diff --git a/etcdserver/api/rafthttp/probing_status.go b/etcdserver/api/rafthttp/probing_status.go index 474d9a0e437..672a579ce62 100644 --- a/etcdserver/api/rafthttp/probing_status.go +++ b/etcdserver/api/rafthttp/probing_status.go @@ -49,9 +49,7 @@ func addPeerToProber(lg *zap.Logger, p probing.Prober, id string, us []string, r s, err := p.Status(id) if err != nil { if lg != nil { - lg.Warn("failed to add peer into prober", zap.String("remote-peer-id", id)) - } else { - plog.Errorf("failed to add peer %s into prober", id) + lg.Warn("failed to add peer into prober", zap.String("remote-peer-id", id), zap.Error(err)) } return } @@ -74,8 +72,6 @@ func monitorProbingStatus(lg *zap.Logger, s probing.Status, id string, roundTrip zap.Duration("rtt", s.SRTT()), zap.Error(s.Err()), ) - } else { - plog.Warningf("health check for peer %s could not connect: %v", id, s.Err()) } interval = statusErrorInterval } else { @@ -91,8 +87,6 @@ func monitorProbingStatus(lg *zap.Logger, s probing.Status, id string, roundTrip zap.Duration("rtt", s.SRTT()), zap.Error(s.Err()), ) - } else { - plog.Warningf("the clock difference against peer %s is too high [%v > %v]", id, s.ClockDiff(), time.Second) } } rttSecProm.WithLabelValues(id).Observe(s.SRTT().Seconds()) diff --git a/etcdserver/api/rafthttp/remote.go b/etcdserver/api/rafthttp/remote.go index 1ef2493ed45..92b29b96c3d 100644 --- a/etcdserver/api/rafthttp/remote.go +++ b/etcdserver/api/rafthttp/remote.go @@ -65,8 +65,6 @@ func (g *remote) send(m raftpb.Message) { zap.String("remote-peer-id", g.id.String()), zap.Bool("remote-peer-active", g.status.isActive()), ) - } else { - plog.MergeWarningf("dropped internal raft message to %s since sending buffer is full (bad/overloaded network)", g.id) } } else { if g.lg != nil { @@ -78,8 +76,6 @@ func (g *remote) send(m raftpb.Message) { zap.String("remote-peer-id", g.id.String()), zap.Bool("remote-peer-active", g.status.isActive()), ) - } else { - plog.Debugf("dropped %s to %s since sending buffer is full", m.Type, g.id) } } sentFailures.WithLabelValues(types.ID(m.To).String()).Inc() diff --git a/etcdserver/api/rafthttp/snapshot_sender.go b/etcdserver/api/rafthttp/snapshot_sender.go index 62efb0cdc3d..99d27f91af9 100644 --- a/etcdserver/api/rafthttp/snapshot_sender.go +++ b/etcdserver/api/rafthttp/snapshot_sender.go @@ -76,7 +76,7 @@ func (s *snapshotSender) send(merged snap.Message) { defer body.Close() u := s.picker.pick() - req := createPostRequest(u, RaftSnapshotPrefix, body, "application/octet-stream", s.tr.URLs, s.from, s.cid) + req := createPostRequest(s.tr.Logger, u, RaftSnapshotPrefix, body, "application/octet-stream", s.tr.URLs, s.from, s.cid) if s.tr.Logger != nil { s.tr.Logger.Info( @@ -86,8 +86,6 @@ func (s *snapshotSender) send(merged snap.Message) { zap.Int64("bytes", merged.TotalSize), zap.String("size", humanize.Bytes(uint64(merged.TotalSize))), ) - } else { - plog.Infof("start to send database snapshot [index: %d, to %s]...", m.Snapshot.Metadata.Index, types.ID(m.To)) } snapshotSendInflights.WithLabelValues(to).Inc() @@ -107,8 +105,6 @@ func (s *snapshotSender) send(merged snap.Message) { zap.String("size", humanize.Bytes(uint64(merged.TotalSize))), zap.Error(err), ) - } else { - plog.Warningf("database snapshot [index: %d, to: %s] failed to be sent out (%v)", m.Snapshot.Metadata.Index, types.ID(m.To), err) } // errMemberRemoved is a critical error since a removed member should @@ -139,8 +135,6 @@ func (s *snapshotSender) send(merged snap.Message) { zap.Int64("bytes", merged.TotalSize), zap.String("size", humanize.Bytes(uint64(merged.TotalSize))), ) - } else { - plog.Infof("database snapshot [index: %d, to: %s] sent out successfully", m.Snapshot.Metadata.Index, types.ID(m.To)) } sentBytes.WithLabelValues(to).Add(float64(merged.TotalSize)) @@ -184,7 +178,7 @@ func (s *snapshotSender) post(req *http.Request) (err error) { if r.err != nil { return r.err } - return checkPostResponse(r.resp, r.body, req, s.to) + return checkPostResponse(s.tr.Logger, r.resp, r.body, req, s.to) } } @@ -195,8 +189,6 @@ func createSnapBody(lg *zap.Logger, merged snap.Message) io.ReadCloser { if err := enc.encode(&merged.Message); err != nil { if lg != nil { lg.Panic("failed to encode message", zap.Error(err)) - } else { - plog.Panicf("encode message error (%v)", err) } } diff --git a/etcdserver/api/rafthttp/stream.go b/etcdserver/api/rafthttp/stream.go index ba9b2e8fea3..f49d47d427f 100644 --- a/etcdserver/api/rafthttp/stream.go +++ b/etcdserver/api/rafthttp/stream.go @@ -64,14 +64,16 @@ var ( type streamType string -func (t streamType) endpoint() string { +func (t streamType) endpoint(lg *zap.Logger) string { switch t { case streamTypeMsgAppV2: return path.Join(RaftStreamPrefix, "msgapp") case streamTypeMessage: return path.Join(RaftStreamPrefix, "message") default: - plog.Panicf("unhandled stream type %v", t) + if lg != nil { + lg.Panic("unhandled stream type", zap.String("stream-type", t.String())) + } return "" } } @@ -169,8 +171,6 @@ func (cw *streamWriter) run() { zap.String("local-member-id", cw.localID.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Infof("started streaming with peer %s (writer)", cw.peerID) } for { @@ -197,8 +197,6 @@ func (cw *streamWriter) run() { zap.String("local-member-id", cw.localID.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Warningf("lost the TCP streaming connection with peer %s (%s writer)", cw.peerID, t) } heartbeatc, msgc = nil, nil @@ -228,8 +226,6 @@ func (cw *streamWriter) run() { zap.String("local-member-id", cw.localID.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Warningf("lost the TCP streaming connection with peer %s (%s writer)", cw.peerID, t) } heartbeatc, msgc = nil, nil cw.r.ReportUnreachable(m.To) @@ -245,7 +241,9 @@ func (cw *streamWriter) run() { case streamTypeMessage: enc = &messageEncoder{w: conn.Writer} default: - plog.Panicf("unhandled stream type %s", conn.t) + if cw.lg != nil { + cw.lg.Panic("unhandled stream type", zap.String("stream-type", t.String())) + } } if cw.lg != nil { cw.lg.Info( @@ -270,19 +268,15 @@ func (cw *streamWriter) run() { zap.String("local-member-id", cw.localID.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Warningf("closed an existing TCP streaming connection with peer %s (%s writer)", cw.peerID, t) } } if cw.lg != nil { - cw.lg.Warn( + cw.lg.Info( "established TCP streaming connection with remote peer", zap.String("stream-writer-type", t.String()), zap.String("local-member-id", cw.localID.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Infof("established a TCP streaming connection with peer %s (%s writer)", cw.peerID, t) } heartbeatc, msgc = tickc.C, cw.msgc @@ -294,18 +288,14 @@ func (cw *streamWriter) run() { zap.String("stream-writer-type", t.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Infof("closed the TCP streaming connection with peer %s (%s writer)", cw.peerID, t) } } if cw.lg != nil { - cw.lg.Warn( + cw.lg.Info( "stopped TCP streaming connection with remote peer", zap.String("stream-writer-type", t.String()), zap.String("remote-peer-id", cw.peerID.String()), ) - } else { - plog.Infof("stopped streaming with peer %s (writer)", cw.peerID) } close(cw.done) return @@ -336,8 +326,6 @@ func (cw *streamWriter) closeUnlocked() bool { zap.String("remote-peer-id", cw.peerID.String()), zap.Error(err), ) - } else { - plog.Errorf("peer %s (writer) connection close error: %v", cw.peerID, err) } } if len(cw.msgc) > 0 { @@ -410,8 +398,6 @@ func (cr *streamReader) run() { zap.String("local-member-id", cr.tr.ID.String()), zap.String("remote-peer-id", cr.peerID.String()), ) - } else { - plog.Infof("started streaming with peer %s (%s reader)", cr.peerID, t) } for { @@ -429,8 +415,6 @@ func (cr *streamReader) run() { zap.String("local-member-id", cr.tr.ID.String()), zap.String("remote-peer-id", cr.peerID.String()), ) - } else { - plog.Infof("established a TCP streaming connection with peer %s (%s reader)", cr.peerID, cr.typ) } err = cr.decodeLoop(rc, t) if cr.lg != nil { @@ -441,8 +425,6 @@ func (cr *streamReader) run() { zap.String("remote-peer-id", cr.peerID.String()), zap.Error(err), ) - } else { - plog.Warningf("lost the TCP streaming connection with peer %s (%s reader)", cr.peerID, cr.typ) } switch { // all data is read out @@ -463,8 +445,6 @@ func (cr *streamReader) run() { zap.String("local-member-id", cr.tr.ID.String()), zap.String("remote-peer-id", cr.peerID.String()), ) - } else { - plog.Infof("stopped streaming with peer %s (%s reader)", cr.peerID, t) } close(cr.done) return @@ -478,8 +458,6 @@ func (cr *streamReader) run() { zap.String("remote-peer-id", cr.peerID.String()), zap.Error(err), ) - } else { - plog.Errorf("streaming with peer %s (%s reader) rate limiter error: %v", cr.peerID, t, err) } } } @@ -496,8 +474,6 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error { default: if cr.lg != nil { cr.lg.Panic("unknown stream type", zap.String("type", t.String())) - } else { - plog.Panicf("unhandled stream type %s", t) } } select { @@ -559,8 +535,6 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error { zap.String("remote-peer-id", types.ID(m.To).String()), zap.Bool("remote-peer-active", cr.status.isActive()), ) - } else { - plog.MergeWarningf("dropped internal raft message from %s since receiving buffer is full (overloaded network)", types.ID(m.From)) } } else { if cr.lg != nil { @@ -572,8 +546,6 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error { zap.String("remote-peer-id", types.ID(m.To).String()), zap.Bool("remote-peer-active", cr.status.isActive()), ) - } else { - plog.Debugf("dropped %s from %s since receiving buffer is full", m.Type, types.ID(m.From)) } } recvFailures.WithLabelValues(types.ID(m.From).String()).Inc() @@ -592,7 +564,7 @@ func (cr *streamReader) stop() { func (cr *streamReader) dial(t streamType) (io.ReadCloser, error) { u := cr.picker.pick() uu := u - uu.Path = path.Join(t.endpoint(), cr.tr.ID.String()) + uu.Path = path.Join(t.endpoint(cr.lg), cr.tr.ID.String()) if cr.lg != nil { cr.lg.Debug( @@ -673,8 +645,6 @@ func (cr *streamReader) dial(t streamType) (io.ReadCloser, error) { zap.String("remote-peer-id", cr.peerID.String()), zap.Error(errIncompatibleVersion), ) - } else { - plog.Errorf("request sent was ignored by peer %s (server version incompatible)", cr.peerID) } return nil, errIncompatibleVersion @@ -688,9 +658,6 @@ func (cr *streamReader) dial(t streamType) (io.ReadCloser, error) { zap.String("local-member-cluster-id", cr.tr.ClusterID.String()), zap.Error(errClusterIDMismatch), ) - } else { - plog.Errorf("request sent was ignored (cluster ID mismatch: peer[%s]=%s, local=%s)", - cr.peerID, resp.Header.Get("X-Etcd-Cluster-ID"), cr.tr.ClusterID) } return nil, errClusterIDMismatch @@ -715,8 +682,6 @@ func (cr *streamReader) close() { zap.String("remote-peer-id", cr.peerID.String()), zap.Error(err), ) - } else { - plog.Errorf("peer %s (reader) connection close error: %v", cr.peerID, err) } } } diff --git a/etcdserver/api/rafthttp/stream_test.go b/etcdserver/api/rafthttp/stream_test.go index 21d69fe5375..b7fca99238b 100644 --- a/etcdserver/api/rafthttp/stream_test.go +++ b/etcdserver/api/rafthttp/stream_test.go @@ -127,7 +127,7 @@ func TestStreamReaderDialRequest(t *testing.T) { } req := act[0].Params[0].(*http.Request) - wurl := fmt.Sprintf("http://localhost:2380" + tt.endpoint() + "/1") + wurl := fmt.Sprintf("http://localhost:2380" + tt.endpoint(zap.NewExample()) + "/1") if req.URL.String() != wurl { t.Errorf("#%d: url = %s, want %s", i, req.URL.String(), wurl) } diff --git a/etcdserver/api/rafthttp/transport.go b/etcdserver/api/rafthttp/transport.go index 7191c3d6063..4de6a677283 100644 --- a/etcdserver/api/rafthttp/transport.go +++ b/etcdserver/api/rafthttp/transport.go @@ -22,20 +22,16 @@ import ( "go.etcd.io/etcd/etcdserver/api/snap" stats "go.etcd.io/etcd/etcdserver/api/v2stats" - "go.etcd.io/etcd/pkg/logutil" "go.etcd.io/etcd/pkg/transport" "go.etcd.io/etcd/pkg/types" "go.etcd.io/etcd/raft" "go.etcd.io/etcd/raft/raftpb" - "github.com/coreos/pkg/capnslog" "github.com/xiang90/probing" "go.uber.org/zap" "golang.org/x/time/rate" ) -var plog = logutil.NewMergeLogger(capnslog.NewPackageLogger("go.etcd.io/etcd", "rafthttp")) - type Raft interface { Process(ctx context.Context, m raftpb.Message) error IsIDRemoved(id uint64) bool @@ -208,8 +204,6 @@ func (t *Transport) Send(msgs []raftpb.Message) { zap.String("type", m.Type.String()), zap.String("unknown-target-peer-id", to.String()), ) - } else { - plog.Debugf("ignored message %s (sent to unknown peer %s)", m.Type, to) } } } @@ -284,8 +278,6 @@ func (t *Transport) AddRemote(id types.ID, us []string) { if err != nil { if t.Logger != nil { t.Logger.Panic("failed NewURLs", zap.Strings("urls", us), zap.Error(err)) - } else { - plog.Panicf("newURLs %+v should never fail: %+v", us, err) } } t.remotes[id] = startRemote(t, urls, id) @@ -314,8 +306,6 @@ func (t *Transport) AddPeer(id types.ID, us []string) { if err != nil { if t.Logger != nil { t.Logger.Panic("failed NewURLs", zap.Strings("urls", us), zap.Error(err)) - } else { - plog.Panicf("newURLs %+v should never fail: %+v", us, err) } } fs := t.LeaderStats.Follower(id.String()) @@ -330,8 +320,6 @@ func (t *Transport) AddPeer(id types.ID, us []string) { zap.String("remote-peer-id", id.String()), zap.Strings("remote-peer-urls", us), ) - } else { - plog.Infof("added peer %s", id) } } @@ -356,8 +344,6 @@ func (t *Transport) removePeer(id types.ID) { } else { if t.Logger != nil { t.Logger.Panic("unexpected removal of unknown remote peer", zap.String("remote-peer-id", id.String())) - } else { - plog.Panicf("unexpected removal of unknown peer '%d'", id) } } delete(t.peers, id) @@ -371,8 +357,6 @@ func (t *Transport) removePeer(id types.ID) { zap.String("local-member-id", t.ID.String()), zap.String("removed-remote-peer-id", id.String()), ) - } else { - plog.Infof("removed peer %s", id) } } @@ -387,8 +371,6 @@ func (t *Transport) UpdatePeer(id types.ID, us []string) { if err != nil { if t.Logger != nil { t.Logger.Panic("failed NewURLs", zap.Strings("urls", us), zap.Error(err)) - } else { - plog.Panicf("newURLs %+v should never fail: %+v", us, err) } } t.peers[id].update(urls) @@ -405,8 +387,6 @@ func (t *Transport) UpdatePeer(id types.ID, us []string) { zap.String("updated-remote-peer-id", id.String()), zap.Strings("updated-remote-peer-urls", us), ) - } else { - plog.Infof("updated peer %s", id) } } diff --git a/etcdserver/api/rafthttp/transport_bench_test.go b/etcdserver/api/rafthttp/transport_bench_test.go index 9e6aabd4fd4..64c9acc7d4b 100644 --- a/etcdserver/api/rafthttp/transport_bench_test.go +++ b/etcdserver/api/rafthttp/transport_bench_test.go @@ -25,6 +25,8 @@ import ( "go.etcd.io/etcd/pkg/types" "go.etcd.io/etcd/raft" "go.etcd.io/etcd/raft/raftpb" + + "go.uber.org/zap" ) func BenchmarkSendingMsgApp(b *testing.B) { @@ -34,7 +36,7 @@ func BenchmarkSendingMsgApp(b *testing.B) { ClusterID: types.ID(1), Raft: &fakeRaft{}, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("1"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "1"), } tr.Start() srv := httptest.NewServer(tr.Handler()) @@ -47,7 +49,7 @@ func BenchmarkSendingMsgApp(b *testing.B) { ClusterID: types.ID(1), Raft: r, ServerStats: newServerStats(), - LeaderStats: stats.NewLeaderStats("2"), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), "2"), } tr2.Start() srv2 := httptest.NewServer(tr2.Handler()) diff --git a/etcdserver/api/rafthttp/transport_test.go b/etcdserver/api/rafthttp/transport_test.go index 7c8641fec4f..e48b4c62b80 100644 --- a/etcdserver/api/rafthttp/transport_test.go +++ b/etcdserver/api/rafthttp/transport_test.go @@ -26,6 +26,7 @@ import ( "go.etcd.io/etcd/raft/raftpb" "github.com/xiang90/probing" + "go.uber.org/zap" ) // TestTransportSend tests that transport can send messages using correct @@ -95,7 +96,7 @@ func TestTransportCutMend(t *testing.T) { } func TestTransportAdd(t *testing.T) { - ls := stats.NewLeaderStats("") + ls := stats.NewLeaderStats(zap.NewExample(), "") tr := &Transport{ LeaderStats: ls, streamRt: &roundTripperRecorder{}, @@ -126,7 +127,7 @@ func TestTransportAdd(t *testing.T) { func TestTransportRemove(t *testing.T) { tr := &Transport{ - LeaderStats: stats.NewLeaderStats(""), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), ""), streamRt: &roundTripperRecorder{}, peers: make(map[types.ID]Peer), pipelineProber: probing.NewProber(nil), @@ -160,7 +161,7 @@ func TestTransportErrorc(t *testing.T) { errorc := make(chan error, 1) tr := &Transport{ Raft: &fakeRaft{}, - LeaderStats: stats.NewLeaderStats(""), + LeaderStats: stats.NewLeaderStats(zap.NewExample(), ""), ErrorC: errorc, streamRt: newRespRoundTripper(http.StatusForbidden, nil), pipelineRt: newRespRoundTripper(http.StatusForbidden, nil), diff --git a/etcdserver/api/rafthttp/util.go b/etcdserver/api/rafthttp/util.go index 20938647c7a..d63f9e2ba67 100644 --- a/etcdserver/api/rafthttp/util.go +++ b/etcdserver/api/rafthttp/util.go @@ -28,6 +28,7 @@ import ( "go.etcd.io/etcd/version" "github.com/coreos/go-semver/semver" + "go.uber.org/zap" ) var ( @@ -60,12 +61,14 @@ func newStreamRoundTripper(tlsInfo transport.TLSInfo, dialTimeout time.Duration) } // createPostRequest creates a HTTP POST request that sends raft message. -func createPostRequest(u url.URL, path string, body io.Reader, ct string, urls types.URLs, from, cid types.ID) *http.Request { +func createPostRequest(lg *zap.Logger, u url.URL, path string, body io.Reader, ct string, urls types.URLs, from, cid types.ID) *http.Request { uu := u uu.Path = path req, err := http.NewRequest("POST", uu.String(), body) if err != nil { - plog.Panicf("unexpected new request error (%v)", err) + if lg != nil { + lg.Panic("unexpected new request error", zap.Error(err)) + } } req.Header.Set("Content-Type", ct) req.Header.Set("X-Server-From", from.String()) @@ -79,16 +82,27 @@ func createPostRequest(u url.URL, path string, body io.Reader, ct string, urls t // checkPostResponse checks the response of the HTTP POST request that sends // raft message. -func checkPostResponse(resp *http.Response, body []byte, req *http.Request, to types.ID) error { +func checkPostResponse(lg *zap.Logger, resp *http.Response, body []byte, req *http.Request, to types.ID) error { switch resp.StatusCode { case http.StatusPreconditionFailed: switch strings.TrimSuffix(string(body), "\n") { case errIncompatibleVersion.Error(): - plog.Errorf("request sent was ignored by peer %s (server version incompatible)", to) + if lg != nil { + lg.Error( + "request sent was ignored by peer", + zap.String("remote-peer-id", to.String()), + ) + } return errIncompatibleVersion case errClusterIDMismatch.Error(): - plog.Errorf("request sent was ignored (cluster ID mismatch: remote[%s]=%s, local=%s)", - to, resp.Header.Get("X-Etcd-Cluster-ID"), req.Header.Get("X-Etcd-Cluster-ID")) + if lg != nil { + lg.Error( + "request sent was ignored due to cluster ID mismatch", + zap.String("remote-peer-id", to.String()), + zap.String("remote-peer-cluster-id", resp.Header.Get("X-Etcd-Cluster-ID")), + zap.String("local-member-cluster-id", req.Header.Get("X-Etcd-Cluster-ID")), + ) + } return errClusterIDMismatch default: return fmt.Errorf("unhandled error %q when precondition failed", string(body)) diff --git a/etcdserver/api/snap/db.go b/etcdserver/api/snap/db.go index 3002ccdccea..121e7a8c1aa 100644 --- a/etcdserver/api/snap/db.go +++ b/etcdserver/api/snap/db.go @@ -63,16 +63,12 @@ func (s *Snapshotter) SaveDBFrom(r io.Reader, id uint64) (int64, error) { return n, err } - if s.lg != nil { - s.lg.Info( - "saved database snapshot to disk", - zap.String("path", fn), - zap.Int64("bytes", n), - zap.String("size", humanize.Bytes(uint64(n))), - ) - } else { - plog.Infof("saved database snapshot to disk [total bytes: %d]", n) - } + s.lg.Info( + "saved database snapshot to disk", + zap.String("path", fn), + zap.Int64("bytes", n), + zap.String("size", humanize.Bytes(uint64(n))), + ) snapDBSaveSec.Observe(time.Since(start).Seconds()) return n, nil diff --git a/etcdserver/api/snap/snapshotter.go b/etcdserver/api/snap/snapshotter.go index 7e7933374c9..a933b3519b3 100644 --- a/etcdserver/api/snap/snapshotter.go +++ b/etcdserver/api/snap/snapshotter.go @@ -31,15 +31,12 @@ import ( "go.etcd.io/etcd/raft" "go.etcd.io/etcd/raft/raftpb" - "github.com/coreos/pkg/capnslog" "go.uber.org/zap" ) const snapSuffix = ".snap" var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd/v3", "snap") - ErrNoSnapshot = errors.New("snap: no available snapshot") ErrEmptySnapshot = errors.New("snap: empty snapshot") ErrCRCMismatch = errors.New("snap: crc mismatch") @@ -57,6 +54,9 @@ type Snapshotter struct { } func New(lg *zap.Logger, dir string) *Snapshotter { + if lg == nil { + lg = zap.NewNop() + } return &Snapshotter{ lg: lg, dir: dir, @@ -90,16 +90,10 @@ func (s *Snapshotter) save(snapshot *raftpb.Snapshot) error { snapFsyncSec.Observe(time.Since(fsyncStart).Seconds()) if err != nil { - if s.lg != nil { - s.lg.Warn("failed to write a snap file", zap.String("path", spath), zap.Error(err)) - } + s.lg.Warn("failed to write a snap file", zap.String("path", spath), zap.Error(err)) rerr := os.Remove(spath) if rerr != nil { - if s.lg != nil { - s.lg.Warn("failed to remove a broken snap file", zap.String("path", spath), zap.Error(err)) - } else { - plog.Errorf("failed to remove broken snapshot file %s", spath) - } + s.lg.Warn("failed to remove a broken snap file", zap.String("path", spath), zap.Error(err)) } return err } @@ -136,8 +130,6 @@ func loadSnap(lg *zap.Logger, dir, name string) (*raftpb.Snapshot, error) { if rerr := os.Rename(fpath, brokenPath); rerr != nil { if lg != nil { lg.Warn("failed to rename a broken snap file", zap.String("path", fpath), zap.String("broken-path", brokenPath), zap.Error(rerr)) - } else { - plog.Warningf("cannot rename broken snapshot file %v to %v: %v", fpath, brokenPath, rerr) } } else { if lg != nil { @@ -154,8 +146,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { if err != nil { if lg != nil { lg.Warn("failed to read a snap file", zap.String("path", snapname), zap.Error(err)) - } else { - plog.Errorf("cannot read file %v: %v", snapname, err) } return nil, err } @@ -163,8 +153,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { if len(b) == 0 { if lg != nil { lg.Warn("failed to read empty snapshot file", zap.String("path", snapname)) - } else { - plog.Errorf("unexpected empty snapshot") } return nil, ErrEmptySnapshot } @@ -173,8 +161,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { if err = serializedSnap.Unmarshal(b); err != nil { if lg != nil { lg.Warn("failed to unmarshal snappb.Snapshot", zap.String("path", snapname), zap.Error(err)) - } else { - plog.Errorf("corrupted snapshot file %v: %v", snapname, err) } return nil, err } @@ -182,8 +168,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { if len(serializedSnap.Data) == 0 || serializedSnap.Crc == 0 { if lg != nil { lg.Warn("failed to read empty snapshot data", zap.String("path", snapname)) - } else { - plog.Errorf("unexpected empty snapshot") } return nil, ErrEmptySnapshot } @@ -196,8 +180,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { zap.Uint32("prev-crc", serializedSnap.Crc), zap.Uint32("new-crc", crc), ) - } else { - plog.Errorf("corrupted snapshot file %v: crc mismatch", snapname) } return nil, ErrCRCMismatch } @@ -206,8 +188,6 @@ func Read(lg *zap.Logger, snapname string) (*raftpb.Snapshot, error) { if err = snap.Unmarshal(serializedSnap.Data); err != nil { if lg != nil { lg.Warn("failed to unmarshal raftpb.Snapshot", zap.String("path", snapname), zap.Error(err)) - } else { - plog.Errorf("corrupted snapshot file %v: %v", snapname, err) } return nil, err } @@ -245,8 +225,6 @@ func checkSuffix(lg *zap.Logger, names []string) []string { if _, ok := validFiles[names[i]]; !ok { if lg != nil { lg.Warn("found unexpected non-snap file; skipping", zap.String("path", names[i])) - } else { - plog.Warningf("skipped unexpected non snapshot file %v", names[i]) } } } diff --git a/etcdserver/api/v2auth/auth.go b/etcdserver/api/v2auth/auth.go index b438074a449..a511501ed54 100644 --- a/etcdserver/api/v2auth/auth.go +++ b/etcdserver/api/v2auth/auth.go @@ -31,7 +31,6 @@ import ( "go.etcd.io/etcd/etcdserver/etcdserverpb" "go.etcd.io/etcd/pkg/types" - "github.com/coreos/pkg/capnslog" "go.uber.org/zap" "golang.org/x/crypto/bcrypt" ) @@ -47,10 +46,6 @@ const ( GuestRoleName = "guest" ) -var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd/v3", "etcdserver/auth") -) - var rootRole = Role{ Role: RootRoleName, Permissions: Permissions{ @@ -148,6 +143,9 @@ func authErr(hs int, s string, v ...interface{}) Error { } func NewStore(lg *zap.Logger, server doer, timeout time.Duration) Store { + if lg == nil { + lg = zap.NewNop() + } s := &store{ lg: lg, server: server, @@ -211,11 +209,7 @@ func (s *store) CreateUser(user User) (User, error) { } u, err := s.createUserInternal(user) if err == nil { - if s.lg != nil { - s.lg.Info("created a user", zap.String("user-name", user.User)) - } else { - plog.Noticef("created user %s", user.User) - } + s.lg.Info("created a user", zap.String("user-name", user.User)) } return u, err } @@ -254,11 +248,7 @@ func (s *store) DeleteUser(name string) error { } return err } - if s.lg != nil { - s.lg.Info("deleted a user", zap.String("user-name", name)) - } else { - plog.Noticef("deleted user %s", name) - } + s.lg.Info("deleted a user", zap.String("user-name", name)) return nil } @@ -282,11 +272,7 @@ func (s *store) UpdateUser(user User) (User, error) { } _, err = s.updateResource("/users/"+user.User, newUser) if err == nil { - if s.lg != nil { - s.lg.Info("updated a user", zap.String("user-name", user.User)) - } else { - plog.Noticef("updated user %s", user.User) - } + s.lg.Info("updated a user", zap.String("user-name", user.User)) } return newUser, err } @@ -325,11 +311,7 @@ func (s *store) CreateRole(role Role) error { } } if err == nil { - if s.lg != nil { - s.lg.Info("created a new role", zap.String("role-name", role.Role)) - } else { - plog.Noticef("created new role %s", role.Role) - } + s.lg.Info("created a new role", zap.String("role-name", role.Role)) } return err } @@ -347,11 +329,7 @@ func (s *store) DeleteRole(name string) error { } } if err == nil { - if s.lg != nil { - s.lg.Info("delete a new role", zap.String("role-name", name)) - } else { - plog.Noticef("deleted role %s", name) - } + s.lg.Info("delete a new role", zap.String("role-name", name)) } return err } @@ -378,11 +356,7 @@ func (s *store) UpdateRole(role Role) (Role, error) { } _, err = s.updateResource("/roles/"+role.Role, newRole) if err == nil { - if s.lg != nil { - s.lg.Info("updated a new role", zap.String("role-name", role.Role)) - } else { - plog.Noticef("updated role %s", role.Role) - } + s.lg.Info("updated a new role", zap.String("role-name", role.Role)) } return newRole, err } @@ -400,42 +374,26 @@ func (s *store) EnableAuth() error { return authErr(http.StatusConflict, "No root user available, please create one") } if _, err := s.getRole(GuestRoleName, true); err != nil { - if s.lg != nil { - s.lg.Info( - "no guest role access found; creating default", + s.lg.Info( + "no guest role access found; creating default", + zap.String("role-name", GuestRoleName), + ) + if err := s.CreateRole(guestRole); err != nil { + s.lg.Warn( + "failed to create a guest role; aborting auth enable", zap.String("role-name", GuestRoleName), + zap.Error(err), ) - } else { - plog.Printf("no guest role access found, creating default") - } - if err := s.CreateRole(guestRole); err != nil { - if s.lg != nil { - s.lg.Warn( - "failed to create a guest role; aborting auth enable", - zap.String("role-name", GuestRoleName), - zap.Error(err), - ) - } else { - plog.Errorf("error creating guest role. aborting auth enable.") - } return err } } if err := s.enableAuth(); err != nil { - if s.lg != nil { - s.lg.Warn("failed to enable auth", zap.Error(err)) - } else { - plog.Errorf("error enabling auth (%v)", err) - } + s.lg.Warn("failed to enable auth", zap.Error(err)) return err } - if s.lg != nil { - s.lg.Info("enabled auth") - } else { - plog.Noticef("auth: enabled auth") - } + s.lg.Info("enabled auth") return nil } @@ -446,17 +404,9 @@ func (s *store) DisableAuth() error { err := s.disableAuth() if err == nil { - if s.lg != nil { - s.lg.Info("disabled auth") - } else { - plog.Noticef("auth: disabled auth") - } + s.lg.Info("disabled auth") } else { - if s.lg != nil { - s.lg.Warn("failed to disable auth", zap.Error(err)) - } else { - plog.Errorf("error disabling auth (%v)", err) - } + s.lg.Warn("failed to disable auth", zap.Error(err)) } return err } @@ -483,30 +433,22 @@ func (ou User) merge(lg *zap.Logger, nu User, s PasswordStore) (User, error) { currentRoles := types.NewUnsafeSet(ou.Roles...) for _, g := range nu.Grant { if currentRoles.Contains(g) { - if lg != nil { - lg.Warn( - "attempted to grant a duplicate role for a user", - zap.String("user-name", nu.User), - zap.String("role-name", g), - ) - } else { - plog.Noticef("granting duplicate role %s for user %s", g, nu.User) - } + lg.Warn( + "attempted to grant a duplicate role for a user", + zap.String("user-name", nu.User), + zap.String("role-name", g), + ) return User{}, authErr(http.StatusConflict, fmt.Sprintf("Granting duplicate role %s for user %s", g, nu.User)) } currentRoles.Add(g) } for _, r := range nu.Revoke { if !currentRoles.Contains(r) { - if lg != nil { - lg.Warn( - "attempted to revoke a ungranted role for a user", - zap.String("user-name", nu.User), - zap.String("role-name", r), - ) - } else { - plog.Noticef("revoking ungranted role %s for user %s", r, nu.User) - } + lg.Warn( + "attempted to revoke a ungranted role for a user", + zap.String("user-name", nu.User), + zap.String("role-name", r), + ) return User{}, authErr(http.StatusConflict, fmt.Sprintf("Revoking ungranted role %s for user %s", r, nu.User)) } currentRoles.Remove(r) @@ -603,14 +545,10 @@ func (rw RWPermission) Revoke(lg *zap.Logger, n RWPermission) (RWPermission, err currentRead := types.NewUnsafeSet(rw.Read...) for _, r := range n.Read { if !currentRead.Contains(r) { - if lg != nil { - lg.Info( - "revoking ungranted read permission", - zap.String("read-permission", r), - ) - } else { - plog.Noticef("revoking ungranted read permission %s", r) - } + lg.Info( + "revoking ungranted read permission", + zap.String("read-permission", r), + ) continue } currentRead.Remove(r) @@ -618,14 +556,10 @@ func (rw RWPermission) Revoke(lg *zap.Logger, n RWPermission) (RWPermission, err currentWrite := types.NewUnsafeSet(rw.Write...) for _, w := range n.Write { if !currentWrite.Contains(w) { - if lg != nil { - lg.Info( - "revoking ungranted write permission", - zap.String("write-permission", w), - ) - } else { - plog.Noticef("revoking ungranted write permission %s", w) - } + lg.Info( + "revoking ungranted write permission", + zap.String("write-permission", w), + ) continue } currentWrite.Remove(w) diff --git a/etcdserver/api/v2auth/auth_requests.go b/etcdserver/api/v2auth/auth_requests.go index d6574ecca63..7de52b4aa6c 100644 --- a/etcdserver/api/v2auth/auth_requests.go +++ b/etcdserver/api/v2auth/auth_requests.go @@ -47,14 +47,10 @@ func (s *store) ensureAuthDirectories() error { continue } } - if s.lg != nil { - s.lg.Warn( - "failed to create auth directories", - zap.Error(err), - ) - } else { - plog.Errorf("failed to create auth directories in the store (%v)", err) - } + s.lg.Warn( + "failed to create auth directories", + zap.Error(err), + ) return err } } @@ -101,28 +97,20 @@ func (s *store) detectAuth() bool { return false } } - if s.lg != nil { - s.lg.Warn( - "failed to detect auth settings", - zap.Error(err), - ) - } else { - plog.Errorf("failed to detect auth settings (%s)", err) - } + s.lg.Warn( + "failed to detect auth settings", + zap.Error(err), + ) return false } var u bool err = json.Unmarshal([]byte(*value.Event.Node.Value), &u) if err != nil { - if s.lg != nil { - s.lg.Warn( - "internal bookkeeping value for enabled isn't valid JSON", - zap.Error(err), - ) - } else { - plog.Errorf("internal bookkeeping value for enabled isn't valid JSON (%v)", err) - } + s.lg.Warn( + "internal bookkeeping value for enabled isn't valid JSON", + zap.Error(err), + ) return false } return u diff --git a/etcdserver/api/v2discovery/discovery.go b/etcdserver/api/v2discovery/discovery.go index 16ccfde8e27..58e6f8d5599 100644 --- a/etcdserver/api/v2discovery/discovery.go +++ b/etcdserver/api/v2discovery/discovery.go @@ -33,14 +33,11 @@ import ( "go.etcd.io/etcd/pkg/transport" "go.etcd.io/etcd/pkg/types" - "github.com/coreos/pkg/capnslog" "github.com/jonboulle/clockwork" "go.uber.org/zap" ) var ( - plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "discovery") - ErrInvalidURL = errors.New("discovery: invalid URL") ErrBadSizeKey = errors.New("discovery: size key is bad") ErrSizeNotFound = errors.New("discovery: size key not found") @@ -93,6 +90,9 @@ type discovery struct { // represent a URL that can be used as a proxy. It performs basic // sanitization of the URL and returns any error encountered. func newProxyFunc(lg *zap.Logger, proxy string) (func(*http.Request) (*url.URL, error), error) { + if lg == nil { + lg = zap.NewNop() + } if proxy == "" { return nil, nil } @@ -113,15 +113,14 @@ func newProxyFunc(lg *zap.Logger, proxy string) (func(*http.Request) (*url.URL, return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err) } - if lg != nil { - lg.Info("running proxy with discovery", zap.String("proxy-url", proxyURL.String())) - } else { - plog.Infof("using proxy %q", proxyURL.String()) - } + lg.Info("running proxy with discovery", zap.String("proxy-url", proxyURL.String())) return http.ProxyURL(proxyURL), nil } func newDiscovery(lg *zap.Logger, durl, dproxyurl string, id types.ID) (*discovery, error) { + if lg == nil { + lg = zap.NewNop() + } u, err := url.Parse(durl) if err != nil { return nil, err @@ -232,17 +231,13 @@ func (d *discovery) checkCluster() ([]*client.Node, uint64, uint64, error) { return nil, 0, 0, ErrBadDiscoveryEndpoint } if ce, ok := err.(*client.ClusterError); ok { - if d.lg != nil { - d.lg.Warn( - "failed to get from discovery server", - zap.String("discovery-url", d.url.String()), - zap.String("path", path.Join(configKey, "size")), - zap.Error(err), - zap.String("err-detail", ce.Detail()), - ) - } else { - plog.Error(ce.Detail()) - } + d.lg.Warn( + "failed to get from discovery server", + zap.String("discovery-url", d.url.String()), + zap.String("path", path.Join(configKey, "size")), + zap.Error(err), + zap.String("err-detail", ce.Detail()), + ) return d.checkClusterRetry() } return nil, 0, 0, err @@ -257,17 +252,13 @@ func (d *discovery) checkCluster() ([]*client.Node, uint64, uint64, error) { cancel() if err != nil { if ce, ok := err.(*client.ClusterError); ok { - if d.lg != nil { - d.lg.Warn( - "failed to get from discovery server", - zap.String("discovery-url", d.url.String()), - zap.String("path", d.cluster), - zap.Error(err), - zap.String("err-detail", ce.Detail()), - ) - } else { - plog.Error(ce.Detail()) - } + d.lg.Warn( + "failed to get from discovery server", + zap.String("discovery-url", d.url.String()), + zap.String("path", d.cluster), + zap.Error(err), + zap.String("err-detail", ce.Detail()), + ) return d.checkClusterRetry() } return nil, 0, 0, err @@ -303,16 +294,12 @@ func (d *discovery) logAndBackoffForRetry(step string) { retries = maxExpoentialRetries } retryTimeInSecond := time.Duration(0x1<