changes per Frey comments (Refs #493)

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

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)
+	for _, c := range correctLogLevels {
+		logger, err := tmflags.ParseLogLevel(c.lvl, jsonLogger)
+		if err != nil {
+			t.Fatal(err)
 		}
-	}
-}
 
-func TestParseComplexLogLevel(t *testing.T) {
-	logger := log.TestingLogger()
+		logger = logger.With("module", "mempool")
 
-	correctLogLevels := []string{"mempool:error", "mempool:error,*:debug", "*:debug,wire:none"}
-	for _, lvl := range correctLogLevels {
-		if _, err := tmflags.ParseComplexLogLevel(lvl, logger); err != nil {
-			t.Fatal(err)
+		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)
 		}
 	}

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

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 {