Automatic merge of gitlab-org/gitaly master

23 files changed, 569 insertions, 321 deletions

diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go
index db33fbab3..5e6b9e6f8 100644
--- a/cmd/gitaly-ssh/auth_test.go
+++ b/cmd/gitaly-ssh/auth_test.go
@@ -4,26 +4,15 @@ package main
 
 import (
 	"fmt"
-	"net"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v15/client"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/backchannel"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/cache"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/server"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -141,45 +130,3 @@ func TestConnectivity(t *testing.T) {
 		})
 	}
 }
-
-func runServer(t *testing.T, secure bool, cfg config.Cfg, connectionType string, addr string) (int, func()) {
-	conns := client.NewPool()
-	locator := config.NewLocator(cfg)
-	registry := backchannel.NewRegistry()
-	txManager := transaction.NewManager(cfg, registry)
-	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
-	hookManager := hook.NewManager(cfg, locator, gitCmdFactory, txManager, gitlab.NewMockClient(
-		t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
-	))
-	limitHandler := limithandler.New(cfg, limithandler.LimitConcurrencyByRepo, limithandler.WithConcurrencyLimiters)
-	diskCache := cache.New(cfg, locator)
-	srv, err := server.New(secure, cfg, testhelper.NewDiscardingLogEntry(t), registry, diskCache, []*limithandler.LimiterMiddleware{limitHandler})
-	require.NoError(t, err)
-	setup.RegisterAll(srv, &service.Dependencies{
-		Cfg:                cfg,
-		GitalyHookManager:  hookManager,
-		TransactionManager: txManager,
-		StorageLocator:     locator,
-		ClientPool:         conns,
-		GitCmdFactory:      gitCmdFactory,
-	})
-
-	listener, err := net.Listen(connectionType, addr)
-	require.NoError(t, err)
-
-	go testhelper.MustServe(t, srv, listener)
-
-	port := 0
-	if connectionType != "unix" {
-		addrSplit := strings.Split(listener.Addr().String(), ":")
-		portString := addrSplit[len(addrSplit)-1]
-
-		port, err = strconv.Atoi(portString)
-		require.NoError(t, err)
-	}
-
-	return port, func() {
-		conns.Close()
-		srv.Stop()
-	}
-}
diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go
index 844e000b1..efcf881d5 100644
--- a/cmd/gitaly-ssh/upload_pack_test.go
+++ b/cmd/gitaly-ssh/upload_pack_test.go
@@ -5,6 +5,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -12,8 +13,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/protobuf/encoding/protojson"
 )
@@ -21,19 +24,15 @@ import (
 const keepAroundNamespace = "refs/keep-around"
 
 func TestVisibilityOfHiddenRefs(t *testing.T) {
+	t.Parallel()
+
 	ctx := testhelper.Context(t)
 
 	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalySSH(t, cfg)
 	testcfg.BuildGitalyHooks(t, cfg)
 
-	socketPath := testhelper.GetTemporaryGitalySocketFileName(t)
-
-	_, clean := runServer(t, false, cfg, "unix", socketPath)
-	defer clean()
-
-	_, clean = runServer(t, false, cfg, "unix", cfg.InternalSocketPath())
-	defer clean()
+	address := testserver.RunGitalyServer(t, cfg, nil, setup.RegisterAll, testserver.WithDisablePraefect())
 
 	// Create a keep-around ref
 	existingSha := git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312")
@@ -84,7 +83,7 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 			stdout := gittest.ExecOpts(t, cfg, gittest.ExecConfig{
 				Env: []string{
 					fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
-					fmt.Sprintf("GITALY_ADDRESS=unix:%s", socketPath),
+					fmt.Sprintf("GITALY_ADDRESS=unix:%s", strings.TrimPrefix(address, "unix://")),
 					fmt.Sprintf("GITALY_WD=%s", wd),
 					fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")),
 					fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", cfg.BinaryPath("gitaly-ssh")),
diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go
index aa8e64f50..b0322c6f6 100644
--- a/internal/gitaly/server/auth_test.go
+++ b/internal/gitaly/server/auth_test.go
@@ -206,7 +206,7 @@ func runServer(t *testing.T, cfg config.Cfg) string {
 	limitHandler := limithandler.New(cfg, limithandler.LimitConcurrencyByRepo, limithandler.WithConcurrencyLimiters)
 	updaterWithHooks := updateref.NewUpdaterWithHooks(cfg, locator, hookManager, gitCmdFactory, catfileCache)
 
-	srv, err := New(false, cfg, testhelper.NewDiscardingLogEntry(t), registry, diskCache, []*limithandler.LimiterMiddleware{limitHandler})
+	srv, err := NewGitalyServerFactory(cfg, testhelper.NewDiscardingLogEntry(t), registry, diskCache, []*limithandler.LimiterMiddleware{limitHandler}).New(false)
 	require.NoError(t, err)
 
 	setup.RegisterAll(srv, &service.Dependencies{
@@ -241,14 +241,13 @@ func runSecureServer(t *testing.T, cfg config.Cfg) string {
 	conns := client.NewPool()
 	t.Cleanup(func() { conns.Close() })
 
-	srv, err := New(
-		true,
+	srv, err := NewGitalyServerFactory(
 		cfg,
 		testhelper.NewDiscardingLogEntry(t),
 		backchannel.NewRegistry(),
 		cache.New(cfg, config.NewLocator(cfg)),
 		[]*limithandler.LimiterMiddleware{limithandler.New(cfg, limithandler.LimitConcurrencyByRepo, limithandler.WithConcurrencyLimiters)},
-	)
+	).New(true)
 	require.NoError(t, err)
 
 	healthpb.RegisterHealthServer(srv, health.NewServer())
diff --git a/internal/gitaly/server/server.go b/internal/gitaly/server/server.go
index fe1b87891..94f11279b 100644
--- a/internal/gitaly/server/server.go
+++ b/internal/gitaly/server/server.go
@@ -9,11 +9,8 @@ import (
 	grpcmwlogrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	grpcmwtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
 	grpcprometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
-	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/backchannel"
-	diskcache "gitlab.com/gitlab-org/gitaly/v15/internal/cache"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/client"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/server/auth"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/grpcstats"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/fieldextractors"
@@ -23,7 +20,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/cache"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/commandstatshandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/featureflag"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/metadatahandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/panichandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/sentryhandler"
@@ -53,16 +49,35 @@ func init() {
 	grpcmwlogrus.ReplaceGrpcLogger(gitalylog.GrpcGo())
 }
 
+type serverConfig struct {
+	unaryInterceptors  []grpc.UnaryServerInterceptor
+	streamInterceptors []grpc.StreamServerInterceptor
+}
+
+// Option is an option that can be passed to `New()`.
+type Option func(*serverConfig)
+
+// WithUnaryInterceptor adds another interceptor that shall be executed for unary RPC calls.
+func WithUnaryInterceptor(interceptor grpc.UnaryServerInterceptor) Option {
+	return func(cfg *serverConfig) {
+		cfg.unaryInterceptors = append(cfg.unaryInterceptors, interceptor)
+	}
+}
+
+// WithStreamInterceptor adds another interceptor that shall be executed for streaming RPC calls.
+func WithStreamInterceptor(interceptor grpc.StreamServerInterceptor) Option {
+	return func(cfg *serverConfig) {
+		cfg.streamInterceptors = append(cfg.streamInterceptors, interceptor)
+	}
+}
+
 // New returns a GRPC server instance with a set of interceptors configured.
-// If logrusEntry is nil the default logger will be used.
-func New(
-	secure bool,
-	cfg config.Cfg,
-	logrusEntry *log.Entry,
-	registry *backchannel.Registry,
-	cacheInvalidator diskcache.Invalidator,
-	limitHandlers []*limithandler.LimiterMiddleware,
-) (*grpc.Server, error) {
+func (s *GitalyServerFactory) New(secure bool, opts ...Option) (*grpc.Server, error) {
+	var cfg serverConfig
+	for _, opt := range opts {
+		opt(&cfg)
+	}
+
 	ctxTagOpts := []grpcmwtags.Option{
 		grpcmwtags.WithFieldExtractorForInitialReq(fieldextractors.FieldExtractor),
 	}
@@ -71,7 +86,7 @@ func New(
 	// If tls config is specified attempt to extract tls options and use it
 	// as a grpc.ServerOption
 	if secure {
-		cert, err := tls.LoadX509KeyPair(cfg.TLS.CertPath, cfg.TLS.KeyPath)
+		cert, err := tls.LoadX509KeyPair(s.cfg.TLS.CertPath, s.cfg.TLS.KeyPath)
 		if err != nil {
 			return nil, fmt.Errorf("error reading certificate and key paths: %v", err)
 		}
@@ -84,8 +99,8 @@ func New(
 
 	lm := listenmux.New(transportCredentials)
 	lm.Register(backchannel.NewServerHandshaker(
-		logrusEntry,
-		registry,
+		s.logger,
+		s.registry,
 		[]grpc.DialOption{client.UnaryInterceptor()},
 	))
 
@@ -105,7 +120,7 @@ func New(
 		metadatahandler.StreamInterceptor,
 		grpcprometheus.StreamServerInterceptor,
 		commandstatshandler.StreamInterceptor,
-		grpcmwlogrus.StreamServerInterceptor(logrusEntry,
+		grpcmwlogrus.StreamServerInterceptor(s.logger,
 			grpcmwlogrus.WithTimestampFormat(gitalylog.LogTimestampFormat),
 			logMsgProducer,
 			gitalylog.DeciderOption(),
@@ -113,7 +128,7 @@ func New(
 		gitalylog.StreamLogDataCatcherServerInterceptor(),
 		sentryhandler.StreamLogHandler,
 		statushandler.Stream, // Should be below LogHandler
-		auth.StreamServerInterceptor(cfg.Auth),
+		auth.StreamServerInterceptor(s.cfg.Auth),
 	}
 	unaryServerInterceptors := []grpc.UnaryServerInterceptor{
 		grpcmwtags.UnaryServerInterceptor(ctxTagOpts...),
@@ -121,7 +136,7 @@ func New(
 		metadatahandler.UnaryInterceptor,
 		grpcprometheus.UnaryServerInterceptor,
 		commandstatshandler.UnaryInterceptor,
-		grpcmwlogrus.UnaryServerInterceptor(logrusEntry,
+		grpcmwlogrus.UnaryServerInterceptor(s.logger,
 			grpcmwlogrus.WithTimestampFormat(gitalylog.LogTimestampFormat),
 			logMsgProducer,
 			gitalylog.DeciderOption(),
@@ -129,30 +144,33 @@ func New(
 		gitalylog.UnaryLogDataCatcherServerInterceptor(),
 		sentryhandler.UnaryLogHandler,
 		statushandler.Unary, // Should be below LogHandler
-		auth.UnaryServerInterceptor(cfg.Auth),
+		auth.UnaryServerInterceptor(s.cfg.Auth),
 	}
 	// Should be below auth handler to prevent v2 hmac tokens from timing out while queued
-	for _, limitHandler := range limitHandlers {
+	for _, limitHandler := range s.limitHandlers {
 		streamServerInterceptors = append(streamServerInterceptors, limitHandler.StreamInterceptor())
 		unaryServerInterceptors = append(unaryServerInterceptors, limitHandler.UnaryInterceptor())
 	}
 
 	streamServerInterceptors = append(streamServerInterceptors,
 		grpctracing.StreamServerTracingInterceptor(),
-		cache.StreamInvalidator(cacheInvalidator, protoregistry.GitalyProtoPreregistered),
+		cache.StreamInvalidator(s.cacheInvalidator, protoregistry.GitalyProtoPreregistered),
 		// Panic handler should remain last so that application panics will be
 		// converted to errors and logged
 		panichandler.StreamPanicHandler,
 	)
 
 	unaryServerInterceptors = append(unaryServerInterceptors,
-		cache.UnaryInvalidator(cacheInvalidator, protoregistry.GitalyProtoPreregistered),
+		cache.UnaryInvalidator(s.cacheInvalidator, protoregistry.GitalyProtoPreregistered),
 		// Panic handler should remain last so that application panics will be
 		// converted to errors and logged
 		panichandler.UnaryPanicHandler,
 	)
 
-	opts := []grpc.ServerOption{
+	streamServerInterceptors = append(streamServerInterceptors, cfg.streamInterceptors...)
+	unaryServerInterceptors = append(unaryServerInterceptors, cfg.unaryInterceptors...)
+
+	serverOptions := []grpc.ServerOption{
 		grpc.StatsHandler(gitalylog.PerRPCLogHandler{
 			Underlying:     &grpcstats.PayloadBytes{},
 			FieldProducers: []gitalylog.FieldsProducer{grpcstats.FieldsProducer},
@@ -173,5 +191,5 @@ func New(
 		}),
 	}
 
-	return grpc.NewServer(opts...), nil
+	return grpc.NewServer(serverOptions...), nil
 }
diff --git a/internal/gitaly/server/server_factory.go b/internal/gitaly/server/server_factory.go
index 2a88ba4ba..f9de03235 100644
--- a/internal/gitaly/server/server_factory.go
+++ b/internal/gitaly/server/server_factory.go
@@ -77,15 +77,8 @@ func (s *GitalyServerFactory) GracefulStop() {
 
 // CreateExternal creates a new external gRPC server. The external servers are closed
 // before the internal servers when gracefully shutting down.
-func (s *GitalyServerFactory) CreateExternal(secure bool) (*grpc.Server, error) {
-	server, err := New(
-		secure,
-		s.cfg,
-		s.logger,
-		s.registry,
-		s.cacheInvalidator,
-		s.limitHandlers,
-	)
+func (s *GitalyServerFactory) CreateExternal(secure bool, opts ...Option) (*grpc.Server, error) {
+	server, err := s.New(secure, opts...)
 	if err != nil {
 		return nil, err
 	}
@@ -96,14 +89,8 @@ func (s *GitalyServerFactory) CreateExternal(secure bool) (*grpc.Server, error)
 
 // CreateInternal creates a new internal gRPC server. Internal servers are closed
 // after the external ones when gracefully shutting down.
-func (s *GitalyServerFactory) CreateInternal() (*grpc.Server, error) {
-	server, err := New(
-		false,
-		s.cfg,
-		s.logger,
-		s.registry,
-		s.cacheInvalidator,
-		s.limitHandlers)
+func (s *GitalyServerFactory) CreateInternal(opts ...Option) (*grpc.Server, error) {
+	server, err := s.New(false, opts...)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitaly/service/commit/check_objects_exist_test.go b/internal/gitaly/service/commit/check_objects_exist_test.go
index ab6d0383e..09d41d07b 100644
--- a/internal/gitaly/service/commit/check_objects_exist_test.go
+++ b/internal/gitaly/service/commit/check_objects_exist_test.go
@@ -7,12 +7,10 @@ import (
 	"io"
 	"testing"
 
-	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -22,8 +20,7 @@ func TestCheckObjectsExist(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	logger, hook := test.NewNullLogger()
-	cfg, client := setupCommitService(t, ctx, testserver.WithLogger(logger))
+	cfg, client := setupCommitService(t, ctx)
 
 	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
@@ -38,11 +35,10 @@ func TestCheckObjectsExist(t *testing.T) {
 	)
 
 	for _, tc := range []struct {
-		desc                  string
-		requests              []*gitalypb.CheckObjectsExistRequest
-		expectedResults       map[string]bool
-		expectedErr           error
-		expectedErrorMetadata any
+		desc            string
+		requests        []*gitalypb.CheckObjectsExistRequest
+		expectedResults map[string]bool
+		expectedErr     error
 	}{
 		{
 			desc:     "no repository provided",
@@ -170,10 +166,8 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'"),
-			expectedErrorMetadata: map[string]any{
-				"revision": "-not-a-rev",
-			},
+			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'").
+				WithInterceptedMetadata("revision", "-not-a-rev"),
 		},
 		{
 			desc: "input with whitespace",
@@ -185,10 +179,8 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't contain whitespace"),
-			expectedErrorMetadata: map[string]any{
-				"revision": fmt.Sprintf("%s\n%s", commitID1, commitID2),
-			},
+			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't contain whitespace").
+				WithInterceptedMetadata("revision", fmt.Sprintf("%s\n%s", commitID1, commitID2)),
 		},
 		{
 			desc: "chunked invalid input",
@@ -205,15 +197,11 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'"),
-			expectedErrorMetadata: map[string]any{
-				"revision": "-not-a-rev",
-			},
+			expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'").
+				WithInterceptedMetadata("revision", "-not-a-rev"),
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			hook.Reset()
-
 			client, err := client.CheckObjectsExist(ctx)
 			require.NoError(t, err)
 
@@ -244,23 +232,6 @@ func TestCheckObjectsExist(t *testing.T) {
 
 			testhelper.RequireGrpcError(t, tc.expectedErr, err)
 			require.Equal(t, tc.expectedResults, results)
-
-			if tc.expectedErrorMetadata == nil {
-				tc.expectedErrorMetadata = map[string]any{}
-			}
-
-			metadata := map[string]any{}
-			for _, entry := range hook.AllEntries() {
-				errorMetadata, ok := entry.Data["error_metadata"]
-				if !ok {
-					continue
-				}
-
-				for key, value := range errorMetadata.(map[string]any) {
-					metadata[key] = value
-				}
-			}
-			require.Equal(t, tc.expectedErrorMetadata, metadata)
 		})
 	}
 }
diff --git a/internal/gitaly/service/commit/tree_entry_test.go b/internal/gitaly/service/commit/tree_entry_test.go
index 781f1cd1b..0077201b5 100644
--- a/internal/gitaly/service/commit/tree_entry_test.go
+++ b/internal/gitaly/service/commit/tree_entry_test.go
@@ -214,17 +214,17 @@ func TestFailedTreeEntry(t *testing.T) {
 		{
 			name:        "Path is outside of repository",
 			req:         &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("913c66a37b4a45b9769037c55c2d238bd0942d2e"), Path: []byte("../bar/.gitkeep")}, // Git blows up on paths like this
-			expectedErr: structerr.NewNotFound("tree entry not found"),
+			expectedErr: structerr.NewNotFound("tree entry not found").WithInterceptedMetadata("path", "../bar/.gitkeep"),
 		},
 		{
 			name:        "Missing file with space in path",
 			req:         &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("deadfacedeadfacedeadfacedeadfacedeadface"), Path: []byte("with space/README.md")},
-			expectedErr: structerr.NewNotFound("tree entry not found"),
+			expectedErr: structerr.NewNotFound("tree entry not found").WithInterceptedMetadata("path", "with space/README.md"),
 		},
 		{
 			name:        "Missing file",
 			req:         &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"), Path: []byte("missing.rb")},
-			expectedErr: structerr.NewNotFound("tree entry not found"),
+			expectedErr: structerr.NewNotFound("tree entry not found").WithInterceptedMetadata("path", "missing.rb"),
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/internal/gitaly/service/diff/patch_id_test.go b/internal/gitaly/service/diff/patch_id_test.go
index ef12951c6..aefee8d6b 100644
--- a/internal/gitaly/service/diff/patch_id_test.go
+++ b/internal/gitaly/service/diff/patch_id_test.go
@@ -21,15 +21,19 @@ func TestGetPatchID(t *testing.T) {
 	gitVersion, err := gittest.NewCommandFactory(t, cfg).GitVersion(ctx)
 	require.NoError(t, err)
 
-	testCases := []struct {
-		desc             string
-		setup            func(t *testing.T) *gitalypb.GetPatchIDRequest
+	type setupData struct {
+		request          *gitalypb.GetPatchIDRequest
 		expectedResponse *gitalypb.GetPatchIDResponse
 		expectedErr      error
+	}
+
+	testCases := []struct {
+		desc  string
+		setup func(t *testing.T) setupData
 	}{
 		{
 			desc: "retruns patch-id successfully",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath,
@@ -45,19 +49,21 @@ func TestGetPatchID(t *testing.T) {
 					),
 				)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte("main~"),
-					NewRevision: []byte("main"),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte("main~"),
+						NewRevision: []byte("main"),
+					},
+					expectedResponse: &gitalypb.GetPatchIDResponse{
+						PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
+					},
 				}
 			},
-			expectedResponse: &gitalypb.GetPatchIDResponse{
-				PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
-			},
 		},
 		{
 			desc: "returns patch-id successfully with commit ids",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath,
@@ -71,19 +77,21 @@ func TestGetPatchID(t *testing.T) {
 					),
 				)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(oldCommit),
-					NewRevision: []byte(newCommit),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(oldCommit),
+						NewRevision: []byte(newCommit),
+					},
+					expectedResponse: &gitalypb.GetPatchIDResponse{
+						PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
+					},
 				}
 			},
-			expectedResponse: &gitalypb.GetPatchIDResponse{
-				PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
-			},
 		},
 		{
 			desc: "returns patch-id successfully for a specific file",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath,
@@ -97,19 +105,21 @@ func TestGetPatchID(t *testing.T) {
 					),
 				)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(fmt.Sprintf("%s:file", oldCommit)),
-					NewRevision: []byte(fmt.Sprintf("%s:file", newCommit)),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(fmt.Sprintf("%s:file", oldCommit)),
+						NewRevision: []byte(fmt.Sprintf("%s:file", newCommit)),
+					},
+					expectedResponse: &gitalypb.GetPatchIDResponse{
+						PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
+					},
 				}
 			},
-			expectedResponse: &gitalypb.GetPatchIDResponse{
-				PatchId: "a79c7e9df0094ee44fa7a2a9ae27e914e6b7e00b",
-			},
 		},
 		{
 			desc: "returns patch-id with binary file",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath,
@@ -123,33 +133,35 @@ func TestGetPatchID(t *testing.T) {
 					),
 				)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(oldCommit),
-					NewRevision: []byte(newCommit),
-				}
-			},
-			expectedResponse: &gitalypb.GetPatchIDResponse{
-				PatchId: func() string {
-					// Before Git v2.39.0, git-patch-id(1) would skip over any
-					// lines that have an "index " prefix. This causes issues
-					// with diffs of binaries though: we don't generate the diff
-					// with `--binary`, so the "index" line that contains the
-					// pre- and post-image blob IDs of the binary is the only
-					// bit of information we have that something changed. But
-					// because Git used to skip over it we wouldn't actually
-					// take into account the contents of the changed blob at
-					// all.
-					//
-					// This was fixed in Git v2.39.0 so that "index" lines will
-					// now be hashed to correctly account for binary changes. As
-					// a result, the patch ID has changed.
-					if gitVersion.PatchIDRespectsBinaries() {
-						return "13e4e9b9cd44ec511bac24fdbdeab9b74ba3000b"
-					}
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(oldCommit),
+						NewRevision: []byte(newCommit),
+					},
+					expectedResponse: &gitalypb.GetPatchIDResponse{
+						PatchId: func() string {
+							// Before Git v2.39.0, git-patch-id(1) would skip over any
+							// lines that have an "index " prefix. This causes issues
+							// with diffs of binaries though: we don't generate the diff
+							// with `--binary`, so the "index" line that contains the
+							// pre- and post-image blob IDs of the binary is the only
+							// bit of information we have that something changed. But
+							// because Git used to skip over it we wouldn't actually
+							// take into account the contents of the changed blob at
+							// all.
+							//
+							// This was fixed in Git v2.39.0 so that "index" lines will
+							// now be hashed to correctly account for binary changes. As
+							// a result, the patch ID has changed.
+							if gitVersion.PatchIDRespectsBinaries() {
+								return "13e4e9b9cd44ec511bac24fdbdeab9b74ba3000b"
+							}
 
-					return "715883c1b90a5b4450072e22fefec769ad346266"
-				}(),
+							return "715883c1b90a5b4450072e22fefec769ad346266"
+						}(),
+					},
+				}
 			},
 		},
 		{
@@ -157,7 +169,7 @@ func TestGetPatchID(t *testing.T) {
 			// with different binary contents. This is done to ensure that we indeed
 			// generate different patch IDs as expected.
 			desc: "different binary diff has different patch ID",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath,
@@ -171,31 +183,33 @@ func TestGetPatchID(t *testing.T) {
 					),
 				)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(oldCommit),
-					NewRevision: []byte(newCommit),
-				}
-			},
-			expectedResponse: &gitalypb.GetPatchIDResponse{
-				PatchId: func() string {
-					if gitVersion.PatchIDRespectsBinaries() {
-						// When respecting binary diffs we indeed have a
-						// different patch ID compared to the preceding
-						// testcase.
-						return "f678855867b112ac2c5466260b3b3a5e75fca875"
-					}
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(oldCommit),
+						NewRevision: []byte(newCommit),
+					},
+					expectedResponse: &gitalypb.GetPatchIDResponse{
+						PatchId: func() string {
+							if gitVersion.PatchIDRespectsBinaries() {
+								// When respecting binary diffs we indeed have a
+								// different patch ID compared to the preceding
+								// testcase.
+								return "f678855867b112ac2c5466260b3b3a5e75fca875"
+							}
 
-					// But when git-patch-id(1) is not paying respect to binary
-					// diffs we incorrectly return the same patch ID. This is
-					// nothing we can easily fix though.
-					return "715883c1b90a5b4450072e22fefec769ad346266"
-				}(),
+							// But when git-patch-id(1) is not paying respect to binary
+							// diffs we incorrectly return the same patch ID. This is
+							// nothing we can easily fix though.
+							return "715883c1b90a5b4450072e22fefec769ad346266"
+						}(),
+					},
+				}
 			},
 		},
 		{
 			desc: "file didn't exist in the old revision",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				oldCommit := gittest.WriteCommit(t, cfg, repoPath)
@@ -203,81 +217,95 @@ func TestGetPatchID(t *testing.T) {
 					gittest.TreeEntry{Path: "file", Mode: "100644", Content: "new"},
 				))
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(fmt.Sprintf("%s:file", oldCommit)),
-					NewRevision: []byte(fmt.Sprintf("%s:file", newCommit)),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(fmt.Sprintf("%s:file", oldCommit)),
+						NewRevision: []byte(fmt.Sprintf("%s:file", newCommit)),
+					},
+					expectedErr: structerr.New("waiting for git-diff: exit status 128").
+						WithInterceptedMetadata("stderr", fmt.Sprintf("fatal: path 'file' does not exist in '%s'\n", oldCommit)),
 				}
 			},
-			expectedErr: structerr.New("waiting for git-diff: exit status 128"),
 		},
 		{
 			desc: "unknown revisions",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, _ := gittest.CreateRepository(t, ctx, cfg)
 
 				newRevision := strings.Replace(string(gittest.DefaultObjectHash.ZeroOID), "0", "1", -1)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(gittest.DefaultObjectHash.ZeroOID),
-					NewRevision: []byte(newRevision),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(gittest.DefaultObjectHash.ZeroOID),
+						NewRevision: []byte(newRevision),
+					},
+					expectedErr: structerr.New("waiting for git-diff: exit status 128").
+						WithInterceptedMetadata("stderr", fmt.Sprintf("fatal: bad object %s\n", gittest.DefaultObjectHash.ZeroOID)),
 				}
 			},
-			expectedErr: structerr.New("waiting for git-diff: exit status 128").WithMetadata("stderr", ""),
 		},
 		{
 			desc: "no diff from the given revisions",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
 				commit := gittest.WriteCommit(t, cfg, repoPath)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte(commit),
-					NewRevision: []byte(commit),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte(commit),
+						NewRevision: []byte(commit),
+					},
+					expectedErr: structerr.NewFailedPrecondition("no difference between old and new revision"),
 				}
 			},
-			expectedErr: structerr.NewFailedPrecondition("no difference between old and new revision"),
 		},
 		{
 			desc: "empty repository",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  nil,
-					OldRevision: []byte("HEAD~1"),
-					NewRevision: []byte("HEAD"),
+			setup: func(t *testing.T) setupData {
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  nil,
+						OldRevision: []byte("HEAD~1"),
+						NewRevision: []byte("HEAD"),
+					},
+					expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect(
+						"empty Repository",
+						"repo scoped: empty Repository",
+					)),
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect(
-				"empty Repository",
-				"repo scoped: empty Repository",
-			)),
 		},
 		{
 			desc: "empty old revision",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, _ := gittest.CreateRepository(t, ctx, cfg)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					NewRevision: []byte("HEAD"),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						NewRevision: []byte("HEAD"),
+					},
+					expectedErr: structerr.NewInvalidArgument("empty OldRevision"),
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument("empty OldRevision"),
 		},
 		{
 			desc: "empty new revision",
-			setup: func(t *testing.T) *gitalypb.GetPatchIDRequest {
+			setup: func(t *testing.T) setupData {
 				repoProto, _ := gittest.CreateRepository(t, ctx, cfg)
 
-				return &gitalypb.GetPatchIDRequest{
-					Repository:  repoProto,
-					OldRevision: []byte("HEAD~1"),
+				return setupData{
+					request: &gitalypb.GetPatchIDRequest{
+						Repository:  repoProto,
+						OldRevision: []byte("HEAD~1"),
+					},
+					expectedErr: structerr.NewInvalidArgument("empty NewRevision"),
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument("empty NewRevision"),
 		},
 	}
 
@@ -286,11 +314,11 @@ func TestGetPatchID(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			t.Parallel()
 
-			request := tc.setup(t)
-			response, err := client.GetPatchID(ctx, request)
+			setupData := tc.setup(t)
+			response, err := client.GetPatchID(ctx, setupData.request)
 
-			testhelper.RequireGrpcError(t, tc.expectedErr, err)
-			testhelper.ProtoEqual(t, tc.expectedResponse, response)
+			testhelper.RequireGrpcError(t, setupData.expectedErr, err)
+			testhelper.ProtoEqual(t, setupData.expectedResponse, response)
 		})
 	}
 }
diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go
index b90b5e6d2..9948de9e0 100644
--- a/internal/gitaly/service/objectpool/create_test.go
+++ b/internal/gitaly/service/objectpool/create_test.go
@@ -113,6 +113,7 @@ func testCreateUnsuccessful(t *testing.T, ctx context.Context) {
 		Origin:     repo,
 	})
 	require.NoError(t, err)
+	preexistingPoolPath := filepath.Join(cfg.Storages[0].Path, preexistingPool.Repository.RelativePath)
 
 	for _, tc := range []struct {
 		desc        string
@@ -218,7 +219,9 @@ func testCreateUnsuccessful(t *testing.T, ctx context.Context) {
 				if featureflag.AtomicCreateObjectPool.IsEnabled(ctx) {
 					return structerr.NewFailedPrecondition("creating object pool: repository exists already")
 				}
-				return structerr.NewFailedPrecondition("creating object pool: target path exists already")
+
+				return structerr.NewFailedPrecondition("creating object pool: target path exists already").
+					WithInterceptedMetadata("object_pool_path", preexistingPoolPath)
 			}(),
 		},
 	} {
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index 73cbd5667..4c93de134 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -549,8 +549,9 @@ func TestUserDeleteBranch(t *testing.T) {
 						BranchName:     []byte(branchName),
 						ExpectedOldOid: "foobar",
 					},
-					repoPath:     repoPath,
-					expectedErr:  structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\""),
+					repoPath: repoPath,
+					expectedErr: structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\"").
+						WithInterceptedMetadata("old_object_id", "foobar"),
 					expectedRefs: []string{"master", branchName},
 				}
 			},
@@ -574,8 +575,9 @@ func TestUserDeleteBranch(t *testing.T) {
 						BranchName:     []byte(branchName),
 						ExpectedOldOid: gittest.DefaultObjectHash.ZeroOID.String(),
 					},
-					repoPath:     repoPath,
-					expectedErr:  structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+					repoPath: repoPath,
+					expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+						WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 					expectedRefs: []string{"master", branchName},
 				}
 			},
diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go
index 8d5b6b183..71cae333f 100644
--- a/internal/gitaly/service/operations/cherry_pick_test.go
+++ b/internal/gitaly/service/operations/cherry_pick_test.go
@@ -205,7 +205,8 @@ func TestUserCherryPick(t *testing.T) {
 					ExpectedOldOid: "foobar",
 				}, &gitalypb.UserCherryPickResponse{}
 			},
-			expectedErr: structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\""),
+			expectedErr: structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\"").
+				WithInterceptedMetadata("old_object_id", "foobar"),
 		},
 		{
 			desc: "valid but non-existent expectedOldOID",
@@ -221,7 +222,8 @@ func TestUserCherryPick(t *testing.T) {
 					ExpectedOldOid: gittest.DefaultObjectHash.ZeroOID.String(),
 				}, &gitalypb.UserCherryPickResponse{}
 			},
-			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+				WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 		},
 		{
 			desc: "incorrect expectedOldOID",
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 7c764ae77..2bff1f4db 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -1154,7 +1154,8 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
 			repoPath:       repoPath,
 			branchName:     "few-commits",
 			expectedOldOID: "foobar",
-			expectedError:  structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`),
+			expectedError: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`).
+				WithInterceptedMetadata("old_object_id", "foobar"),
 		},
 		{
 			desc:           "existing repo and branch + valid expectedOldOID but invalid object",
@@ -1162,7 +1163,8 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
 			repoPath:       repoPath,
 			branchName:     "few-commits",
 			expectedOldOID: gittest.DefaultObjectHash.ZeroOID.String(),
-			expectedError:  structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+			expectedError: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+				WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 		},
 		{
 			desc:           "existing repo and branch + incorrect expectedOldOID",
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 80a4b8f65..f799d0b11 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -135,7 +135,8 @@ func TestUserMergeBranch(t *testing.T) {
 						Message:        []byte(data.message),
 						ExpectedOldOid: "foobar",
 					},
-					firstExpectedErr: structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\""),
+					firstExpectedErr: structerr.NewInvalidArgument("invalid expected old object ID: invalid object ID: \"foobar\"").
+						WithInterceptedMetadata("old_object_id", "foobar"),
 				}
 			},
 		},
@@ -152,7 +153,8 @@ func TestUserMergeBranch(t *testing.T) {
 						Message:        []byte(data.message),
 						ExpectedOldOid: gittest.DefaultObjectHash.ZeroOID.String(),
 					},
-					firstExpectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+					firstExpectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+						WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 				}
 			},
 		},
@@ -1254,7 +1256,8 @@ func TestUserFFBranch(t *testing.T) {
 					},
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`),
+			expectedErr: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`).
+				WithInterceptedMetadata("old_object_id", "foobar"),
 		},
 		{
 			desc: "valid SHA, but not existing expectedOldOID",
@@ -1275,7 +1278,8 @@ func TestUserFFBranch(t *testing.T) {
 					},
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+				WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 		},
 		{
 			desc: "expectedOldOID pointing to old commit",
diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go
index 088739913..78e100b59 100644
--- a/internal/gitaly/service/operations/revert_test.go
+++ b/internal/gitaly/service/operations/revert_test.go
@@ -320,7 +320,8 @@ func TestUserRevert(t *testing.T) {
 					},
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`),
+			expectedErr: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "foobar"`).
+				WithInterceptedMetadata("old_object_id", "foobar"),
 		},
 		{
 			desc: "expectedOldOID with valid SHA, but not present in repo",
@@ -342,7 +343,8 @@ func TestUserRevert(t *testing.T) {
 					},
 				}
 			},
-			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+				WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 		},
 		{
 			desc: "expectedOldOID pointing to old commit",
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index b2aac01dd..7dee225ed 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -190,7 +190,8 @@ func TestUserDeleteTag(t *testing.T) {
 					ExpectedOldOid: "io",
 				}
 			},
-			expectedErr:  structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "io"`),
+			expectedErr: structerr.NewInvalidArgument(`invalid expected old object ID: invalid object ID: "io"`).
+				WithInterceptedMetadata("old_object_id", "io"),
 			expectedTags: []string{"europa"},
 		},
 		{
@@ -207,7 +208,8 @@ func TestUserDeleteTag(t *testing.T) {
 					ExpectedOldOid: gittest.DefaultObjectHash.ZeroOID.String(),
 				}
 			},
-			expectedErr:  structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found"),
+			expectedErr: structerr.NewInvalidArgument("cannot resolve expected old object ID: reference not found").
+				WithInterceptedMetadata("old_object_id", gittest.DefaultObjectHash.ZeroOID),
 			expectedTags: []string{"europa"},
 		},
 		{
diff --git a/internal/structerr/error.go b/internal/structerr/error.go
index 7cff2bb0d..fdaf132df 100644
--- a/internal/structerr/error.go
+++ b/internal/structerr/error.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 
+	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb/testproto"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
@@ -288,6 +289,17 @@ func (e Error) WithMetadata(key string, value any) Error {
 	return e
 }
 
+// WithInterceptedMetadata adds an additional metadata item to the Error in the form of an error
+// detail. Note that this is only intended to be used in the context of tests where we convert error
+// metadata into structured errors via the UnaryInterceptor and StreamInterceptor so that we can
+// test that metadata has been set as expected on the client-side of a gRPC call.
+func (e Error) WithInterceptedMetadata(key string, value any) Error {
+	return e.WithDetail(&testproto.ErrorMetadata{
+		Key:   []byte(key),
+		Value: []byte(fmt.Sprintf("%v", value)),
+	})
+}
+
 // Details returns the chain error details set by this and any wrapped Error. The returned array
 // contains error details ordered from top-level error details to bottom-level error details.
 func (e Error) Details() []proto.Message {
diff --git a/internal/structerr/fields_producer.go b/internal/structerr/fields_producer.go
deleted file mode 100644
index 367f149e7..000000000
--- a/internal/structerr/fields_producer.go
+++ /dev/null
@@ -1,26 +0,0 @@
-package structerr
-
-import (
-	"context"
-	"errors"
-
-	"github.com/sirupsen/logrus"
-)
-
-// FieldsProducer extracts metadata from err if it contains a `structerr.Error` and exposes it as
-// logged fields. This function is supposed to be used with `log.MessageProducer()`.
-func FieldsProducer(_ context.Context, err error) logrus.Fields {
-	var structErr Error
-	if errors.As(err, &structErr) {
-		metadata := structErr.Metadata()
-		if len(metadata) == 0 {
-			return nil
-		}
-
-		return logrus.Fields{
-			"error_metadata": metadata,
-		}
-	}
-
-	return nil
-}
diff --git a/internal/structerr/grpc_server.go b/internal/structerr/grpc_server.go
new file mode 100644
index 000000000..dec2d44d2
--- /dev/null
+++ b/internal/structerr/grpc_server.go
@@ -0,0 +1,70 @@
+package structerr
+
+import (
+	"context"
+	"errors"
+	"sort"
+
+	"github.com/sirupsen/logrus"
+	"google.golang.org/grpc"
+)
+
+// UnaryInterceptor is an interceptor for unary RPC calls that injects error metadata as detailed
+// error. This is only supposed to be used for testing purposes as error metadata is considered to
+// be a server-side detail. No clients should start to rely on it.
+func UnaryInterceptor(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
+	response, err := handler(ctx, req)
+	return response, interceptedError(err)
+}
+
+// StreamInterceptor is an interceptor for streaming RPC calls that injects error metadata as
+// detailed error. This is only supposed to be used for testing purposes as error metadata is
+// considered to be a server-side detail. No clients should start to rely on it.
+func StreamInterceptor(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
+	return interceptedError(handler(srv, stream))
+}
+
+func interceptedError(err error) error {
+	var structErr Error
+	if errors.As(err, &structErr) {
+		metadata := structErr.Metadata()
+		if len(metadata) == 0 {
+			return err
+		}
+
+		keys := make([]string, 0, len(metadata))
+		for key := range metadata {
+			keys = append(keys, key)
+		}
+		sort.Strings(keys)
+
+		// We need to wrap the top-level error with the expected intercepted metadata, or
+		// otherwise we lose error context.
+		errWithMetadata := New("%w", err)
+		for _, key := range keys {
+			errWithMetadata = errWithMetadata.WithInterceptedMetadata(key, metadata[key])
+		}
+
+		return errWithMetadata
+	}
+
+	return err
+}
+
+// FieldsProducer extracts metadata from err if it contains a `structerr.Error` and exposes it as
+// logged fields. This function is supposed to be used with `log.MessageProducer()`.
+func FieldsProducer(_ context.Context, err error) logrus.Fields {
+	var structErr Error
+	if errors.As(err, &structErr) {
+		metadata := structErr.Metadata()
+		if len(metadata) == 0 {
+			return nil
+		}
+
+		return logrus.Fields{
+			"error_metadata": metadata,
+		}
+	}
+
+	return nil
+}
diff --git a/internal/structerr/fields_producer_test.go b/internal/structerr/grpc_server_test.go
index 27bde9a32..b9c48d6b9 100644
--- a/internal/structerr/fields_producer_test.go
+++ b/internal/structerr/grpc_server_test.go
@@ -3,6 +3,7 @@ package structerr
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"testing"
 
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb/testproto"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials/insecure"
@@ -20,6 +22,57 @@ import (
 	"google.golang.org/grpc/test/grpc_testing"
 )
 
+func TestInterceptedError(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		desc        string
+		err         error
+		expectedErr error
+	}{
+		{
+			desc:        "normal error",
+			err:         fmt.Errorf("test"),
+			expectedErr: fmt.Errorf("test"),
+		},
+		{
+			desc:        "structured error",
+			err:         NewNotFound("not found"),
+			expectedErr: NewNotFound("not found"),
+		},
+		{
+			desc:        "wrapped structured error",
+			err:         fmt.Errorf("wrapped: %w", NewNotFound("not found")),
+			expectedErr: fmt.Errorf("wrapped: %w", NewNotFound("not found")),
+		},
+		{
+			desc: "metadata",
+			err:  NewNotFound("not found").WithMetadata("key", "value"),
+			expectedErr: NewNotFound("not found").WithDetail(
+				&testproto.ErrorMetadata{
+					Key:   []byte("key"),
+					Value: []byte("value"),
+				},
+			),
+		},
+		{
+			desc: "wrapped error with metadata",
+			err:  fmt.Errorf("wrapped: %w", NewNotFound("not found").WithMetadata("key", "value")),
+			expectedErr: NewNotFound("wrapped: not found").WithDetail(
+				&testproto.ErrorMetadata{
+					Key:   []byte("key"),
+					Value: []byte("value"),
+				},
+			),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := interceptedError(tc.err)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+		})
+	}
+}
+
 type mockService struct {
 	grpc_testing.UnimplementedTestServiceServer
 	err error
diff --git a/internal/structerr/testhelper_test.go b/internal/structerr/testhelper_test.go
deleted file mode 100644
index b9934fecc..000000000
--- a/internal/structerr/testhelper_test.go
+++ /dev/null
@@ -1,11 +0,0 @@
-package structerr
-
-import (
-	"testing"
-
-	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
-)
-
-func TestMain(m *testing.M) {
-	testhelper.Run(m)
-}
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index 6401cd25b..d2a377710 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -33,6 +33,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	praefectconfig "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/streamcache"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testdb"
 	"google.golang.org/grpc"
@@ -153,6 +154,13 @@ func runGitaly(tb testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, reg
 		gsd = opt(gsd)
 	}
 
+	// We set up the structerr interceptors so that any error metadata that gets set via
+	// `structerr.WithMetadata()` is not only logged, but also present in the error details.
+	serverOpts := []server.Option{
+		server.WithUnaryInterceptor(structerr.UnaryInterceptor),
+		server.WithStreamInterceptor(structerr.StreamInterceptor),
+	}
+
 	deps := gsd.createDependencies(tb, cfg, rubyServer)
 	tb.Cleanup(func() { gsd.conns.Close() })
 
@@ -165,7 +173,7 @@ func runGitaly(tb testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, reg
 	)
 
 	if cfg.RuntimeDir != "" {
-		internalServer, err := serverFactory.CreateInternal()
+		internalServer, err := serverFactory.CreateInternal(serverOpts...)
 		require.NoError(tb, err)
 		tb.Cleanup(internalServer.Stop)
 
@@ -185,7 +193,9 @@ func runGitaly(tb testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, reg
 		waitHealthy(tb, ctx, "unix://"+internalListener.Addr().String(), cfg.Auth.Token)
 	}
 
-	externalServer, err := serverFactory.CreateExternal(cfg.TLS.CertPath != "" && cfg.TLS.KeyPath != "")
+	secure := cfg.TLS.CertPath != "" && cfg.TLS.KeyPath != ""
+
+	externalServer, err := serverFactory.CreateExternal(secure, serverOpts...)
 	require.NoError(tb, err)
 	tb.Cleanup(externalServer.Stop)
 
diff --git a/proto/go/gitalypb/testproto/error_metadata.pb.go b/proto/go/gitalypb/testproto/error_metadata.pb.go
new file mode 100644
index 000000000..56f0b4a17
--- /dev/null
+++ b/proto/go/gitalypb/testproto/error_metadata.pb.go
@@ -0,0 +1,159 @@
+// Code generated by protoc-gen-go. DO NOT EDIT.
+// versions:
+// 	protoc-gen-go v1.28.1
+// 	protoc        v3.21.7
+// source: testproto/error_metadata.proto
+
+package testproto
+
+import (
+	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
+	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
+	reflect "reflect"
+	sync "sync"
+)
+
+const (
+	// Verify that this generated code is sufficiently up-to-date.
+	_ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion)
+	// Verify that runtime/protoimpl is sufficiently up-to-date.
+	_ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20)
+)
+
+// ErrorMetadata is a key-value metadata item that may be attached to errors. We only use this
+// infrastructure for testing purposes to assert that we add error metadata as expected that would
+// otherwise only get logged.
+type ErrorMetadata struct {
+	state         protoimpl.MessageState
+	sizeCache     protoimpl.SizeCache
+	unknownFields protoimpl.UnknownFields
+
+	// Key is the key of the item.
+	Key []byte `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"`
+	// Value is the value of the item.
+	Value []byte `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"`
+}
+
+func (x *ErrorMetadata) Reset() {
+	*x = ErrorMetadata{}
+	if protoimpl.UnsafeEnabled {
+		mi := &file_testproto_error_metadata_proto_msgTypes[0]
+		ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
+		ms.StoreMessageInfo(mi)
+	}
+}
+
+func (x *ErrorMetadata) String() string {
+	return protoimpl.X.MessageStringOf(x)
+}
+
+func (*ErrorMetadata) ProtoMessage() {}
+
+func (x *ErrorMetadata) ProtoReflect() protoreflect.Message {
+	mi := &file_testproto_error_metadata_proto_msgTypes[0]
+	if protoimpl.UnsafeEnabled && x != nil {
+		ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
+		if ms.LoadMessageInfo() == nil {
+			ms.StoreMessageInfo(mi)
+		}
+		return ms
+	}
+	return mi.MessageOf(x)
+}
+
+// Deprecated: Use ErrorMetadata.ProtoReflect.Descriptor instead.
+func (*ErrorMetadata) Descriptor() ([]byte, []int) {
+	return file_testproto_error_metadata_proto_rawDescGZIP(), []int{0}
+}
+
+func (x *ErrorMetadata) GetKey() []byte {
+	if x != nil {
+		return x.Key
+	}
+	return nil
+}
+
+func (x *ErrorMetadata) GetValue() []byte {
+	if x != nil {
+		return x.Value
+	}
+	return nil
+}
+
+var File_testproto_error_metadata_proto protoreflect.FileDescriptor
+
+var file_testproto_error_metadata_proto_rawDesc = []byte{
+	0x0a, 0x1e, 0x74, 0x65, 0x73, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x65, 0x72, 0x72, 0x6f,
+	0x72, 0x5f, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f,
+	0x12, 0x09, 0x74, 0x65, 0x73, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0x37, 0x0a, 0x0d, 0x45,
+	0x72, 0x72, 0x6f, 0x72, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, 0x10, 0x0a, 0x03,
+	0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14,
+	0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x76,
+	0x61, 0x6c, 0x75, 0x65, 0x42, 0x32, 0x5a, 0x30, 0x67, 0x69, 0x74, 0x6c, 0x61, 0x62, 0x2e, 0x63,
+	0x6f, 0x6d, 0x2f, 0x67, 0x69, 0x74, 0x6c, 0x61, 0x62, 0x2d, 0x6f, 0x72, 0x67, 0x2f, 0x67, 0x69,
+	0x74, 0x61, 0x6c, 0x79, 0x2f, 0x76, 0x31, 0x35, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x74,
+	0x65, 0x73, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
+}
+
+var (
+	file_testproto_error_metadata_proto_rawDescOnce sync.Once
+	file_testproto_error_metadata_proto_rawDescData = file_testproto_error_metadata_proto_rawDesc
+)
+
+func file_testproto_error_metadata_proto_rawDescGZIP() []byte {
+	file_testproto_error_metadata_proto_rawDescOnce.Do(func() {
+		file_testproto_error_metadata_proto_rawDescData = protoimpl.X.CompressGZIP(file_testproto_error_metadata_proto_rawDescData)
+	})
+	return file_testproto_error_metadata_proto_rawDescData
+}
+
+var file_testproto_error_metadata_proto_msgTypes = make([]protoimpl.MessageInfo, 1)
+var file_testproto_error_metadata_proto_goTypes = []interface{}{
+	(*ErrorMetadata)(nil), // 0: testproto.ErrorMetadata
+}
+var file_testproto_error_metadata_proto_depIdxs = []int32{
+	0, // [0:0] is the sub-list for method output_type
+	0, // [0:0] is the sub-list for method input_type
+	0, // [0:0] is the sub-list for extension type_name
+	0, // [0:0] is the sub-list for extension extendee
+	0, // [0:0] is the sub-list for field type_name
+}
+
+func init() { file_testproto_error_metadata_proto_init() }
+func file_testproto_error_metadata_proto_init() {
+	if File_testproto_error_metadata_proto != nil {
+		return
+	}
+	if !protoimpl.UnsafeEnabled {
+		file_testproto_error_metadata_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} {
+			switch v := v.(*ErrorMetadata); i {
+			case 0:
+				return &v.state
+			case 1:
+				return &v.sizeCache
+			case 2:
+				return &v.unknownFields
+			default:
+				return nil
+			}
+		}
+	}
+	type x struct{}
+	out := protoimpl.TypeBuilder{
+		File: protoimpl.DescBuilder{
+			GoPackagePath: reflect.TypeOf(x{}).PkgPath(),
+			RawDescriptor: file_testproto_error_metadata_proto_rawDesc,
+			NumEnums:      0,
+			NumMessages:   1,
+			NumExtensions: 0,
+			NumServices:   0,
+		},
+		GoTypes:           file_testproto_error_metadata_proto_goTypes,
+		DependencyIndexes: file_testproto_error_metadata_proto_depIdxs,
+		MessageInfos:      file_testproto_error_metadata_proto_msgTypes,
+	}.Build()
+	File_testproto_error_metadata_proto = out.File
+	file_testproto_error_metadata_proto_rawDesc = nil
+	file_testproto_error_metadata_proto_goTypes = nil
+	file_testproto_error_metadata_proto_depIdxs = nil
+}
diff --git a/proto/testproto/error_metadata.proto b/proto/testproto/error_metadata.proto
new file mode 100644
index 000000000..e97c9dc0d
--- /dev/null
+++ b/proto/testproto/error_metadata.proto
@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+package testproto;
+
+option go_package = "gitlab.com/gitlab-org/gitaly/v15/proto/testproto";
+
+// ErrorMetadata is a key-value metadata item that may be attached to errors. We only use this
+// infrastructure for testing purposes to assert that we add error metadata as expected that would
+// otherwise only get logged.
+message ErrorMetadata {
+  // Key is the key of the item.
+  bytes key = 1;
+  // Value is the value of the item.
+  bytes value = 2;
+}