Merge branch 'pks-log-stop-using-logrus-new' into 'master'

log: Stop using `logrus.New()`

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

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>

14 files changed, 82 insertions, 94 deletions

diff --git a/.golangci.yml b/.golangci.yml
index 80720509c..b995e9795 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -74,6 +74,8 @@ linters-settings:
         msg: Use the logger provided by `log.Default()` or `ctxlogrus.Extract()`.
       - p: ^logrus\.StandardLogger$
         msg: Use the logger provided by `log.Default()` or `ctxlogrus.Extract()`.
+      - p: ^logrus\.New$
+        msg: Use the logger provided by `log.Default()` or `ctxlogrus.Extract()`.
     analyze-types: true
   paralleltest:
     # Ignore missing calls to `t.Parallel()` and only report incorrect uses of it.
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 6d737acd3..20dfba9ce 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -17,11 +17,13 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/env"
-	gitalylog "gitlab.com/gitlab-org/gitaly/v16/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/stream"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/tracing"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v16/streamio"
+	"gitlab.com/gitlab-org/labkit/correlation"
 	labkitcorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
 	labkittracing "gitlab.com/gitlab-org/labkit/tracing"
 	"google.golang.org/grpc"
@@ -74,7 +76,8 @@ func main() {
 	ctx, finished := labkittracing.ExtractFromEnv(ctx)
 	defer finished()
 
-	logger := gitalylog.NewHookLogger(ctx)
+	logger := configureLogger(ctx)
+
 	if err := run(ctx, os.Args); err != nil {
 		var hookError hookError
 		if errors.As(err, &hookError) {
@@ -94,6 +97,25 @@ 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 {
+	writer := io.Discard
+
+	if logDir := os.Getenv(log.GitalyLogDirEnvKey); logDir != "" {
+		logFile, err := os.OpenFile(filepath.Join(logDir, "gitaly_hooks.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, perm.SharedFile)
+		if err != nil {
+			// Ignore this error as we cannot do anything about it anyway. We cannot write anything to
+			// stdout or stderr as that might break hooks, and we have no other destination to log to.
+		} else {
+			writer = logFile
+		}
+	}
+
+	return log.Configure(writer, "text", "info").WithField(correlation.FieldName, correlation.ExtractFromContext(ctx))
+}
+
 // Both stderr and stdout of gitaly-hooks are streamed back to clients. stdout is processed by client
 // git process transparently. stderr is dumped directly to client's stdout. Thus, we must be cautious
 // what to write into stderr.
diff --git a/cmd/gitaly/main_test.go b/cmd/gitaly/main_test.go
index d9335f74c..f0f10b2d7 100644
--- a/cmd/gitaly/main_test.go
+++ b/cmd/gitaly/main_test.go
@@ -49,8 +49,7 @@ func TestGitalyCLI(t *testing.T) {
 			desc:     "check with non-existent config",
 			args:     []string{"check", "non-existent-file"},
 			exitCode: 1,
-			stdout:   "Checking GitLab API access: FAILED",
-			stderr:   "load config: config_path \"non-existent-file\": open non-existent-file: no such file or directory\n",
+			stderr:   "loading configuration \"non-existent-file\": open non-existent-file: no such file or directory\n",
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
diff --git a/internal/cli/gitaly/check.go b/internal/cli/gitaly/check.go
index 40d5f925e..9dc99e731 100644
--- a/internal/cli/gitaly/check.go
+++ b/internal/cli/gitaly/check.go
@@ -3,6 +3,7 @@ package gitaly
 import (
 	"context"
 	"fmt"
+	"os"
 
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
@@ -11,6 +12,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 )
 
 func newCheckCommand() *cli.Command {
@@ -27,8 +29,6 @@ Example: gitaly check gitaly.config.toml`,
 }
 
 func checkAction(ctx *cli.Context) error {
-	logrus.SetLevel(logrus.ErrorLevel)
-
 	if ctx.NArg() != 1 || ctx.Args().First() == "" {
 		if err := cli.ShowSubcommandHelp(ctx); err != nil {
 			return err
@@ -38,9 +38,15 @@ func checkAction(ctx *cli.Context) error {
 	}
 
 	configPath := ctx.Args().First()
+	cfg, err := loadConfig(configPath)
+	if err != nil {
+		return fmt.Errorf("loading configuration %q: %w", configPath, err)
+	}
+
+	logger := log.Configure(os.Stderr, "text", "error")
 
 	fmt.Fprint(ctx.App.Writer, "Checking GitLab API access: ")
-	info, err := checkAPI(configPath)
+	info, err := checkAPI(cfg, logger)
 	if err != nil {
 		fmt.Fprintln(ctx.App.Writer, "FAILED")
 		return err
@@ -56,13 +62,8 @@ func checkAction(ctx *cli.Context) error {
 	return nil
 }
 
-func checkAPI(configPath string) (*gitlab.CheckInfo, error) {
-	cfg, err := loadConfig(configPath)
-	if err != nil {
-		return nil, fmt.Errorf("load config: config_path %q: %w", configPath, err)
-	}
-
-	gitlabAPI, err := gitlab.NewHTTPClient(logrus.New(), cfg.Gitlab, cfg.TLS, prometheus.Config{})
+func checkAPI(cfg config.Cfg, logger logrus.FieldLogger) (*gitlab.CheckInfo, error) {
+	gitlabAPI, err := gitlab.NewHTTPClient(logger, cfg.Gitlab, cfg.TLS, prometheus.Config{})
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/grpc/backchannel/backchannel_test.go b/internal/grpc/backchannel/backchannel_test.go
index a7c8fed6c..d9a4ae4ee 100644
--- a/internal/grpc/backchannel/backchannel_test.go
+++ b/internal/grpc/backchannel/backchannel_test.go
@@ -10,7 +10,6 @@ import (
 	"sync/atomic"
 	"testing"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/listenmux"
@@ -31,18 +30,12 @@ func (m mockTransactionServer) VoteTransaction(ctx context.Context, req *gitalyp
 	return m.voteTransactionFunc(ctx, req)
 }
 
-func newLogger() *logrus.Entry {
-	logger := logrus.New()
-	logger.Out = io.Discard
-	return logrus.NewEntry(logger)
-}
-
 func TestBackchannel_concurrentRequestsFromMultipleClients(t *testing.T) {
 	var interceptorInvoked int32
 	registry := NewRegistry()
 	lm := listenmux.New(insecure.NewCredentials())
 	lm.Register(NewServerHandshaker(
-		newLogger(),
+		testhelper.NewDiscardingLogEntry(t),
 		registry,
 		[]grpc.DialOption{
 			grpc.WithUnaryInterceptor(func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
@@ -109,7 +102,7 @@ func TestBackchannel_concurrentRequestsFromMultipleClients(t *testing.T) {
 
 			expectedErr := status.Error(codes.Internal, fmt.Sprintf("multiplexed %d", i))
 
-			clientHandshaker := NewClientHandshaker(newLogger(), func() Server {
+			clientHandshaker := NewClientHandshaker(testhelper.NewDiscardingLogEntry(t), func() Server {
 				srv := grpc.NewServer()
 				gitalypb.RegisterRefTransactionServer(srv, mockTransactionServer{
 					voteTransactionFunc: func(ctx context.Context, req *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
@@ -246,7 +239,7 @@ func Benchmark(b *testing.B) {
 					var serverOpts []grpc.ServerOption
 					if tc.multiplexed {
 						lm := listenmux.New(insecure.NewCredentials())
-						lm.Register(NewServerHandshaker(newLogger(), NewRegistry(), nil))
+						lm.Register(NewServerHandshaker(testhelper.NewDiscardingLogEntry(b), NewRegistry(), nil))
 						serverOpts = []grpc.ServerOption{
 							grpc.Creds(lm),
 						}
@@ -274,7 +267,7 @@ func Benchmark(b *testing.B) {
 
 					opts := []grpc.DialOption{grpc.WithBlock(), grpc.WithTransportCredentials(insecure.NewCredentials())}
 					if tc.multiplexed {
-						clientHandshaker := NewClientHandshaker(newLogger(), func() Server { return grpc.NewServer() }, DefaultConfiguration())
+						clientHandshaker := NewClientHandshaker(testhelper.NewDiscardingLogEntry(b), func() Server { return grpc.NewServer() }, DefaultConfiguration())
 						opts = []grpc.DialOption{
 							grpc.WithBlock(),
 							grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials())),
diff --git a/internal/grpc/sidechannel/proxy_test.go b/internal/grpc/sidechannel/proxy_test.go
index 7ab65bb1f..68f1c7322 100644
--- a/internal/grpc/sidechannel/proxy_test.go
+++ b/internal/grpc/sidechannel/proxy_test.go
@@ -7,7 +7,6 @@ import (
 	"net"
 	"testing"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/backchannel"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/listenmux"
@@ -93,7 +92,7 @@ func testUnaryProxy(t *testing.T, closeWrite bool) {
 	proxyAddr := startServer(
 		t,
 		func(ctx context.Context, request *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) {
-			conn, err := dialProxy(upstreamAddr)
+			conn, err := dialProxy(t, upstreamAddr)
 			if err != nil {
 				return nil, err
 			}
@@ -109,9 +108,7 @@ func testUnaryProxy(t *testing.T, closeWrite bool) {
 	require.NoError(t, call(ctx, conn, registry, testProxyClient(closeWrite)))
 }
 
-func newLogger() *logrus.Entry { return logrus.NewEntry(logrus.New()) }
-
-func dialProxy(upstreamAddr string) (*grpc.ClientConn, error) {
+func dialProxy(tb testing.TB, upstreamAddr string) (*grpc.ClientConn, error) {
 	registry := NewRegistry()
 	factory := func() backchannel.Server {
 		lm := listenmux.New(insecure.NewCredentials())
@@ -119,7 +116,7 @@ func dialProxy(upstreamAddr string) (*grpc.ClientConn, error) {
 		return grpc.NewServer(grpc.Creds(lm))
 	}
 
-	clientHandshaker := backchannel.NewClientHandshaker(newLogger(), factory, backchannel.DefaultConfiguration())
+	clientHandshaker := backchannel.NewClientHandshaker(testhelper.NewDiscardingLogEntry(tb), factory, backchannel.DefaultConfiguration())
 	dialOpts := []grpc.DialOption{
 		grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials())),
 		grpc.WithUnaryInterceptor(NewUnaryProxy(registry)),
@@ -144,7 +141,7 @@ func testStreamProxy(t *testing.T, closeWrite bool) {
 	proxyAddr := startStreamServer(
 		t,
 		func(stream gitalypb.SSHService_SSHUploadPackServer) error {
-			conn, err := dialProxy(upstreamAddr)
+			conn, err := dialProxy(t, upstreamAddr)
 			if err != nil {
 				return err
 			}
@@ -192,7 +189,7 @@ func startStreamServer(t *testing.T, handler func(gitalypb.SSHService_SSHUploadP
 
 	lm := listenmux.New(insecure.NewCredentials())
 	lm.Register(backchannel.NewServerHandshaker(
-		newLogger(), backchannel.NewRegistry(), nil,
+		testhelper.NewDiscardingLogEntry(t), backchannel.NewRegistry(), nil,
 	))
 
 	srv := grpc.NewServer(grpc.Creds(lm))
diff --git a/internal/grpc/sidechannel/sidechannel_test.go b/internal/grpc/sidechannel/sidechannel_test.go
index dace862e4..7465e27be 100644
--- a/internal/grpc/sidechannel/sidechannel_test.go
+++ b/internal/grpc/sidechannel/sidechannel_test.go
@@ -150,7 +150,7 @@ func startServer(t *testing.T, th testHandler, opts ...grpc.ServerOption) string
 	t.Helper()
 
 	lm := listenmux.New(insecure.NewCredentials())
-	lm.Register(backchannel.NewServerHandshaker(newLogger(), backchannel.NewRegistry(), nil))
+	lm.Register(backchannel.NewServerHandshaker(testhelper.NewDiscardingLogEntry(t), backchannel.NewRegistry(), nil))
 
 	opts = append(opts, grpc.Creds(lm))
 
@@ -170,7 +170,7 @@ func startServer(t *testing.T, th testHandler, opts ...grpc.ServerOption) string
 
 func dial(t *testing.T, addr string) (*grpc.ClientConn, *Registry) {
 	registry := NewRegistry()
-	clientHandshaker := NewClientHandshaker(newLogger(), registry)
+	clientHandshaker := NewClientHandshaker(testhelper.NewDiscardingLogEntry(t), registry)
 	dialOpt := grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials()))
 
 	conn, err := grpc.Dial(addr, dialOpt)
diff --git a/internal/log/hook.go b/internal/log/hook.go
deleted file mode 100644
index 503b76b93..000000000
--- a/internal/log/hook.go
+++ /dev/null
@@ -1,40 +0,0 @@
-package log
-
-import (
-	"context"
-	"io"
-	"os"
-	"path/filepath"
-
-	"github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
-	"gitlab.com/gitlab-org/labkit/correlation"
-)
-
-// NewHookLogger creates a file logger, since both stderr and stdout will be displayed in git output
-func NewHookLogger(ctx context.Context) *logrus.Entry {
-	logger := logrus.New()
-
-	logDir := os.Getenv(GitalyLogDirEnvKey)
-	if logDir == "" {
-		logger.SetOutput(io.Discard)
-		return logrus.NewEntry(logger)
-	}
-
-	logFile, err := os.OpenFile(filepath.Join(logDir, "gitaly_hooks.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, perm.SharedFile)
-	if err != nil {
-		logger.SetOutput(io.Discard)
-	} else {
-		logger.SetOutput(logFile)
-	}
-
-	logger.SetFormatter(UTCTextFormatter())
-
-	return logger.WithFields(logFieldsFromContext(ctx))
-}
-
-func logFieldsFromContext(ctx context.Context) logrus.Fields {
-	return logrus.Fields{
-		correlation.FieldName: correlation.ExtractFromContext(ctx),
-	}
-}
diff --git a/internal/log/logger.go b/internal/log/logger.go
index fd45e3676..ee7adb7cb 100644
--- a/internal/log/logger.go
+++ b/internal/log/logger.go
@@ -62,7 +62,7 @@ func Configure(out io.Writer, format string, level string, hooks ...logrus.Hook)
 	configure(defaultLogger, out, format, level, hooks...)
 
 	// We replace the gRPC logger with a custom one because the default one is too chatty.
-	grpcLogger := logrus.New()
+	grpcLogger := logrus.New() //nolint:forbidigo
 	configure(grpcLogger, out, format, mapGRPCLogLevel(level), hooks...)
 	grpcmwlogrus.ReplaceGrpcLogger(grpcLogger.WithField("pid", os.Getpid()))
 
diff --git a/internal/log/logger_test.go b/internal/log/logger_test.go
index 404e37fa5..d710eebe4 100644
--- a/internal/log/logger_test.go
+++ b/internal/log/logger_test.go
@@ -28,7 +28,7 @@ func TestConfigure(t *testing.T) {
 			desc:   "json format with info level",
 			format: "json",
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCJsonFormatter()
 				logger.Level = logrus.InfoLevel
@@ -39,7 +39,7 @@ func TestConfigure(t *testing.T) {
 			desc:   "text format with info level",
 			format: "text",
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCTextFormatter()
 				logger.Level = logrus.InfoLevel
@@ -49,7 +49,7 @@ func TestConfigure(t *testing.T) {
 		{
 			desc: "empty format with info level",
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCTextFormatter()
 				logger.Level = logrus.InfoLevel
@@ -61,7 +61,7 @@ func TestConfigure(t *testing.T) {
 			format: "text",
 			level:  "debug",
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCTextFormatter()
 				logger.Level = logrus.DebugLevel
@@ -73,7 +73,7 @@ func TestConfigure(t *testing.T) {
 			format: "text",
 			level:  "invalid-level",
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCTextFormatter()
 				logger.Level = logrus.InfoLevel
@@ -88,7 +88,7 @@ func TestConfigure(t *testing.T) {
 				testHook,
 			},
 			expectedLogger: func() *logrus.Logger {
-				logger := logrus.New()
+				logger := newLogger()
 				logger.Out = &out
 				logger.Formatter = UTCTextFormatter()
 				logger.Level = logrus.InfoLevel
@@ -100,7 +100,7 @@ func TestConfigure(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			out.Reset()
 
-			logger := logrus.New()
+			logger := newLogger()
 			configure(logger, &out, tc.format, tc.level, tc.hooks...)
 
 			// We cannot directly compare the loggers with each other because they contain function
diff --git a/internal/log/middleware_test.go b/internal/log/middleware_test.go
index 734d91d97..8e333997c 100644
--- a/internal/log/middleware_test.go
+++ b/internal/log/middleware_test.go
@@ -273,7 +273,7 @@ func TestPerRPCLogHandler(t *testing.T) {
 	})
 
 	t.Run("log handling", func(t *testing.T) {
-		ctx := ctxlogrus.ToContext(createContext(), logrus.NewEntry(logrus.New()))
+		ctx := ctxlogrus.ToContext(createContext(), logrus.NewEntry(newLogger()))
 		ctx = lh.TagRPC(ctx, &stats.RPCTagInfo{})
 		mpp := ctx.Value(messageProducerHolderKey{}).(*messageProducerHolder)
 		mpp.format = "message"
@@ -340,7 +340,7 @@ func TestUnaryLogDataCatcherServerInterceptor(t *testing.T) {
 	t.Run("caught", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
 		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
-		ctx = ctxlogrus.ToContext(ctx, logrus.New().WithField("a", 1))
+		ctx = ctxlogrus.ToContext(ctx, newLogger().WithField("a", 1))
 		interceptor := UnaryLogDataCatcherServerInterceptor()
 		_, _ = interceptor(ctx, nil, nil, handlerStub)
 		assert.Equal(t, logrus.Fields{"a": 1}, mpp.fields)
@@ -370,7 +370,7 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 	t.Run("caught", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
 		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
-		ctx = ctxlogrus.ToContext(ctx, logrus.New().WithField("a", 1))
+		ctx = ctxlogrus.ToContext(ctx, newLogger().WithField("a", 1))
 
 		interceptor := StreamLogDataCatcherServerInterceptor()
 		ss := &grpcmw.WrappedServerStream{WrappedContext: ctx}
diff --git a/internal/log/testhelper_test.go b/internal/log/testhelper_test.go
new file mode 100644
index 000000000..c96b77e61
--- /dev/null
+++ b/internal/log/testhelper_test.go
@@ -0,0 +1,17 @@
+package log
+
+import (
+	"io"
+
+	"github.com/sirupsen/logrus"
+)
+
+// newLogger creates a new logger for testing purposes. Use of `logrus.New()` is forbidden globally, but required here
+// to verify that we correctly configure our logging infrastructure.
+//
+//nolint:forbidigo
+func newLogger() *logrus.Logger {
+	logger := logrus.New()
+	logger.Out = io.Discard
+	return logger
+}
diff --git a/internal/log/url_sanitizer_test.go b/internal/log/url_sanitizer_test.go
index 67b90f807..756122423 100644
--- a/internal/log/url_sanitizer_test.go
+++ b/internal/log/url_sanitizer_test.go
@@ -3,7 +3,6 @@ package log
 import (
 	"bytes"
 	"fmt"
-	"io"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -20,7 +19,7 @@ func TestUrlSanitizerHook(t *testing.T) {
 		"FetchRemote",
 	)
 
-	logger := logrus.New()
+	logger := newLogger()
 	logger.Out = outBuf
 	logger.Hooks.Add(urlSanitizer)
 
@@ -100,8 +99,7 @@ func TestUrlSanitizerHook(t *testing.T) {
 func BenchmarkUrlSanitizerWithoutSanitization(b *testing.B) {
 	urlSanitizer := NewURLSanitizerHook()
 
-	logger := logrus.New()
-	logger.Out = io.Discard
+	logger := newLogger()
 	logger.Hooks.Add(urlSanitizer)
 
 	benchmarkLogging(b, logger)
@@ -114,8 +112,7 @@ func BenchmarkUrlSanitizerWithSanitization(b *testing.B) {
 		"CreateRepositoryFromURL",
 	)
 
-	logger := logrus.New()
-	logger.Out = io.Discard
+	logger := newLogger()
 	logger.Hooks.Add(urlSanitizer)
 
 	benchmarkLogging(b, logger)
diff --git a/internal/testhelper/logger.go b/internal/testhelper/logger.go
index debb875e6..63f165ae3 100644
--- a/internal/testhelper/logger.go
+++ b/internal/testhelper/logger.go
@@ -14,7 +14,7 @@ import (
 
 // NewDiscardingLogger creates a logger that discards everything.
 func NewDiscardingLogger(tb testing.TB) *logrus.Logger {
-	logger := logrus.New()
+	logger := logrus.New() //nolint:forbidigo
 	logger.Out = io.Discard
 	return logger
 }
@@ -36,7 +36,7 @@ func newServerLogger(tb testing.TB, logName string) *logrus.Logger {
 
 	tb.Cleanup(func() { require.NoError(tb, f.Close()) })
 
-	logger := logrus.New()
+	logger := logrus.New() //nolint:forbidigo
 	logger.SetOutput(f)
 	logger.SetLevel(logrus.InfoLevel)
 	logger.SetFormatter(&logrus.JSONFormatter{})