From 9c23d8196bc84a8560dc0d72370729fe8703f41e Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Tue, 2 Nov 2021 15:56:19 +0000 Subject: [PATCH 1/9] Read log levels from flags --- cmd/gossamer/config.go | 150 +++++++++++++----------------- cmd/gossamer/config_test.go | 181 ++++++++++++++++++++++++++++++++++++ cmd/gossamer/flags.go | 51 +++++++++- 3 files changed, 298 insertions(+), 84 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 1d083972f7..fac43eb6e0 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -18,6 +18,7 @@ package main import ( "fmt" + "regexp" "strconv" "strings" "time" @@ -287,115 +288,98 @@ func createExportConfig(ctx *cli.Context) (*dot.Config, error) { return cfg, nil } -func setLogConfig(ctx *cli.Context, cfg *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) error { - if cfg == nil { - cfg = new(ctoml.Config) - } - - if lvlStr := ctx.String(LogFlag.Name); lvlStr != "" { - if lvlToInt, err := strconv.Atoi(lvlStr); err == nil { - lvlStr = log.Lvl(lvlToInt).String() - } - cfg.Global.LogLvl = lvlStr - } +type getStringer interface { + String(key string) (value string) +} - if cfg.Global.LogLvl == "" { - cfg.Global.LogLvl = gssmr.DefaultLvl.String() +// getLogLevel obtains the log level in the following order: +// 1. Try to obtain it from the flag value corresponding to flagName. +// 2. Try to obtain it from the TOML value given, if step 1. failed. +// 3. Return the default value given if both previous steps failed. +// For steps 1 and 2, it tries to parse the level as an integer to convert it +// to a level, and also tries to parse it as a string. +func getLogLevel(ctx getStringer, flagName, tomlValue string, defaultLevel log.Lvl) ( + level log.Lvl, err error) { + if flagValue := ctx.String(flagName); flagValue != "" { + return parseLogLevelString(flagValue) } - var err error - globalCfg.LogLvl, err = log.LvlFromString(cfg.Global.LogLvl) - if err != nil { - return err + if tomlValue == "" { + return defaultLevel, nil } - // check and set log levels for each pkg - if cfg.Log.CoreLvl == "" { - logCfg.CoreLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.CoreLvl) - if err != nil { - return err - } + return parseLogLevelString(tomlValue) +} - logCfg.CoreLvl = lvl - } +var regexDigits = regexp.MustCompile("^[0-9]+$") - if cfg.Log.SyncLvl == "" { - logCfg.SyncLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.SyncLvl) - if err != nil { - return err +func parseLogLevelString(logLevelString string) (logLevel log.Lvl, err error) { + if regexDigits.MatchString(logLevelString) { + levelInt, err := strconv.Atoi(logLevelString) + if err != nil { // should never happen + return 0, fmt.Errorf("cannot parse log level digits: %w", err) } + logLevel = log.Lvl(levelInt) + return logLevel, nil + } - logCfg.SyncLvl = lvl + logLevel, err = log.LvlFromString(logLevelString) + if err != nil { + return 0, fmt.Errorf("cannot parse log level string: %w", err) } - if cfg.Log.NetworkLvl == "" { - logCfg.NetworkLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.NetworkLvl) - if err != nil { - return err - } + return logLevel, nil +} - logCfg.NetworkLvl = lvl +func setLogConfig(ctx getStringer, cfg *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) (err error) { + if cfg == nil { + cfg = new(ctoml.Config) } - if cfg.Log.RPCLvl == "" { - logCfg.RPCLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.RPCLvl) - if err != nil { - return err - } - - logCfg.RPCLvl = lvl + globalCfg.LogLvl, err = getLogLevel(ctx, LogFlag.Name, cfg.Global.LogLvl, gssmr.DefaultLvl) + if err != nil { + return fmt.Errorf("cannot get global log level: %w", err) } + cfg.Global.LogLvl = globalCfg.LogLvl.String() - if cfg.Log.StateLvl == "" { - logCfg.StateLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.StateLvl) - if err != nil { - return err - } + logCfg.CoreLvl, err = getLogLevel(ctx, LogCoreLevelFlag.Name, cfg.Log.CoreLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get core log level: %w", err) + } - logCfg.StateLvl = lvl + logCfg.SyncLvl, err = getLogLevel(ctx, LogSyncLevelFlag.Name, cfg.Log.SyncLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get sync log level: %w", err) } - if cfg.Log.RuntimeLvl == "" { - logCfg.RuntimeLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.RuntimeLvl) - if err != nil { - return err - } + logCfg.NetworkLvl, err = getLogLevel(ctx, LogNetworkLevelFlag.Name, cfg.Log.NetworkLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get network log level: %w", err) + } - logCfg.RuntimeLvl = lvl + logCfg.RPCLvl, err = getLogLevel(ctx, LogRPCLevelFlag.Name, cfg.Log.RPCLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get RPC log level: %w", err) } - if cfg.Log.BlockProducerLvl == "" { - logCfg.BlockProducerLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.BlockProducerLvl) - if err != nil { - return err - } + logCfg.StateLvl, err = getLogLevel(ctx, LogStateLevelFlag.Name, cfg.Log.StateLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get state log level: %w", err) + } - logCfg.BlockProducerLvl = lvl + logCfg.RuntimeLvl, err = getLogLevel(ctx, LogRuntimeLevelFlag.Name, cfg.Log.RuntimeLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get runtime log level: %w", err) } - if cfg.Log.FinalityGadgetLvl == "" { - logCfg.FinalityGadgetLvl = globalCfg.LogLvl - } else { - lvl, err := log.LvlFromString(cfg.Log.FinalityGadgetLvl) - if err != nil { - return err - } + logCfg.BlockProducerLvl, err = getLogLevel(ctx, LogBlockProducerLevelFlag.Name, cfg.Log.BlockProducerLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get block producer log level: %w", err) + } - logCfg.FinalityGadgetLvl = lvl + logCfg.FinalityGadgetLvl, err = getLogLevel(ctx, LogFinalityGadgetLevelFlag.Name, cfg.Log.FinalityGadgetLvl, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get finality gadget log level: %w", err) } logger.Debug("set log configuration", "--log", ctx.String(LogFlag.Name), "global", globalCfg.LogLvl) diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index 64ef0ee06c..e1c2697d20 100644 --- a/cmd/gossamer/config_test.go +++ b/cmd/gossamer/config_test.go @@ -17,6 +17,7 @@ package main import ( + "errors" "io/ioutil" "testing" "time" @@ -24,12 +25,14 @@ import ( "github.com/ChainSafe/gossamer/chain/dev" "github.com/ChainSafe/gossamer/chain/gssmr" "github.com/ChainSafe/gossamer/dot" + ctoml "github.com/ChainSafe/gossamer/dot/config/toml" "github.com/ChainSafe/gossamer/dot/state" "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/genesis" "github.com/ChainSafe/gossamer/lib/utils" log "github.com/ChainSafe/log15" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/urfave/cli" ) @@ -1065,3 +1068,181 @@ func TestGlobalNodeNamePriorityOrder(t *testing.T) { require.NotEqual(t, cfg.Global.Name, createdCfg.Global.Name) }) } + +type mockGetStringer struct { + kv map[string]string +} + +func (m *mockGetStringer) String(key string) (value string) { + return m.kv[key] +} + +func newMockGetStringer(keyValue map[string]string) *mockGetStringer { + kv := make(map[string]string, len(keyValue)) + for k, v := range keyValue { + kv[k] = v + } + return &mockGetStringer{kv: kv} +} + +func Test_getLogLevel(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + ctx getStringer + flagName string + tomlValue string + defaultLevel log.Lvl + level log.Lvl + err error + }{ + "no value with default": { + ctx: newMockGetStringer(map[string]string{}), + defaultLevel: log.LvlError, + level: log.LvlError, + }, + "flag integer value": { + ctx: newMockGetStringer(map[string]string{"x": "1"}), + flagName: "x", + level: log.LvlError, + }, + "flag string value": { + ctx: newMockGetStringer(map[string]string{"x": "eror"}), + flagName: "x", + level: log.LvlError, + }, + "flag bad string value": { + ctx: newMockGetStringer(map[string]string{"x": "garbage"}), + flagName: "x", + err: errors.New("cannot parse log level string: Unknown level: garbage"), + }, + "toml integer value": { + ctx: newMockGetStringer(map[string]string{}), + tomlValue: "1", + level: log.LvlError, + }, + "toml string value": { + ctx: newMockGetStringer(map[string]string{}), + tomlValue: "eror", + level: log.LvlError, + }, + "toml bad string value": { + ctx: newMockGetStringer(map[string]string{}), + tomlValue: "garbage", + err: errors.New("cannot parse log level string: Unknown level: garbage"), + }, + "flag takes precedence": { + ctx: newMockGetStringer(map[string]string{"x": "eror"}), + flagName: "x", + tomlValue: "warn", + level: log.LvlError, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + level, err := getLogLevel(testCase.ctx, testCase.flagName, + testCase.tomlValue, testCase.defaultLevel) + + if testCase.err != nil { + assert.EqualError(t, err, testCase.err.Error()) + } else { + assert.NoError(t, err) + } + assert.Equal(t, testCase.level, level) + }) + } +} + +func Test_setLogConfig(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + ctx getStringer + initialCfg ctoml.Config + initialGlobalCfg dot.GlobalConfig + initialLogCfg dot.LogConfig + expectedCfg ctoml.Config + expectedGlobalCfg dot.GlobalConfig + expectedLogCfg dot.LogConfig + err error + }{ + "no value": { + ctx: newMockGetStringer(map[string]string{}), + expectedCfg: ctoml.Config{ + Global: ctoml.GlobalConfig{ + LogLvl: log.LvlInfo.String(), + }, + }, + expectedGlobalCfg: dot.GlobalConfig{ + LogLvl: log.LvlInfo, + }, + expectedLogCfg: dot.LogConfig{ + CoreLvl: log.LvlInfo, + SyncLvl: log.LvlInfo, + NetworkLvl: log.LvlInfo, + RPCLvl: log.LvlInfo, + StateLvl: log.LvlInfo, + RuntimeLvl: log.LvlInfo, + BlockProducerLvl: log.LvlInfo, + FinalityGadgetLvl: log.LvlInfo, + }, + }, + "some values": { + ctx: newMockGetStringer(map[string]string{}), + initialCfg: ctoml.Config{ + Log: ctoml.LogConfig{ + CoreLvl: log.LvlError.String(), + SyncLvl: log.LvlDebug.String(), + StateLvl: log.LvlWarn.String(), + }, + }, + expectedCfg: ctoml.Config{ + Global: ctoml.GlobalConfig{ + LogLvl: log.LvlInfo.String(), + }, + Log: ctoml.LogConfig{ + CoreLvl: log.LvlError.String(), + SyncLvl: log.LvlDebug.String(), + StateLvl: log.LvlWarn.String(), + }, + }, + expectedGlobalCfg: dot.GlobalConfig{ + LogLvl: log.LvlInfo, + }, + expectedLogCfg: dot.LogConfig{ + CoreLvl: log.LvlError, + SyncLvl: log.LvlDebug, + NetworkLvl: log.LvlInfo, + RPCLvl: log.LvlInfo, + StateLvl: log.LvlWarn, + RuntimeLvl: log.LvlInfo, + BlockProducerLvl: log.LvlInfo, + FinalityGadgetLvl: log.LvlInfo, + }, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := setLogConfig(testCase.ctx, &testCase.initialCfg, + &testCase.initialGlobalCfg, &testCase.initialLogCfg) + + if testCase.err != nil { + assert.EqualError(t, err, testCase.err.Error()) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, testCase.expectedCfg, testCase.initialCfg) + assert.Equal(t, testCase.expectedGlobalCfg, testCase.initialGlobalCfg) + assert.Equal(t, testCase.expectedLogCfg, testCase.initialLogCfg) + }) + } +} diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index 93b2d6d472..646cc03120 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -56,9 +56,50 @@ var ( // LogFlag cli service settings LogFlag = cli.StringFlag{ Name: "log", - Usage: "Supports levels crit (silent) to trce (trace)", + Usage: "Global log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: log.LvlInfo.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, + } + LogBlockProducerLevelFlag = cli.StringFlag{ + Name: "log-blockproducer", + Usage: "Block producer package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Value: LogFlag.Value, + } + LogFinalityGadgetLevelFlag = cli.StringFlag{ + Name: "log-finalitygadget", + Usage: "Finality Gadget package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Value: LogFlag.Value, + } + // NameFlag node implementation name NameFlag = cli.StringFlag{ Name: "name", @@ -342,6 +383,14 @@ var ( // GlobalFlags are flags that are valid for use with the root command and all subcommands GlobalFlags = []cli.Flag{ LogFlag, + LogCoreLevelFlag, + LogSyncLevelFlag, + LogNetworkLevelFlag, + LogRPCLevelFlag, + LogStateLevelFlag, + LogRuntimeLevelFlag, + LogBlockProducerLevelFlag, + LogFinalityGadgetLevelFlag, NameFlag, ChainFlag, ConfigFlag, From 854d0ac29e4e0836f529960103c8fba434868cfd Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Tue, 2 Nov 2021 17:04:18 +0000 Subject: [PATCH 2/9] Remove regex digits and add comment --- cmd/gossamer/config.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index fac43eb6e0..2e9bc8a548 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -18,7 +18,6 @@ package main import ( "fmt" - "regexp" "strconv" "strings" "time" @@ -311,14 +310,9 @@ func getLogLevel(ctx getStringer, flagName, tomlValue string, defaultLevel log.L return parseLogLevelString(tomlValue) } -var regexDigits = regexp.MustCompile("^[0-9]+$") - func parseLogLevelString(logLevelString string) (logLevel log.Lvl, err error) { - if regexDigits.MatchString(logLevelString) { - levelInt, err := strconv.Atoi(logLevelString) - if err != nil { // should never happen - return 0, fmt.Errorf("cannot parse log level digits: %w", err) - } + levelInt, err := strconv.Atoi(logLevelString) + if err == nil { // level given as an integer logLevel = log.Lvl(levelInt) return logLevel, nil } From 5e41c421fcf129ce4032353b46742db8591b6722 Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Tue, 2 Nov 2021 17:09:11 +0000 Subject: [PATCH 3/9] Add check for level integer value bounds --- cmd/gossamer/config.go | 6 +++++ cmd/gossamer/config_test.go | 46 +++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 2e9bc8a548..a9b47735aa 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -17,6 +17,7 @@ package main import ( + "errors" "fmt" "strconv" "strings" @@ -310,9 +311,14 @@ func getLogLevel(ctx getStringer, flagName, tomlValue string, defaultLevel log.L return parseLogLevelString(tomlValue) } +var ErrLogLevelIntegerOutOfRange = errors.New("log level integer can only be between 0 and 5 included") + func parseLogLevelString(logLevelString string) (logLevel log.Lvl, err error) { levelInt, err := strconv.Atoi(logLevelString) if err == nil { // level given as an integer + if levelInt < 0 || levelInt > 5 { + return 0, fmt.Errorf("%w: log level given: %d", ErrLogLevelIntegerOutOfRange, levelInt) + } logLevel = log.Lvl(levelInt) return logLevel, nil } diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index e1c2697d20..c314556109 100644 --- a/cmd/gossamer/config_test.go +++ b/cmd/gossamer/config_test.go @@ -1157,6 +1157,52 @@ func Test_getLogLevel(t *testing.T) { } } +func Test_parseLogLevelString(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + logLevelString string + logLevel log.Lvl + err error + }{ + "empty string": { + err: errors.New("cannot parse log level string: Unknown level: "), + }, + "valid integer": { + logLevelString: "1", + logLevel: log.LvlError, + }, + "minus one": { + logLevelString: "-1", + err: errors.New("log level integer can only be between 0 and 5 included: log level given: -1"), + }, + "over 5": { + logLevelString: "6", + err: errors.New("log level integer can only be between 0 and 5 included: log level given: 6"), + }, + "valid string": { + logLevelString: "error", + logLevel: log.LvlError, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + logLevel, err := parseLogLevelString(testCase.logLevelString) + + if testCase.err != nil { + assert.EqualError(t, err, testCase.err.Error()) + } else { + assert.NoError(t, err) + } + assert.Equal(t, testCase.logLevel, logLevel) + }) + } +} + func Test_setLogConfig(t *testing.T) { t.Parallel() From 039b3c5cc3047c05abad4a742624be01f24b6323 Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Tue, 2 Nov 2021 17:10:41 +0000 Subject: [PATCH 4/9] Change `log-blockproducer` to `log-babe` --- cmd/gossamer/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index 646cc03120..a3521915d0 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -90,7 +90,7 @@ var ( Value: LogFlag.Value, } LogBlockProducerLevelFlag = cli.StringFlag{ - Name: "log-blockproducer", + Name: "log-babe", Usage: "Block producer package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } From 63c1ec6f4545de929d04b82559c58b23dd0a29ad Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Tue, 2 Nov 2021 17:10:58 +0000 Subject: [PATCH 5/9] Change `log-finalitygadget` to `log-grandpa` --- cmd/gossamer/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index a3521915d0..09c32ea928 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -95,7 +95,7 @@ var ( Value: LogFlag.Value, } LogFinalityGadgetLevelFlag = cli.StringFlag{ - Name: "log-finalitygadget", + Name: "log-grandpa", Usage: "Finality Gadget package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } From 26cd469a8456beb4c8573e8d8e69c78efb7d6498 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 10 Nov 2021 11:24:38 +0000 Subject: [PATCH 6/9] Apply Eclesio's suggestion --- cmd/gossamer/config.go | 106 ++++++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 38 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index a9b47735aa..dbea880b5b 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -342,44 +342,74 @@ func setLogConfig(ctx getStringer, cfg *ctoml.Config, globalCfg *dot.GlobalConfi } cfg.Global.LogLvl = globalCfg.LogLvl.String() - logCfg.CoreLvl, err = getLogLevel(ctx, LogCoreLevelFlag.Name, cfg.Log.CoreLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get core log level: %w", err) - } - - logCfg.SyncLvl, err = getLogLevel(ctx, LogSyncLevelFlag.Name, cfg.Log.SyncLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get sync log level: %w", err) - } - - logCfg.NetworkLvl, err = getLogLevel(ctx, LogNetworkLevelFlag.Name, cfg.Log.NetworkLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get network log level: %w", err) - } - - logCfg.RPCLvl, err = getLogLevel(ctx, LogRPCLevelFlag.Name, cfg.Log.RPCLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get RPC log level: %w", err) - } - - logCfg.StateLvl, err = getLogLevel(ctx, LogStateLevelFlag.Name, cfg.Log.StateLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get state log level: %w", err) - } - - logCfg.RuntimeLvl, err = getLogLevel(ctx, LogRuntimeLevelFlag.Name, cfg.Log.RuntimeLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get runtime log level: %w", err) - } - - logCfg.BlockProducerLvl, err = getLogLevel(ctx, LogBlockProducerLevelFlag.Name, cfg.Log.BlockProducerLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get block producer log level: %w", err) - } - - logCfg.FinalityGadgetLvl, err = getLogLevel(ctx, LogFinalityGadgetLevelFlag.Name, cfg.Log.FinalityGadgetLvl, globalCfg.LogLvl) - if err != nil { - return fmt.Errorf("cannot get finality gadget log level: %w", err) + levelsData := []struct { + name string + flagName string + tomlValue string + levelPtr *log.Lvl // pointer to value to modify + }{ + { + name: "core", + flagName: LogCoreLevelFlag.Name, + tomlValue: cfg.Log.CoreLvl, + levelPtr: &logCfg.CoreLvl, + }, + { + name: "sync", + flagName: LogSyncLevelFlag.Name, + tomlValue: cfg.Log.SyncLvl, + levelPtr: &logCfg.SyncLvl, + }, + { + name: "network", + flagName: LogNetworkLevelFlag.Name, + tomlValue: cfg.Log.NetworkLvl, + levelPtr: &logCfg.NetworkLvl, + }, + { + name: "RPC", + flagName: LogRPCLevelFlag.Name, + tomlValue: cfg.Log.RPCLvl, + levelPtr: &logCfg.RPCLvl, + }, + { + name: "state", + flagName: LogStateLevelFlag.Name, + tomlValue: cfg.Log.StateLvl, + levelPtr: &logCfg.StateLvl, + }, + { + name: "runtime", + flagName: LogRuntimeLevelFlag.Name, + tomlValue: cfg.Log.RuntimeLvl, + levelPtr: &logCfg.RuntimeLvl, + }, + { + name: "block producer", + flagName: LogBlockProducerLevelFlag.Name, + tomlValue: cfg.Log.BlockProducerLvl, + levelPtr: &logCfg.BlockProducerLvl, + }, + { + name: "finality gadget", + flagName: LogFinalityGadgetLevelFlag.Name, + tomlValue: cfg.Log.FinalityGadgetLvl, + levelPtr: &logCfg.FinalityGadgetLvl, + }, + { + name: "sync", + flagName: LogSyncLevelFlag.Name, + tomlValue: cfg.Log.SyncLvl, + levelPtr: &logCfg.SyncLvl, + }, + } + + for _, levelData := range levelsData { + level, err := getLogLevel(ctx, levelData.flagName, levelData.tomlValue, globalCfg.LogLvl) + if err != nil { + return fmt.Errorf("cannot get %s log level: %w", levelData.name, err) + } + *levelData.levelPtr = level } logger.Debug("set log configuration", "--log", ctx.String(LogFlag.Name), "global", globalCfg.LogLvl) From c367d7be57e40adedf87e158547978fe3c8201c9 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 10 Nov 2021 11:58:17 +0000 Subject: [PATCH 7/9] Rename `ctx` to `flagsKVStore` --- cmd/gossamer/config.go | 14 +++++----- cmd/gossamer/config_test.go | 52 ++++++++++++++++++------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index dbea880b5b..21d13dfd14 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -288,7 +288,7 @@ func createExportConfig(ctx *cli.Context) (*dot.Config, error) { return cfg, nil } -type getStringer interface { +type stringKVStore interface { String(key string) (value string) } @@ -298,9 +298,9 @@ type getStringer interface { // 3. Return the default value given if both previous steps failed. // For steps 1 and 2, it tries to parse the level as an integer to convert it // to a level, and also tries to parse it as a string. -func getLogLevel(ctx getStringer, flagName, tomlValue string, defaultLevel log.Lvl) ( +func getLogLevel(flagsKVStore stringKVStore, flagName, tomlValue string, defaultLevel log.Lvl) ( level log.Lvl, err error) { - if flagValue := ctx.String(flagName); flagValue != "" { + if flagValue := flagsKVStore.String(flagName); flagValue != "" { return parseLogLevelString(flagValue) } @@ -331,12 +331,12 @@ func parseLogLevelString(logLevelString string) (logLevel log.Lvl, err error) { return logLevel, nil } -func setLogConfig(ctx getStringer, cfg *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) (err error) { +func setLogConfig(flagsKVStore stringKVStore, cfg *ctoml.Config, globalCfg *dot.GlobalConfig, logCfg *dot.LogConfig) (err error) { if cfg == nil { cfg = new(ctoml.Config) } - globalCfg.LogLvl, err = getLogLevel(ctx, LogFlag.Name, cfg.Global.LogLvl, gssmr.DefaultLvl) + globalCfg.LogLvl, err = getLogLevel(flagsKVStore, LogFlag.Name, cfg.Global.LogLvl, gssmr.DefaultLvl) if err != nil { return fmt.Errorf("cannot get global log level: %w", err) } @@ -405,14 +405,14 @@ func setLogConfig(ctx getStringer, cfg *ctoml.Config, globalCfg *dot.GlobalConfi } for _, levelData := range levelsData { - level, err := getLogLevel(ctx, levelData.flagName, levelData.tomlValue, globalCfg.LogLvl) + level, err := getLogLevel(flagsKVStore, levelData.flagName, levelData.tomlValue, globalCfg.LogLvl) if err != nil { return fmt.Errorf("cannot get %s log level: %w", levelData.name, err) } *levelData.levelPtr = level } - logger.Debug("set log configuration", "--log", ctx.String(LogFlag.Name), "global", globalCfg.LogLvl) + logger.Debug("set log configuration", "--log", flagsKVStore.String(LogFlag.Name), "global", globalCfg.LogLvl) return nil } diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index c314556109..c7a8b23799 100644 --- a/cmd/gossamer/config_test.go +++ b/cmd/gossamer/config_test.go @@ -1089,7 +1089,7 @@ func Test_getLogLevel(t *testing.T) { t.Parallel() testCases := map[string]struct { - ctx getStringer + flagsKVStore stringKVStore flagName string tomlValue string defaultLevel log.Lvl @@ -1097,45 +1097,45 @@ func Test_getLogLevel(t *testing.T) { err error }{ "no value with default": { - ctx: newMockGetStringer(map[string]string{}), + flagsKVStore: newMockGetStringer(map[string]string{}), defaultLevel: log.LvlError, level: log.LvlError, }, "flag integer value": { - ctx: newMockGetStringer(map[string]string{"x": "1"}), - flagName: "x", - level: log.LvlError, + flagsKVStore: newMockGetStringer(map[string]string{"x": "1"}), + flagName: "x", + level: log.LvlError, }, "flag string value": { - ctx: newMockGetStringer(map[string]string{"x": "eror"}), - flagName: "x", - level: log.LvlError, + flagsKVStore: newMockGetStringer(map[string]string{"x": "eror"}), + flagName: "x", + level: log.LvlError, }, "flag bad string value": { - ctx: newMockGetStringer(map[string]string{"x": "garbage"}), - flagName: "x", - err: errors.New("cannot parse log level string: Unknown level: garbage"), + flagsKVStore: newMockGetStringer(map[string]string{"x": "garbage"}), + flagName: "x", + err: errors.New("cannot parse log level string: Unknown level: garbage"), }, "toml integer value": { - ctx: newMockGetStringer(map[string]string{}), - tomlValue: "1", - level: log.LvlError, + flagsKVStore: newMockGetStringer(map[string]string{}), + tomlValue: "1", + level: log.LvlError, }, "toml string value": { - ctx: newMockGetStringer(map[string]string{}), - tomlValue: "eror", - level: log.LvlError, + flagsKVStore: newMockGetStringer(map[string]string{}), + tomlValue: "eror", + level: log.LvlError, }, "toml bad string value": { - ctx: newMockGetStringer(map[string]string{}), - tomlValue: "garbage", - err: errors.New("cannot parse log level string: Unknown level: garbage"), + flagsKVStore: newMockGetStringer(map[string]string{}), + tomlValue: "garbage", + err: errors.New("cannot parse log level string: Unknown level: garbage"), }, "flag takes precedence": { - ctx: newMockGetStringer(map[string]string{"x": "eror"}), - flagName: "x", - tomlValue: "warn", - level: log.LvlError, + flagsKVStore: newMockGetStringer(map[string]string{"x": "eror"}), + flagName: "x", + tomlValue: "warn", + level: log.LvlError, }, } @@ -1144,7 +1144,7 @@ func Test_getLogLevel(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - level, err := getLogLevel(testCase.ctx, testCase.flagName, + level, err := getLogLevel(testCase.flagsKVStore, testCase.flagName, testCase.tomlValue, testCase.defaultLevel) if testCase.err != nil { @@ -1207,7 +1207,7 @@ func Test_setLogConfig(t *testing.T) { t.Parallel() testCases := map[string]struct { - ctx getStringer + ctx stringKVStore initialCfg ctoml.Config initialGlobalCfg dot.GlobalConfig initialLogCfg dot.LogConfig From e4079a8712097d201ff97c43d995621e58af95ce Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 10 Nov 2021 12:01:47 +0000 Subject: [PATCH 8/9] Rename `LogFinalityGadgetLevelFlag` to `LogGrandpaLevelFlag` --- cmd/gossamer/config.go | 2 +- cmd/gossamer/flags.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 21d13dfd14..a0f0da4f35 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -392,7 +392,7 @@ func setLogConfig(flagsKVStore stringKVStore, cfg *ctoml.Config, globalCfg *dot. }, { name: "finality gadget", - flagName: LogFinalityGadgetLevelFlag.Name, + flagName: LogGrandpaLevelFlag.Name, tomlValue: cfg.Log.FinalityGadgetLvl, levelPtr: &logCfg.FinalityGadgetLvl, }, diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index 09c32ea928..5d9f934215 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -94,9 +94,9 @@ var ( Usage: "Block producer package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } - LogFinalityGadgetLevelFlag = cli.StringFlag{ + LogGrandpaLevelFlag = cli.StringFlag{ Name: "log-grandpa", - Usage: "Finality Gadget package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Grandpa package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } @@ -390,7 +390,7 @@ var ( LogStateLevelFlag, LogRuntimeLevelFlag, LogBlockProducerLevelFlag, - LogFinalityGadgetLevelFlag, + LogGrandpaLevelFlag, NameFlag, ChainFlag, ConfigFlag, From 895d2baaa6b63c10b710ca54ed495b2a142c21f6 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 10 Nov 2021 12:02:31 +0000 Subject: [PATCH 9/9] Rename `LogBlockProducerLevelFlag` to `LogBabeLevelFlag` --- cmd/gossamer/config.go | 2 +- cmd/gossamer/flags.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index a0f0da4f35..054d2a4f9e 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -386,7 +386,7 @@ func setLogConfig(flagsKVStore stringKVStore, cfg *ctoml.Config, globalCfg *dot. }, { name: "block producer", - flagName: LogBlockProducerLevelFlag.Name, + flagName: LogBabeLevelFlag.Name, tomlValue: cfg.Log.BlockProducerLvl, levelPtr: &logCfg.BlockProducerLvl, }, diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index 5d9f934215..5ae6a3118b 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -89,9 +89,9 @@ var ( Usage: "Runtime package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } - LogBlockProducerLevelFlag = cli.StringFlag{ + LogBabeLevelFlag = cli.StringFlag{ Name: "log-babe", - Usage: "Block producer package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "BABE package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", Value: LogFlag.Value, } LogGrandpaLevelFlag = cli.StringFlag{ @@ -389,7 +389,7 @@ var ( LogRPCLevelFlag, LogStateLevelFlag, LogRuntimeLevelFlag, - LogBlockProducerLevelFlag, + LogBabeLevelFlag, LogGrandpaLevelFlag, NameFlag, ChainFlag,