From 37ec68eebb533a545765458badd8236033a184f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90urica=20Yuri=20Nikoli=C4=87?= Date: Mon, 4 Sep 2023 12:47:11 +0200 Subject: [PATCH] Remove unnecessary code after dskit log.Interface has been dereleased (#10434) This PR removes unnecessary code used to convert the dereleased `dslog.Interface` (see https://github.com/grafana/dskit/pull/359) into `go-kit.Logger`. Additionally, it modifies the signature and implementation of `pkg/util/log/log.go`'s `InitLogger` as follows: - previous signature ``` InitLogger(*server.Config, prometheus.Registerer, bool, bool) ``` has been replaced with ``` InitLogger(*server.Config, prometheus.Registerer, bool, bool) log.Logger ``` Namely, the new `InitLogger` initialises the global logger and returns it. - the new implementation initializes the global logger, but it does not override the default logger for the server. This is now done by the callers of `InitLogger`. Previously, it was done inside `InitLogger` for convenience, since the types of the 2 loggers were not the same. Signed-off-by: Yuri Nikolic --- clients/cmd/docker-driver/main.go | 3 +- clients/cmd/fluent-bit/loki.go | 4 +-- clients/cmd/promtail/main.go | 3 +- .../promtail/targets/lokipush/pushtarget.go | 3 +- cmd/loki/main.go | 3 +- pkg/util/log.go | 18 ---------- pkg/util/log/log.go | 33 +++++-------------- tools/tsdb/index-analyzer/main.go | 3 +- tools/tsdb/migrate-versions/main.go | 3 +- 9 files changed, 20 insertions(+), 53 deletions(-) diff --git a/clients/cmd/docker-driver/main.go b/clients/cmd/docker-driver/main.go index bff182a39c..5aba041f6b 100644 --- a/clients/cmd/docker-driver/main.go +++ b/clients/cmd/docker-driver/main.go @@ -12,7 +12,6 @@ import ( dslog "github.com/grafana/dskit/log" "github.com/prometheus/common/version" - "github.com/grafana/loki/pkg/util" _ "github.com/grafana/loki/pkg/util/build" util_log "github.com/grafana/loki/pkg/util/log" ) @@ -54,7 +53,7 @@ func main() { func newLogger(lvl dslog.Level) log.Logger { // plugin logs must be stdout to appear. logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stdout)) - logger = level.NewFilter(logger, util.LogFilter(lvl.String())) + logger = level.NewFilter(logger, lvl.Option) logger = log.With(logger, "ts", log.DefaultTimestampUTC) logger = log.With(logger, "caller", log.Caller(3)) return logger diff --git a/clients/cmd/fluent-bit/loki.go b/clients/cmd/fluent-bit/loki.go index 5dbb7031b6..ea3de0261f 100644 --- a/clients/cmd/fluent-bit/loki.go +++ b/clients/cmd/fluent-bit/loki.go @@ -20,8 +20,6 @@ import ( "github.com/grafana/loki/clients/pkg/promtail/api" "github.com/grafana/loki/clients/pkg/promtail/client" - "github.com/grafana/loki/pkg/util" - "github.com/grafana/loki/pkg/logproto" ) @@ -270,7 +268,7 @@ func (l *loki) createLine(records map[string]interface{}, f format) (string, err func newLogger(logLevel dslog.Level) log.Logger { logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)) - logger = level.NewFilter(logger, util.LogFilter(logLevel.String())) + logger = level.NewFilter(logger, logLevel.Option) logger = log.With(logger, "caller", log.Caller(3)) return logger } diff --git a/clients/cmd/promtail/main.go b/clients/cmd/promtail/main.go index 833677bfb0..4d9065ddb5 100644 --- a/clients/cmd/promtail/main.go +++ b/clients/cmd/promtail/main.go @@ -106,7 +106,8 @@ func main() { fmt.Println("Invalid log level") exit(1) } - util_log.InitLogger(&config.Config.ServerConfig.Config, prometheus.DefaultRegisterer, true, false) + serverCfg := &config.Config.ServerConfig.Config + serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.DefaultRegisterer, true, false) // Use Stderr instead of files for the klog. klog.SetOutput(os.Stderr) diff --git a/clients/pkg/promtail/targets/lokipush/pushtarget.go b/clients/pkg/promtail/targets/lokipush/pushtarget.go index abf08baf82..a204227df5 100644 --- a/clients/pkg/promtail/targets/lokipush/pushtarget.go +++ b/clients/pkg/promtail/targets/lokipush/pushtarget.go @@ -79,7 +79,8 @@ func (t *PushTarget) run() error { // The logger registers a metric which will cause a duplicate registry panic unless we provide an empty registry // The metric created is for counting log lines and isn't likely to be missed. - util_log.InitLogger(&t.config.Server, prometheus.NewRegistry(), true, false) + serverCfg := &t.config.Server + serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.NewRegistry(), true, false) srv, err := server.New(t.config.Server) if err != nil { diff --git a/cmd/loki/main.go b/cmd/loki/main.go index 6c41dd61f6..16846dbb09 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -48,7 +48,8 @@ func main() { level.Error(util_log.Logger).Log("msg", "invalid log level") exit(1) } - util_log.InitLogger(&config.Server, prometheus.DefaultRegisterer, config.UseBufferedLogger, config.UseSyncLogger) + serverCfg := &config.Server + serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.DefaultRegisterer, config.UseBufferedLogger, config.UseSyncLogger) // Validate the config once both the config file has been loaded // and CLI flags parsed. diff --git a/pkg/util/log.go b/pkg/util/log.go index d6862dc4de..877904f6f2 100644 --- a/pkg/util/log.go +++ b/pkg/util/log.go @@ -72,21 +72,3 @@ func (l LogAdapter) Printf(format string, v ...interface{}) { func (l LogAdapter) Println(v ...interface{}) { level.Info(l).Log("msg", fmt.Sprint(v...)) } - -// LogFilter translates a log level into a go-kit/log level filter. -// -// TODO(dannyk): remove once weaveworks/common updates to go-kit/log, we can then revert to using Level.Gokit -func LogFilter(l string) level.Option { - switch l { - case "debug": - return level.AllowDebug() - case "info": - return level.AllowInfo() - case "warn": - return level.AllowWarn() - case "error": - return level.AllowError() - default: - return level.AllowAll() - } -} diff --git a/pkg/util/log/log.go b/pkg/util/log/log.go index 2e376f4042..c6f59217be 100644 --- a/pkg/util/log/log.go +++ b/pkg/util/log/log.go @@ -28,14 +28,13 @@ var ( plogger *prometheusLogger ) -// InitLogger initialises the global gokit logger (util_log.Logger) and overrides the -// default logger for the server. -func InitLogger(cfg *server.Config, reg prometheus.Registerer, buffered bool, sync bool) { - l := newPrometheusLogger(cfg.LogLevel, cfg.LogFormat, reg, buffered, sync) - +// InitLogger initialises the global gokit logger (util_log.Logger) and returns that logger. +func InitLogger(cfg *server.Config, reg prometheus.Registerer, buffered bool, sync bool) log.Logger { + logger := newPrometheusLogger(cfg.LogLevel, cfg.LogFormat, reg, buffered, sync) // when using util_log.Logger, skip 3 stack frames. - Logger = log.With(l, "caller", log.Caller(3)) - cfg.Log = Logger + Logger = log.With(logger, "caller", log.Caller(3)) + + return Logger } type Flusher interface { @@ -92,7 +91,7 @@ func LevelHandler(currentLogLevel *dslog.Level) http.HandlerFunc { Status: "failed", } } else { - plogger.Set(levelFilter(logLevel)) + plogger.Set(currentLogLevel.Option) msg := fmt.Sprintf("Log level set to %s", logLevel) level.Info(Logger).Log("msg", msg) @@ -159,7 +158,7 @@ func newPrometheusLogger(l dslog.Level, format string, reg prometheus.Registerer } baseLogger := dslog.NewGoKitWithWriter(format, writer) - logger := level.NewFilter(baseLogger, levelFilter(l.String())) + logger := level.NewFilter(baseLogger, l.Option) plogger = &prometheusLogger{ baseLogger: baseLogger, @@ -224,19 +223,3 @@ func CheckFatal(location string, err error, logger log.Logger) { } os.Exit(1) } - -// TODO: remove once weaveworks/common updates to go-kit/log, we can then revert to using Level.Gokit -func levelFilter(l string) level.Option { - switch l { - case "debug": - return level.AllowDebug() - case "info": - return level.AllowInfo() - case "warn": - return level.AllowWarn() - case "error": - return level.AllowError() - default: - return level.AllowAll() - } -} diff --git a/tools/tsdb/index-analyzer/main.go b/tools/tsdb/index-analyzer/main.go index b59d1ea22a..2bd11b36dc 100644 --- a/tools/tsdb/index-analyzer/main.go +++ b/tools/tsdb/index-analyzer/main.go @@ -90,7 +90,8 @@ func setup() (loki.Config, string, error) { } c.Config.StorageConfig.TSDBShipperConfig.Mode = indexshipper.ModeReadOnly - util_log.InitLogger(&c.Server, prometheus.DefaultRegisterer, c.UseBufferedLogger, c.UseSyncLogger) + serverCfg := &c.Server + serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.DefaultRegisterer, c.UseBufferedLogger, c.UseSyncLogger) c.Config.StorageConfig.TSDBShipperConfig.ActiveIndexDirectory = filepath.Join(dir, "tsdb-active") c.Config.StorageConfig.TSDBShipperConfig.CacheLocation = filepath.Join(dir, "tsdb-cache") diff --git a/tools/tsdb/migrate-versions/main.go b/tools/tsdb/migrate-versions/main.go index ebf62c388f..443e6b766e 100644 --- a/tools/tsdb/migrate-versions/main.go +++ b/tools/tsdb/migrate-versions/main.go @@ -298,7 +298,8 @@ func setup() loki.Config { os.Exit(1) } - util_log.InitLogger(&c.Server, prometheus.DefaultRegisterer, c.UseBufferedLogger, c.UseSyncLogger) + serverCfg := &c.Server + serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.DefaultRegisterer, c.UseBufferedLogger, c.UseSyncLogger) if err := c.Validate(); err != nil { level.Error(util_log.Logger).Log("msg", "validating config", "err", err.Error())