Merge branch 'jc-add-grpc_method-to-metadatahandler' into 'master'

metadatahandler: Add grpc_method to prometheus metric

Closes #4289

See merge request gitlab-org/gitaly!4625

2 files changed, 39 insertions, 20 deletions

diff --git a/internal/middleware/metadatahandler/metadatahandler.go b/internal/middleware/metadatahandler/metadatahandler.go
index 8dca74e0e..5c42cd411 100644
--- a/internal/middleware/metadatahandler/metadatahandler.go
+++ b/internal/middleware/metadatahandler/metadatahandler.go
@@ -17,12 +17,18 @@ import (
 
 var requests = promauto.NewCounterVec(
 	prometheus.CounterOpts{
-		Namespace: "gitaly",
-		Subsystem: "service",
-		Name:      "client_requests_total",
-		Help:      "Counter of client requests received by client, call_site, auth version, response code and deadline_type",
+		Name: "gitaly_service_client_requests_total",
+		Help: "Counter of client requests received by client, call_site, auth version, response code and deadline_type",
+	},
+	[]string{
+		"client_name",
+		"grpc_service",
+		"grpc_method",
+		"call_site",
+		"auth_version",
+		"grpc_code",
+		"deadline_type",
 	},
-	[]string{"client_name", "grpc_service", "call_site", "auth_version", "grpc_code", "deadline_type"},
 )
 
 type metadataTags struct {
@@ -149,12 +155,12 @@ func addMetadataTags(ctx context.Context, grpcMethodType string) metadataTags {
 	return metaTags
 }
 
-func extractServiceName(fullMethodName string) string {
+func extractServiceAndMethodName(fullMethodName string) (string, string) {
 	fullMethodName = strings.TrimPrefix(fullMethodName, "/") // remove leading slash
 	if i := strings.Index(fullMethodName, "/"); i >= 0 {
-		return fullMethodName[:i]
+		return fullMethodName[:i], fullMethodName[i+1:]
 	}
-	return unknownValue
+	return unknownValue, unknownValue
 }
 
 func streamRPCType(info *grpc.StreamServerInfo) string {
@@ -168,11 +174,12 @@ func streamRPCType(info *grpc.StreamServerInfo) string {
 
 func reportWithPrometheusLabels(metaTags metadataTags, fullMethod string, err error) {
 	grpcCode := helper.GrpcCode(err)
-	serviceName := extractServiceName(fullMethod)
+	serviceName, methodName := extractServiceAndMethodName(fullMethod)
 
 	requests.WithLabelValues(
 		metaTags.clientName,   // client_name
 		serviceName,           // grpc_service
+		methodName,            // grpc_method
 		metaTags.callSite,     // call_site
 		metaTags.authVersion,  // auth_version
 		grpcCode.String(),     // grpc_code
diff --git a/internal/middleware/metadatahandler/metadatahandler_test.go b/internal/middleware/metadatahandler/metadatahandler_test.go
index ee4c59b4a..879bdd38a 100644
--- a/internal/middleware/metadatahandler/metadatahandler_test.go
+++ b/internal/middleware/metadatahandler/metadatahandler_test.go
@@ -7,6 +7,7 @@ import (
 	"time"
 
 	grpcmwtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/labkit/correlation"
@@ -152,29 +153,40 @@ func TestGRPCTags(t *testing.T) {
 
 func Test_extractServiceName(t *testing.T) {
 	tests := []struct {
-		name           string
-		fullMethodName string
-		want           string
+		name                    string
+		fullMethodName          string
+		wantService, wantMethod string
 	}{
 		{
 			name:           "blank",
 			fullMethodName: "",
-			want:           unknownValue,
-		}, {
+			wantService:    unknownValue,
+			wantMethod:     unknownValue,
+		},
+		{
 			name:           "normal",
 			fullMethodName: "/gitaly.OperationService/method",
-			want:           "gitaly.OperationService",
-		}, {
+			wantService:    "gitaly.OperationService",
+			wantMethod:     "method",
+		},
+		{
 			name:           "malformed",
 			fullMethodName: "//method",
-			want:           "",
+			wantService:    "",
+			wantMethod:     "method",
+		},
+		{
+			name:           "malformed",
+			fullMethodName: "/gitaly.OperationService/",
+			wantService:    "gitaly.OperationService",
+			wantMethod:     "",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := extractServiceName(tt.fullMethodName); got != tt.want {
-				t.Errorf("extractServiceName() = %v, want %v", got, tt.want)
-			}
+			gotService, gotMethod := extractServiceAndMethodName(tt.fullMethodName)
+			assert.Equal(t, tt.wantService, gotService)
+			assert.Equal(t, tt.wantMethod, gotMethod)
 		})
 	}
 }