Move getting a logger from config to the config package (#4816)

* Move getting a logger from config to the config package Signed-off-by: Radoslav Dimitrov <[email protected]> * Fix linting errors Signed-off-by: Radoslav Dimitrov <[email protected]> --------- Signed-off-by: Radoslav Dimitrov <[email protected]>
mindersec · Oct 24, 2024 · f27c512 · f27c512
1 parent 29e0706
commit f27c512
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 102 deletions.
diff --git a/cmd/dev/app/rule_type/rttst.go b/cmd/dev/app/rule_type/rttst.go
@@ -25,7 +25,6 @@ import (
 	engif "github.com/mindersec/minder/internal/engine/interfaces"
 	entModels "github.com/mindersec/minder/internal/entities/models"
 	entProps "github.com/mindersec/minder/internal/entities/properties"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/internal/profiles"
 	"github.com/mindersec/minder/internal/profiles/models"
 	"github.com/mindersec/minder/internal/providers/credentials"
@@ -251,7 +250,7 @@ func runEvaluationForRules(
 		// Enable logging for the engine
 		ctx := context.Background()
 		logConfig := serverconfig.LoggingConfig{Level: cmd.Flag("log-level").Value.String()}
-		ctx = logger.FromFlags(logConfig).WithContext(ctx)
+		ctx = serverconfig.LoggerFromConfigFlags(logConfig).WithContext(ctx)
 
 		// convert to EntityInfoWrapper as that's what the engine operates on
 		inf, err := entityWithPropertiesToEntityInfoWrapper(ewp, prov)

diff --git a/cmd/dev/app/testserver/testserver.go b/cmd/dev/app/testserver/testserver.go
@@ -23,7 +23,6 @@ import (
 	mockauthz "github.com/mindersec/minder/internal/authz/mock"
 	"github.com/mindersec/minder/internal/controlplane/metrics"
 	"github.com/mindersec/minder/internal/db/embedded"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/internal/metrics/meters"
 	"github.com/mindersec/minder/internal/providers/ratecache"
 	provtelemetry "github.com/mindersec/minder/internal/providers/telemetry"
@@ -54,7 +53,7 @@ func runTestServer(cmd *cobra.Command, _ []string) error {
 	ctx, cancel := signal.NotifyContext(cmd.Context(), os.Interrupt)
 	defer cancel()
 
-	ctx = logger.FromFlags(cfg.LoggingConfig).WithContext(ctx)
+	ctx = serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(ctx)
 	l := zerolog.Ctx(ctx)
 	l.Info().Msgf("Initializing logger in level: %s", cfg.LoggingConfig.Level)
 

diff --git a/cmd/reminder/app/start.go b/cmd/reminder/app/start.go
@@ -16,7 +16,6 @@ import (
 
 	"github.com/mindersec/minder/internal/db"
 	"github.com/mindersec/minder/internal/reminder"
-	"github.com/mindersec/minder/internal/reminder/logger"
 	"github.com/mindersec/minder/pkg/config"
 	reminderconfig "github.com/mindersec/minder/pkg/config/reminder"
 )
@@ -42,7 +41,7 @@ func start(cmd *cobra.Command, _ []string) error {
 		return fmt.Errorf("error validating config: %w", err)
 	}
 
-	ctx = logger.FromFlags(cfg.LoggingConfig).WithContext(ctx)
+	ctx = reminderconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(ctx)
 
 	dbConn, _, err := cfg.Database.GetDBConnection(ctx)
 	if err != nil {

diff --git a/cmd/server/app/encryption_purge_sessions.go b/cmd/server/app/encryption_purge_sessions.go
@@ -15,7 +15,6 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/mindersec/minder/internal/db"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -31,7 +30,7 @@ var purgeCmd = &cobra.Command{
 			cliErrorf(cmd, "unable to read config: %s", err)
 		}
 
-		ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+		ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 		// instantiate `db.Store` so we can run queries
 		store, closer, err := wireUpDB(ctx, cfg)

diff --git a/cmd/server/app/encryption_rotate.go b/cmd/server/app/encryption_rotate.go
@@ -17,7 +17,6 @@ import (
 
 	"github.com/mindersec/minder/internal/crypto"
 	"github.com/mindersec/minder/internal/db"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -44,7 +43,7 @@ var rotateCmd = &cobra.Command{
 			cliErrorf(cmd, "default key ID not defined in crypto config - exiting")
 		}
 
-		ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+		ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 		zerolog.Ctx(ctx).Debug().
 			Str("default_key_id", cfg.Crypto.Default.KeyID).

diff --git a/cmd/server/app/history_purge.go b/cmd/server/app/history_purge.go
@@ -15,7 +15,6 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/mindersec/minder/internal/db"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -37,7 +36,7 @@ func historyPurgeCommand(cmd *cobra.Command, _ []string) error {
 		cliErrorf(cmd, "unable to read config: %s", err)
 	}
 
-	ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+	ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 	// instantiate `db.Store` so we can run queries
 	store, closer, err := wireUpDB(ctx, cfg)

diff --git a/cmd/server/app/migrate_down.go b/cmd/server/app/migrate_down.go
@@ -13,7 +13,6 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/mindersec/minder/database"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -28,7 +27,7 @@ var downCmd = &cobra.Command{
 			return fmt.Errorf("unable to read config: %w", err)
 		}
 
-		ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+		ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 		// Database configuration
 		dbConn, connString, err := cfg.Database.GetDBConnection(ctx)

diff --git a/cmd/server/app/migrate_up.go b/cmd/server/app/migrate_up.go
@@ -18,7 +18,6 @@ import (
 
 	"github.com/mindersec/minder/database"
 	"github.com/mindersec/minder/internal/authz"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -34,7 +33,7 @@ var upCmd = &cobra.Command{
 			return fmt.Errorf("unable to read config: %w", err)
 		}
 
-		ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+		ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 		// Database configuration
 		dbConn, connString, err := cfg.Database.GetDBConnection(ctx)

diff --git a/cmd/server/app/migrate_version.go b/cmd/server/app/migrate_version.go
@@ -15,7 +15,6 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/mindersec/minder/database"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/pkg/config"
 	serverconfig "github.com/mindersec/minder/pkg/config/server"
 )
@@ -31,7 +30,7 @@ var versionCmd = &cobra.Command{
 			return fmt.Errorf("unable to read config: %w", err)
 		}
 
-		ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+		ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 		// Database configuration
 		dbConn, connString, err := cfg.Database.GetDBConnection(ctx)

diff --git a/cmd/server/app/serve.go b/cmd/server/app/serve.go
@@ -47,7 +47,7 @@ var serveCmd = &cobra.Command{
 			os.Exit(0)
 		}
 
-		ctx = logger.FromFlags(cfg.LoggingConfig).WithContext(ctx)
+		ctx = serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(ctx)
 		l := zerolog.Ctx(ctx)
 		l.Info().Msgf("Initializing logger in level: %s", cfg.LoggingConfig.Level)
 

diff --git a/cmd/server/app/webhook_update.go b/cmd/server/app/webhook_update.go
@@ -20,7 +20,6 @@ import (
 	"github.com/mindersec/minder/internal/crypto"
 	"github.com/mindersec/minder/internal/db"
 	propssvc "github.com/mindersec/minder/internal/entities/properties/service"
-	"github.com/mindersec/minder/internal/logger"
 	"github.com/mindersec/minder/internal/providers"
 	ghprovider "github.com/mindersec/minder/internal/providers/github"
 	"github.com/mindersec/minder/internal/providers/github/clients"
@@ -60,7 +59,7 @@ func runCmdWebhookUpdate(cmd *cobra.Command, _ []string) error {
 		return fmt.Errorf("unable to read config: %w", err)
 	}
 
-	ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+	ctx := serverconfig.LoggerFromConfigFlags(cfg.LoggingConfig).WithContext(context.Background())
 
 	providerName := cmd.Flag("provider").Value.String()
 

diff --git a/internal/logger/logger_setup.go b/internal/logger/logger_setup.go
diff --git a/internal/reminder/logger/logger_setup.go b/internal/reminder/logger/logger_setup.go
diff --git a/pkg/config/reminder/logging.go b/pkg/config/reminder/logging.go
@@ -3,7 +3,21 @@
 
 package reminder
 
+import (
+	"os"
+
+	"github.com/rs/zerolog"
+
+	"github.com/mindersec/minder/internal/util"
+)
+
 // LoggingConfig is the configuration for the logger
 type LoggingConfig struct {
 	Level string `mapstructure:"level" default:"info"`
 }
+
+// LoggerFromConfigFlags creates a new logger from the provided configuration
+func LoggerFromConfigFlags(cfg LoggingConfig) zerolog.Logger {
+	level := util.ViperLogLevelToZerologLevel(cfg.Level)
+	return zerolog.New(os.Stdout).Level(level).With().Timestamp().Logger()
+}
diff --git a/pkg/config/server/logging.go b/pkg/config/server/logging.go
@@ -3,14 +3,72 @@
 
 package server
 
+import (
+	"io"
+	"os"
+	"path/filepath"
+
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
+
+	"github.com/mindersec/minder/internal/util"
+)
+
+// Text is the constant for the text format
+const Text = "text"
+
 // LoggingConfig is the configuration for the logging package
 type LoggingConfig struct {
 	Level   string `mapstructure:"level" default:"debug"`
 	Format  string `mapstructure:"format" default:"json"`
 	LogFile string `mapstructure:"logFile" default:""`
 
-	// LogPayloads controls whether or not message payloads are ever logged.
+	// LogPayloads controls whether message payloads are ever logged.
 	// For debugging purposes, it may be useful to log the payloads that result
 	// in error conditions, but could also leak PII.
 	LogPayloads bool `mapstructure:"logPayloads" default:"false"`
 }
+
+// LoggerFromConfigFlags configures logging and returns a logger with settings matching
+// the supplied cfg. It also performs some global initialization, because
+// that's how zerolog works.
+func LoggerFromConfigFlags(cfg LoggingConfig) zerolog.Logger {
+	zlevel := util.ViperLogLevelToZerologLevel(cfg.Level)
+	zerolog.SetGlobalLevel(zlevel)
+
+	loggers := []io.Writer{}
+
+	// Conform to https://github.com/open-telemetry/oteps/blob/main/text/logs/0097-log-data-model.md#example-log-records
+	// Unfortunately, these can't be set on a per-logger basis except by ConsoleWriter
+	zerolog.ErrorFieldName = "exception.message"
+	zerolog.TimestampFieldName = "Timestamp"
+	zerolog.TimeFieldFormat = zerolog.TimeFormatUnixNano
+
+	if cfg.LogFile != "" {
+		cfg.LogFile = filepath.Clean(cfg.LogFile)
+		file, err := os.OpenFile(cfg.LogFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
+		// NOTE: we are leaking the open file here
+		if err != nil {
+			log.Err(err).Msg("Failed to open log file, defaulting to stdout")
+		} else {
+			loggers = append(loggers, file)
+		}
+	}
+
+	if cfg.Format == Text {
+		loggers = append(loggers, zerolog.NewConsoleWriter())
+	} else {
+		loggers = append(loggers, os.Stdout)
+	}
+
+	logger := zerolog.New(zerolog.MultiLevelWriter(loggers...)).With().
+		Caller().
+		Timestamp().
+		Logger()
+
+	// Use this logger when calling zerolog.Ctx(nil), etc
+	zerolog.DefaultContextLogger = &logger
+	log.Logger = logger
+
+	return logger
+}