Merge branch 'sh-add-lfs-smudge-logging' into 'master'

Log LFS smudge activity to gitaly_lfs_smudge.log

See merge request gitlab-org/gitaly!2734

13 files changed, 158 insertions, 62 deletions

diff --git a/changelogs/unreleased/sh-add-lfs-smudge-logging.yml b/changelogs/unreleased/sh-add-lfs-smudge-logging.yml
new file mode 100644
index 000000000..9f8b1ceb6
--- /dev/null
+++ b/changelogs/unreleased/sh-add-lfs-smudge-logging.yml
@@ -0,0 +1,5 @@
+---
+title: Log LFS smudge activity to gitaly_lfs_smudge.log
+merge_request: 2734
+author:
+type: added
diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge.go b/cmd/gitaly-lfs-smudge/lfs_smudge.go
index 70cdf62c1..d08716404 100644
--- a/cmd/gitaly-lfs-smudge/lfs_smudge.go
+++ b/cmd/gitaly-lfs-smudge/lfs_smudge.go
@@ -5,11 +5,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"path/filepath"
 
 	"github.com/git-lfs/git-lfs/lfs"
-	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
+	gitalylog "gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/labkit/log"
 	"gitlab.com/gitlab-org/labkit/tracing"
 )
@@ -18,6 +19,21 @@ type configProvider interface {
 	Get(key string) string
 }
 
+func initLogging(p configProvider) (io.Closer, error) {
+	path := p.Get(gitalylog.GitalyLogDirEnvKey)
+	if path == "" {
+		return nil, nil
+	}
+
+	filepath := filepath.Join(path, "gitaly_lfs_smudge.log")
+
+	return log.Initialize(
+		log.WithFormatter("json"),
+		log.WithLogLevel("info"),
+		log.WithOutputName(filepath),
+	)
+}
+
 func smudge(to io.Writer, from io.Reader, cfgProvider configProvider) error {
 	output, err := handleSmudge(to, from, cfgProvider)
 	if err != nil {
@@ -35,44 +51,40 @@ func smudge(to io.Writer, from io.Reader, cfgProvider configProvider) error {
 }
 
 func handleSmudge(to io.Writer, from io.Reader, config configProvider) (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()
+
+	logger := log.ContextLogger(ctx)
+
 	ptr, contents, err := lfs.DecodeFrom(from)
 	if err != nil {
 		// This isn't a valid LFS pointer. Just copy the existing pointer data.
 		return contents, nil
 	}
 
-	log.WithField("oid", ptr.Oid).Debug("decoded LFS OID")
+	logger.WithField("oid", ptr.Oid).Debug("decoded LFS OID")
 
 	cfg, glRepository, err := loadConfig(config)
 	if err != nil {
 		return contents, err
 	}
 
-	log.WithField("gitlab_config", cfg).Debug("loaded GitLab API config")
+	logger.WithField("gitlab_config", cfg).Debug("loaded GitLab API config")
 
 	client, err := hook.NewGitlabNetClient(cfg)
 	if err != nil {
 		return contents, err
 	}
 
-	// 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()
-
-	url := fmt.Sprintf("/lfs?oid=%s&gl_repository=%s", ptr.Oid, glRepository)
-	response, err := client.Get(ctx, url)
+	path := fmt.Sprintf("/lfs?oid=%s&gl_repository=%s", ptr.Oid, glRepository)
+	response, err := client.Get(ctx, path)
 	if err != nil {
 		return contents, fmt.Errorf("error loading LFS object: %v", err)
 	}
 
-	// This cannot go to STDOUT or it will corrupt the stream
-	log.WithFields(logrus.Fields{
-		"status_code": response.StatusCode,
-		"url":         url,
-	}).Info("completed HTTP request")
-
 	if response.StatusCode == 200 {
 		return response.Body, nil
 	}
diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
index 65a2b595f..4a05d1e58 100644
--- a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
+++ b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
@@ -3,7 +3,9 @@ package main
 import (
 	"bytes"
 	"encoding/json"
+	"io/ioutil"
 	"net/http"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -67,24 +69,42 @@ func TestSuccessfulLfsSmudge(t *testing.T) {
 	cfg, err := json.Marshal(c)
 	require.NoError(t, err)
 
+	tmpDir, err := ioutil.TempDir("", "")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
 	env := map[string]string{
 		"GL_REPOSITORY":      "project-1",
 		"GL_INTERNAL_CONFIG": string(cfg),
+		"GITALY_LOG_DIR":     tmpDir,
 	}
 	cfgProvider := &mapConfig{env: env}
+	initLogging(cfgProvider)
 
 	err = smudge(&b, reader, cfgProvider)
 	require.NoError(t, err)
 	require.Equal(t, testData, b.String())
+
+	logFilename := filepath.Join(tmpDir, "gitaly_lfs_smudge.log")
+	require.FileExists(t, logFilename)
+
+	data, err := ioutil.ReadFile(logFilename)
+	require.NoError(t, err)
+	d := string(data)
+
+	require.Contains(t, d, `"msg":"Finished HTTP request"`)
+	require.Contains(t, d, `"status":200`)
+	require.Contains(t, d, `"content_length_bytes":`)
 }
 
 func TestUnsuccessfulLfsSmudge(t *testing.T) {
 	testCases := []struct {
-		desc          string
-		data          string
-		missingEnv    string
-		expectedError bool
-		options       testhelper.GitlabTestServerOptions
+		desc               string
+		data               string
+		missingEnv         string
+		expectedError      bool
+		options            testhelper.GitlabTestServerOptions
+		expectedLogMessage string
 	}{
 		{
 			desc:          "bad LFS pointer",
@@ -93,18 +113,20 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 			expectedError: false,
 		},
 		{
-			desc:          "missing GL_REPOSITORY",
-			data:          lfsPointer,
-			missingEnv:    "GL_REPOSITORY",
-			options:       defaultOptions,
-			expectedError: true,
+			desc:               "missing GL_REPOSITORY",
+			data:               lfsPointer,
+			missingEnv:         "GL_REPOSITORY",
+			options:            defaultOptions,
+			expectedError:      true,
+			expectedLogMessage: "GL_REPOSITORY is not defined",
 		},
 		{
-			desc:          "missing GL_INTERNAL_CONFIG",
-			data:          lfsPointer,
-			missingEnv:    "GL_INTERNAL_CONFIG",
-			options:       defaultOptions,
-			expectedError: true,
+			desc:               "missing GL_INTERNAL_CONFIG",
+			data:               lfsPointer,
+			missingEnv:         "GL_INTERNAL_CONFIG",
+			options:            defaultOptions,
+			expectedError:      true,
+			expectedLogMessage: "unable to retrieve GL_INTERNAL_CONFIG",
 		},
 		{
 			desc: "failed HTTP response",
@@ -116,7 +138,8 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 				GlRepository:  glRepository,
 				LfsStatusCode: http.StatusInternalServerError,
 			},
-			expectedError: true,
+			expectedError:      true,
+			expectedLogMessage: "error loading LFS object",
 		},
 	}
 
@@ -128,9 +151,14 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 			cfg, err := json.Marshal(c)
 			require.NoError(t, err)
 
+			tmpDir, err := ioutil.TempDir("", "")
+			require.NoError(t, err)
+			defer os.RemoveAll(tmpDir)
+
 			env := map[string]string{
 				"GL_REPOSITORY":      "project-1",
 				"GL_INTERNAL_CONFIG": string(cfg),
+				"GITALY_LOG_DIR":     tmpDir,
 			}
 
 			if tc.missingEnv != "" {
@@ -142,6 +170,7 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 			var b bytes.Buffer
 			reader := strings.NewReader(tc.data)
 
+			initLogging(cfgProvider)
 			err = smudge(&b, reader, cfgProvider)
 
 			if tc.expectedError {
@@ -150,6 +179,16 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, tc.data, b.String())
 			}
+
+			logFilename := filepath.Join(tmpDir, "gitaly_lfs_smudge.log")
+			require.FileExists(t, logFilename)
+
+			data, err := ioutil.ReadFile(logFilename)
+			require.NoError(t, err)
+
+			if tc.expectedLogMessage != "" {
+				require.Contains(t, string(data), tc.expectedLogMessage)
+			}
 		})
 	}
 }
diff --git a/cmd/gitaly-lfs-smudge/main.go b/cmd/gitaly-lfs-smudge/main.go
index ec99c127d..314105b50 100644
--- a/cmd/gitaly-lfs-smudge/main.go
+++ b/cmd/gitaly-lfs-smudge/main.go
@@ -29,8 +29,14 @@ func requireStdin(msg string) {
 
 func main() {
 	requireStdin("This command should be run by the Git 'smudge' filter")
-	err := smudge(os.Stdout, os.Stdin, &envConfig{})
 
+	closer, err := initLogging(&envConfig{})
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error initializing log file for gitaly-lfs-smudge: %v", err)
+	}
+	defer closer.Close()
+
+	err = smudge(os.Stdout, os.Stdin, &envConfig{})
 	if err != nil {
 		os.Exit(1)
 	}
diff --git a/go.mod b/go.mod
index 0e9af60ca..65ca39f76 100644
--- a/go.mod
+++ b/go.mod
@@ -26,7 +26,7 @@ require (
 	github.com/sirupsen/logrus v1.7.0
 	github.com/stretchr/testify v1.5.1
 	github.com/uber/jaeger-client-go v2.15.0+incompatible
-	gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201015160834-d2f888583c77
+	gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201106143703-da924afd346d
 	gitlab.com/gitlab-org/labkit v0.0.0-20201014124351-eb1fe6499318
 	go.uber.org/atomic v1.4.0 // indirect
 	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a // indirect
diff --git a/go.sum b/go.sum
index b594d87c5..f011711ee 100644
--- a/go.sum
+++ b/go.sum
@@ -411,8 +411,8 @@ github.com/yudai/pp v2.0.1+incompatible/go.mod h1:PuxR/8QJ7cyCkFp/aUDS+JY727OFEZ
 github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=
 github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
 gitlab.com/gitlab-org/gitaly v1.68.0/go.mod h1:/pCsB918Zu5wFchZ9hLYin9WkJ2yQqdVNz0zlv5HbXg=
-gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201015160834-d2f888583c77 h1:4OJ1ZTsIKyKSmTGrb8vne+kQGWIp20zZ2jv54ny/cY8=
-gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201015160834-d2f888583c77/go.mod h1:5QSTbpAHY2v0iIH5uHh2KA9w7sPUqPmnLjDApI/sv1U=
+gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201106143703-da924afd346d h1:C5BHPkyOWx6mDFz2LA3O/3o2ixSGsQBnaLSSQV7KOj8=
+gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20201106143703-da924afd346d/go.mod h1:5QSTbpAHY2v0iIH5uHh2KA9w7sPUqPmnLjDApI/sv1U=
 gitlab.com/gitlab-org/labkit v0.0.0-20190221122536-0c3fc7cdd57c/go.mod h1:rYhLgfrbEcyfinG+R3EvKu6bZSsmwQqcXzLfHWSfUKM=
 gitlab.com/gitlab-org/labkit v0.0.0-20190221122536-0c3fc7cdd57c/go.mod h1:rYhLgfrbEcyfinG+R3EvKu6bZSsmwQqcXzLfHWSfUKM=
 gitlab.com/gitlab-org/labkit v0.0.0-20200908084045-45895e129029 h1:L7b9YLsU3zBfTShAPl4fjhgFdfSvuo9tu4VobJdcKDs=
diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index fb2fc7b5b..61abdee28 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -15,12 +15,26 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/commit"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"gitlab.com/gitlab-org/labkit/correlation"
 )
 
+type archiveParams struct {
+	ctx         context.Context
+	writer      io.Writer
+	in          *gitalypb.GetArchiveRequest
+	compressCmd *exec.Cmd
+	format      string
+	archivePath string
+	exclude     []string
+	internalCfg []byte
+	binDir      string
+	loggingDir  string
+}
+
 func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.RepositoryService_GetArchiveServer) error {
 	ctx := stream.Context()
 	compressCmd, format := parseArchiveFormat(in.GetFormat())
@@ -72,7 +86,18 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo
 		return err
 	}
 
-	return handleArchive(ctx, writer, in, compressCmd, format, path, exclude, gitlabConfig, s.binDir)
+	return handleArchive(archiveParams{
+		ctx:         ctx,
+		writer:      writer,
+		in:          in,
+		compressCmd: compressCmd,
+		format:      format,
+		archivePath: path,
+		exclude:     exclude,
+		internalCfg: gitlabConfig,
+		binDir:      s.binDir,
+		loggingDir:  s.loggingCfg.Dir,
+	})
 }
 
 func parseArchiveFormat(format gitalypb.GetArchiveRequest_Format) (*exec.Cmd, string) {
@@ -140,42 +165,43 @@ func findGetArchivePath(f *commit.TreeEntryFinder, commitID, path string) (ok bo
 	return true, nil
 }
 
-func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiveRequest, compressCmd *exec.Cmd, format string, archivePath string, exclude []string, internalCfg []byte, binDir string) error {
+func handleArchive(p archiveParams) error {
 	var args []string
-	pathspecs := make([]string, 0, len(exclude)+1)
-	if !in.GetElidePath() {
+	pathspecs := make([]string, 0, len(p.exclude)+1)
+	if !p.in.GetElidePath() {
 		// git archive [options] <commit ID> -- <path> [exclude*]
-		args = []string{in.GetCommitId()}
-		pathspecs = append(pathspecs, archivePath)
-	} else if archivePath != "." {
+		args = []string{p.in.GetCommitId()}
+		pathspecs = append(pathspecs, p.archivePath)
+	} else if p.archivePath != "." {
 		// git archive [options] <commit ID>:<path> -- [exclude*]
-		args = []string{in.GetCommitId() + ":" + archivePath}
+		args = []string{p.in.GetCommitId() + ":" + p.archivePath}
 	} else {
 		// git archive [options] <commit ID> -- [exclude*]
-		args = []string{in.GetCommitId()}
+		args = []string{p.in.GetCommitId()}
 	}
 
-	for _, exclude := range exclude {
+	for _, exclude := range p.exclude {
 		pathspecs = append(pathspecs, ":(exclude)"+exclude)
 	}
 
 	env := []string{
-		fmt.Sprintf("GL_REPOSITORY=%s", in.GetRepository().GetGlRepository()),
-		fmt.Sprintf("GL_PROJECT_PATH=%s", in.GetRepository().GetGlProjectPath()),
-		fmt.Sprintf("GL_INTERNAL_CONFIG=%s", internalCfg),
-		fmt.Sprintf("CORRELATION_ID=%s", correlation.ExtractFromContext(ctx)),
+		fmt.Sprintf("GL_REPOSITORY=%s", p.in.GetRepository().GetGlRepository()),
+		fmt.Sprintf("GL_PROJECT_PATH=%s", p.in.GetRepository().GetGlProjectPath()),
+		fmt.Sprintf("GL_INTERNAL_CONFIG=%s", p.internalCfg),
+		fmt.Sprintf("CORRELATION_ID=%s", correlation.ExtractFromContext(p.ctx)),
+		fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, p.loggingDir),
 	}
 
 	var globals []git.Option
 
-	if in.GetIncludeLfsBlobs() {
-		binary := filepath.Join(binDir, "gitaly-lfs-smudge")
+	if p.in.GetIncludeLfsBlobs() {
+		binary := filepath.Join(p.binDir, "gitaly-lfs-smudge")
 		globals = append(globals, git.ValueFlag{"-c", fmt.Sprintf("filter.lfs.smudge=%s", binary)})
 	}
 
-	archiveCommand, err := git.SafeCmdWithEnv(ctx, env, in.GetRepository(), globals, git.SubCmd{
+	archiveCommand, err := git.SafeCmdWithEnv(p.ctx, env, p.in.GetRepository(), globals, git.SubCmd{
 		Name:        "archive",
-		Flags:       []git.Option{git.ValueFlag{"--format", format}, git.ValueFlag{"--prefix", in.GetPrefix() + "/"}},
+		Flags:       []git.Option{git.ValueFlag{"--format", p.format}, git.ValueFlag{"--prefix", p.in.GetPrefix() + "/"}},
 		Args:        args,
 		PostSepArgs: pathspecs,
 	})
@@ -183,8 +209,8 @@ func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiv
 		return err
 	}
 
-	if compressCmd != nil {
-		command, err := command.New(ctx, compressCmd, archiveCommand, writer, nil)
+	if p.compressCmd != nil {
+		command, err := command.New(p.ctx, p.compressCmd, archiveCommand, p.writer, nil)
 		if err != nil {
 			return err
 		}
@@ -192,7 +218,7 @@ func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiv
 		if err := command.Wait(); err != nil {
 			return err
 		}
-	} else if _, err = io.Copy(writer, archiveCommand); err != nil {
+	} else if _, err = io.Copy(p.writer, archiveCommand); err != nil {
 		return err
 	}
 
diff --git a/internal/gitaly/service/repository/archive_test.go b/internal/gitaly/service/repository/archive_test.go
index 1620b686f..26b872e38 100644
--- a/internal/gitaly/service/repository/archive_test.go
+++ b/internal/gitaly/service/repository/archive_test.go
@@ -521,7 +521,7 @@ func TestGetArchiveEnv(t *testing.T) {
 	require.NoError(t, err)
 
 	tmpFile.Write([]byte(`#!/bin/sh
-env | grep -E "^GL_|CORRELATION"`))
+env | grep -E "^GL_|CORRELATION|GITALY_"`))
 	tmpFile.Close()
 
 	oldBinPath := config.Config.Git.BinPath
@@ -540,6 +540,7 @@ env | grep -E "^GL_|CORRELATION"`))
 	require.Contains(t, string(data), "GL_PROJECT_PATH="+testhelper.GlProjectPath)
 	require.Contains(t, string(data), "GL_INTERNAL_CONFIG="+string(cfgData))
 	require.Contains(t, string(data), "CORRELATION_ID="+correlationID)
+	require.Contains(t, string(data), "GITALY_LOG_DIR="+config.Config.Logging.Dir)
 }
 
 func compressedFileContents(t *testing.T, format gitalypb.GetArchiveRequest_Format, name string) []byte {
diff --git a/internal/gitaly/service/repository/server.go b/internal/gitaly/service/repository/server.go
index 020f37b8f..f82abaa05 100644
--- a/internal/gitaly/service/repository/server.go
+++ b/internal/gitaly/service/repository/server.go
@@ -15,6 +15,7 @@ type server struct {
 	locator              storage.Locator
 	cfg                  config.Gitlab
 	binDir               string
+	loggingCfg           config.Logging
 }
 
 // NewServer creates a new instance of a gRPC repo server
@@ -26,5 +27,6 @@ func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator, i
 		internalGitalySocket: internalGitalySocket,
 		cfg:                  cfg.Gitlab,
 		binDir:               cfg.BinDir,
+		loggingCfg:           cfg.Logging,
 	}
 }
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 763001318..6a859c967 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -2,8 +2,10 @@ package gitlabshell
 
 import (
 	"encoding/json"
+	"fmt"
 
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/log"
 )
 
 // Env is a helper that returns a slice with environment variables used by gitlab shell
@@ -45,7 +47,7 @@ func EnvFromConfig(cfg config.Cfg) ([]string, error) {
 	return []string{
 		//TODO: remove GITALY_GITLAB_SHELL_DIR: https://gitlab.com/gitlab-org/gitaly/-/issues/2679
 		"GITALY_GITLAB_SHELL_DIR=" + cfg.GitlabShell.Dir,
-		"GITALY_LOG_DIR=" + cfg.Logging.Dir,
+		fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir),
 		"GITALY_LOG_FORMAT=" + cfg.Logging.Format,
 		"GITALY_LOG_LEVEL=" + cfg.Logging.Level,
 		"GITALY_BIN_DIR=" + cfg.BinDir,
diff --git a/internal/log/hook.go b/internal/log/hook.go
index a8219b376..e0fe7959b 100644
--- a/internal/log/hook.go
+++ b/internal/log/hook.go
@@ -18,7 +18,7 @@ type HookLogger struct {
 func NewHookLogger() *HookLogger {
 	logger := logrus.New()
 
-	filepath := filepath.Join(os.Getenv("GITALY_LOG_DIR"), "gitaly_hooks.log")
+	filepath := filepath.Join(os.Getenv(GitalyLogDirEnvKey), "gitaly_hooks.log")
 
 	logFile, err := os.OpenFile(filepath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644)
 	if err != nil {
diff --git a/internal/log/log.go b/internal/log/log.go
index cd2271413..1e3a251f5 100644
--- a/internal/log/log.go
+++ b/internal/log/log.go
@@ -6,6 +6,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+const GitalyLogDirEnvKey = "GITALY_LOG_DIR"
+
 var (
 	defaultLogger = logrus.StandardLogger()
 	grpcGo        = logrus.New()
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index c542d9d28..b8d1d94f4 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -33,6 +33,7 @@ import (
 	serverauth "gitlab.com/gitlab-org/gitaly/internal/gitaly/server/auth"
 	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/fieldextractors"
+	gitalylog "gitlab.com/gitlab-org/gitaly/internal/log"
 	praefectconfig "gitlab.com/gitlab-org/gitaly/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	grpccorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
@@ -924,7 +925,7 @@ func EnvForHooks(t testing.TB, gitlabShellDir, gitalySocket, gitalyToken string,
 		fmt.Sprintf("GITALY_TOKEN=%s", gitalyToken),
 		fmt.Sprintf("GITALY_REPO=%v", repoString),
 		fmt.Sprintf("GITALY_GITLAB_SHELL_DIR=%s", gitlabShellDir),
-		fmt.Sprintf("GITALY_LOG_DIR=%s", gitlabShellDir),
+		fmt.Sprintf("%s=%s", gitalylog.GitalyLogDirEnvKey, gitlabShellDir),
 	}...)
 	env = append(env, hooks.GitPushOptions(gitPushOptions)...)