cmd/gitaly-lfs-smudge: Stop using labkit logger

Stop using the labkit logger in favor of our own logging mechanisms
provided by the `internal/log` package.

3 files changed, 52 insertions, 38 deletions

diff --git a/cmd/gitaly-lfs-smudge/main.go b/cmd/gitaly-lfs-smudge/main.go
index dbd67c7ce..ac62e6c31 100644
--- a/cmd/gitaly-lfs-smudge/main.go
+++ b/cmd/gitaly-lfs-smudge/main.go
@@ -7,10 +7,12 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/smudge"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/env"
-	gitalylog "gitlab.com/gitlab-org/gitaly/v16/internal/log"
-	"gitlab.com/gitlab-org/labkit/log"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
+	"gitlab.com/gitlab-org/labkit/correlation"
 	"gitlab.com/gitlab-org/labkit/tracing"
 )
 
@@ -33,40 +35,54 @@ func requireStdin(msg string) {
 func main() {
 	requireStdin("This command should be run by the Git 'smudge' filter")
 
-	closer, err := initLogging(os.Environ())
+	// Since the environment is sanitized at the moment, we're only
+	// using this to extract the correlation ID. The finished() call
+	// to clean up the tracing will be a NOP here.
+	ctx, finished := tracing.ExtractFromEnv(context.Background())
+	defer finished()
+
+	logger, closer, err := configureLogging(ctx, os.Environ())
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "error initializing log file for gitaly-lfs-smudge: %v", err)
 	}
 	defer closer.Close()
 
-	if err := run(os.Environ(), os.Stdout, os.Stdin); err != nil {
-		log.WithError(err).Error(err)
+	if err := run(ctx, os.Environ(), os.Stdout, os.Stdin, logger); err != nil {
+		logger.WithError(err).Error(err)
 		os.Exit(1)
 	}
 }
 
-func initLogging(environment []string) (io.Closer, error) {
-	path := env.ExtractValue(environment, gitalylog.GitalyLogDirEnvKey)
-	if path == "" {
-		return log.Initialize(log.WithWriter(io.Discard))
+type nopCloser struct{}
+
+func (nopCloser) Close() error {
+	return nil
+}
+
+func configureLogging(ctx context.Context, environment []string) (logrus.FieldLogger, io.Closer, error) {
+	var closer io.Closer = nopCloser{}
+	writer := io.Discard
+
+	if logDir := env.ExtractValue(environment, log.GitalyLogDirEnvKey); logDir != "" {
+		logFile, err := os.OpenFile(filepath.Join(logDir, "gitaly_lfs_smudge.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
+		}
 	}
 
-	filepath := filepath.Join(path, "gitaly_lfs_smudge.log")
+	logger, err := log.Configure(writer, "json", "info")
+	if err != nil {
+		closer.Close()
+		return nil, nil, err
+	}
 
-	return log.Initialize(
-		log.WithFormatter("json"),
-		log.WithLogLevel("info"),
-		log.WithOutputName(filepath),
-	)
+	return logger.WithField(correlation.FieldName, correlation.ExtractFromContext(ctx)), closer, nil
 }
 
-func run(environment []string, out io.Writer, in io.Reader) error {
-	// Since the environment is sanitized at the moment, we're only
-	// using this to extract the correlation ID. The finished() call
-	// to clean up the tracing will be a NOP here.
-	ctx, finished := tracing.ExtractFromEnv(context.Background())
-	defer finished()
-
+func run(ctx context.Context, environment []string, out io.Writer, in io.Reader, logger logrus.FieldLogger) error {
 	cfg, err := smudge.ConfigFromEnvironment(environment)
 	if err != nil {
 		return fmt.Errorf("loading configuration: %w", err)
@@ -74,13 +90,13 @@ func run(environment []string, out io.Writer, in io.Reader) error {
 
 	switch cfg.DriverType {
 	case smudge.DriverTypeFilter:
-		if err := filter(ctx, cfg, out, in); err != nil {
+		if err := filter(ctx, cfg, out, in, logger); err != nil {
 			return fmt.Errorf("running smudge filter: %w", err)
 		}
 
 		return nil
 	case smudge.DriverTypeProcess:
-		if err := process(ctx, cfg, out, in); err != nil {
+		if err := process(ctx, cfg, out, in, logger); err != nil {
 			return fmt.Errorf("running smudge process: %w", err)
 		}
 
diff --git a/cmd/gitaly-lfs-smudge/smudge.go b/cmd/gitaly-lfs-smudge/smudge.go
index 405dcfa00..17d8a9f16 100644
--- a/cmd/gitaly-lfs-smudge/smudge.go
+++ b/cmd/gitaly-lfs-smudge/smudge.go
@@ -10,20 +10,20 @@ import (
 	"net/url"
 
 	"github.com/git-lfs/git-lfs/v3/lfs"
+	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/pktline"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/smudge"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
-	"gitlab.com/gitlab-org/labkit/log"
 )
 
-func filter(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reader) (returnedErr error) {
-	client, err := gitlab.NewHTTPClient(log.ContextLogger(ctx), cfg.Gitlab, cfg.TLS, prometheus.Config{})
+func filter(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reader, logger logrus.FieldLogger) (returnedErr error) {
+	client, err := gitlab.NewHTTPClient(logger, cfg.Gitlab, cfg.TLS, prometheus.Config{})
 	if err != nil {
 		return fmt.Errorf("creating HTTP client: %w", err)
 	}
 
-	output, err := smudgeOneObject(ctx, cfg, client, from)
+	output, err := smudgeOneObject(ctx, cfg, client, from, logger)
 	if err != nil {
 		return fmt.Errorf("smudging contents: %w", err)
 	}
@@ -63,8 +63,8 @@ const (
 	processStateSmudgeContent
 )
 
-func process(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reader) error {
-	client, err := gitlab.NewHTTPClient(log.ContextLogger(ctx), cfg.Gitlab, cfg.TLS, prometheus.Config{})
+func process(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reader, logger logrus.FieldLogger) error {
+	client, err := gitlab.NewHTTPClient(logger, cfg.Gitlab, cfg.TLS, prometheus.Config{})
 	if err != nil {
 		return fmt.Errorf("creating HTTP client: %w", err)
 	}
@@ -194,9 +194,9 @@ func process(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reade
 			// When we receive a flush packet we know that the client is done sending us
 			// the clean data.
 			if pktline.IsFlush(line) {
-				smudgedReader, err := smudgeOneObject(ctx, cfg, client, &content)
+				smudgedReader, err := smudgeOneObject(ctx, cfg, client, &content, logger)
 				if err != nil {
-					log.ContextLogger(ctx).WithError(err).Error("failed smudging LFS pointer")
+					logger.WithError(err).Error("failed smudging LFS pointer")
 
 					if _, err := pktline.WriteString(writer, "status=error\n"); err != nil {
 						return fmt.Errorf("reporting failure: %w", err)
@@ -287,9 +287,7 @@ func process(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reade
 	return nil
 }
 
-func smudgeOneObject(ctx context.Context, cfg smudge.Config, gitlabClient *gitlab.HTTPClient, from io.Reader) (io.ReadCloser, error) {
-	logger := log.ContextLogger(ctx)
-
+func smudgeOneObject(ctx context.Context, cfg smudge.Config, gitlabClient *gitlab.HTTPClient, from io.Reader, logger logrus.FieldLogger) (io.ReadCloser, error) {
 	ptr, contents, err := lfs.DecodeFrom(from)
 	if err != nil {
 		// This isn't a valid LFS pointer. Just copy the existing pointer data.
diff --git a/cmd/gitaly-lfs-smudge/smudge_test.go b/cmd/gitaly-lfs-smudge/smudge_test.go
index e24c7a7ca..7ab9c0ac0 100644
--- a/cmd/gitaly-lfs-smudge/smudge_test.go
+++ b/cmd/gitaly-lfs-smudge/smudge_test.go
@@ -84,7 +84,7 @@ func TestFilter_successful(t *testing.T) {
 				},
 			}
 
-			require.NoError(t, filter(ctx, cfg, &b, reader))
+			require.NoError(t, filter(ctx, cfg, &b, reader, testhelper.NewDiscardingLogger(t)))
 			require.Equal(t, testData, b.String())
 		})
 	}
@@ -185,7 +185,7 @@ func TestFilter_unsuccessful(t *testing.T) {
 			cfg := tc.setupCfg(t, gitlabCfg)
 
 			var b bytes.Buffer
-			err := filter(ctx, cfg, &b, strings.NewReader(tc.data))
+			err := filter(ctx, cfg, &b, strings.NewReader(tc.data), testhelper.NewDiscardingLogger(t))
 
 			if tc.expectedError {
 				require.Error(t, err)
@@ -558,7 +558,7 @@ func TestProcess(t *testing.T) {
 			}
 
 			var outputBuffer bytes.Buffer
-			require.Equal(t, tc.expectedErr, process(ctx, tc.cfg, &outputBuffer, &inputBuffer))
+			require.Equal(t, tc.expectedErr, process(ctx, tc.cfg, &outputBuffer, &inputBuffer, testhelper.NewDiscardingLogger(t)))
 			require.Equal(t, tc.expectedOutput, outputBuffer.String())
 		})
 	}