refactor(cmd/influxd): parse log-level CLI opts directly to correct type

cmd/influxd/launcher/cmd.go

@@ -93,7 +93,7 @@ type InfluxdOpts struct {
 	Testing                 bool
 	TestingAlwaysAllowSetup bool
 
-	LogLevel          string
+	LogLevel          zapcore.Level
 	TracingType       string
 	ReportingDisabled bool
 
@@ -142,7 +142,7 @@ func newOpts(viper *viper.Viper) *InfluxdOpts {
 		StorageConfig:     storage.NewConfig(),
 		CoordinatorConfig: coordinator.NewConfig(),
 
-		LogLevel:          zapcore.InfoLevel.String(),
+		LogLevel:          zapcore.InfoLevel,
 		ReportingDisabled: false,
 
 		BoltPath:   filepath.Join(dir, bolt.DefaultFilename),

cmd/influxd/launcher/launcher.go

@@ -77,7 +77,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	jaegerconfig "github.com/uber/jaeger-client-go/config"
 	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
 )
 
 const (
@@ -212,16 +211,11 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 
 	ctx, m.cancel = context.WithCancel(ctx)
 
-	var logLevel zapcore.Level
-	if err := logLevel.Set(opts.LogLevel); err != nil {
-		return fmt.Errorf("unknown log level; supported levels are debug, info, and error")
-	}
-
 	if m.log == nil {
 		// Create top level logger
 		logconf := &influxlogger.Config{
 			Format: "auto",
-			Level:  logLevel,
+			Level:  opts.LogLevel,
 		}
 		m.log, err = logconf.New(m.Stdout)
 		if err != nil {
@@ -920,7 +914,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 			http.WithAPIHandler(platformHandler),
 		)
 
-		if logLevel == zap.DebugLevel {
+		if opts.LogLevel == zap.DebugLevel {
 			m.httpServer.Handler = http.LoggingMW(httpLogger)(m.httpServer.Handler)
 		}
 		// If we are in testing mode we allow all data to be flushed and removed.

cmd/influxd/launcher/launcher_helpers.go

@@ -115,7 +115,7 @@ func (tl *TestLauncher) Run(ctx context.Context, setters ...OptSetter) error {
 	opts.BoltPath = filepath.Join(tl.Path, bolt.DefaultFilename)
 	opts.EnginePath = filepath.Join(tl.Path, "engine")
 	opts.HttpBindAddress = "127.0.0.1:0"
-	opts.LogLevel = zap.DebugLevel.String()
+	opts.LogLevel = zap.DebugLevel
 	opts.ReportingDisabled = true
 
 	for _, setter := range setters {

cmd/influxd/launcher/pkger_test.go

@@ -28,7 +28,7 @@ var ctx = context.Background()
 
 func TestLauncher_Pkger(t *testing.T) {
 	l := RunTestLauncherOrFail(t, ctx, nil, func(o *InfluxdOpts) {
-		o.LogLevel = zap.ErrorLevel.String()
+		o.LogLevel = zap.ErrorLevel
 	})
 	l.SetupOrFail(t)
 	defer l.ShutdownOrFail(t, ctx)

cmd/influxd/launcher/query_test.go

@@ -340,7 +340,7 @@ func TestLauncher_QueryMemoryManager_ExceedMemory(t *testing.T) {
 	t.Skip("this test is flaky, occasionally get error: \"memory allocation limit reached\" on OK query")
 
 	l := launcher.RunTestLauncherOrFail(t, ctx, nil, func(o *launcher.InfluxdOpts) {
-		o.LogLevel = zap.ErrorLevel.String()
+		o.LogLevel = zap.ErrorLevel
 		o.ConcurrencyQuota = 1
 		o.InitialMemoryBytesQuotaPerQuery = 100
 		o.MemoryBytesQuotaPerQuery = 50000
@@ -385,7 +385,7 @@ func TestLauncher_QueryMemoryManager_ContextCanceled(t *testing.T) {
 	t.Skip("this test is flaky, occasionally get error: \"memory allocation limit reached\"")
 
 	l := launcher.RunTestLauncherOrFail(t, ctx, nil, func(o *launcher.InfluxdOpts) {
-		o.LogLevel = zap.ErrorLevel.String()
+		o.LogLevel = zap.ErrorLevel
 		o.ConcurrencyQuota = 1
 		o.InitialMemoryBytesQuotaPerQuery = 100
 		o.MemoryBytesQuotaPerQuery = 50000
@@ -429,7 +429,7 @@ func TestLauncher_QueryMemoryManager_ConcurrentQueries(t *testing.T) {
 	t.Skip("this test is flaky, occasionally get error: \"dial tcp 127.0.0.1:59654: connect: connection reset by peer\"")
 
 	l := launcher.RunTestLauncherOrFail(t, ctx, nil, func(o *launcher.InfluxdOpts) {
-		o.LogLevel = zap.ErrorLevel.String()
+		o.LogLevel = zap.ErrorLevel
 		o.QueueSize = 1024
 		o.ConcurrencyQuota = 1
 		o.InitialMemoryBytesQuotaPerQuery = 10000

cmd/influxd/upgrade/upgrade.go

@@ -113,7 +113,7 @@ var options = struct {
 	verbose bool
 
 	// logging
-	logLevel string
+	logLevel zapcore.Level
 	logPath  string
 
 	force bool
@@ -247,7 +247,7 @@ func NewCommand(v *viper.Viper) *cobra.Command {
 		{
 			DestP:   &options.logLevel,
 			Flag:    "log-level",
-			Default: zapcore.InfoLevel.String(),
+			Default: zapcore.InfoLevel,
 			Desc:    "supported log levels are debug, info, warn and error",
 		},
 		{
@@ -316,11 +316,6 @@ func runUpgradeE(*cobra.Command, []string) error {
 		fluxInitialized = true
 	}
 
-	var lvl zapcore.Level
-	if err := lvl.Set(options.logLevel); err != nil {
-		return errors.New("unknown log level; supported levels are debug, info, warn and error")
-	}
-
 	if options.source.configFile != "" && options.source.dbDir != "" {
 		return errors.New("only one of --v1-dir or --config-file may be specified")
 	}
@@ -340,7 +335,7 @@ func runUpgradeE(*cobra.Command, []string) error {
 
 	ctx := context.Background()
 	config := zap.NewProductionConfig()
-	config.Level = zap.NewAtomicLevelAt(lvl)
+	config.Level = zap.NewAtomicLevelAt(options.logLevel)
 	config.OutputPaths = append(config.OutputPaths, options.logPath)
 	config.ErrorOutputPaths = append(config.ErrorOutputPaths, options.logPath)
 	log, err := config.Build()

kit/cli/loglevelflag.go

@@ -0,0 +1,42 @@
+package cli
+
+import (
+	"fmt"
+
+	"github.com/spf13/pflag"
+	"go.uber.org/zap/zapcore"
+)
+
+type levelValue zapcore.Level
+
+func newLevelValue(val zapcore.Level, p *zapcore.Level) *levelValue {
+	*p = val
+	return (*levelValue)(p)
+}
+
+func (l *levelValue) String() string {
+	return zapcore.Level(*l).String()
+}
+func (l *levelValue) Set(s string) error {
+	var level zapcore.Level
+	if err := level.Set(s); err != nil {
+		return fmt.Errorf("unknown log level; supported levels are debug, info, warn, error")
+	}
+	*l = levelValue(level)
+	return nil
+}
+
+func (l *levelValue) Type() string {
+	return "Log-Level"
+}
+
+// LevelVar defines a zapcore.Level flag with specified name, default value, and usage string.
+// The argument p points to a zapcore.Level variable in which to store the value of the flag.
+func LevelVar(fs *pflag.FlagSet, p *zapcore.Level, name string, value zapcore.Level, usage string) {
+	LevelVarP(fs, p, name, "", value, usage)
+}
+
+// LevelVarP is like LevelVar, but accepts a shorthand letter that can be used after a single dash.
+func LevelVarP(fs *pflag.FlagSet, p *zapcore.Level, name, shorthand string, value zapcore.Level, usage string) {
+	fs.VarP(newLevelValue(value, p), name, shorthand, usage)
+}

kit/cli/viper.go

@@ -11,6 +11,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
+	"go.uber.org/zap/zapcore"
 )
 
 // Opt is a single command-line option
@@ -274,6 +275,19 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
 			if s := v.GetString(envVar); s != "" {
 				_ = (*destP).DecodeFromString(v.GetString(envVar))
 			}
+		case *zapcore.Level:
+			var l zapcore.Level
+			if o.Default != nil {
+				l = o.Default.(zapcore.Level)
+			}
+			if hasShort {
+				LevelVarP(flagset, destP, o.Flag, string(o.Short), l, o.Desc)
+			} else {
+				LevelVar(flagset, destP, o.Flag, l, o.Desc)
+			}
+			if s := v.GetString(envVar); s != "" {
+				_ = (*destP).Set(v.GetString(envVar))
+			}
 		default:
 			// if you get a panic here, sorry about that!
 			// anyway, go ahead and make a PR and add another type.

kit/cli/viper_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zapcore"
 )
 
 type customFlag bool
@@ -47,6 +48,7 @@ func ExampleNewCommand() {
 	var duration time.Duration
 	var stringSlice []string
 	var fancyBool customFlag
+	var logLevel zapcore.Level
 	cmd := NewCommand(viper.New(), &Program{
 		Run: func() error {
 			fmt.Println(monitorHost)
@@ -58,6 +60,7 @@ func ExampleNewCommand() {
 			fmt.Println(duration)
 			fmt.Println(stringSlice)
 			fmt.Println(fancyBool)
+			fmt.Println(logLevel.String())
 			return nil
 		},
 		Name: "myprogram",
@@ -110,6 +113,11 @@ func ExampleNewCommand() {
 				Default: "on",
 				Desc:    "things that implement pflag.Value",
 			},
+			{
+				DestP:   &logLevel,
+				Flag:    "log-level",
+				Default: zapcore.WarnLevel,
+			},
 		},
 	})
 
@@ -125,6 +133,7 @@ func ExampleNewCommand() {
 	// 1m0s
 	// [foo bar]
 	// on
+	// warn
 }
 
 func Test_NewProgram(t *testing.T) {
@@ -134,6 +143,7 @@ func Test_NewProgram(t *testing.T) {
 		"shoe-fly":    "yadon",
 		"number":      "2147483647",
 		"long-number": "9223372036854775807",
+		"log-level":   "debug",
 	})
 	defer cleanup()
 	defer setEnvVar("TEST_CONFIG_PATH", testFilePath)()
@@ -176,6 +186,7 @@ func Test_NewProgram(t *testing.T) {
 			var testFly string
 			var testNumber int32
 			var testLongNumber int64
+			var logLevel zapcore.Level
 			program := &Program{
 				Name: "test",
 				Opts: []Opt{
@@ -196,6 +207,10 @@ func Test_NewProgram(t *testing.T) {
 						DestP: &testLongNumber,
 						Flag:  "long-number",
 					},
+					{
+						DestP: &logLevel,
+						Flag:  "log-level",
+					},
 				},
 				Run: func() error { return nil },
 			}
@@ -208,6 +223,7 @@ func Test_NewProgram(t *testing.T) {
 			assert.Equal(t, "yadon", testFly)
 			assert.Equal(t, int32(math.MaxInt32), testNumber)
 			assert.Equal(t, int64(math.MaxInt64), testLongNumber)
+			assert.Equal(t, zapcore.DebugLevel, logLevel)
 		}
 
 		t.Run(tt.name, fn)