Merge pull request #12968 from serathius/logger-simplify

server: Simplify passing logger setup by passing only logger

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.

server/embed/config.go

@@ -40,7 +40,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"
@@ -372,15 +371,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"`

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
 	}
 }

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,

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: