Skip to content

Commit

Permalink
chore(log): fix spelling and add padding for logger (#2743)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jimjbrettj authored Aug 16, 2022
1 parent af5c63f commit 83eac8a
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 62 deletions.
6 changes: 3 additions & 3 deletions cmd/gossamer/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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": {
Expand All @@ -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,
Expand Down
20 changes: 10 additions & 10 deletions cmd/gossamer/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/gossamer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions dot/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 {
Expand Down
22 changes: 10 additions & 12 deletions internal/log/level.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 (
Expand All @@ -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)
Expand Down
26 changes: 13 additions & 13 deletions internal/log/level_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: "??? ",
},
}

Expand All @@ -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.
Expand Down Expand Up @@ -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": {
Expand All @@ -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": {
Expand Down
3 changes: 1 addition & 2 deletions internal/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand All @@ -42,7 +42,6 @@ func (l *Logger) log(logLevel Level, s string, args ...interface{}) {
}

line += "\n"

_, _ = io.WriteString(l.settings.writer, line)
}

Expand Down
34 changes: 17 additions & 17 deletions internal/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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$",
},
}

Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 83eac8a

Please sign in to comment.