fix: Logger not correctly applying named config (#696)

There was a bug in the #658 that got merged which effectively broke the entire feature. Tests didn't catch it as they were accidentally poorly defined, as they would pass with the default logger settings as well as the custom ones set in the test.

There are two fixes in this PR that resolves the problem. During review, we decided to switch back the Config.Log from Config.Logger, but the viper Bind calls were still using Bind(logger.loglevel) (old name). Additionally, the validate call on the config.Log struct previously always returned nil (no errors), but was changed recently to validate the available options. But, if the --loglevel option is provided it will set the config.Log.Level to error,defra.cli=info,... for example, which would fail validation. I have no moved the validation rules, as it will be validated anyways later in the call stack (in the ToLoggerConfig).

Lastly, it fixes #694, as an accidental oversight, potentially during a rebase/merge-conflict.

cli/cli.go

@@ -162,6 +162,12 @@ func parseAndConfigLogAllParams(ctx context.Context, cfg *config.LoggingConfig,
 				return fmt.Errorf("couldn't parse kv bool: %w", err)
 			}
 			logcfg.NoColor = boolValue
+		case "caller": // bool
+			boolValue, err := strconv.ParseBool(parsedKV[1])
+			if err != nil {
+				return fmt.Errorf("couldn't parse kv bool: %w", err)
+			}
+			logcfg.Caller = boolValue
 		default:
 			return fmt.Errorf("unknown parameter for logger: %s", param)
 		}

cli/root.go

@@ -80,7 +80,7 @@ func init() {
 		"loglevel", cfg.Log.Level,
 		"Log level to use. Options are debug, info, error, fatal",
 	)
-	err := viper.BindPFlag("logging.level", rootCmd.PersistentFlags().Lookup("loglevel"))
+	err := viper.BindPFlag("log.level", rootCmd.PersistentFlags().Lookup("loglevel"))
 	if err != nil {
 		log.FatalE(context.Background(), "Could not bind logging.loglevel", err)
 	}
@@ -94,7 +94,7 @@ func init() {
 		"logoutput", cfg.Log.OutputPath,
 		"Log output path",
 	)
-	err = viper.BindPFlag("logging.outputpath", rootCmd.PersistentFlags().Lookup("logoutput"))
+	err = viper.BindPFlag("log.outputpath", rootCmd.PersistentFlags().Lookup("logoutput"))
 	if err != nil {
 		log.FatalE(context.Background(), "Could not bind log.outputpath", err)
 	}
@@ -103,7 +103,7 @@ func init() {
 		"logformat", cfg.Log.Format,
 		"Log format to use. Options are csv, json",
 	)
-	err = viper.BindPFlag("logging.format", rootCmd.PersistentFlags().Lookup("logformat"))
+	err = viper.BindPFlag("log.format", rootCmd.PersistentFlags().Lookup("logformat"))
 	if err != nil {
 		log.FatalE(context.Background(), "Could not bind log.format", err)
 	}
@@ -112,7 +112,7 @@ func init() {
 		"logtrace", cfg.Log.Stacktrace,
 		"Include stacktrace in error and fatal logs",
 	)
-	err = viper.BindPFlag("logging.stacktrace", rootCmd.PersistentFlags().Lookup("logtrace"))
+	err = viper.BindPFlag("log.stacktrace", rootCmd.PersistentFlags().Lookup("logtrace"))
 	if err != nil {
 		log.FatalE(context.Background(), "Could not bind log.stacktrace", err)
 	}

config/config.go

@@ -362,11 +362,6 @@ func defaultLogConfig() *LoggingConfig {
 }
 
 func (logcfg *LoggingConfig) validate() error {
-	switch logcfg.Level {
-	case logLevelDebug, logLevelInfo, logLevelError, logLevelFatal:
-	default:
-		return fmt.Errorf("invalid log level: %s", logcfg.Level)
-	}
 	return nil
 }
 
@@ -408,6 +403,7 @@ func (logcfg LoggingConfig) ToLoggerConfig() (logging.Config, error) {
 		DisableColor:          logging.NewDisableColorOption(logcfg.NoColor),
 		EncoderFormat:         logging.NewEncoderFormatOption(encfmt),
 		OutputPaths:           []string{logcfg.OutputPath},
+		EnableCaller:          logging.NewEnableCallerOption(logcfg.Caller),
 		OverridesByLoggerName: overrides,
 	}, nil
 }

tests/integration/cli/log_config_test.go

@@ -52,7 +52,8 @@ func TestCLILogsToStderrGivenNamedLogLevel(t *testing.T) {
 			log1.Error(ctx, "error")
 			log1.Debug(ctx, "debug")
 			log2.Info(ctx, "info")
-			log3.Debug(ctx, "info")
+			log3.Debug(ctx, "debug") // wont print, as logger3 will use global level defined above as 'error'
+			log3.Info(ctx, "info")   // wont print, as logger3 will use global level defined above as 'error'
 		},
 	)