From fcf2e7f687b1909effb68a7d1e7e1428aa838bb4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 16 Nov 2020 11:18:45 +0100 Subject: [PATCH 1/3] build/log_test: add tests for parsing log levels --- build/log_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 build/log_test.go diff --git a/build/log_test.go b/build/log_test.go new file mode 100644 index 00000000..1c7e8e61 --- /dev/null +++ b/build/log_test.go @@ -0,0 +1,104 @@ +package build_test + +import ( + "testing" + + "github.com/btcsuite/btclog" + "github.com/lightningnetwork/lnd/build" + "github.com/stretchr/testify/require" +) + +type mockSubLogger struct { + globalLogLevel string + subLogLevels map[string]string +} + +func (m *mockSubLogger) SubLoggers() build.SubLoggers { + return build.SubLoggers{ + "PEER": btclog.Disabled, + "SRVR": btclog.Disabled, + } +} + +func (m *mockSubLogger) SupportedSubsystems() []string { + return nil +} + +func (m *mockSubLogger) SetLogLevel(subsystemID string, logLevel string) { + m.subLogLevels[subsystemID] = logLevel +} + +func (m *mockSubLogger) SetLogLevels(logLevel string) { + m.globalLogLevel = logLevel +} + +// TestParseAndSetDebugLevels tests tha we can properly set the log levels for +// all andspecified subsystems. +func TestParseAndSetDebugLevels(t *testing.T) { + testCases := []struct { + name string + debugLevel string + expErr string + expGlobal string + expSubLevels map[string]string + }{ + { + name: "empty log level", + debugLevel: "", + expErr: "invalid", + }, + { + name: "invalid global debug level", + debugLevel: "ddddddebug", + expErr: "invalid", + }, + { + name: "global debug level", + debugLevel: "debug", + expGlobal: "debug", + }, + { + name: "invalid global debug level#2", + debugLevel: "debug,info", + expErr: "invalid", + }, + { + name: "invalid subsystem debug level", + debugLevel: "AAAA=debug", + expErr: "invalid", + }, + { + name: "valid subsystem debug level", + debugLevel: "PEER=info,SRVR=debug", + expSubLevels: map[string]string{ + "PEER": "info", + "SRVR": "debug", + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.name, func(t *testing.T) { + m := &mockSubLogger{ + subLogLevels: make(map[string]string), + } + + // If the subsystem map is empty, make and empty one to ensure + // the equal test later succeeds. + if len(test.expSubLevels) == 0 { + test.expSubLevels = make(map[string]string) + } + + err := build.ParseAndSetDebugLevels(test.debugLevel, m) + if test.expErr != "" { + require.Contains(t, err.Error(), test.expErr) + return + } + require.NoError(t, err) + + require.Equal(t, test.expGlobal, m.globalLogLevel) + require.Equal(t, test.expSubLevels, m.subLogLevels) + }) + } +} From c1d423dc07e305c034102a3ce6b1a2f0cc709459 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 16 Nov 2020 11:22:57 +0100 Subject: [PATCH 2/3] build/log: support parsing global+subsystem levels This makes it possible to specify both a global+subsystem loglevels, like: --debuglevel=debug,PEER=info,SRVR=trace --- build/log.go | 28 ++++++++++++++++++---------- build/log_test.go | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/build/log.go b/build/log.go index e8ce253a..21c8b953 100644 --- a/build/log.go +++ b/build/log.go @@ -122,24 +122,32 @@ type LeveledSubLogger interface { // the levels accordingly on the given logger. An appropriate error is returned // if anything is invalid. func ParseAndSetDebugLevels(level string, logger LeveledSubLogger) error { - // When the specified string doesn't have any delimiters, treat it as - // the log level for all subsystems. - if !strings.Contains(level, ",") && !strings.Contains(level, "=") { + // Split at the delimiter. + levels := strings.Split(level, ",") + if len(levels) == 0 { + return fmt.Errorf("invalid log level: %v", level) + } + + // If the first entry has no =, treat is as the log level for all + // subsystems. + globalLevel := levels[0] + if !strings.Contains(globalLevel, "=") { // Validate debug log level. - if !validLogLevel(level) { + if !validLogLevel(globalLevel) { str := "the specified debug level [%v] is invalid" - return fmt.Errorf(str, level) + return fmt.Errorf(str, globalLevel) } // Change the logging level for all subsystems. - logger.SetLogLevels(level) + logger.SetLogLevels(globalLevel) - return nil + // The rest will target specific subsystems. + levels = levels[1:] } - // Split the specified string into subsystem/level pairs while detecting - // issues and update the log levels accordingly. - for _, logLevelPair := range strings.Split(level, ",") { + // Go through the subsystem/level pairs while detecting issues and + // update the log levels accordingly. + for _, logLevelPair := range levels { if !strings.Contains(logLevelPair, "=") { str := "the specified debug level contains an " + "invalid subsystem/level pair [%v]" diff --git a/build/log_test.go b/build/log_test.go index 1c7e8e61..b26c125b 100644 --- a/build/log_test.go +++ b/build/log_test.go @@ -75,6 +75,20 @@ func TestParseAndSetDebugLevels(t *testing.T) { "SRVR": "debug", }, }, + { + name: "valid global+subsystem debug level", + debugLevel: "trace,PEER=info,SRVR=debug", + expGlobal: "trace", + expSubLevels: map[string]string{ + "PEER": "info", + "SRVR": "debug", + }, + }, + { + name: "invalid global+subsystem debug level", + debugLevel: "PEER=info,debug,SRVR=debug", + expErr: "invalid", + }, } for _, test := range testCases { From a2f45cb81260e0282c4852dee774aee2f2f0126a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 16 Nov 2020 11:23:26 +0100 Subject: [PATCH 3/3] config: update debuglevel description + sample conf --- config.go | 2 +- sample-lnd.conf | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config.go b/config.go index 338deb27..a07eb76a 100644 --- a/config.go +++ b/config.go @@ -216,7 +216,7 @@ type Config struct { MaxBackoff time.Duration `long:"maxbackoff" description:"Longest backoff when reconnecting to persistent peers. Valid time units are {s, m, h}."` ConnectionTimeout time.Duration `long:"connectiontimeout" description:"The timeout value for network connections. Valid time units are {ms, s, m, h}."` - DebugLevel string `short:"d" long:"debuglevel" description:"Logging level for all subsystems {trace, debug, info, warn, error, critical} -- You may also specify =,=,... to set the log level for individual subsystems -- Use show to list available subsystems"` + DebugLevel string `short:"d" long:"debuglevel" description:"Logging level for all subsystems {trace, debug, info, warn, error, critical} -- You may also specify ,=,=,... to set the log level for individual subsystems -- Use show to list available subsystems"` CPUProfile string `long:"cpuprofile" description:"Write CPU profile to the specified file"` diff --git a/sample-lnd.conf b/sample-lnd.conf index 01bc5eaa..00bdb012 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -193,10 +193,10 @@ ; Debug logging level. ; Valid levels are {trace, debug, info, warn, error, critical} -; You may also specify =,=,... to set -; log level for individual subsystems. Use lncli debuglevel --show to list -; available subsystems. -; debuglevel=info +; You may also specify ,=,=,... +; to set log level for individual subsystems. Use lncli debuglevel --show to +; list available subsystems. +; debuglevel=debug,PEER=info ; Write CPU profile to the specified file. ; cpuprofile=