objectpool: Fix data race with logger

In our test to verify that we correctly collect objectpool statistics
via the logger, we create our own logger with a backing buffer. This
buffer is being accessed concurrently though, once by the gRPC
middleware which tries to write into it and once from the test itself
after the RPC call has finished to verify it contains what we expect it
to contain. Given that we don't use locking for this the result is a
data race.

Fix the data race by instead using a NullLogger and extracting the log
entries via a hook. This also allows us to tighten our checks to assert
we see all expected stats.

1 files changed, 22 insertions, 14 deletions

diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 185b1173d..42175a8fb 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -1,16 +1,13 @@
 package objectpool
 
 import (
-	"bytes"
-	"encoding/json"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
-	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
@@ -117,8 +114,8 @@ func TestFetchIntoObjectPool_CollectLogStatistics(t *testing.T) {
 	testcfg.BuildGitalyHooks(t, cfg)
 
 	locator := config.NewLocator(cfg)
-	logBuffer := &bytes.Buffer{}
-	logger := &logrus.Logger{Out: logBuffer, Formatter: &logrus.JSONFormatter{}, Level: logrus.InfoLevel}
+
+	logger, hook := test.NewNullLogger()
 	serverSocketPath := runObjectPoolServer(t, cfg, locator, logger)
 
 	conn, err := grpc.Dial(serverSocketPath, grpc.WithInsecure())
@@ -141,15 +138,26 @@ func TestFetchIntoObjectPool_CollectLogStatistics(t *testing.T) {
 	_, err = client.FetchIntoObjectPool(ctx, req)
 	require.NoError(t, err)
 
-	msgs := strings.Split(logBuffer.String(), "\n")
 	const key = "count_objects"
-	for _, msg := range msgs {
-		if strings.Contains(msg, key) {
-			var out map[string]interface{}
-			require.NoError(t, json.NewDecoder(strings.NewReader(msg)).Decode(&out))
-			require.Contains(t, out, key, "there is no any information about statistics")
-			countObjects := out[key].(map[string]interface{})
-			assert.Contains(t, countObjects, "count")
+	for _, logEntry := range hook.AllEntries() {
+		if stats, ok := logEntry.Data[key]; ok {
+			require.IsType(t, map[string]interface{}{}, stats)
+
+			var keys []string
+			for key := range stats.(map[string]interface{}) {
+				keys = append(keys, key)
+			}
+
+			require.ElementsMatch(t, []string{
+				"count",
+				"garbage",
+				"in-pack",
+				"packs",
+				"prune-packable",
+				"size",
+				"size-garbage",
+				"size-pack",
+			}, keys)
 			return
 		}
 	}