Browse Source

config: multiple logging fixes

First, don't panic with invalid --log-outputs. For example:

  $> ./bin/etcd --log-outputs foo
  2018-12-20 15:05:47.988652 C | embed: unknown log-output "foo" (only supports "default", "stderr", "stdout")
  panic: unknown log-output "foo" (only supports "default", "stderr", "stdout")

  goroutine 1 [running]:
  go.etcd.io/etcd/vendor/github.com/coreos/pkg/capnslog.(*PackageLogger).Panicf(0xc000294b00, 0x10fe067, 0x30, 0xc0001fa398, 0x4, 0x4)
        go.etcd.io/etcd/vendor/github.com/coreos/pkg/capnslog/pkg_logger.go:75 +0x161
  go.etcd.io/etcd/embed.(*Config).setupLogging(0xc000291400, 0xc0002a85b0, 0x1)
        go.etcd.io/etcd/embed/config_logging.go:120 +0x1939
  ...

Or:

 $> ./bin/etcd --log-outputs foo,default --logger zap
 panic: multi logoutput for "default" is not supported yet

 goroutine 1 [running]:
 go.etcd.io/etcd/embed.(*Config).setupLogging(0xc000314500, 0xc0001b2f70, 0x1)
        go.etcd.io/etcd/embed/config_logging.go:129 +0x2437
 go.etcd.io/etcd/embed.(*Config).Validate(0xc000314500, 0xc000268a98, 0x127e440)
        go.etcd.io/etcd/embed/config.go:543 +0x43

Second, don't exit in embed.setupLogging(). Before:

  $> ./bin/etcd --log-outputs foo,bar
  --logger=capnslog supports only 1 value in '--log-outputs', got ["bar" "foo"]

and after:

  $> ./bin/etcd --log-outputs foo,bar
  2018-12-20 15:10:24.317982 E | etcdmain: error verifying flags, --logger=capnslog supports only 1 value in '--log-outputs', got ["bar" "foo"]. See 'etcd --help'.

Third, remove duplicated unique strings code. UniqueStringsFromFlag()
is already available to return a sorted slice of values, so just use
that.

Lastly, fix a tiny logging typo in config.
Mark McLoughlin 7 years ago
parent
commit
fcc29894c2
2 changed files with 6 additions and 21 deletions
  1. 3 4
      embed/config_logging.go
  2. 3 17
      etcdmain/config.go

+ 3 - 4
embed/config_logging.go

@@ -103,8 +103,7 @@ func (cfg *Config) setupLogging() error {
 		}
 
 		if len(cfg.LogOutputs) != 1 {
-			fmt.Printf("--logger=capnslog supports only 1 value in '--log-outputs', got %q\n", cfg.LogOutputs)
-			os.Exit(1)
+			return fmt.Errorf("--logger=capnslog supports only 1 value in '--log-outputs', got %q", cfg.LogOutputs)
 		}
 		// capnslog initially SetFormatter(NewDefaultFormatter(os.Stderr))
 		// where NewDefaultFormatter returns NewJournaldFormatter when syscall.Getppid() == 1
@@ -117,7 +116,7 @@ func (cfg *Config) setupLogging() error {
 			capnslog.SetFormatter(capnslog.NewPrettyFormatter(os.Stdout, cfg.Debug))
 		case DefaultLogOutput:
 		default:
-			plog.Panicf(`unknown log-output %q (only supports %q, %q, %q)`, output, DefaultLogOutput, StdErrLogOutput, StdOutLogOutput)
+			return fmt.Errorf("unknown log-output %q (only supports %q, %q, %q)", output, DefaultLogOutput, StdErrLogOutput, StdOutLogOutput)
 		}
 
 	case "zap":
@@ -127,7 +126,7 @@ func (cfg *Config) setupLogging() error {
 		if len(cfg.LogOutputs) > 1 {
 			for _, v := range cfg.LogOutputs {
 				if v == DefaultLogOutput {
-					panic(fmt.Errorf("multi logoutput for %q is not supported yet", DefaultLogOutput))
+					return fmt.Errorf("multi logoutput for %q is not supported yet", DefaultLogOutput)
 				}
 			}
 		}

+ 3 - 17
etcdmain/config.go

@@ -24,7 +24,6 @@ import (
 	"net/url"
 	"os"
 	"runtime"
-	"sort"
 	"strings"
 
 	"go.etcd.io/etcd/embed"
@@ -284,7 +283,7 @@ func (cfg *config) parse(arguments []string) error {
 		err = cfg.configFromFile(cfg.configFile)
 		if lg := cfg.ec.GetLogger(); lg != nil {
 			lg.Info(
-				"loaded server configuraionl, other configuration command line flags and environment variables will be ignored if provided",
+				"loaded server configuration, other configuration command line flags and environment variables will be ignored if provided",
 				zap.String("path", cfg.configFile),
 			)
 		} else {
@@ -315,21 +314,8 @@ func (cfg *config) configFromCmdLine() error {
 	cfg.ec.CipherSuites = flags.StringsFromFlag(cfg.cf.flagSet, "cipher-suites")
 
 	// TODO: remove this in v3.5
-	output := flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "log-output")
-	oss1 := make([]string, 0, len(output))
-	for v := range output {
-		oss1 = append(oss1, v)
-	}
-	sort.Strings(oss1)
-	cfg.ec.DeprecatedLogOutput = oss1
-
-	outputs := flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "log-outputs")
-	oss2 := make([]string, 0, len(outputs))
-	for v := range outputs {
-		oss2 = append(oss2, v)
-	}
-	sort.Strings(oss2)
-	cfg.ec.LogOutputs = oss2
+	cfg.ec.DeprecatedLogOutput = flags.UniqueStringsFromFlag(cfg.cf.flagSet, "log-output")
+	cfg.ec.LogOutputs = flags.UniqueStringsFromFlag(cfg.cf.flagSet, "log-outputs")
 
 	cfg.ec.ClusterState = cfg.cf.clusterState.String()
 	cfg.cp.Fallback = cfg.cf.fallback.String()