Merge branch 'pks-log-drop-with' into 'master'

log: Disallow usage of `logrus.With` functions

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6240

Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <ps@pks.im>

6 files changed, 62 insertions, 25 deletions

diff --git a/cmd/gitaly-backup/main.go b/cmd/gitaly-backup/main.go
index 287342dec..a37121a3e 100644
--- a/cmd/gitaly-backup/main.go
+++ b/cmd/gitaly-backup/main.go
@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"flag"
+	"fmt"
 	"io"
 	"os"
 
@@ -22,7 +23,11 @@ var subcommands = map[string]subcmd{
 }
 
 func main() {
-	logger := log.Configure(os.Stdout, "json", "")
+	logger, err := log.Configure(os.Stdout, "json", "")
+	if err != nil {
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
+	}
 
 	flags := flag.NewFlagSet("gitaly-backup", flag.ExitOnError)
 	_ = flags.Parse(os.Args)
diff --git a/cmd/gitaly-blackbox/main.go b/cmd/gitaly-blackbox/main.go
index 9173f4e42..d9d604e0b 100644
--- a/cmd/gitaly-blackbox/main.go
+++ b/cmd/gitaly-blackbox/main.go
@@ -6,7 +6,6 @@ import (
 	"os"
 
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/blackbox"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/version"
@@ -35,26 +34,36 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err := run(flag.Arg(0)); err != nil {
-		logrus.WithError(err).Fatal()
-	}
-}
-
-func run(configPath string) error {
-	configRaw, err := os.ReadFile(configPath)
+	cfg, err := readConfig(flag.Arg(0))
 	if err != nil {
-		return err
+		fmt.Printf("reading configuration: %v", err)
+		os.Exit(1)
 	}
 
-	config, err := blackbox.ParseConfig(string(configRaw))
+	logger, err := log.Configure(os.Stdout, cfg.Logging.Format, cfg.Logging.Level)
 	if err != nil {
-		return err
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
 	}
 
-	bb := blackbox.New(config)
+	bb := blackbox.New(cfg)
 	prometheus.MustRegister(bb)
 
-	log.Configure(os.Stdout, config.Logging.Format, config.Logging.Level)
+	if err := bb.Run(); err != nil {
+		logger.WithError(err).Fatal()
+	}
+}
+
+func readConfig(path string) (blackbox.Config, error) {
+	contents, err := os.ReadFile(path)
+	if err != nil {
+		return blackbox.Config{}, err
+	}
+
+	cfg, err := blackbox.ParseConfig(string(contents))
+	if err != nil {
+		return blackbox.Config{}, err
+	}
 
-	return bb.Run()
+	return cfg, nil
 }
diff --git a/cmd/gitaly-git2go/main.go b/cmd/gitaly-git2go/main.go
index 17b4d046c..73e79c99d 100644
--- a/cmd/gitaly-git2go/main.go
+++ b/cmd/gitaly-git2go/main.go
@@ -68,10 +68,15 @@ func main() {
 		correlationID = correlation.SafeRandomID()
 	}
 
-	glog.Configure(os.Stderr, logFormat, logLevel)
-
 	ctx := correlation.ContextWithCorrelation(context.Background(), correlationID)
-	logger := glog.Default().WithFields(logrus.Fields{
+
+	logger, err := glog.Configure(os.Stderr, logFormat, logLevel)
+	if err != nil {
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
+	}
+
+	logger = logger.WithFields(logrus.Fields{
 		"command.name":           git2go.BinaryName,
 		"correlation_id":         correlationID,
 		"enabled_feature_flags":  enabledFeatureFlags,
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 20dfba9ce..a19873cf0 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -76,7 +76,11 @@ func main() {
 	ctx, finished := labkittracing.ExtractFromEnv(ctx)
 	defer finished()
 
-	logger := configureLogger(ctx)
+	logger, err := configureLogger(ctx)
+	if err != nil {
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
+	}
 
 	if err := run(ctx, os.Args); err != nil {
 		var hookError hookError
@@ -100,7 +104,7 @@ func main() {
 // configureLogger configures the logger used by gitaly-hooks. As both stdout and stderr might be interpreted by Git, we
 // need to log to a file instead. If the `log.GitalyLogDirEnvKey` environment variable is set, we thus log to a file
 // contained in the directory pointed to by it, otherwise we discard any log messages.
-func configureLogger(ctx context.Context) logrus.FieldLogger {
+func configureLogger(ctx context.Context) (logrus.FieldLogger, error) {
 	writer := io.Discard
 
 	if logDir := os.Getenv(log.GitalyLogDirEnvKey); logDir != "" {
@@ -113,7 +117,12 @@ func configureLogger(ctx context.Context) logrus.FieldLogger {
 		}
 	}
 
-	return log.Configure(writer, "text", "info").WithField(correlation.FieldName, correlation.ExtractFromContext(ctx))
+	logger, err := log.Configure(writer, "text", "info")
+	if err != nil {
+		return nil, err
+	}
+
+	return logger.WithField(correlation.FieldName, correlation.ExtractFromContext(ctx)), nil
 }
 
 // Both stderr and stdout of gitaly-hooks are streamed back to clients. stdout is processed by client
diff --git a/cmd/gitaly-ssh/main.go b/cmd/gitaly-ssh/main.go
index f5f0fad92..121186378 100644
--- a/cmd/gitaly-ssh/main.go
+++ b/cmd/gitaly-ssh/main.go
@@ -42,7 +42,11 @@ type gitalySSHCommand struct {
 // GITALY_USE_SIDECHANNEL=1 if desired
 // gitaly-ssh upload-pack <git-garbage-x2>
 func main() {
-	logger := log.Configure(os.Stderr, "", "info")
+	logger, err := log.Configure(os.Stderr, "", "info")
+	if err != nil {
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
+	}
 
 	// < 4 since git throws on 2x garbage here
 	if n := len(os.Args); n < 4 {
diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index dfc101dc1..b83af7bb9 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -29,7 +29,12 @@ func main() {
 		logFormat = "json"
 	}
 
-	logger := log.Configure(os.Stdout, logFormat, "").WithField("wrapper", os.Getpid())
+	logger, err := log.Configure(os.Stdout, logFormat, "")
+	if err != nil {
+		fmt.Printf("configuring logger failed: %v", err)
+		os.Exit(1)
+	}
+	logger = logger.WithField("wrapper", os.Getpid())
 
 	if len(os.Args) < 2 {
 		logger.Fatalf("usage: %s forking_binary [args]", os.Args[0])
@@ -102,7 +107,7 @@ func findProcess(pidFilePath string) (*os.Process, error) {
 	return nil, nil
 }
 
-func spawnProcess(logger *logrus.Entry, bin string, args []string) (*os.Process, error) {
+func spawnProcess(logger logrus.FieldLogger, bin string, args []string) (*os.Process, error) {
 	cmd := exec.Command(bin, args...)
 	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=true", bootstrap.EnvUpgradesEnabled))
 
@@ -128,7 +133,7 @@ func isRuntimeSig(s os.Signal) bool {
 	return s == unix.SIGURG
 }
 
-func forwardSignals(gitaly *os.Process, log *logrus.Entry) {
+func forwardSignals(gitaly *os.Process, log logrus.FieldLogger) {
 	sigs := make(chan os.Signal, 1)
 	go func() {
 		for sig := range sigs {