From 0a3eab47ee53eab7fd0333a42ef6b4d908506f11 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 13:54:19 +0200 Subject: [PATCH 01/10] set logging dest to be stdout, and simplify cmd logging --- cmd/cli/devstack/devstack.go | 5 +-- cmd/cli/root.go | 20 ++------- cmd/cli/serve/serve.go | 5 ++- pkg/logger/logger.go | 82 ++++++++++++++---------------------- pkg/setup/setup.go | 4 -- 5 files changed, 41 insertions(+), 75 deletions(-) diff --git a/cmd/cli/devstack/devstack.go b/cmd/cli/devstack/devstack.go index 8eb9102566..80fb3ed4d2 100644 --- a/cmd/cli/devstack/devstack.go +++ b/cmd/cli/devstack/devstack.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" + "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/viper" "k8s.io/kubectl/pkg/util/i18n" @@ -103,9 +104,7 @@ func NewCmd() *cobra.Command { RunE: func(cmd *cobra.Command, _ []string) error { // TODO: a hack to force debug logging for devstack // until I figure out why flags and env vars are not working - if err := logger.ConfigureLogging(string(logger.LogModeDefault), "debug"); err != nil { - return fmt.Errorf("failed to configure logging: %w", err) - } + logger.ConfigureLogging(logger.LogModeDefault, zerolog.DebugLevel) return runDevstack(cmd, ODs) }, } diff --git a/cmd/cli/root.go b/cmd/cli/root.go index 9677911d71..7607518635 100644 --- a/cmd/cli/root.go +++ b/cmd/cli/root.go @@ -6,6 +6,7 @@ import ( "os" "strings" + "github.com/rs/zerolog" "github.com/spf13/cobra" "github.com/spf13/viper" "go.opentelemetry.io/otel/trace" @@ -24,7 +25,6 @@ import ( "github.com/bacalhau-project/bacalhau/cmd/util" "github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags" "github.com/bacalhau-project/bacalhau/cmd/util/flags/configflags" - "github.com/bacalhau-project/bacalhau/pkg/config/types" "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/system" "github.com/bacalhau-project/bacalhau/pkg/telemetry" @@ -72,22 +72,8 @@ func NewRootCmd() *cobra.Command { return err } - // Configure logging - // While we allow users to configure logging via the config file, they are applied - // and will override this configuration at a later stage when the config is loaded. - // This is needed to ensure any logs before the config is loaded are captured. - logMode := viper.GetString(types.LoggingModeKey) - if logMode == "" { - logMode = string(logger.LogModeDefault) - } - logLevel := viper.GetString(types.LoggingLevelKey) - if logLevel == "" { - logLevel = "Info" - } - if err := logger.ConfigureLogging(logMode, logLevel); err != nil { - return fmt.Errorf("failed to configure logging: %w", err) - } - + // set cmd log mode by default. + logger.ConfigureLogging(logger.LogModeCmd, zerolog.InfoLevel) return nil } diff --git a/cmd/cli/serve/serve.go b/cmd/cli/serve/serve.go index 2ca436e133..8886b58ae4 100644 --- a/cmd/cli/serve/serve.go +++ b/cmd/cli/serve/serve.go @@ -95,6 +95,10 @@ func NewCmd() *cobra.Command { return fmt.Errorf("failed to setup config: %w", err) } + if err = logger.ParseAndConfigureLogging(cfg.Logging.Mode, cfg.Logging.Level); err != nil { + return fmt.Errorf("failed to configure logging: %w", err) + } + log.Info().Msgf("Config loaded from: %s, and with data-dir %s", rawCfg.Paths(), rawCfg.Get(types.DataDirKey)) @@ -103,7 +107,6 @@ func NewCmd() *cobra.Command { if err != nil { return fmt.Errorf("failed to reconcile repo: %w", err) } - return serve(cmd, cfg, fsr) }, } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index c053cd9b94..f8241d5558 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -13,6 +13,7 @@ import ( "runtime/debug" "strconv" "strings" + "sync" "time" "github.com/rs/zerolog/pkgerrors" @@ -29,22 +30,23 @@ type LogMode string // Available logging modes const ( - LogModeDefault LogMode = "default" - LogModeStation LogMode = "station" - LogModeJSON LogMode = "json" - LogModeCombined LogMode = "combined" - LogModeEvent LogMode = "event" + LogModeDefault LogMode = "default" + LogModeJSON LogMode = "json" + LogModeCmd LogMode = "cmd" +) + +var ( + logMu sync.Mutex ) func ParseLogMode(s string) (LogMode, error) { - lm := []LogMode{LogModeDefault, LogModeStation, LogModeJSON, LogModeCombined, LogModeEvent} + lm := []LogMode{LogModeDefault, LogModeJSON, LogModeCmd} for _, logMode := range lm { if strings.ToLower(s) == strings.ToLower(string(logMode)) { return logMode, nil } } - return "Error", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", - s, lm) + return "", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) } var nodeIDFieldName = "NodeID" @@ -57,7 +59,7 @@ func init() { //nolint:gochecknoinits strings.HasSuffix(os.Args[0], ".test") || flag.Lookup("test.v") != nil || flag.Lookup("test.run") != nil { - configureLogging(zerolog.DebugLevel, defaultLogging()) + ConfigureLogging(LogModeDefault, zerolog.DebugLevel) return } @@ -89,9 +91,7 @@ func ConfigureTestLogging(t tTesting) { }) } -func ConfigureLogging(modeStr, levelStr string) error { - logModeConfig := defaultLogging() - +func ParseAndConfigureLogging(modeStr, levelStr string) error { mode, err := ParseLogMode(modeStr) if err != nil { return fmt.Errorf("invalid log mode: %w", err) @@ -101,25 +101,31 @@ func ConfigureLogging(modeStr, levelStr string) error { return fmt.Errorf("invalid log level: %w", err) } + ConfigureLogging(mode, level) + return nil +} + +func ConfigureLogging(mode LogMode, level zerolog.Level) { + var logWriter io.Writer switch mode { case LogModeDefault: - logModeConfig = defaultLogging() - case LogModeStation: - logModeConfig = defaultStationLogging() + logWriter = defaultLogging() case LogModeJSON: - logModeConfig = jsonLogging() - case LogModeEvent: - logModeConfig = eventLogging() - case LogModeCombined: - logModeConfig = combinedLogging() + logWriter = jsonLogging() + case LogModeCmd: + logWriter = clientLogging() + default: + logWriter = defaultLogging() } - configureLogging(level, logModeConfig) - LogBufferedLogs(logModeConfig) - return nil + configureLogging(level, logWriter) + LogBufferedLogs(logWriter) } func configureLogging(level zerolog.Level, logWriter io.Writer) { + logMu.Lock() + defer logMu.Unlock() + zerolog.TimeFieldFormat = time.RFC3339Nano zerolog.SetGlobalLevel(level) @@ -148,21 +154,12 @@ func jsonLogging() io.Writer { return os.Stdout } -func eventLogging() io.Writer { - return io.Discard -} - -func combinedLogging() io.Writer { - return zerolog.MultiLevelWriter(defaultLogging(), os.Stdout) -} - func defaultLogging() io.Writer { return zerolog.NewConsoleWriter(defaultLogFormat) } func defaultLogFormat(w *zerolog.ConsoleWriter) { isTerminal := isatty.IsTerminal(os.Stdout.Fd()) - w.Out = os.Stderr w.NoColor = !isTerminal w.TimeFormat = "15:04:05.999 |" w.PartsOrder = []string{ @@ -187,25 +184,10 @@ func defaultLogFormat(w *zerolog.ConsoleWriter) { } } -func defaultStationLogging() io.Writer { +func clientLogging() io.Writer { return zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { - isTerminal := isatty.IsTerminal(os.Stdout.Fd()) - w.Out = os.Stdout - w.NoColor = !isTerminal - w.PartsOrder = []string{ - zerolog.LevelFieldName, - zerolog.MessageFieldName, - } - - w.FormatLevel = func(i interface{}) string { - return strings.ToUpper(i.(string)) + ":" - } - w.FormatErrFieldName = func(i interface{}) string { - return "- " - } - w.FormatErrFieldValue = func(i interface{}) string { - return strings.Trim(i.(string), "\"") - } + defaultLogFormat(w) + w.PartsOrder = []string{zerolog.MessageFieldName} }) } diff --git a/pkg/setup/setup.go b/pkg/setup/setup.go index 345268831e..d270600bec 100644 --- a/pkg/setup/setup.go +++ b/pkg/setup/setup.go @@ -9,7 +9,6 @@ import ( "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" - "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo/migrations" "github.com/bacalhau-project/bacalhau/pkg/repo" @@ -25,9 +24,6 @@ func SetupMigrationManager() (*repo.MigrationManager, error) { // SetupBacalhauRepo ensures that a bacalhau repo and config exist and are initialized. func SetupBacalhauRepo(cfg types.Bacalhau) (*repo.FsRepo, error) { - if err := logger.ConfigureLogging(cfg.Logging.Mode, cfg.Logging.Level); err != nil { - return nil, fmt.Errorf("failed to configure logging: %w", err) - } migrationManger, err := SetupMigrationManager() if err != nil { return nil, fmt.Errorf("failed to create migration manager: %w", err) From e44cdae8b3891f7c3c38697beac3986256933ac0 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 14:26:55 +0200 Subject: [PATCH 02/10] cleanup config list logging --- cmd/cli/config/list.go | 2 +- cmd/cli/root.go | 16 +++++++++++++--- cmd/util/repo.go | 7 +++++++ pkg/logger/logger.go | 35 ++++++++++++++++++++++++++++++----- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/cmd/cli/config/list.go b/cmd/cli/config/list.go index c3d970b132..905f443811 100644 --- a/cmd/cli/config/list.go +++ b/cmd/cli/config/list.go @@ -41,7 +41,7 @@ Each key shown can be used with: if err != nil { return err } - log.Info().Msgf("Config loaded from: %s, and with data-dir %s", cfg.Paths(), cfg.Get(types.DataDirKey)) + log.Debug().Msgf("Config loaded from: %s, and with data-dir %s", cfg.Paths(), cfg.Get(types.DataDirKey)) return list(cmd, cfg, o) }, } diff --git a/cmd/cli/root.go b/cmd/cli/root.go index 7607518635..8df28fa5ee 100644 --- a/cmd/cli/root.go +++ b/cmd/cli/root.go @@ -6,7 +6,6 @@ import ( "os" "strings" - "github.com/rs/zerolog" "github.com/spf13/cobra" "github.com/spf13/viper" "go.opentelemetry.io/otel/trace" @@ -25,6 +24,7 @@ import ( "github.com/bacalhau-project/bacalhau/cmd/util" "github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags" "github.com/bacalhau-project/bacalhau/cmd/util/flags/configflags" + "github.com/bacalhau-project/bacalhau/pkg/config/types" "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/system" "github.com/bacalhau-project/bacalhau/pkg/telemetry" @@ -72,8 +72,18 @@ func NewRootCmd() *cobra.Command { return err } - // set cmd log mode by default. - logger.ConfigureLogging(logger.LogModeCmd, zerolog.InfoLevel) + // Configure logging + // While we allow users to configure logging via the config file, they are applied + // and will override this configuration at a later stage when the config is loaded. + // This is needed to ensure any logs before the config is loaded are captured. + logLevel := viper.GetString(types.LoggingLevelKey) + if logLevel == "" { + logLevel = "Info" + } + if err := logger.ParseAndConfigureLogging(string(logger.LogModeCmd), logLevel); err != nil { + return fmt.Errorf("failed to configure logging: %w", err) + } + return nil } diff --git a/cmd/util/repo.go b/cmd/util/repo.go index 042041a056..a295d53661 100644 --- a/cmd/util/repo.go +++ b/cmd/util/repo.go @@ -12,6 +12,7 @@ import ( "github.com/bacalhau-project/bacalhau/cmd/util/hook" "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" + "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo" "github.com/bacalhau-project/bacalhau/pkg/setup" ) @@ -75,6 +76,12 @@ func SetupConfigType(cmd *cobra.Command) (*config.Config, error) { if err != nil { return nil, err } + + // We always apply the configured logging level. Logging mode on the other hand is only applied with serve cmd + if err = logger.ParseAndConfigureLoggingLevel(cfg.Get(types.LoggingLevelKey).(string)); err != nil { + return nil, fmt.Errorf("failed to configure logging: %w", err) + } + return cfg, nil } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index f8241d5558..dfe6ec2bad 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -49,6 +49,14 @@ func ParseLogMode(s string) (LogMode, error) { return "", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) } +func ParseLogLevel(s string) (zerolog.Level, error) { + l, err := zerolog.ParseLevel(s) + if err != nil { + return l, fmt.Errorf("%q is an invalid log-level", s) + } + return l, nil +} + var nodeIDFieldName = "NodeID" func init() { //nolint:gochecknoinits @@ -64,7 +72,8 @@ func init() { //nolint:gochecknoinits } // the default log level when not running a test is ERROR - configureLogging(zerolog.ErrorLevel, bufferLogs()) + ConfigureLoggingLevel(zerolog.ErrorLevel) + configureLogging(bufferLogs()) } func ErrOrDebug(err error) zerolog.Level { @@ -84,7 +93,8 @@ type tTesting interface { func ConfigureTestLogging(t tTesting) { oldLogger := log.Logger oldContextLogger := zerolog.DefaultContextLogger - configureLogging(zerolog.DebugLevel, zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) + ConfigureLoggingLevel(zerolog.DebugLevel) + configureLogging(zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) t.Cleanup(func() { log.Logger = oldLogger zerolog.DefaultContextLogger = oldContextLogger @@ -118,16 +128,31 @@ func ConfigureLogging(mode LogMode, level zerolog.Level) { logWriter = defaultLogging() } - configureLogging(level, logWriter) + ConfigureLoggingLevel(level) + configureLogging(logWriter) LogBufferedLogs(logWriter) } -func configureLogging(level zerolog.Level, logWriter io.Writer) { +func ParseAndConfigureLoggingLevel(level string) error { + l, err := ParseLogLevel(level) + if err != nil { + return err + } + ConfigureLoggingLevel(l) + return nil +} + +func ConfigureLoggingLevel(level zerolog.Level) { + logMu.Lock() + defer logMu.Unlock() + zerolog.SetGlobalLevel(level) +} + +func configureLogging(logWriter io.Writer) { logMu.Lock() defer logMu.Unlock() zerolog.TimeFieldFormat = time.RFC3339Nano - zerolog.SetGlobalLevel(level) info, ok := debug.ReadBuildInfo() if ok && info.Main.Path != "" { From 58566b043df656cf0a0f5f25faad8bd7c4f284cb Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 14:39:36 +0200 Subject: [PATCH 03/10] fix tests --- pkg/logger/logger_test.go | 71 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index ac25c6536d..062540c8d0 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -23,11 +23,15 @@ func TestConfigureLogging(t *testing.T) { }) var logging strings.Builder - configureLogging(zerolog.InfoLevel, zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { + writer := zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { defaultLogFormat(w) w.Out = &logging w.NoColor = true - })) + }) + + // Configure logging with the test writer + ConfigureLogging(LogModeDefault, zerolog.InfoLevel) + configureLogging(writer) subsubpackage.TestLog("testing error logging", "testing message") @@ -40,3 +44,66 @@ func TestConfigureLogging(t *testing.T) { assert.Contains(t, actual, "pkg/logger/testpackage/subpackage/subsubpackage/testutil.go", "Log statement doesn't contain the full package path") assert.Contains(t, actual, `stack:[{"func":"TestLog","line":`, "Log statement didn't automatically include the error's stacktrace") } + +func TestParseAndConfigureLogging(t *testing.T) { + err := ParseAndConfigureLogging("default", "debug") + assert.NoError(t, err) + assert.Equal(t, zerolog.DebugLevel, zerolog.GlobalLevel()) + + err = ParseAndConfigureLogging("json", "info") + assert.NoError(t, err) + assert.Equal(t, zerolog.InfoLevel, zerolog.GlobalLevel()) + + err = ParseAndConfigureLogging("invalid", "error") + assert.Error(t, err) + + err = ParseAndConfigureLogging("default", "invalid") + assert.Error(t, err) +} + +func TestParseLogMode(t *testing.T) { + tests := []struct { + input string + expected LogMode + hasError bool + }{ + {"default", LogModeDefault, false}, + {"json", LogModeJSON, false}, + {"cmd", LogModeCmd, false}, + {"invalid", "", true}, + } + + for _, test := range tests { + result, err := ParseLogMode(test.input) + if test.hasError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, result) + } + } +} + +func TestParseLogLevel(t *testing.T) { + tests := []struct { + input string + expected zerolog.Level + hasError bool + }{ + {"debug", zerolog.DebugLevel, false}, + {"info", zerolog.InfoLevel, false}, + {"warn", zerolog.WarnLevel, false}, + {"error", zerolog.ErrorLevel, false}, + {"invalid", zerolog.NoLevel, true}, + } + + for _, test := range tests { + result, err := ParseLogLevel(test.input) + if test.hasError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, result) + } + } +} From 8d5afa1944c4e68fbf88e44dd64dd100610fa4fa Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 14:56:19 +0200 Subject: [PATCH 04/10] fix failing tests --- pkg/logger/logger.go | 3 ++- pkg/logger/logger_test.go | 9 +++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index dfe6ec2bad..e56f6628b0 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -67,7 +67,8 @@ func init() { //nolint:gochecknoinits strings.HasSuffix(os.Args[0], ".test") || flag.Lookup("test.v") != nil || flag.Lookup("test.run") != nil { - ConfigureLogging(LogModeDefault, zerolog.DebugLevel) + ConfigureLoggingLevel(zerolog.DebugLevel) + configureLogging(defaultLogging()) return } diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index 062540c8d0..7786c4b979 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -23,15 +23,12 @@ func TestConfigureLogging(t *testing.T) { }) var logging strings.Builder - writer := zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { + ConfigureLoggingLevel(zerolog.InfoLevel) + configureLogging(zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { defaultLogFormat(w) w.Out = &logging w.NoColor = true - }) - - // Configure logging with the test writer - ConfigureLogging(LogModeDefault, zerolog.InfoLevel) - configureLogging(writer) + })) subsubpackage.TestLog("testing error logging", "testing message") From 31fe782d3894e7db5c3014953a74426a66f70c3e Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:03:14 +0200 Subject: [PATCH 05/10] return error by default --- pkg/logger/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index e56f6628b0..410523dd3f 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -46,7 +46,7 @@ func ParseLogMode(s string) (LogMode, error) { return logMode, nil } } - return "", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) + return "Error", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) } func ParseLogLevel(s string) (zerolog.Level, error) { From 01e8e79bfbee00eac7e2b295ff5590a05b3d7e05 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:09:22 +0200 Subject: [PATCH 06/10] set logging in repo setup --- pkg/setup/setup.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/setup/setup.go b/pkg/setup/setup.go index d270600bec..7c6d11e0fb 100644 --- a/pkg/setup/setup.go +++ b/pkg/setup/setup.go @@ -9,6 +9,7 @@ import ( "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" + "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo/migrations" "github.com/bacalhau-project/bacalhau/pkg/repo" @@ -24,6 +25,9 @@ func SetupMigrationManager() (*repo.MigrationManager, error) { // SetupBacalhauRepo ensures that a bacalhau repo and config exist and are initialized. func SetupBacalhauRepo(cfg types.Bacalhau) (*repo.FsRepo, error) { + if err := logger.ParseAndConfigureLogging(cfg.Logging.Mode, cfg.Logging.Level); err != nil { + return nil, fmt.Errorf("failed to configure logging: %w", err) + } migrationManger, err := SetupMigrationManager() if err != nil { return nil, fmt.Errorf("failed to create migration manager: %w", err) From b2c039ce8157ef2b64f2f7bb68e201f6701dc0bc Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:14:32 +0200 Subject: [PATCH 07/10] revert config setup --- cmd/util/repo.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cmd/util/repo.go b/cmd/util/repo.go index a295d53661..042041a056 100644 --- a/cmd/util/repo.go +++ b/cmd/util/repo.go @@ -12,7 +12,6 @@ import ( "github.com/bacalhau-project/bacalhau/cmd/util/hook" "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" - "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo" "github.com/bacalhau-project/bacalhau/pkg/setup" ) @@ -76,12 +75,6 @@ func SetupConfigType(cmd *cobra.Command) (*config.Config, error) { if err != nil { return nil, err } - - // We always apply the configured logging level. Logging mode on the other hand is only applied with serve cmd - if err = logger.ParseAndConfigureLoggingLevel(cfg.Get(types.LoggingLevelKey).(string)); err != nil { - return nil, fmt.Errorf("failed to configure logging: %w", err) - } - return cfg, nil } From c7f0f767ad0faa00f56b6a9f82e9e42fe9cecd93 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:22:07 +0200 Subject: [PATCH 08/10] revert more changes --- pkg/logger/logger.go | 32 +++++++++----------------------- pkg/logger/logger_test.go | 3 +-- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 410523dd3f..067e6daa80 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -13,7 +13,6 @@ import ( "runtime/debug" "strconv" "strings" - "sync" "time" "github.com/rs/zerolog/pkgerrors" @@ -35,10 +34,6 @@ const ( LogModeCmd LogMode = "cmd" ) -var ( - logMu sync.Mutex -) - func ParseLogMode(s string) (LogMode, error) { lm := []LogMode{LogModeDefault, LogModeJSON, LogModeCmd} for _, logMode := range lm { @@ -67,14 +62,12 @@ func init() { //nolint:gochecknoinits strings.HasSuffix(os.Args[0], ".test") || flag.Lookup("test.v") != nil || flag.Lookup("test.run") != nil { - ConfigureLoggingLevel(zerolog.DebugLevel) - configureLogging(defaultLogging()) + configureLogging(zerolog.DebugLevel, defaultLogging()) return } // the default log level when not running a test is ERROR - ConfigureLoggingLevel(zerolog.ErrorLevel) - configureLogging(bufferLogs()) + configureLogging(zerolog.ErrorLevel, bufferLogs()) } func ErrOrDebug(err error) zerolog.Level { @@ -94,8 +87,7 @@ type tTesting interface { func ConfigureTestLogging(t tTesting) { oldLogger := log.Logger oldContextLogger := zerolog.DefaultContextLogger - ConfigureLoggingLevel(zerolog.DebugLevel) - configureLogging(zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) + configureLogging(zerolog.DebugLevel, zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) t.Cleanup(func() { log.Logger = oldLogger zerolog.DefaultContextLogger = oldContextLogger @@ -105,11 +97,11 @@ func ConfigureTestLogging(t tTesting) { func ParseAndConfigureLogging(modeStr, levelStr string) error { mode, err := ParseLogMode(modeStr) if err != nil { - return fmt.Errorf("invalid log mode: %w", err) + return err } - level, err := zerolog.ParseLevel(levelStr) + level, err := ParseLogLevel(levelStr) if err != nil { - return fmt.Errorf("invalid log level: %w", err) + return err } ConfigureLogging(mode, level) @@ -128,9 +120,7 @@ func ConfigureLogging(mode LogMode, level zerolog.Level) { default: logWriter = defaultLogging() } - - ConfigureLoggingLevel(level) - configureLogging(logWriter) + configureLogging(level, logWriter) LogBufferedLogs(logWriter) } @@ -144,16 +134,12 @@ func ParseAndConfigureLoggingLevel(level string) error { } func ConfigureLoggingLevel(level zerolog.Level) { - logMu.Lock() - defer logMu.Unlock() zerolog.SetGlobalLevel(level) } -func configureLogging(logWriter io.Writer) { - logMu.Lock() - defer logMu.Unlock() - +func configureLogging(level zerolog.Level, logWriter io.Writer) { zerolog.TimeFieldFormat = time.RFC3339Nano + ConfigureLoggingLevel(level) info, ok := debug.ReadBuildInfo() if ok && info.Main.Path != "" { diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index 7786c4b979..b3b22e9b1e 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -23,8 +23,7 @@ func TestConfigureLogging(t *testing.T) { }) var logging strings.Builder - ConfigureLoggingLevel(zerolog.InfoLevel) - configureLogging(zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { + configureLogging(zerolog.InfoLevel, zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { defaultLogFormat(w) w.Out = &logging w.NoColor = true From 9a2d19f4c75652fab1291efa0a3764635b5cccd1 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:32:50 +0200 Subject: [PATCH 09/10] set out --- pkg/logger/logger.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 067e6daa80..850acbba3b 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -172,6 +172,7 @@ func defaultLogging() io.Writer { func defaultLogFormat(w *zerolog.ConsoleWriter) { isTerminal := isatty.IsTerminal(os.Stdout.Fd()) + w.Out = os.Stdout w.NoColor = !isTerminal w.TimeFormat = "15:04:05.999 |" w.PartsOrder = []string{ From bc68e01d40a5ea67761bd397e136c65802b94c76 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Mon, 7 Oct 2024 15:41:12 +0200 Subject: [PATCH 10/10] revert reverts --- cmd/util/repo.go | 7 +++++++ pkg/logger/logger.go | 28 +++++++++++++++++++++------- pkg/logger/logger_test.go | 3 ++- pkg/setup/setup.go | 4 ---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/cmd/util/repo.go b/cmd/util/repo.go index 042041a056..a295d53661 100644 --- a/cmd/util/repo.go +++ b/cmd/util/repo.go @@ -12,6 +12,7 @@ import ( "github.com/bacalhau-project/bacalhau/cmd/util/hook" "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" + "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo" "github.com/bacalhau-project/bacalhau/pkg/setup" ) @@ -75,6 +76,12 @@ func SetupConfigType(cmd *cobra.Command) (*config.Config, error) { if err != nil { return nil, err } + + // We always apply the configured logging level. Logging mode on the other hand is only applied with serve cmd + if err = logger.ParseAndConfigureLoggingLevel(cfg.Get(types.LoggingLevelKey).(string)); err != nil { + return nil, fmt.Errorf("failed to configure logging: %w", err) + } + return cfg, nil } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 850acbba3b..dc34a033c0 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -13,6 +13,7 @@ import ( "runtime/debug" "strconv" "strings" + "sync" "time" "github.com/rs/zerolog/pkgerrors" @@ -34,6 +35,10 @@ const ( LogModeCmd LogMode = "cmd" ) +var ( + logMu sync.Mutex +) + func ParseLogMode(s string) (LogMode, error) { lm := []LogMode{LogModeDefault, LogModeJSON, LogModeCmd} for _, logMode := range lm { @@ -41,7 +46,7 @@ func ParseLogMode(s string) (LogMode, error) { return logMode, nil } } - return "Error", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) + return "", fmt.Errorf("%q is an invalid log-mode (valid modes: %q)", s, lm) } func ParseLogLevel(s string) (zerolog.Level, error) { @@ -62,12 +67,14 @@ func init() { //nolint:gochecknoinits strings.HasSuffix(os.Args[0], ".test") || flag.Lookup("test.v") != nil || flag.Lookup("test.run") != nil { - configureLogging(zerolog.DebugLevel, defaultLogging()) + ConfigureLoggingLevel(zerolog.DebugLevel) + configureLogging(defaultLogging()) return } // the default log level when not running a test is ERROR - configureLogging(zerolog.ErrorLevel, bufferLogs()) + ConfigureLoggingLevel(zerolog.ErrorLevel) + configureLogging(bufferLogs()) } func ErrOrDebug(err error) zerolog.Level { @@ -87,7 +94,8 @@ type tTesting interface { func ConfigureTestLogging(t tTesting) { oldLogger := log.Logger oldContextLogger := zerolog.DefaultContextLogger - configureLogging(zerolog.DebugLevel, zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) + ConfigureLoggingLevel(zerolog.DebugLevel) + configureLogging(zerolog.NewConsoleWriter(zerolog.ConsoleTestWriter(t), defaultLogFormat)) t.Cleanup(func() { log.Logger = oldLogger zerolog.DefaultContextLogger = oldContextLogger @@ -120,7 +128,9 @@ func ConfigureLogging(mode LogMode, level zerolog.Level) { default: logWriter = defaultLogging() } - configureLogging(level, logWriter) + + ConfigureLoggingLevel(level) + configureLogging(logWriter) LogBufferedLogs(logWriter) } @@ -134,12 +144,16 @@ func ParseAndConfigureLoggingLevel(level string) error { } func ConfigureLoggingLevel(level zerolog.Level) { + logMu.Lock() + defer logMu.Unlock() zerolog.SetGlobalLevel(level) } -func configureLogging(level zerolog.Level, logWriter io.Writer) { +func configureLogging(logWriter io.Writer) { + logMu.Lock() + defer logMu.Unlock() + zerolog.TimeFieldFormat = time.RFC3339Nano - ConfigureLoggingLevel(level) info, ok := debug.ReadBuildInfo() if ok && info.Main.Path != "" { diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index b3b22e9b1e..7786c4b979 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -23,7 +23,8 @@ func TestConfigureLogging(t *testing.T) { }) var logging strings.Builder - configureLogging(zerolog.InfoLevel, zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { + ConfigureLoggingLevel(zerolog.InfoLevel) + configureLogging(zerolog.NewConsoleWriter(func(w *zerolog.ConsoleWriter) { defaultLogFormat(w) w.Out = &logging w.NoColor = true diff --git a/pkg/setup/setup.go b/pkg/setup/setup.go index 7c6d11e0fb..d270600bec 100644 --- a/pkg/setup/setup.go +++ b/pkg/setup/setup.go @@ -9,7 +9,6 @@ import ( "github.com/bacalhau-project/bacalhau/pkg/config" "github.com/bacalhau-project/bacalhau/pkg/config/types" - "github.com/bacalhau-project/bacalhau/pkg/logger" "github.com/bacalhau-project/bacalhau/pkg/repo/migrations" "github.com/bacalhau-project/bacalhau/pkg/repo" @@ -25,9 +24,6 @@ func SetupMigrationManager() (*repo.MigrationManager, error) { // SetupBacalhauRepo ensures that a bacalhau repo and config exist and are initialized. func SetupBacalhauRepo(cfg types.Bacalhau) (*repo.FsRepo, error) { - if err := logger.ParseAndConfigureLogging(cfg.Logging.Mode, cfg.Logging.Level); err != nil { - return nil, fmt.Errorf("failed to configure logging: %w", err) - } migrationManger, err := SetupMigrationManager() if err != nil { return nil, fmt.Errorf("failed to create migration manager: %w", err)