From 44c9b7af38a8a2609b5627f8d93fd40fd9c8dd0d Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 15 Nov 2021 17:23:27 +0100 Subject: [PATCH] fix(log-levels): do not ignore configuration file log levels (#2016) - Fix config log levels being ignored (remove default flag values) - Use `parseLogLevelString` in other places to reduce code duplication - Rename `cfg` to `tomlConfig` in `setLogConfig` --- cmd/gossamer/config.go | 40 ++++++++++++++++++++------------------ cmd/gossamer/flags.go | 10 ---------- cmd/gossamer/utils.go | 8 +++----- cmd/gossamer/utils_test.go | 2 +- 4 files changed, 25 insertions(+), 35 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 7f79f942a0..c381499059 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -315,16 +315,16 @@ func parseLogLevelString(logLevelString string) (logLevel log.Level, err error) return logLevel, nil } -func setLogConfig(flagsKVStore stringKVStore, cfg *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) (err error) { - if cfg == nil { - cfg = new(ctoml.Config) +func setLogConfig(flagsKVStore stringKVStore, tomlConfig *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) (err error) { + if tomlConfig == nil { + tomlConfig = new(ctoml.Config) } - globalCfg.LogLvl, err = getLogLevel(flagsKVStore, LogFlag.Name, cfg.Global.LogLvl, gssmr.DefaultLvl) + globalCfg.LogLvl, err = getLogLevel(flagsKVStore, LogFlag.Name, tomlConfig.Global.LogLvl, gssmr.DefaultLvl) if err != nil { return fmt.Errorf("cannot get global log level: %w", err) } - cfg.Global.LogLvl = globalCfg.LogLvl.String() + tomlConfig.Global.LogLvl = globalCfg.LogLvl.String() levelsData := []struct { name string @@ -335,55 +335,55 @@ func setLogConfig(flagsKVStore stringKVStore, cfg *ctoml.Config, globalCfg *dot. { name: "core", flagName: LogCoreLevelFlag.Name, - tomlValue: cfg.Log.CoreLvl, + tomlValue: tomlConfig.Log.CoreLvl, levelPtr: &logCfg.CoreLvl, }, { name: "sync", flagName: LogSyncLevelFlag.Name, - tomlValue: cfg.Log.SyncLvl, + tomlValue: tomlConfig.Log.SyncLvl, levelPtr: &logCfg.SyncLvl, }, { name: "network", flagName: LogNetworkLevelFlag.Name, - tomlValue: cfg.Log.NetworkLvl, + tomlValue: tomlConfig.Log.NetworkLvl, levelPtr: &logCfg.NetworkLvl, }, { name: "RPC", flagName: LogRPCLevelFlag.Name, - tomlValue: cfg.Log.RPCLvl, + tomlValue: tomlConfig.Log.RPCLvl, levelPtr: &logCfg.RPCLvl, }, { name: "state", flagName: LogStateLevelFlag.Name, - tomlValue: cfg.Log.StateLvl, + tomlValue: tomlConfig.Log.StateLvl, levelPtr: &logCfg.StateLvl, }, { name: "runtime", flagName: LogRuntimeLevelFlag.Name, - tomlValue: cfg.Log.RuntimeLvl, + tomlValue: tomlConfig.Log.RuntimeLvl, levelPtr: &logCfg.RuntimeLvl, }, { name: "block producer", flagName: LogBabeLevelFlag.Name, - tomlValue: cfg.Log.BlockProducerLvl, + tomlValue: tomlConfig.Log.BlockProducerLvl, levelPtr: &logCfg.BlockProducerLvl, }, { name: "finality gadget", flagName: LogGrandpaLevelFlag.Name, - tomlValue: cfg.Log.FinalityGadgetLvl, + tomlValue: tomlConfig.Log.FinalityGadgetLvl, levelPtr: &logCfg.FinalityGadgetLvl, }, { name: "sync", flagName: LogSyncLevelFlag.Name, - tomlValue: cfg.Log.SyncLvl, + tomlValue: tomlConfig.Log.SyncLvl, levelPtr: &logCfg.SyncLvl, }, } @@ -442,7 +442,10 @@ func setDotGlobalConfigFromToml(tomlCfg *ctoml.Config, cfg *dot.GlobalConfig) { } if tomlCfg.Global.LogLvl != "" { - cfg.LogLvl, _ = log.ParseLevel(tomlCfg.Global.LogLvl) + level, err := parseLogLevelString(tomlCfg.Global.LogLvl) + if err == nil { + cfg.LogLvl = level + } } cfg.MetricsPort = tomlCfg.Global.MetricsPort @@ -465,10 +468,9 @@ func setDotGlobalConfigFromFlags(ctx *cli.Context, cfg *dot.GlobalConfig) error } // check --log flag - if lvlToInt, err := strconv.Atoi(ctx.String(LogFlag.Name)); err == nil { - cfg.LogLvl = log.Level(lvlToInt) - } else if lvl, err := log.ParseLevel(ctx.String(LogFlag.Name)); err == nil { - cfg.LogLvl = lvl + logLevel, err := parseLogLevelString(ctx.String(LogFlag.Name)) + if err == nil { + cfg.LogLvl = logLevel } cfg.PublishMetrics = ctx.Bool("publish-metrics") diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index 61c9b8edf5..11dc3b05e7 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -5,7 +5,6 @@ package main import ( "github.com/ChainSafe/gossamer/chain/dev" - "github.com/ChainSafe/gossamer/internal/log" "github.com/urfave/cli" ) @@ -44,47 +43,38 @@ var ( LogFlag = cli.StringFlag{ Name: "log", Usage: "Global log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: log.Info.String(), } LogCoreLevelFlag = cli.StringFlag{ Name: "log-core", Usage: "Core package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogSyncLevelFlag = cli.StringFlag{ Name: "log-sync", Usage: "Sync package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogNetworkLevelFlag = cli.StringFlag{ Name: "log-network", Usage: "Network package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogRPCLevelFlag = cli.StringFlag{ Name: "log-rpc", Usage: "RPC package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogStateLevelFlag = cli.StringFlag{ Name: "log-state", Usage: "State package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogRuntimeLevelFlag = cli.StringFlag{ Name: "log-runtime", Usage: "Runtime package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogBabeLevelFlag = cli.StringFlag{ Name: "log-babe", Usage: "BABE package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } LogGrandpaLevelFlag = cli.StringFlag{ Name: "log-grandpa", Usage: "Grandpa package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", - Value: LogFlag.Value, } // NameFlag node implementation name diff --git a/cmd/gossamer/utils.go b/cmd/gossamer/utils.go index 5c4ee2c174..8c3567f479 100644 --- a/cmd/gossamer/utils.go +++ b/cmd/gossamer/utils.go @@ -7,7 +7,6 @@ import ( "bufio" "fmt" "os" - "strconv" "strings" "syscall" "testing" @@ -24,10 +23,9 @@ const confirmCharacter = "Y" // setupLogger sets up the global Gossamer logger. func setupLogger(ctx *cli.Context) (level log.Level, err error) { - if lvlToInt, err := strconv.Atoi(ctx.String(LogFlag.Name)); err == nil { - level = log.Level(lvlToInt) - } else if level, err = log.ParseLevel(ctx.String(LogFlag.Name)); err != nil { - return 0, err + level, err = getLogLevel(ctx, LogFlag.Name, "", log.Info) + if err != nil { + return level, err } log.Patch( diff --git a/cmd/gossamer/utils_test.go b/cmd/gossamer/utils_test.go index 9c4b942378..34e01fce49 100644 --- a/cmd/gossamer/utils_test.go +++ b/cmd/gossamer/utils_test.go @@ -112,7 +112,7 @@ func Test_setupLogger(t *testing.T) { "Test gossamer --log blah", []string{"log"}, []interface{}{"blah"}, - errors.New("level is not recognised: blah"), + errors.New("cannot parse log level string: level is not recognised: blah"), }, }