From fb0df75de097c2b92b4dfb5396db36e02e526bbe Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 16 May 2017 15:16:50 +0200 Subject: [PATCH] changes per Frey comments (Refs #493) --- cmd/tendermint/commands/flags/log_level.go | 20 +++--- .../commands/flags/log_level_test.go | 64 +++++++++++++------ cmd/tendermint/commands/root.go | 24 ++----- cmd/tendermint/commands/run_node.go | 9 --- 4 files changed, 61 insertions(+), 56 deletions(-) diff --git a/cmd/tendermint/commands/flags/log_level.go b/cmd/tendermint/commands/flags/log_level.go index 361ec9d2..acdd24ec 100644 --- a/cmd/tendermint/commands/flags/log_level.go +++ b/cmd/tendermint/commands/flags/log_level.go @@ -14,22 +14,24 @@ const ( defaultLogLevelKey = "*" ) -// IsLogLevelSimple returns true if log level is a single word ("info", ...). -func IsLogLevelSimple(l string) bool { - return !strings.Contains(l, ":") -} - -// ParseComplexLogLevel parses complex log level - comma-separated +// ParseLogLevel parses complex log level - comma-separated // list of module:level pairs with an optional *:level pair (* means // all other modules). // // Example: -// ParseComplexLogLevel("consensus:debug,mempool:debug,*:error", log.NewTMLogger(os.Stdout)) -func ParseComplexLogLevel(l string, logger log.Logger) (log.Logger, error) { - if l == "" { +// ParseLogLevel("consensus:debug,mempool:debug,*:error", log.NewTMLogger(os.Stdout)) +func ParseLogLevel(lvl string, logger log.Logger) (log.Logger, error) { + if lvl == "" { return nil, errors.New("Empty log level") } + l := lvl + + // prefix simple one word levels (e.g. "info") with "*" + if !strings.Contains(l, ":") { + l = defaultLogLevelKey + ":" + l + } + options := make([]log.Option, 0) isDefaultLogLevelSet := false diff --git a/cmd/tendermint/commands/flags/log_level_test.go b/cmd/tendermint/commands/flags/log_level_test.go index 6ccd5c84..c89f3f88 100644 --- a/cmd/tendermint/commands/flags/log_level_test.go +++ b/cmd/tendermint/commands/flags/log_level_test.go @@ -1,41 +1,63 @@ package flags_test import ( + "bytes" + "strings" "testing" tmflags "github.com/tendermint/tendermint/cmd/tendermint/commands/flags" "github.com/tendermint/tmlibs/log" ) -func TestIsLogLevelSimple(t *testing.T) { - simpleFlags := []string{"info", "debug", "some"} - for _, f := range simpleFlags { - if !tmflags.IsLogLevelSimple(f) { - t.Fatalf("%s is a simple flag", f) - } +func TestParseLogLevel(t *testing.T) { + var buf bytes.Buffer + jsonLogger := log.NewTMJSONLogger(&buf) + + correctLogLevels := []struct { + lvl string + expectedLogLines []string + }{ + {"mempool:error", []string{``, ``, `{"_msg":"Mesmero","level":"error","module":"mempool"}`}}, + {"mempool:error,*:debug", []string{``, ``, `{"_msg":"Mesmero","level":"error","module":"mempool"}`}}, + {"*:debug,wire:none", []string{ + `{"_msg":"Kingpin","level":"debug","module":"mempool"}`, + `{"_msg":"Kitty Pryde","level":"info","module":"mempool"}`, + `{"_msg":"Mesmero","level":"error","module":"mempool"}`}}, } - complexFlags := []string{"mempool:error", "mempool:error,*:debug"} - for _, f := range complexFlags { - if tmflags.IsLogLevelSimple(f) { - t.Fatalf("%s is a complex flag", f) - } - } -} - -func TestParseComplexLogLevel(t *testing.T) { - logger := log.TestingLogger() - - correctLogLevels := []string{"mempool:error", "mempool:error,*:debug", "*:debug,wire:none"} - for _, lvl := range correctLogLevels { - if _, err := tmflags.ParseComplexLogLevel(lvl, logger); err != nil { + for _, c := range correctLogLevels { + logger, err := tmflags.ParseLogLevel(c.lvl, jsonLogger) + if err != nil { t.Fatal(err) } + + logger = logger.With("module", "mempool") + + buf.Reset() + + logger.Debug("Kingpin") + if have := strings.TrimSpace(buf.String()); c.expectedLogLines[0] != have { + t.Errorf("\nwant '%s'\nhave '%s'\nlevel '%s'", c.expectedLogLines[0], have, c.lvl) + } + + buf.Reset() + + logger.Info("Kitty Pryde") + if have := strings.TrimSpace(buf.String()); c.expectedLogLines[1] != have { + t.Errorf("\nwant '%s'\nhave '%s'\nlevel '%s'", c.expectedLogLines[1], have, c.lvl) + } + + buf.Reset() + + logger.Error("Mesmero") + if have := strings.TrimSpace(buf.String()); c.expectedLogLines[2] != have { + t.Errorf("\nwant '%s'\nhave '%s'\nlevel '%s'", c.expectedLogLines[2], have, c.lvl) + } } incorrectLogLevel := []string{"some", "mempool:some", "*:some,mempool:error"} for _, lvl := range incorrectLogLevel { - if _, err := tmflags.ParseComplexLogLevel(lvl, logger); err == nil { + if _, err := tmflags.ParseLogLevel(lvl, jsonLogger); err == nil { t.Fatalf("Expected %s to produce error", lvl) } } diff --git a/cmd/tendermint/commands/root.go b/cmd/tendermint/commands/root.go index 79448b98..3565f3bb 100644 --- a/cmd/tendermint/commands/root.go +++ b/cmd/tendermint/commands/root.go @@ -1,7 +1,6 @@ package commands import ( - "fmt" "os" "github.com/spf13/cobra" @@ -26,24 +25,15 @@ var RootCmd = &cobra.Command{ Short: "Tendermint Core (BFT Consensus) in Go", PersistentPreRunE: func(cmd *cobra.Command, args []string) error { err := viper.Unmarshal(config) + if err != nil { + return err + } config.SetRoot(config.RootDir) cfg.EnsureRoot(config.RootDir) - if tmflags.IsLogLevelSimple(config.LogLevel) { - var option log.Option - switch config.LogLevel { - case "info": - option = log.AllowInfo() - case "debug": - option = log.AllowDebug() - case "error": - option = log.AllowError() - case "none": - option = log.AllowNone() - default: - return fmt.Errorf("Expected log level to be either \"info\", \"debug\", \"error\" or \"none\", given %s", config.LogLevel) - } - logger = log.NewFilter(logger, option) + logger, err = tmflags.ParseLogLevel(config.LogLevel, logger) + if err != nil { + return err } - return err + return nil }, } diff --git a/cmd/tendermint/commands/run_node.go b/cmd/tendermint/commands/run_node.go index 5eb477f7..5e38f2b5 100644 --- a/cmd/tendermint/commands/run_node.go +++ b/cmd/tendermint/commands/run_node.go @@ -7,7 +7,6 @@ import ( "github.com/spf13/cobra" - tmflags "github.com/tendermint/tendermint/cmd/tendermint/commands/flags" "github.com/tendermint/tendermint/node" "github.com/tendermint/tendermint/types" cmn "github.com/tendermint/tmlibs/common" @@ -80,14 +79,6 @@ func runNode(cmd *cobra.Command, args []string) error { } } - if !tmflags.IsLogLevelSimple(config.LogLevel) { - var err error - logger, err = tmflags.ParseComplexLogLevel(config.LogLevel, logger) - if err != nil { - return err - } - } - // Create & start node n := node.NewNodeDefault(config, logger.With("module", "node")) if _, err := n.Start(); err != nil {