From bdd53c966c3a87e268ed05fedad5d03cd70783d3 Mon Sep 17 00:00:00 2001
From: James Liu <jliu@gitlab.com>
Date: Mon, 29 Apr 2024 11:34:24 +1000
Subject: [PATCH] middleware: Stop extracting object pool repo

f6aa35a (middleware: Log nested Repository fields) modified the gRPC
middleware such that gitalypb.Repository fields which were nested in a
request message were properly extracted for logging purposes. This was
accomplished by using the `MethodInfo.TargetRepo()` helper to extract
the appropriate field.

That change had the by-product of duplicating the repository data for
object pool-scoped requests. This happened because object pool-scoped
requests were handled by the old interface casting method in addition
to the `TargetRepo()` helper.

Remove the interface casting as well as the associated `objectPool`
field in the `RequestInfo` struct. All of the repository data are
already captured in the `Repository` field of the struct anyway.
---
 .../requestinfohandler/requestinfohandler.go  | 20 -------------
 .../requestinfohandler_test.go                | 28 ++++++-------------
 2 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/internal/grpc/middleware/requestinfohandler/requestinfohandler.go b/internal/grpc/middleware/requestinfohandler/requestinfohandler.go
index 3c182d180a..3d43e9f780 100644
--- a/internal/grpc/middleware/requestinfohandler/requestinfohandler.go
+++ b/internal/grpc/middleware/requestinfohandler/requestinfohandler.go
@@ -54,7 +54,6 @@ type RequestInfo struct {
 	methodScope     string
 
 	Repository  *gitalypb.Repository
-	objectPool  *gitalypb.ObjectPool
 	storageName string
 }
 
@@ -172,14 +171,6 @@ func newRequestInfo(ctx context.Context, fullMethod, grpcMethodType string) *Req
 }
 
 func (i *RequestInfo) extractRequestInfo(request any) {
-	type poolScopedRequest interface {
-		GetObjectPool() *gitalypb.ObjectPool
-	}
-
-	if poolScoped, ok := request.(poolScopedRequest); ok {
-		i.objectPool = poolScoped.GetObjectPool()
-	}
-
 	if reqMsg, ok := request.(proto.Message); ok {
 		// This handles extracting nested and non-nested *gitalypb.Repository fields from the request. In cases of
 		// multiple such fields, it will choose the one with the `target_repository` extension.
@@ -252,17 +243,6 @@ func (i *RequestInfo) Tags() map[string]string {
 		}
 	}
 
-	// Same for the object pool repository.
-	if pool := i.objectPool.GetRepository(); pool != nil {
-		for key, value := range map[string]string{
-			"grpc.request.pool.storage":           pool.GetStorageName(),
-			"grpc.request.pool.relativePath":      pool.GetRelativePath(),
-			"grpc.request.pool.sourceProjectPath": pool.GetGlProjectPath(),
-		} {
-			tags[key] = value
-		}
-	}
-
 	return tags
 }
 
diff --git a/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go b/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go
index 8968dc6112..ac9150cb42 100644
--- a/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go
+++ b/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go
@@ -513,13 +513,6 @@ func TestInterceptors(t *testing.T) {
 				methodScope:     "repository",
 				methodType:      "unary",
 				FullMethod:      "/gitaly.ObjectPoolService/FetchIntoObjectPool",
-				objectPool: &gitalypb.ObjectPool{
-					Repository: &gitalypb.Repository{
-						StorageName:   "storage",
-						RelativePath:  "path",
-						GlProjectPath: "glProject",
-					},
-				},
 				Repository: &gitalypb.Repository{
 					StorageName:   "storage",
 					RelativePath:  "path",
@@ -527,18 +520,15 @@ func TestInterceptors(t *testing.T) {
 				},
 			},
 			expectedTags: map[string]any{
-				"grpc.meta.deadline_type":             "none",
-				"grpc.meta.method_operation":          "mutator",
-				"grpc.meta.method_scope":              "repository",
-				"grpc.meta.method_type":               "unary",
-				"grpc.request.fullMethod":             "/gitaly.ObjectPoolService/FetchIntoObjectPool",
-				"grpc.request.glProjectPath":          "glProject",
-				"grpc.request.glRepository":           "",
-				"grpc.request.repoPath":               "path",
-				"grpc.request.repoStorage":            "storage",
-				"grpc.request.pool.relativePath":      "path",
-				"grpc.request.pool.storage":           "storage",
-				"grpc.request.pool.sourceProjectPath": "glProject",
+				"grpc.meta.deadline_type":    "none",
+				"grpc.meta.method_operation": "mutator",
+				"grpc.meta.method_scope":     "repository",
+				"grpc.meta.method_type":      "unary",
+				"grpc.request.fullMethod":    "/gitaly.ObjectPoolService/FetchIntoObjectPool",
+				"grpc.request.glProjectPath": "glProject",
+				"grpc.request.glRepository":  "",
+				"grpc.request.repoPath":      "path",
+				"grpc.request.repoStorage":   "storage",
 			},
 		},
 		{
-- 
GitLab