Merge pull request #9579 from gyuho/log-output-multiple-files

*: support output logs to multiple files

CHANGELOG-3.4.md

@@ -71,6 +71,9 @@ See [code changes](https://github.com/coreos/etcd/compare/v3.3.0...v3.4.0) and [
   - Previously, `Create(dirpath string, metadata []byte) (*WAL, error)`, now `Create(lg *zap.Logger, dirpath string, metadata []byte) (*WAL, error)`.
 - Remove [`embed.Config.SetupLogging`](https://github.com/coreos/etcd/pull/9572).
   - Now logger is set up automatically based on [`embed.Config.Logger`, `embed.Config.LogOutput`, `embed.Config.Debug` fields](https://github.com/coreos/etcd/pull/9572).
+- Change [`embed.Config.LogOutput` type from `string` to `[]string`](https://github.com/coreos/etcd/pull/9579) to support multiple log outputs.
+  - Now that `log-output` accepts multiple writers, etcd configuration YAML file `log-output` field must be changed to `[]string` type.
+  - Previously, `etcd.config.yaml` can have `log-output: default` field, now must be `log-output: [default]`.
 - Remove [`pkg/cors` package](https://github.com/coreos/etcd/pull/9490).
 - Move `"github.com/coreos/etcd/snap"` to [`"github.com/coreos/etcd/raftsnap"`](https://github.com/coreos/etcd/pull/9211).
 - Move `"github.com/coreos/etcd/etcdserver/auth"` to [`"github.com/coreos/etcd/etcdserver/v2auth"`](https://github.com/coreos/etcd/pull/9275).
@@ -140,10 +143,11 @@ See [security doc](https://github.com/coreos/etcd/blob/master/Documentation/op-g
 - Add [`--logger`](https://github.com/coreos/etcd/pull/9572) flag to support [structured logger and logging to file](https://github.com/coreos/etcd/issues/9438) in server-side.
   - e.g. `--logger=capnslog --log-output=default` is the default setting and same as previous etcd server logging format.
   - TODO: `--logger=zap` is experimental, and journald logging may not work when etcd runs as PID 1.
-  - e.g. `--logger=zap --log-output=/tmp/test.log` will log server operations with [JSON-encoded format](TODO) and writes logs to the specified file `/tmp/test.log`.
-  - e.g. `--logger=zap --log-output=default` will log server operations with [JSON-encoded format](TODO) and writes logs to `os.Stderr` (detect systemd journald TODO).
-  - e.g. `--logger=zap --log-output=stderr` will log server operations with [JSON-encoded format](TODO) and writes logs to `os.Stderr` (bypass journald TODO).
-  - e.g. `--logger=zap --log-output=stdout` will log server operations with [JSON-encoded format](TODO) and writes logs to `os.Stdout` (bypass journald TODO).
+  - e.g. `--logger=zap --log-output=/tmp/test.log` will log server operations in [JSON-encoded format](https://godoc.org/go.uber.org/zap#NewProductionEncoderConfig) and writes logs to the specified file `/tmp/test.log`.
+  - e.g. `--logger=zap --log-output=default` will log server operations in [JSON-encoded format](https://godoc.org/go.uber.org/zap#NewProductionEncoderConfig) and writes logs to `os.Stderr` (detect systemd journald TODO).
+  - e.g. `--logger=zap --log-output=stderr` will log server operations in [JSON-encoded format](https://godoc.org/go.uber.org/zap#NewProductionEncoderConfig) and writes logs to `os.Stderr` (bypass journald TODO).
+  - e.g. `--logger=zap --log-output=stdout` will log server operations in [JSON-encoded format](https://godoc.org/go.uber.org/zap#NewProductionEncoderConfig) and writes logs to `os.Stdout` (bypass journald TODO).
+  - e.g. `--logger=zap --log-output=a.log,b.log,c.log,stdout` [writes server logs to multiple files `a.log`, `b.log` and `c.log` at the same time](https://github.com/coreos/etcd/pull/9579) and outputs to `stdout`, in [JSON-encoded format](https://godoc.org/go.uber.org/zap#NewProductionEncoderConfig).
   - e.g. `--logger=zap --log-output=/dev/null` will discard all server logs.
 
 ### Added: `embed`

Documentation/upgrades/upgrade_3_4.md

@@ -79,6 +79,26 @@ cfg := &embed.Config{Debug: false}
 -cfg.SetupLogging()
 ```
 
+Changed [`embed.Config.LogOutput` type from `string` to `[]string`](https://github.com/coreos/etcd/pull/9579) to support multiple log outputs.
+
+```diff
+import "github.com/coreos/etcd/embed"
+
+cfg := &embed.Config{Debug: false}
+-cfg.LogOutput = "stderr"
++cfg.LogOutput = []string{"stderr"}
+```
+
+#### Change in `etcd --config-file`
+
+Now that `log-output` accepts multiple writers, etcd configuration YAML file `log-output` field must be changed to `[]string` type as below:
+
+```diff
+ # Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd.
+-log-output: default
++log-output: [default]
+```
+
 ### Server upgrade checklists
 
 #### Upgrade requirements

embed/config.go

@@ -23,6 +23,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"sync"
 	"syscall"
@@ -241,11 +242,12 @@ type Config struct {
 	Logger string `json:"logger"`
 
 	// LogOutput is either:
-	//  - "default" as os.Stderr
-	//  - "stderr" as os.Stderr
-	//  - "stdout" as os.Stdout
-	//  - file path to append server logs to
-	LogOutput string `json:"log-output"`
+	//  - "default" as os.Stderr,
+	//  - "stderr" as os.Stderr,
+	//  - "stdout" as os.Stdout,
+	//  - file path to append server logs to.
+	// It can be multiple when "Logger" is zap.
+	LogOutput []string `json:"log-output"`
 	// Debug is true, to enable debug level logging.
 	Debug bool `json:"debug"`
 
@@ -318,7 +320,7 @@ func NewConfig() *Config {
 		loggerMu:     new(sync.RWMutex),
 		logger:       nil,
 		Logger:       "capnslog",
-		LogOutput:    DefaultLogOutput,
+		LogOutput:    []string{DefaultLogOutput},
 		Debug:        false,
 		LogPkgLevels: "",
 	}
@@ -380,21 +382,37 @@ func (cfg *Config) setupLogging() error {
 			repoLog.SetLogLevel(settings)
 		}
 
+		if len(cfg.LogOutput) != 1 {
+			fmt.Printf("expected only 1 value in 'log-output', got %v\n", cfg.LogOutput)
+			os.Exit(1)
+		}
 		// capnslog initially SetFormatter(NewDefaultFormatter(os.Stderr))
 		// where NewDefaultFormatter returns NewJournaldFormatter when syscall.Getppid() == 1
 		// specify 'stdout' or 'stderr' to skip journald logging even when running under systemd
-		switch cfg.LogOutput {
+		output := cfg.LogOutput[0]
+		switch output {
 		case "stdout":
 			capnslog.SetFormatter(capnslog.NewPrettyFormatter(os.Stdout, cfg.Debug))
 		case "stderr":
 			capnslog.SetFormatter(capnslog.NewPrettyFormatter(os.Stderr, cfg.Debug))
 		case DefaultLogOutput:
 		default:
-			plog.Panicf(`unknown log-output %q (only supports %q, "stdout", "stderr")`, cfg.LogOutput, DefaultLogOutput)
+			plog.Panicf(`unknown log-output %q (only supports %q, "stdout", "stderr")`, output, DefaultLogOutput)
 		}
 
 	case "zap":
-		// TODO: make this more configurable
+		if len(cfg.LogOutput) == 0 {
+			cfg.LogOutput = []string{DefaultLogOutput}
+		}
+		if len(cfg.LogOutput) > 1 {
+			for _, v := range cfg.LogOutput {
+				if v == DefaultLogOutput {
+					panic(fmt.Errorf("multi logoutput for %q is not supported yet", DefaultLogOutput))
+				}
+			}
+		}
+
+		// TODO: use zapcore to support more features?
 		lcfg := zap.Config{
 			Level:       zap.NewAtomicLevelAt(zap.InfoLevel),
 			Development: false,
@@ -404,35 +422,50 @@ func (cfg *Config) setupLogging() error {
 			},
 			Encoding:      "json",
 			EncoderConfig: zap.NewProductionEncoderConfig(),
-		}
-		switch cfg.LogOutput {
-		case DefaultLogOutput:
-			if syscall.Getppid() == 1 {
-				// capnslog initially SetFormatter(NewDefaultFormatter(os.Stderr))
-				// where "NewDefaultFormatter" returns "NewJournaldFormatter"
-				// when syscall.Getppid() == 1, specify 'stdout' or 'stderr' to
-				// skip journald logging even when running under systemd
-				fmt.Println("running under init, which may be systemd!")
-				// TODO: capnlog.NewJournaldFormatter()
-				lcfg.OutputPaths = []string{"stderr"}
-				lcfg.ErrorOutputPaths = []string{"stderr"}
-			} else {
-				lcfg.OutputPaths = []string{"stderr"}
-				lcfg.ErrorOutputPaths = []string{"stderr"}
-			}
 
-		case "stderr":
-			lcfg.OutputPaths = []string{"stderr"}
-			lcfg.ErrorOutputPaths = []string{"stderr"}
+			OutputPaths:      make([]string, 0),
+			ErrorOutputPaths: make([]string, 0),
+		}
+		outputPaths, errOutputPaths := make(map[string]struct{}), make(map[string]struct{})
+		for _, v := range cfg.LogOutput {
+			switch v {
+			case DefaultLogOutput:
+				if syscall.Getppid() == 1 {
+					// capnslog initially SetFormatter(NewDefaultFormatter(os.Stderr))
+					// where "NewDefaultFormatter" returns "NewJournaldFormatter"
+					// when syscall.Getppid() == 1, specify 'stdout' or 'stderr' to
+					// skip journald logging even when running under systemd
+					// TODO: capnlog.NewJournaldFormatter()
+					fmt.Println("running under init, which may be systemd!")
+					outputPaths["stderr"] = struct{}{}
+					errOutputPaths["stderr"] = struct{}{}
+					continue
+				}
 
-		case "stdout":
-			lcfg.OutputPaths = []string{"stdout"}
-			lcfg.ErrorOutputPaths = []string{"stdout"}
+				outputPaths["stderr"] = struct{}{}
+				errOutputPaths["stderr"] = struct{}{}
 
-		default:
-			lcfg.OutputPaths = []string{cfg.LogOutput}
-			lcfg.ErrorOutputPaths = []string{cfg.LogOutput}
+			case "stderr":
+				outputPaths["stderr"] = struct{}{}
+				errOutputPaths["stderr"] = struct{}{}
+
+			case "stdout":
+				outputPaths["stdout"] = struct{}{}
+				errOutputPaths["stdout"] = struct{}{}
+
+			default:
+				outputPaths[v] = struct{}{}
+				errOutputPaths[v] = struct{}{}
+			}
+		}
+		for v := range outputPaths {
+			lcfg.OutputPaths = append(lcfg.OutputPaths, v)
+		}
+		for v := range errOutputPaths {
+			lcfg.ErrorOutputPaths = append(lcfg.ErrorOutputPaths, v)
 		}
+		sort.Strings(lcfg.OutputPaths)
+		sort.Strings(lcfg.ErrorOutputPaths)
 
 		if cfg.Debug {
 			lcfg.Level = zap.NewAtomicLevelAt(zap.DebugLevel)

embed/config_test.go

@@ -34,14 +34,14 @@ func TestConfigFileOtherFields(t *testing.T) {
 		PeerSecurityCfgFile   securityConfig `json:"peer-transport-security"`
 		ForceNewCluster       bool           `json:"force-new-cluster"`
 		Logger                string         `json:"logger"`
-		LogOutput             string         `json:"log-output"`
+		LogOutputs            []string       `json:"log-output"`
 		Debug                 bool           `json:"debug"`
 	}{
 		ctls,
 		ptls,
 		true,
 		"zap",
-		"/dev/null",
+		[]string{"/dev/null"},
 		false,
 	}
 
@@ -157,7 +157,7 @@ func mustCreateCfgFile(t *testing.T, b []byte) *os.File {
 func TestAutoCompactionModeInvalid(t *testing.T) {
 	cfg := NewConfig()
 	cfg.Logger = "zap"
-	cfg.LogOutput = "/dev/null"
+	cfg.LogOutput = []string{"/dev/null"}
 	cfg.Debug = false
 	cfg.AutoCompactionMode = "period"
 	err := cfg.Validate()

etcd.conf.yml.sample

@@ -138,7 +138,7 @@ debug: false
 log-package-levels:
 
 # Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd.
-log-output: default
+log-output: [default]
 
 # Force to create a new one member cluster.
 force-new-cluster: false

etcdmain/config.go

@@ -24,6 +24,7 @@ import (
 	"net/url"
 	"os"
 	"runtime"
+	"sort"
 	"strings"
 
 	"github.com/coreos/etcd/embed"
@@ -216,7 +217,7 @@ func newConfig() *config {
 
 	// logging
 	fs.StringVar(&cfg.ec.Logger, "logger", "capnslog", "Specify 'zap' for structured logging or 'capnslog'.")
-	fs.StringVar(&cfg.ec.LogOutput, "log-output", embed.DefaultLogOutput, "Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd.")
+	fs.Var(flags.NewUniqueStringsValue(embed.DefaultLogOutput), "log-output", "Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd, or list of comma separated output targets.")
 	fs.BoolVar(&cfg.ec.Debug, "debug", false, "Enable debug-level logging for etcd.")
 	fs.StringVar(&cfg.ec.LogPkgLevels, "log-package-levels", "", "(To be deprecated) Specify a particular log level for each etcd package (eg: 'etcdmain=CRITICAL,etcdserver=DEBUG').")
 
@@ -304,6 +305,13 @@ func (cfg *config) configFromCmdLine() error {
 
 	cfg.ec.CORS = flags.UniqueURLsMapFromFlag(cfg.cf.flagSet, "cors")
 	cfg.ec.HostWhitelist = flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "host-whitelist")
+	outputs := flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "log-output")
+	oss := make([]string, 0, len(outputs))
+	for v := range outputs {
+		oss = append(oss, v)
+	}
+	sort.Strings(oss)
+	cfg.ec.LogOutput = oss
 
 	cfg.ec.ClusterState = cfg.cf.clusterState.String()
 	cfg.cp.Fallback = cfg.cf.fallback.String()

etcdmain/help.go

@@ -157,7 +157,7 @@ Logging:
   --logger 'capnslog'
     Specify 'zap' for structured logging or 'capnslog'.
   --log-output 'default'
-    Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd.
+    Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd, or list of comma separated output targets.
   --debug 'false'
     Enable debug-level logging for etcd.
 

functional.yaml

@@ -34,7 +34,7 @@ agent-configs:
     pre-vote: true
     initial-corrupt-check: true
     logger: zap
-    log-output: /tmp/etcd-functional-1/etcd.log
+    log-output: [/tmp/etcd-functional-1/etcd.log]
     debug: true
   client-cert-data: ""
   client-cert-path: ""
@@ -85,7 +85,7 @@ agent-configs:
     pre-vote: true
     initial-corrupt-check: true
     logger: zap
-    log-output: /tmp/etcd-functional-2/etcd.log
+    log-output: [/tmp/etcd-functional-2/etcd.log]
     debug: true
   client-cert-data: ""
   client-cert-path: ""
@@ -136,7 +136,7 @@ agent-configs:
     pre-vote: true
     initial-corrupt-check: true
     logger: zap
-    log-output: /tmp/etcd-functional-3/etcd.log
+    log-output: [/tmp/etcd-functional-3/etcd.log]
     debug: true
   client-cert-data: ""
   client-cert-path: ""

functional/agent/handler.go

@@ -85,13 +85,14 @@ func (srv *Server) handleTesterRequest(req *rpcpb.Request) (resp *rpcpb.Response
 	}
 }
 
+// just archive the first file
 func (srv *Server) createEtcdLogFile() error {
 	var err error
-	srv.etcdLogFile, err = os.Create(srv.Member.Etcd.LogOutput)
+	srv.etcdLogFile, err = os.Create(srv.Member.Etcd.LogOutput[0])
 	if err != nil {
 		return err
 	}
-	srv.lg.Info("created etcd log file", zap.String("path", srv.Member.Etcd.LogOutput))
+	srv.lg.Info("created etcd log file", zap.String("path", srv.Member.Etcd.LogOutput[0]))
 	return nil
 }
 
@@ -664,7 +665,7 @@ func (srv *Server) handle_SIGQUIT_ETCD_AND_ARCHIVE_DATA() (*rpcpb.Response, erro
 	// TODO: support separate WAL directory
 	if err = archive(
 		srv.Member.BaseDir,
-		srv.Member.Etcd.LogOutput,
+		srv.Member.Etcd.LogOutput[0],
 		srv.Member.Etcd.DataDir,
 	); err != nil {
 		return nil, err

functional/rpcpb/etcd_config_test.go

@@ -58,7 +58,7 @@ func TestEtcd(t *testing.T) {
 		InitialCorruptCheck: true,
 
 		Logger:    "zap",
-		LogOutput: "/tmp/etcd-functional-1/etcd.log",
+		LogOutput: []string{"/tmp/etcd-functional-1/etcd.log"},
 		Debug:     true,
 	}
 
@@ -133,7 +133,7 @@ func TestEtcd(t *testing.T) {
 	expc.PreVote = true
 	expc.ExperimentalInitialCorruptCheck = true
 	expc.Logger = "zap"
-	expc.LogOutput = "/tmp/etcd-functional-1/etcd.log"
+	expc.LogOutput = []string{"/tmp/etcd-functional-1/etcd.log"}
 	expc.Debug = true
 	cfg, err := e.EmbedConfig()
 	if err != nil {