From 83eac8ad974961e1af713a2b04b05be2cd25e690 Mon Sep 17 00:00:00 2001 From: JimboJ <40345116+jimjbrettj@users.noreply.github.com> Date: Tue, 16 Aug 2022 16:29:24 -0400 Subject: [PATCH] chore(log): fix spelling and add padding for logger (#2743) * fix spelling and padding for logger * remove unneeded test func * fix cmd tests * reduce diff * clean * final fix * lint * fix core tests * feedback * clean up tests * wip/make cleaner * format func for logs * fix tests * reduce diff * trigger build * fix cmd test * replace crit with critical in logger --- cmd/gossamer/config_test.go | 6 +++--- cmd/gossamer/flags.go | 20 ++++++++++---------- cmd/gossamer/main.go | 2 +- dot/config_test.go | 8 ++++---- internal/log/level.go | 22 ++++++++++------------ internal/log/level_test.go | 26 +++++++++++++------------- internal/log/log.go | 3 +-- internal/log/log_test.go | 34 +++++++++++++++++----------------- 8 files changed, 59 insertions(+), 62 deletions(-) diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index e033766e57..b134321e5d 100644 --- a/cmd/gossamer/config_test.go +++ b/cmd/gossamer/config_test.go @@ -1133,7 +1133,7 @@ func Test_getLogLevel(t *testing.T) { level: log.Error, }, "flag string value": { - flagsKVStore: newMockGetStringer(map[string]string{"x": "eror"}), + flagsKVStore: newMockGetStringer(map[string]string{"x": "error"}), flagName: "x", level: log.Error, }, @@ -1149,7 +1149,7 @@ func Test_getLogLevel(t *testing.T) { }, "toml string value": { flagsKVStore: newMockGetStringer(map[string]string{}), - tomlValue: "eror", + tomlValue: "error", level: log.Error, }, "toml bad string value": { @@ -1158,7 +1158,7 @@ func Test_getLogLevel(t *testing.T) { err: errors.New("cannot parse log level string: level is not recognised: garbage"), }, "flag takes precedence": { - flagsKVStore: newMockGetStringer(map[string]string{"x": "eror"}), + flagsKVStore: newMockGetStringer(map[string]string{"x": "error"}), flagName: "x", tomlValue: "warn", level: log.Error, diff --git a/cmd/gossamer/flags.go b/cmd/gossamer/flags.go index f69518b4d8..4f754e80fe 100644 --- a/cmd/gossamer/flags.go +++ b/cmd/gossamer/flags.go @@ -44,43 +44,43 @@ var ( // LogFlag cli service settings LogFlag = cli.StringFlag{ Name: "log", - Usage: "Global log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Global log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogCoreLevelFlag = cli.StringFlag{ Name: "log-core", - Usage: "Core package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Core package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogDigestLevelFlag = cli.StringFlag{ Name: "log-digest", - Usage: "Digest package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Digest package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogSyncLevelFlag = cli.StringFlag{ Name: "log-sync", - Usage: "Sync package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Sync package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogNetworkLevelFlag = cli.StringFlag{ Name: "log-network", - Usage: "Network package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Network package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogRPCLevelFlag = cli.StringFlag{ Name: "log-rpc", - Usage: "RPC package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "RPC package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogStateLevelFlag = cli.StringFlag{ Name: "log-state", - Usage: "State package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "State package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogRuntimeLevelFlag = cli.StringFlag{ Name: "log-runtime", - Usage: "Runtime package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Runtime package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogBabeLevelFlag = cli.StringFlag{ Name: "log-babe", - Usage: "BABE package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "BABE package log level. Supports levels critical (silent), error, warn, info, debug and trace", } LogGrandpaLevelFlag = cli.StringFlag{ Name: "log-grandpa", - Usage: "Grandpa package log level. Supports levels crit (silent), eror, warn, info, dbug and trce (trace)", + Usage: "Grandpa package log level. Supports levels critical (silent), error, warn, info, debug and trace", } // NameFlag node implementation name diff --git a/cmd/gossamer/main.go b/cmd/gossamer/main.go index c85fb0415a..764b93de83 100644 --- a/cmd/gossamer/main.go +++ b/cmd/gossamer/main.go @@ -348,7 +348,7 @@ func initAction(ctx *cli.Context) error { func buildSpecAction(ctx *cli.Context) error { // set logger to critical, so output only contains genesis data - err := ctx.Set("log", "crit") + err := ctx.Set("log", "critical") if err != nil { return err } diff --git a/dot/config_test.go b/dot/config_test.go index 901deaa2bb..b173cfe48f 100644 --- a/dot/config_test.go +++ b/dot/config_test.go @@ -446,8 +446,8 @@ func TestLogConfig_String(t *testing.T) { { name: "default case", logConfig: LogConfig{}, - want: "core: CRIT, digest: CRIT, sync: CRIT, network: CRIT, rpc: CRIT, state: CRIT, runtime: CRIT, " + - "block producer: CRIT, finality gadget: CRIT", + want: "core: CRITICAL, digest: CRITICAL, sync: CRITICAL, network: CRITICAL, rpc: CRITICAL, " + + "state: CRITICAL, runtime: CRITICAL, block producer: CRITICAL, finality gadget: CRITICAL", }, { name: "change fields case", @@ -462,8 +462,8 @@ func TestLogConfig_String(t *testing.T) { BlockProducerLvl: log.Warn, FinalityGadgetLvl: log.Error, }, - want: "core: DBUG, digest: INFO, sync: WARN, network: EROR, rpc: CRIT, state: DBUG, runtime: INFO, " + - "block producer: WARN, finality gadget: EROR", + want: "core: DEBUG, digest: INFO, sync: WARN, network: ERROR, rpc: CRITICAL, " + + "state: DEBUG, runtime: INFO, block producer: WARN, finality gadget: ERROR", }, } for _, tt := range tests { diff --git a/internal/log/level.go b/internal/log/level.go index 495e49d059..7531c7a421 100644 --- a/internal/log/level.go +++ b/internal/log/level.go @@ -36,25 +36,23 @@ const ( func (level Level) String() (s string) { switch level { case Trace: - return "TRCE" + return "TRACE" case Debug: - return "DBUG" + return "DEBUG" case Info: return "INFO" case Warn: return "WARN" case Error: - return "EROR" + return "ERROR" case Critical: - return "CRIT" + return "CRITICAL" default: return "???" } } -// ColouredString returns the corresponding coloured -// string for the level. -func (level Level) ColouredString() (s string) { +func (level Level) format() (s string) { attribute := color.Reset switch level { @@ -73,7 +71,7 @@ func (level Level) ColouredString() (s string) { } c := color.New(attribute) - return c.Sprint(level.String()) + return c.Sprintf("%-8s", level.String()) } var ( @@ -94,17 +92,17 @@ func ParseLevel(s string) (level Level, err error) { } switch strings.ToUpper(s) { - case Trace.String(), "TRACE": + case Trace.String(): return Trace, nil - case Debug.String(), "DEBUG": + case Debug.String(): return Debug, nil case Info.String(): return Info, nil case Warn.String(): return Warn, nil - case Error.String(), "ERROR": + case Error.String(): return Error, nil - case Critical.String(), "CRITICAL": + case Critical.String(): return Critical, nil } return 0, fmt.Errorf("%w: %s", ErrLevelNotRecognised, s) diff --git a/internal/log/level_test.go b/internal/log/level_test.go index 7124252149..a532116bd8 100644 --- a/internal/log/level_test.go +++ b/internal/log/level_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_Level_ColouredString(t *testing.T) { +func Test_Level_format(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -20,31 +20,31 @@ func Test_Level_ColouredString(t *testing.T) { }{ "trace": { level: Trace, - s: "TRCE", + s: "TRACE ", }, "debug": { level: Debug, - s: "DBUG", + s: "DEBUG ", }, "info": { level: Info, - s: "INFO", + s: "INFO ", }, "warn": { level: Warn, - s: "WARN", + s: "WARN ", }, "error": { level: Error, - s: "EROR", + s: "ERROR ", }, "critical": { level: Critical, - s: "CRIT", + s: "CRITICAL", }, "unknown": { level: 178, - s: "???", + s: "??? ", }, } @@ -53,7 +53,7 @@ func Test_Level_ColouredString(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - s := testCase.level.ColouredString() + s := testCase.level.format() // Note: fatih/colour is clever enough to not add colours // when running tests, so the string is effectively without // colour here. @@ -88,11 +88,11 @@ func Test_ParseLevel(t *testing.T) { err: errors.New("level integer can only be between 0 and 5 included: 6"), }, "trace": { - s: "TRCE", + s: "TRACE", level: Trace, }, "debug": { - s: "DBUG", + s: "DEBUG", level: Debug, }, "info": { @@ -104,11 +104,11 @@ func Test_ParseLevel(t *testing.T) { level: Warn, }, "error": { - s: "EROR", + s: "ERROR", level: Error, }, "critical": { - s: "CRIT", + s: "CRITICAL", level: Critical, }, "invalid": { diff --git a/internal/log/log.go b/internal/log/log.go index 84febcaa46..1ba748f349 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -24,7 +24,7 @@ func (l *Logger) log(logLevel Level, s string, args ...interface{}) { s = fmt.Sprintf(s, args...) } - line := time.Now().Format(time.RFC3339) + " " + logLevel.ColouredString() + " " + s + line := time.Now().Format(time.RFC3339) + " " + logLevel.format() + " " + s callerString := getCallerString(l.settings.caller) if callerString != "" { @@ -42,7 +42,6 @@ func (l *Logger) log(logLevel Level, s string, args ...interface{}) { } line += "\n" - _, _ = io.WriteString(l.settings.writer, line) } diff --git a/internal/log/log_test.go b/internal/log/log_test.go index 9159a4b97a..d0aed77da5 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -34,7 +34,7 @@ func Test_Logger_log(t *testing.T) { }, level: Trace, s: "some words", - outputRegex: timePrefixRegex + "TRCE some words\n$", + outputRegex: timePrefixRegex + "TRACE some words\n$", }, "do not log at trace": { logger: &Logger{ @@ -58,7 +58,7 @@ func Test_Logger_log(t *testing.T) { }, level: Debug, s: "some words", - outputRegex: timePrefixRegex + "DBUG some words\n$", + outputRegex: timePrefixRegex + "DEBUG some words\n$", }, "format string": { logger: &Logger{ @@ -71,7 +71,7 @@ func Test_Logger_log(t *testing.T) { level: Trace, s: "some %s", args: []interface{}{"words"}, - outputRegex: timePrefixRegex + "TRCE some words\n$", + outputRegex: timePrefixRegex + "TRACE some words\n$", }, "show caller": { logger: &Logger{ @@ -83,7 +83,7 @@ func Test_Logger_log(t *testing.T) { }, level: Trace, s: "some words", - outputRegex: timePrefixRegex + "TRCE some words\tlog_test.go:L[0-9]+:func[0-9]+\n$", + outputRegex: timePrefixRegex + "TRACE some words\tlog_test.go:L[0-9]+:func[0-9]+\n$", }, "context": { logger: &Logger{ @@ -99,7 +99,7 @@ func Test_Logger_log(t *testing.T) { }, level: Trace, s: "some words", - outputRegex: timePrefixRegex + "TRCE some words\tkey1=a,b key2=c,d\n$", + outputRegex: timePrefixRegex + "TRACE some words\tkey1=a,b key2=c,d\n$", }, } @@ -157,18 +157,18 @@ func Test_Logger_LevelsLog(t *testing.T) { lines = lines[:len(lines)-1] expectedRegexes := []string{ - timePrefixRegex + "TRCE some trace$", - timePrefixRegex + "DBUG some debug$", - timePrefixRegex + "INFO some info$", - timePrefixRegex + "WARN some warn$", - timePrefixRegex + "EROR some error$", - timePrefixRegex + "CRIT some critical$", - timePrefixRegex + "TRCE some 2nd trace$", - timePrefixRegex + "DBUG some 2nd debug$", - timePrefixRegex + "INFO some 2nd info$", - timePrefixRegex + "WARN some 2nd warn$", - timePrefixRegex + "EROR some 2nd error$", - timePrefixRegex + "CRIT some 2nd critical$", + timePrefixRegex + "TRACE some trace$", + timePrefixRegex + "DEBUG some debug$", + timePrefixRegex + "INFO some info$", + timePrefixRegex + "WARN some warn$", + timePrefixRegex + "ERROR some error$", + timePrefixRegex + "CRITICAL some critical$", + timePrefixRegex + "TRACE some 2nd trace$", + timePrefixRegex + "DEBUG some 2nd debug$", + timePrefixRegex + "INFO some 2nd info$", + timePrefixRegex + "WARN some 2nd warn$", + timePrefixRegex + "ERROR some 2nd error$", + timePrefixRegex + "CRITICAL some 2nd critical$", } require.Equal(t, len(expectedRegexes), len(lines))