From 41ed74824e2eab01e3531a7f3d731fd507491d70 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Fri, 14 May 2021 12:27:53 +0200 Subject: [PATCH] server: Simplify passing logger setup by passing only logger --- server/config/config.go | 10 ---------- server/embed/config.go | 10 ---------- server/embed/config_logging.go | 16 +++------------- server/embed/etcd.go | 3 --- server/etcdserver/raft.go | 29 +++-------------------------- 5 files changed, 6 insertions(+), 62 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 7874d8ee0..a2f315f10 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -30,7 +30,6 @@ import ( bolt "go.etcd.io/bbolt" "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) // ServerConfig holds the configuration of etcd as taken from the command line or discovery. @@ -144,17 +143,8 @@ type ServerConfig struct { SocketOpts transport.SocketOpts // Logger logs server-side operations. - // If not nil, it disables "capnslog" and uses the given logger. Logger *zap.Logger - // LoggerConfig is server logger configuration for Raft logger. - // Must be either: "LoggerConfig != nil" or "LoggerCore != nil && LoggerWriteSyncer != nil". - LoggerConfig *zap.Config - // LoggerCore is "zapcore.Core" for raft logger. - // Must be either: "LoggerConfig != nil" or "LoggerCore != nil && LoggerWriteSyncer != nil". - LoggerCore zapcore.Core - LoggerWriteSyncer zapcore.WriteSyncer - ForceNewCluster bool // EnableLeaseCheckpoint enables primary lessor to persist lease remainingTTL to prevent indefinite auto-renewal of long lived leases. diff --git a/server/embed/config.go b/server/embed/config.go index f50ff1663..875733290 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -39,7 +39,6 @@ import ( bolt "go.etcd.io/bbolt" "go.uber.org/multierr" "go.uber.org/zap" - "go.uber.org/zap/zapcore" "golang.org/x/crypto/bcrypt" "google.golang.org/grpc" "sigs.k8s.io/yaml" @@ -371,15 +370,6 @@ type Config struct { // Do not set logger directly. loggerMu *sync.RWMutex logger *zap.Logger - - // loggerConfig is server logger configuration for Raft logger. - // Must be either: "loggerConfig != nil" or "loggerCore != nil && loggerWriteSyncer != nil". - loggerConfig *zap.Config - // loggerCore is "zapcore.Core" for raft logger. - // Must be either: "loggerConfig != nil" or "loggerCore != nil && loggerWriteSyncer != nil". - loggerCore zapcore.Core - loggerWriteSyncer zapcore.WriteSyncer - // EnableGRPCGateway enables grpc gateway. // The gateway translates a RESTful HTTP API into gRPC. EnableGRPCGateway bool `json:"enable-grpc-gateway"` diff --git a/server/embed/config_logging.go b/server/embed/config_logging.go index 261949ef0..511f73159 100644 --- a/server/embed/config_logging.go +++ b/server/embed/config_logging.go @@ -104,16 +104,13 @@ func (cfg *Config) setupLogging() error { copied.Level = zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) if cfg.ZapLoggerBuilder == nil { cfg.ZapLoggerBuilder = func(c *Config) error { + c.loggerMu.Lock() + defer c.loggerMu.Unlock() var err error c.logger, err = copied.Build() if err != nil { return err } - c.loggerMu.Lock() - defer c.loggerMu.Unlock() - c.loggerConfig = &copied - c.loggerCore = nil - c.loggerWriteSyncer = nil return nil } } @@ -143,13 +140,9 @@ func (cfg *Config) setupLogging() error { ) if cfg.ZapLoggerBuilder == nil { cfg.ZapLoggerBuilder = func(c *Config) error { - c.logger = zap.New(cr, zap.AddCaller(), zap.ErrorOutput(syncer)) c.loggerMu.Lock() defer c.loggerMu.Unlock() - c.loggerConfig = nil - c.loggerCore = cr - c.loggerWriteSyncer = syncer - + c.logger = zap.New(cr, zap.AddCaller(), zap.ErrorOutput(syncer)) return nil } } @@ -203,9 +196,6 @@ func NewZapCoreLoggerBuilder(lg *zap.Logger) func(*Config) error { cfg.loggerMu.Lock() defer cfg.loggerMu.Unlock() cfg.logger = lg - cfg.loggerConfig = nil - cfg.loggerCore = nil - cfg.loggerWriteSyncer = nil return nil } } diff --git a/server/embed/etcd.go b/server/embed/etcd.go index d38091573..96d7f7b8c 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -211,9 +211,6 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { CorruptCheckTime: cfg.ExperimentalCorruptCheckTime, PreVote: cfg.PreVote, Logger: cfg.logger, - LoggerConfig: cfg.loggerConfig, - LoggerCore: cfg.loggerCore, - LoggerWriteSyncer: cfg.loggerWriteSyncer, ForceNewCluster: cfg.ForceNewCluster, EnableGRPCGateway: cfg.EnableGRPCGateway, ExperimentalEnableDistributedTracing: cfg.ExperimentalEnableDistributedTracing, diff --git a/server/etcdserver/raft.go b/server/etcdserver/raft.go index bb248eb36..8b9600d39 100644 --- a/server/etcdserver/raft.go +++ b/server/etcdserver/raft.go @@ -460,9 +460,8 @@ func startNode(cfg config.ServerConfig, cl *membership.RaftCluster, ids []types. MaxInflightMsgs: maxInflightMsgs, CheckQuorum: true, PreVote: cfg.PreVote, + Logger: NewRaftLoggerZap(cfg.Logger.Named("raft")), } - c.Logger, _ = getRaftLogger(cfg) - if len(peers) == 0 { n = raft.RestartNode(c) } else { @@ -504,11 +503,7 @@ func restartNode(cfg config.ServerConfig, snapshot *raftpb.Snapshot) (types.ID, MaxInflightMsgs: maxInflightMsgs, CheckQuorum: true, PreVote: cfg.PreVote, - } - var err error - c.Logger, err = getRaftLogger(cfg) - if err != nil { - log.Fatalf("cannot create raft logger %v", err) + Logger: NewRaftLoggerZap(cfg.Logger.Named("raft")), } n := raft.RestartNode(c) @@ -582,11 +577,7 @@ func restartAsStandaloneNode(cfg config.ServerConfig, snapshot *raftpb.Snapshot) MaxInflightMsgs: maxInflightMsgs, CheckQuorum: true, PreVote: cfg.PreVote, - } - - c.Logger, err = getRaftLogger(cfg) - if err != nil { - log.Fatalf("cannot create raft logger %v", err) + Logger: NewRaftLoggerZap(cfg.Logger.Named("raft")), } n := raft.RestartNode(c) @@ -594,20 +585,6 @@ func restartAsStandaloneNode(cfg config.ServerConfig, snapshot *raftpb.Snapshot) return id, cl, n, s, w } -func getRaftLogger(cfg config.ServerConfig) (raft.Logger, error) { - if cfg.Logger != nil { - // called after capnslog setting in "init" function - if cfg.LoggerConfig != nil { - return NewRaftLogger(cfg.LoggerConfig) - } else if cfg.LoggerCore != nil && cfg.LoggerWriteSyncer != nil { - return NewRaftLoggerFromZapCore(cfg.LoggerCore, cfg.LoggerWriteSyncer), nil - } else { - return NewRaftLoggerZap(cfg.Logger.Named("raft")), nil - } - } - return nil, nil -} - // getIDs returns an ordered set of IDs included in the given snapshot and // the entries. The given snapshot/entries can contain three kinds of // ID-related entry: