Merge branch 'qmnguyen0711/expose-test-logs-as-artifaccts' into 'master'

Expose logs as artifacts when running tests on CI

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

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

5 files changed, 59 insertions, 5 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9752897fc..611399929 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -102,6 +102,7 @@ include:
     TEST_REPORT: "${CI_PROJECT_DIR}/_build/reports/go-tests-report.xml"
     TEST_COVERAGE_DIR: "${CI_PROJECT_DIR}/_build/reports/coverage"
     TEST_FULL_OUTPUT: "${CI_PROJECT_DIR}/_build/reports/test-output.log"
+    TEST_LOG_DIR: "${CI_PROJECT_DIR}/_build/reports/test-logs"
   before_script: &test_before_script
     - go version
   script:
@@ -123,10 +124,11 @@ include:
       fi
   artifacts:
     paths:
-      - ruby/tmp/gitaly-rspec-test.log
+      - ${CI_PROJECT_DIR}/ruby/tmp/gitaly-rspec-test.log
+      - ${TEST_LOG_DIR}
     reports:
       junit: ${TEST_REPORT}
-    when: on_failure
+    when: always
     expire_in: 1 week
 
 danger-review:
@@ -337,6 +339,9 @@ test:macos:
     PGUSER: gitlab
     TEST_OPTIONS: "-timeout=20m" # a number of tests may exceed the default 10m
   artifacts:
+    paths:
+      - ${CI_PROJECT_DIR}/ruby/tmp/gitaly-rspec-test.log
+      - ${TEST_LOG_DIR}
     reports:
       junit: ${TEST_REPORT}
     when: always
diff --git a/Makefile b/Makefile
index b50f638ee..4bb2b811a 100644
--- a/Makefile
+++ b/Makefile
@@ -233,6 +233,8 @@ TEST_FULL_OUTPUT  ?= /dev/null
 TEST_COVERAGE_DIR ?= ${BUILD_DIR}/cover
 ## Directory where all runtime test data is being created.
 TEST_TMP_DIR      ?=
+## Directory where Gitaly should write logs to during test execution.
+TEST_LOG_DIR	  ?=
 TEST_REPO_DIR     := ${BUILD_DIR}/testrepos
 TEST_REPO         := ${TEST_REPO_DIR}/gitlab-test.git
 TEST_REPO_MIRROR  := ${TEST_REPO_DIR}/gitlab-test-mirror.git
@@ -253,8 +255,10 @@ find_go_sources              = $(shell find ${SOURCE_DIR} -type d \( -name ruby
 #
 # TEST_OPTIONS: any additional options
 # TEST_PACKAGES: packages which shall be tested
+# TEST_LOG_DIR: specify the output log dir. By default, all logs will be discarded
 run_go_tests = PATH='${SOURCE_DIR}/internal/testhelper/testdata/home/bin:${PATH}' \
     TEST_TMP_DIR='${TEST_TMP_DIR}' \
+    TEST_LOG_DIR='${TEST_LOG_DIR}' \
     ${GOTESTSUM} --format ${TEST_FORMAT} --junitfile ${TEST_REPORT} --jsonfile ${TEST_FULL_OUTPUT} -- -ldflags '${GO_LDFLAGS}' -tags '${SERVER_BUILD_TAGS},${GIT2GO_BUILD_TAGS},gitaly_test_signing' ${TEST_OPTIONS} ${TEST_PACKAGES}
 
 ## Test options passed to `dlv test`.
diff --git a/internal/testhelper/logger.go b/internal/testhelper/logger.go
index 9d61c1fa5..ad7cdbcf5 100644
--- a/internal/testhelper/logger.go
+++ b/internal/testhelper/logger.go
@@ -1,10 +1,14 @@
 package testhelper
 
 import (
+	"fmt"
 	"io"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
 )
 
 // NewDiscardingLogger creates a logger that discards everything.
@@ -18,3 +22,39 @@ func NewDiscardingLogger(tb testing.TB) *logrus.Logger {
 func NewDiscardingLogEntry(tb testing.TB) *logrus.Entry {
 	return logrus.NewEntry(NewDiscardingLogger(tb))
 }
+
+// NewGitalyServerLogger creates a logger for Gitaly servers started by current test. The logger
+// writes logs to gitaly_server.log inside log dir from TEST_LOG_DIR env.
+func NewGitalyServerLogger(tb testing.TB) *logrus.Logger {
+	logDir := CreateTestLogDir(tb)
+	if len(logDir) == 0 {
+		return NewDiscardingLogger(tb)
+	}
+
+	path := filepath.Join(logDir, "gitaly_server.log")
+	f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_RDWR, 0o755)
+	require.NoError(tb, err)
+
+	tb.Cleanup(func() { require.NoError(tb, f.Close()) })
+
+	logger := logrus.New()
+	logger.SetOutput(f)
+	logger.SetLevel(logrus.InfoLevel)
+	logger.SetFormatter(&logrus.JSONFormatter{})
+	logger.Infof(fmt.Sprintf("=== RUN %s", tb.Name()))
+
+	return logger
+}
+
+// CreateTestLogDir creates the log directory specified in the `TEST_LOG_DIR` environment variable.
+// If the variable is unset, then no directory will be created. This method returns the path to
+// the log dir if TEST_LOG_DIR is set.
+func CreateTestLogDir(tb testing.TB) string {
+	testLogDir := os.Getenv("TEST_LOG_DIR")
+	if len(testLogDir) == 0 {
+		return ""
+	}
+	require.NoError(tb, os.MkdirAll(testLogDir, 0o755))
+
+	return testLogDir
+}
diff --git a/internal/testhelper/testcfg/gitaly_builder.go b/internal/testhelper/testcfg/gitaly_builder.go
index 28caf6e01..84df476c5 100644
--- a/internal/testhelper/testcfg/gitaly_builder.go
+++ b/internal/testhelper/testcfg/gitaly_builder.go
@@ -89,8 +89,13 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg {
 	}
 
 	if cfg.Logging.Dir == "" {
-		cfg.Logging.Dir = filepath.Join(root, "log.d")
-		require.NoError(tb, os.Mkdir(cfg.Logging.Dir, 0o755))
+		logDir := testhelper.CreateTestLogDir(tb)
+		if len(logDir) != 0 {
+			cfg.Logging.Dir = logDir
+		} else {
+			cfg.Logging.Dir = filepath.Join(root, "log.d")
+			require.NoError(tb, os.Mkdir(cfg.Logging.Dir, 0o755))
+		}
 	}
 
 	if cfg.GitlabShell.Dir == "" {
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index d27ce2726..fc475c702 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -253,7 +253,7 @@ type gitalyServerDeps struct {
 
 func (gsd *gitalyServerDeps) createDependencies(tb testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server) *service.Dependencies {
 	if gsd.logger == nil {
-		gsd.logger = testhelper.NewDiscardingLogger(tb)
+		gsd.logger = testhelper.NewGitalyServerLogger(tb)
 	}
 
 	if gsd.conns == nil {