From 74d5ba57778ba0fda73f751f5c0cee7f73473de4 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Wed, 5 Feb 2020 10:22:32 -0800 Subject: [PATCH] embed: remove capnslog (#11592) --- embed/config.go | 82 ++++-------- embed/etcd.go | 350 +++++++++++++++++------------------------------- embed/serve.go | 66 ++++----- 3 files changed, 174 insertions(+), 324 deletions(-) diff --git a/embed/config.go b/embed/config.go index dba3a867a..2f5ce4c19 100644 --- a/embed/config.go +++ b/embed/config.go @@ -15,7 +15,6 @@ package embed import ( - "crypto/tls" "fmt" "io/ioutil" "net" @@ -408,19 +407,6 @@ func NewConfig() *Config { return cfg } -func logTLSHandshakeFailure(conn *tls.Conn, err error) { - state := conn.ConnectionState() - remoteAddr := conn.RemoteAddr().String() - serverName := state.ServerName - if len(state.PeerCertificates) > 0 { - cert := state.PeerCertificates[0] - ips, dns := cert.IPAddresses, cert.DNSNames - plog.Infof("rejected connection from %q (error %q, ServerName %q, IPAddresses %q, DNSNames %q)", remoteAddr, err.Error(), serverName, ips, dns) - } else { - plog.Infof("rejected connection from %q (error %q, ServerName %q)", remoteAddr, err.Error(), serverName) - } -} - func ConfigFromFile(path string) (*Config, error) { cfg := &configYAML{Config: *NewConfig()} if err := cfg.configFromFile(path); err != nil { @@ -618,19 +604,11 @@ func (cfg *Config) PeerURLsMapAndToken(which string) (urlsmap types.URLsMap, tok clusterStrs, cerr := cfg.GetDNSClusterNames() lg := cfg.logger if cerr != nil { - if lg != nil { - lg.Warn("failed to resolve during SRV discovery", zap.Error(cerr)) - } else { - plog.Errorf("couldn't resolve during SRV discovery (%v)", cerr) - } + lg.Warn("failed to resolve during SRV discovery", zap.Error(cerr)) return nil, "", cerr } for _, s := range clusterStrs { - if lg != nil { - lg.Info("got bootstrap from DNS for etcd-server", zap.String("node", s)) - } else { - plog.Noticef("got bootstrap from DNS for etcd-server at %s", s) - } + lg.Info("got bootstrap from DNS for etcd-server", zap.String("node", s)) } clusterStr := strings.Join(clusterStrs, ",") if strings.Contains(clusterStr, "https://") && cfg.PeerTLSInfo.TrustedCAFile == "" { @@ -671,35 +649,31 @@ func (cfg *Config) GetDNSClusterNames() ([]string, error) { if cerr != nil { clusterStrs = make([]string, 0) } - if lg != nil { - lg.Info( - "get cluster for etcd-server-ssl SRV", - zap.String("service-scheme", "https"), - zap.String("service-name", "etcd-server-ssl"+serviceNameSuffix), - zap.String("server-name", cfg.Name), - zap.String("discovery-srv", cfg.DNSCluster), - zap.Strings("advertise-peer-urls", cfg.getAPURLs()), - zap.Strings("found-cluster", clusterStrs), - zap.Error(cerr), - ) - } + lg.Info( + "get cluster for etcd-server-ssl SRV", + zap.String("service-scheme", "https"), + zap.String("service-name", "etcd-server-ssl"+serviceNameSuffix), + zap.String("server-name", cfg.Name), + zap.String("discovery-srv", cfg.DNSCluster), + zap.Strings("advertise-peer-urls", cfg.getAPURLs()), + zap.Strings("found-cluster", clusterStrs), + zap.Error(cerr), + ) defaultHTTPClusterStrs, httpCerr := srv.GetCluster("http", "etcd-server"+serviceNameSuffix, cfg.Name, cfg.DNSCluster, cfg.APUrls) if httpCerr != nil { clusterStrs = append(clusterStrs, defaultHTTPClusterStrs...) } - if lg != nil { - lg.Info( - "get cluster for etcd-server SRV", - zap.String("service-scheme", "http"), - zap.String("service-name", "etcd-server"+serviceNameSuffix), - zap.String("server-name", cfg.Name), - zap.String("discovery-srv", cfg.DNSCluster), - zap.Strings("advertise-peer-urls", cfg.getAPURLs()), - zap.Strings("found-cluster", clusterStrs), - zap.Error(httpCerr), - ) - } + lg.Info( + "get cluster for etcd-server SRV", + zap.String("service-scheme", "http"), + zap.String("service-name", "etcd-server"+serviceNameSuffix), + zap.String("server-name", cfg.Name), + zap.String("discovery-srv", cfg.DNSCluster), + zap.Strings("advertise-peer-urls", cfg.getAPURLs()), + zap.Strings("found-cluster", clusterStrs), + zap.Error(httpCerr), + ) return clusterStrs, cerr } @@ -734,11 +708,7 @@ func (cfg *Config) ClientSelfCert() (err error) { return nil } if !cfg.ClientTLSInfo.Empty() { - if cfg.logger != nil { - cfg.logger.Warn("ignoring client auto TLS since certs given") - } else { - plog.Warningf("ignoring client auto TLS since certs given") - } + cfg.logger.Warn("ignoring client auto TLS since certs given") return nil } chosts := make([]string, len(cfg.LCUrls)) @@ -757,11 +727,7 @@ func (cfg *Config) PeerSelfCert() (err error) { return nil } if !cfg.PeerTLSInfo.Empty() { - if cfg.logger != nil { - cfg.logger.Warn("ignoring peer auto TLS since certs given") - } else { - plog.Warningf("ignoring peer auto TLS since certs given") - } + cfg.logger.Warn("ignoring peer auto TLS since certs given") return nil } phosts := make([]string, len(cfg.LPUrls)) diff --git a/embed/etcd.go b/embed/etcd.go index 6c43742bb..4c02c1184 100644 --- a/embed/etcd.go +++ b/embed/etcd.go @@ -42,7 +42,6 @@ import ( "go.etcd.io/etcd/pkg/types" "go.etcd.io/etcd/version" - "github.com/coreos/pkg/capnslog" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/soheilhy/cmux" "go.uber.org/zap" @@ -50,8 +49,6 @@ import ( "google.golang.org/grpc/keepalive" ) -var plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "embed") - const ( // internal fd usage includes disk usage and transport usage. // To read/write snapshot, snap pkg needs 1. In normal case, wal pkg needs @@ -113,22 +110,18 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { e = nil }() - if e.cfg.logger != nil { - e.cfg.logger.Info( - "configuring peer listeners", - zap.Strings("listen-peer-urls", e.cfg.getLPURLs()), - ) - } + e.cfg.logger.Info( + "configuring peer listeners", + zap.Strings("listen-peer-urls", e.cfg.getLPURLs()), + ) if e.Peers, err = configurePeerListeners(cfg); err != nil { return e, err } - if e.cfg.logger != nil { - e.cfg.logger.Info( - "configuring client listeners", - zap.Strings("listen-client-urls", e.cfg.getLCURLs()), - ) - } + e.cfg.logger.Info( + "configuring client listeners", + zap.Strings("listen-client-urls", e.cfg.getLCURLs()), + ) if e.sctxs, err = configureClientListeners(cfg); err != nil { return e, err } @@ -236,107 +229,78 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { return e, err } - if e.cfg.logger != nil { - e.cfg.logger.Info( - "now serving peer/client/metrics", - zap.String("local-member-id", e.Server.ID().String()), - zap.Strings("initial-advertise-peer-urls", e.cfg.getAPURLs()), - zap.Strings("listen-peer-urls", e.cfg.getLPURLs()), - zap.Strings("advertise-client-urls", e.cfg.getACURLs()), - zap.Strings("listen-client-urls", e.cfg.getLCURLs()), - zap.Strings("listen-metrics-urls", e.cfg.getMetricsURLs()), - ) - } + e.cfg.logger.Info( + "now serving peer/client/metrics", + zap.String("local-member-id", e.Server.ID().String()), + zap.Strings("initial-advertise-peer-urls", e.cfg.getAPURLs()), + zap.Strings("listen-peer-urls", e.cfg.getLPURLs()), + zap.Strings("advertise-client-urls", e.cfg.getACURLs()), + zap.Strings("listen-client-urls", e.cfg.getLCURLs()), + zap.Strings("listen-metrics-urls", e.cfg.getMetricsURLs()), + ) serving = true return e, nil } func print(lg *zap.Logger, ec Config, sc etcdserver.ServerConfig, memberInitialized bool) { - // TODO: remove this after dropping "capnslog" - if lg == nil { - plog.Infof("name = %s", ec.Name) - if sc.ForceNewCluster { - plog.Infof("force new cluster") - } - plog.Infof("data dir = %s", sc.DataDir) - plog.Infof("member dir = %s", sc.MemberDir()) - if sc.DedicatedWALDir != "" { - plog.Infof("dedicated WAL dir = %s", sc.DedicatedWALDir) - } - plog.Infof("heartbeat = %dms", sc.TickMs) - plog.Infof("election = %dms", sc.ElectionTicks*int(sc.TickMs)) - plog.Infof("snapshot count = %d", sc.SnapshotCount) - if len(sc.DiscoveryURL) != 0 { - plog.Infof("discovery URL= %s", sc.DiscoveryURL) - if len(sc.DiscoveryProxy) != 0 { - plog.Infof("discovery proxy = %s", sc.DiscoveryProxy) - } - } - plog.Infof("advertise client URLs = %s", sc.ClientURLs) - if memberInitialized { - plog.Infof("initial advertise peer URLs = %s", sc.PeerURLs) - plog.Infof("initial cluster = %s", sc.InitialPeerURLsMap) - } - } else { - cors := make([]string, 0, len(ec.CORS)) - for v := range ec.CORS { - cors = append(cors, v) - } - sort.Strings(cors) - - hss := make([]string, 0, len(ec.HostWhitelist)) - for v := range ec.HostWhitelist { - hss = append(hss, v) - } - sort.Strings(hss) - - quota := ec.QuotaBackendBytes - if quota == 0 { - quota = etcdserver.DefaultQuotaBytes - } - - lg.Info( - "starting an etcd server", - zap.String("etcd-version", version.Version), - zap.String("git-sha", version.GitSHA), - zap.String("go-version", runtime.Version()), - zap.String("go-os", runtime.GOOS), - zap.String("go-arch", runtime.GOARCH), - zap.Int("max-cpu-set", runtime.GOMAXPROCS(0)), - zap.Int("max-cpu-available", runtime.NumCPU()), - zap.Bool("member-initialized", memberInitialized), - zap.String("name", sc.Name), - zap.String("data-dir", sc.DataDir), - zap.String("wal-dir", ec.WalDir), - zap.String("wal-dir-dedicated", sc.DedicatedWALDir), - zap.String("member-dir", sc.MemberDir()), - zap.Bool("force-new-cluster", sc.ForceNewCluster), - zap.String("heartbeat-interval", fmt.Sprintf("%v", time.Duration(sc.TickMs)*time.Millisecond)), - zap.String("election-timeout", fmt.Sprintf("%v", time.Duration(sc.ElectionTicks*int(sc.TickMs))*time.Millisecond)), - zap.Bool("initial-election-tick-advance", sc.InitialElectionTickAdvance), - zap.Uint64("snapshot-count", sc.SnapshotCount), - zap.Uint64("snapshot-catchup-entries", sc.SnapshotCatchUpEntries), - zap.Strings("initial-advertise-peer-urls", ec.getAPURLs()), - zap.Strings("listen-peer-urls", ec.getLPURLs()), - zap.Strings("advertise-client-urls", ec.getACURLs()), - zap.Strings("listen-client-urls", ec.getLCURLs()), - zap.Strings("listen-metrics-urls", ec.getMetricsURLs()), - zap.Strings("cors", cors), - zap.Strings("host-whitelist", hss), - zap.String("initial-cluster", sc.InitialPeerURLsMap.String()), - zap.String("initial-cluster-state", ec.ClusterState), - zap.String("initial-cluster-token", sc.InitialClusterToken), - zap.Int64("quota-size-bytes", quota), - zap.Bool("pre-vote", sc.PreVote), - zap.Bool("initial-corrupt-check", sc.InitialCorruptCheck), - zap.String("corrupt-check-time-interval", sc.CorruptCheckTime.String()), - zap.String("auto-compaction-mode", sc.AutoCompactionMode), - zap.Duration("auto-compaction-retention", sc.AutoCompactionRetention), - zap.String("auto-compaction-interval", sc.AutoCompactionRetention.String()), - zap.String("discovery-url", sc.DiscoveryURL), - zap.String("discovery-proxy", sc.DiscoveryProxy), - ) + cors := make([]string, 0, len(ec.CORS)) + for v := range ec.CORS { + cors = append(cors, v) } + sort.Strings(cors) + + hss := make([]string, 0, len(ec.HostWhitelist)) + for v := range ec.HostWhitelist { + hss = append(hss, v) + } + sort.Strings(hss) + + quota := ec.QuotaBackendBytes + if quota == 0 { + quota = etcdserver.DefaultQuotaBytes + } + + lg.Info( + "starting an etcd server", + zap.String("etcd-version", version.Version), + zap.String("git-sha", version.GitSHA), + zap.String("go-version", runtime.Version()), + zap.String("go-os", runtime.GOOS), + zap.String("go-arch", runtime.GOARCH), + zap.Int("max-cpu-set", runtime.GOMAXPROCS(0)), + zap.Int("max-cpu-available", runtime.NumCPU()), + zap.Bool("member-initialized", memberInitialized), + zap.String("name", sc.Name), + zap.String("data-dir", sc.DataDir), + zap.String("wal-dir", ec.WalDir), + zap.String("wal-dir-dedicated", sc.DedicatedWALDir), + zap.String("member-dir", sc.MemberDir()), + zap.Bool("force-new-cluster", sc.ForceNewCluster), + zap.String("heartbeat-interval", fmt.Sprintf("%v", time.Duration(sc.TickMs)*time.Millisecond)), + zap.String("election-timeout", fmt.Sprintf("%v", time.Duration(sc.ElectionTicks*int(sc.TickMs))*time.Millisecond)), + zap.Bool("initial-election-tick-advance", sc.InitialElectionTickAdvance), + zap.Uint64("snapshot-count", sc.SnapshotCount), + zap.Uint64("snapshot-catchup-entries", sc.SnapshotCatchUpEntries), + zap.Strings("initial-advertise-peer-urls", ec.getAPURLs()), + zap.Strings("listen-peer-urls", ec.getLPURLs()), + zap.Strings("advertise-client-urls", ec.getACURLs()), + zap.Strings("listen-client-urls", ec.getLCURLs()), + zap.Strings("listen-metrics-urls", ec.getMetricsURLs()), + zap.Strings("cors", cors), + zap.Strings("host-whitelist", hss), + zap.String("initial-cluster", sc.InitialPeerURLsMap.String()), + zap.String("initial-cluster-state", ec.ClusterState), + zap.String("initial-cluster-token", sc.InitialClusterToken), + zap.Int64("quota-size-bytes", quota), + zap.Bool("pre-vote", sc.PreVote), + zap.Bool("initial-corrupt-check", sc.InitialCorruptCheck), + zap.String("corrupt-check-time-interval", sc.CorruptCheckTime.String()), + zap.String("auto-compaction-mode", sc.AutoCompactionMode), + zap.Duration("auto-compaction-retention", sc.AutoCompactionRetention), + zap.String("auto-compaction-interval", sc.AutoCompactionRetention.String()), + zap.String("discovery-url", sc.DiscoveryURL), + zap.String("discovery-proxy", sc.DiscoveryProxy), + ) } // Config returns the current configuration. @@ -355,14 +319,10 @@ func (e *Etcd) Close() { zap.Strings("advertise-client-urls", e.cfg.getACURLs()), } lg := e.GetLogger() - if lg != nil { - lg.Info("closing etcd server", fields...) - } + lg.Info("closing etcd server", fields...) defer func() { - if lg != nil { - lg.Info("closed etcd server", fields...) - lg.Sync() - } + lg.Info("closed etcd server", fields...) + lg.Sync() }() e.closeOnce.Do(func() { close(e.stopc) }) @@ -453,22 +413,14 @@ func configurePeerListeners(cfg *Config) (peers []*peerListener, err error) { return nil, err } if err = cfg.PeerSelfCert(); err != nil { - if cfg.logger != nil { - cfg.logger.Fatal("failed to get peer self-signed certs", zap.Error(err)) - } else { - plog.Fatalf("could not get certs (%v)", err) - } + cfg.logger.Fatal("failed to get peer self-signed certs", zap.Error(err)) } if !cfg.PeerTLSInfo.Empty() { - if cfg.logger != nil { - cfg.logger.Info( - "starting with peer TLS", - zap.String("tls-info", fmt.Sprintf("%+v", cfg.PeerTLSInfo)), - zap.Strings("cipher-suites", cfg.CipherSuites), - ) - } else { - plog.Infof("peerTLS: %s", cfg.PeerTLSInfo) - } + cfg.logger.Info( + "starting with peer TLS", + zap.String("tls-info", fmt.Sprintf("%+v", cfg.PeerTLSInfo)), + zap.Strings("cipher-suites", cfg.CipherSuites), + ) } peers = make([]*peerListener, len(cfg.LPUrls)) @@ -478,15 +430,11 @@ func configurePeerListeners(cfg *Config) (peers []*peerListener, err error) { } for i := range peers { if peers[i] != nil && peers[i].close != nil { - if cfg.logger != nil { - cfg.logger.Warn( - "closing peer listener", - zap.String("address", cfg.LPUrls[i].String()), - zap.Error(err), - ) - } else { - plog.Info("stopping listening for peers on ", cfg.LPUrls[i].String()) - } + cfg.logger.Warn( + "closing peer listener", + zap.String("address", cfg.LPUrls[i].String()), + zap.Error(err), + ) ctx, cancel := context.WithTimeout(context.Background(), time.Second) peers[i].close(ctx) cancel() @@ -497,18 +445,10 @@ func configurePeerListeners(cfg *Config) (peers []*peerListener, err error) { for i, u := range cfg.LPUrls { if u.Scheme == "http" { if !cfg.PeerTLSInfo.Empty() { - if cfg.logger != nil { - cfg.logger.Warn("scheme is HTTP while key and cert files are present; ignoring key and cert files", zap.String("peer-url", u.String())) - } else { - plog.Warningf("The scheme of peer url %s is HTTP while peer key/cert files are presented. Ignored peer key/cert files.", u.String()) - } + cfg.logger.Warn("scheme is HTTP while key and cert files are present; ignoring key and cert files", zap.String("peer-url", u.String())) } if cfg.PeerTLSInfo.ClientCertAuth { - if cfg.logger != nil { - cfg.logger.Warn("scheme is HTTP while --peer-client-cert-auth is enabled; ignoring client cert auth for this URL", zap.String("peer-url", u.String())) - } else { - plog.Warningf("The scheme of peer url %s is HTTP while client cert auth (--peer-client-cert-auth) is enabled. Ignored client cert auth for this url.", u.String()) - } + cfg.logger.Warn("scheme is HTTP while --peer-client-cert-auth is enabled; ignoring client cert auth for this URL", zap.String("peer-url", u.String())) } } peers[i] = &peerListener{close: func(context.Context) error { return nil }} @@ -550,19 +490,15 @@ func (e *Etcd) servePeers() (err error) { // gracefully shutdown http.Server // close open listeners, idle connections // until context cancel or time-out - if e.cfg.logger != nil { - e.cfg.logger.Info( - "stopping serving peer traffic", - zap.String("address", u), - ) - } + e.cfg.logger.Info( + "stopping serving peer traffic", + zap.String("address", u), + ) stopServers(ctx, &servers{secure: peerTLScfg != nil, grpc: gs, http: srv}) - if e.cfg.logger != nil { - e.cfg.logger.Info( - "stopped serving peer traffic", - zap.String("address", u), - ) - } + e.cfg.logger.Info( + "stopped serving peer traffic", + zap.String("address", u), + ) return nil } } @@ -571,14 +507,10 @@ func (e *Etcd) servePeers() (err error) { for _, pl := range e.Peers { go func(l *peerListener) { u := l.Addr().String() - if e.cfg.logger != nil { - e.cfg.logger.Info( - "serving peer traffic", - zap.String("address", u), - ) - } else { - plog.Info("listening for peers on ", u) - } + e.cfg.logger.Info( + "serving peer traffic", + zap.String("address", u), + ) e.errHandler(l.serve()) }(pl) } @@ -590,18 +522,10 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro return nil, err } if err = cfg.ClientSelfCert(); err != nil { - if cfg.logger != nil { - cfg.logger.Fatal("failed to get client self-signed certs", zap.Error(err)) - } else { - plog.Fatalf("could not get certs (%v)", err) - } + cfg.logger.Fatal("failed to get client self-signed certs", zap.Error(err)) } if cfg.EnablePprof { - if cfg.logger != nil { - cfg.logger.Info("pprof is enabled", zap.String("path", debugutil.HTTPPrefixPProf)) - } else { - plog.Infof("pprof is enabled under %s", debugutil.HTTPPrefixPProf) - } + cfg.logger.Info("pprof is enabled", zap.String("path", debugutil.HTTPPrefixPProf)) } sctxs = make(map[string]*serveCtx) @@ -609,18 +533,10 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro sctx := newServeCtx(cfg.logger) if u.Scheme == "http" || u.Scheme == "unix" { if !cfg.ClientTLSInfo.Empty() { - if cfg.logger != nil { - cfg.logger.Warn("scheme is HTTP while key and cert files are present; ignoring key and cert files", zap.String("client-url", u.String())) - } else { - plog.Warningf("The scheme of client url %s is HTTP while peer key/cert files are presented. Ignored key/cert files.", u.String()) - } + cfg.logger.Warn("scheme is HTTP while key and cert files are present; ignoring key and cert files", zap.String("client-url", u.String())) } if cfg.ClientTLSInfo.ClientCertAuth { - if cfg.logger != nil { - cfg.logger.Warn("scheme is HTTP while --client-cert-auth is enabled; ignoring client cert auth for this URL", zap.String("client-url", u.String())) - } else { - plog.Warningf("The scheme of client url %s is HTTP while client cert auth (--client-cert-auth) is enabled. Ignored client cert auth for this url.", u.String()) - } + cfg.logger.Warn("scheme is HTTP while --client-cert-auth is enabled; ignoring client cert auth for this URL", zap.String("client-url", u.String())) } } if (u.Scheme == "https" || u.Scheme == "unixs") && cfg.ClientTLSInfo.Empty() { @@ -652,15 +568,11 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro if fdLimit, fderr := runtimeutil.FDLimit(); fderr == nil { if fdLimit <= reservedInternalFDNum { - if cfg.logger != nil { - cfg.logger.Fatal( - "file descriptor limit of etcd process is too low; please set higher", - zap.Uint64("limit", fdLimit), - zap.Int("recommended-limit", reservedInternalFDNum), - ) - } else { - plog.Fatalf("file descriptor limit[%d] of etcd process is too low, and should be set higher than %d to ensure internal usage", fdLimit, reservedInternalFDNum) - } + cfg.logger.Fatal( + "file descriptor limit of etcd process is too low; please set higher", + zap.Uint64("limit", fdLimit), + zap.Int("recommended-limit", reservedInternalFDNum), + ) } sctx.l = transport.LimitListener(sctx.l, int(fdLimit-reservedInternalFDNum)) } @@ -676,15 +588,11 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro return } sctx.l.Close() - if cfg.logger != nil { - cfg.logger.Warn( - "closing peer listener", - zap.String("address", u.Host), - zap.Error(err), - ) - } else { - plog.Info("stopping listening for client requests on ", u.Host) - } + cfg.logger.Warn( + "closing peer listener", + zap.String("address", u.Host), + zap.Error(err), + ) }() for k := range cfg.UserHandlers { sctx.userHandlers[k] = cfg.UserHandlers[k] @@ -703,15 +611,11 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro func (e *Etcd) serveClients() (err error) { if !e.cfg.ClientTLSInfo.Empty() { - if e.cfg.logger != nil { - e.cfg.logger.Info( - "starting with client TLS", - zap.String("tls-info", fmt.Sprintf("%+v", e.cfg.ClientTLSInfo)), - zap.Strings("cipher-suites", e.cfg.CipherSuites), - ) - } else { - plog.Infof("ClientTLS: %s", e.cfg.ClientTLSInfo) - } + e.cfg.logger.Info( + "starting with client TLS", + zap.String("tls-info", fmt.Sprintf("%+v", e.cfg.ClientTLSInfo)), + zap.Strings("cipher-suites", e.cfg.CipherSuites), + ) } // Start a client server goroutine for each listen address @@ -773,14 +677,10 @@ func (e *Etcd) serveMetrics() (err error) { } e.metricsListeners = append(e.metricsListeners, ml) go func(u url.URL, ln net.Listener) { - if e.cfg.logger != nil { - e.cfg.logger.Info( - "serving metrics", - zap.String("address", u.String()), - ) - } else { - plog.Info("listening for metrics on ", u.String()) - } + e.cfg.logger.Info( + "serving metrics", + zap.String("address", u.String()), + ) e.errHandler(http.Serve(ln, metricsMux)) }(murl, ml) } diff --git a/embed/serve.go b/embed/serve.go index a3b20c46c..14e481082 100644 --- a/embed/serve.go +++ b/embed/serve.go @@ -70,6 +70,9 @@ type servers struct { func newServeCtx(lg *zap.Logger) *serveCtx { ctx, cancel := context.WithCancel(context.Background()) + if lg == nil { + lg = zap.NewNop() + } return &serveCtx{ lg: lg, ctx: ctx, @@ -91,9 +94,7 @@ func (sctx *serveCtx) serve( logger := defaultLog.New(ioutil.Discard, "etcdhttp", 0) <-s.ReadyNotify() - if sctx.lg == nil { - plog.Info("ready to serve client requests") - } + sctx.lg.Info("ready to serve client requests") m := cmux.New(sctx.l) v3c := v3client.New(s) @@ -135,14 +136,10 @@ func (sctx *serveCtx) serve( go func() { errHandler(srvhttp.Serve(httpl)) }() sctx.serversC <- &servers{grpc: gs, http: srvhttp} - if sctx.lg != nil { - sctx.lg.Info( - "serving client traffic insecurely; this is strongly discouraged!", - zap.String("address", sctx.l.Addr().String()), - ) - } else { - plog.Noticef("serving insecure client requests on %s, this is strongly discouraged!", sctx.l.Addr().String()) - } + sctx.lg.Info( + "serving client traffic insecurely; this is strongly discouraged!", + zap.String("address", sctx.l.Addr().String()), + ) } if sctx.secure { @@ -187,14 +184,10 @@ func (sctx *serveCtx) serve( go func() { errHandler(srv.Serve(tlsl)) }() sctx.serversC <- &servers{secure: true, grpc: gs, http: srv} - if sctx.lg != nil { - sctx.lg.Info( - "serving client traffic securely", - zap.String("address", sctx.l.Addr().String()), - ) - } else { - plog.Infof("serving client requests on %s", sctx.l.Addr().String()) - } + sctx.lg.Info( + "serving client traffic securely", + zap.String("address", sctx.l.Addr().String()), + ) } close(sctx.serversC) @@ -253,15 +246,11 @@ func (sctx *serveCtx) registerGateway(opts []grpc.DialOption) (*gw.ServeMux, err go func() { <-ctx.Done() if cerr := conn.Close(); cerr != nil { - if sctx.lg != nil { - sctx.lg.Warn( - "failed to close connection", - zap.String("address", sctx.l.Addr().String()), - zap.Error(cerr), - ) - } else { - plog.Warningf("failed to close conn to %s: %v", sctx.l.Addr().String(), cerr) - } + sctx.lg.Warn( + "failed to close connection", + zap.String("address", sctx.l.Addr().String()), + zap.Error(cerr), + ) } }() @@ -300,6 +289,9 @@ func (sctx *serveCtx) createMux(gwmux *gw.ServeMux, handler http.Handler) *http. // - check hostname whitelist // client HTTP requests goes here first func createAccessController(lg *zap.Logger, s *etcdserver.EtcdServer, mux *http.ServeMux) http.Handler { + if lg == nil { + lg = zap.NewNop() + } return &accessController{lg: lg, s: s, mux: mux} } @@ -318,14 +310,10 @@ func (ac *accessController) ServeHTTP(rw http.ResponseWriter, req *http.Request) if req.TLS == nil { // check origin if client connection is not secure host := httputil.GetHostname(req) if !ac.s.AccessController.IsHostWhitelisted(host) { - if ac.lg != nil { - ac.lg.Warn( - "rejecting HTTP request to prevent DNS rebinding attacks", - zap.String("host", host), - ) - } else { - plog.Warningf("rejecting HTTP request from %q to prevent DNS rebinding attacks", host) - } + ac.lg.Warn( + "rejecting HTTP request to prevent DNS rebinding attacks", + zap.String("host", host), + ) // TODO: use Go's "http.StatusMisdirectedRequest" (421) // https://github.com/golang/go/commit/4b8a7eafef039af1834ef9bfa879257c4a72b7b5 http.Error(rw, errCVE20185702(host), 421) @@ -411,11 +399,7 @@ func (ch *corsHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { func (sctx *serveCtx) registerUserHandler(s string, h http.Handler) { if sctx.userHandlers[s] != nil { - if sctx.lg != nil { - sctx.lg.Warn("path is already registered by user handler", zap.String("path", s)) - } else { - plog.Warningf("path %s already registered by user handler", s) - } + sctx.lg.Warn("path is already registered by user handler", zap.String("path", s)) return } sctx.userHandlers[s] = h