Merge branch 'fix-git-protocol-prometheus-metrics' into 'master'

Fix git protocol prometheus metrics

Closes #1362

See merge request gitlab-org/gitaly!908

10 files changed, 82 insertions, 52 deletions

diff --git a/changelogs/unreleased/fix-git-protocol-prometheus-metrics.yml b/changelogs/unreleased/fix-git-protocol-prometheus-metrics.yml
new file mode 100644
index 000000000..502a8f02c
--- /dev/null
+++ b/changelogs/unreleased/fix-git-protocol-prometheus-metrics.yml
@@ -0,0 +1,5 @@
+---
+title: Fix git protocol prometheus metrics
+merge_request: 908
+author:
+type: fixed
diff --git a/internal/git/helper.go b/internal/git/helper.go
index e2067a6fe..8af8511d9 100644
--- a/internal/git/helper.go
+++ b/internal/git/helper.go
@@ -11,15 +11,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 )
 
-// ProtocolV2 Git sets the protocol 2 string as in the environment
-const ProtocolV2 = "version=2"
-
-// GitV1 Indicates Git protocol 1 is used
-const GitV1 = "v1"
-
-// GitV2 Indicates Git protocol 2 is used
-const GitV2 = "v2"
-
 // FallbackTimeValue is the value returned by `SafeTimeParse` in case it
 // encounters a parse error. It's the maximum time value possible in golang.
 // See https://gitlab.com/gitlab-org/gitaly/issues/556#note_40289573
@@ -81,17 +72,3 @@ func BuildGitOptions(gitOpts []string, otherOpts ...string) []string {
 
 	return append(args, otherOpts...)
 }
-
-// AddGitProtocolEnv checks whether the request has Git protocol v2
-// and sets this in the environment.
-func AddGitProtocolEnv(req RequestWithGitProtocol, env []string) []string {
-	if req.GetGitProtocol() == ProtocolV2 {
-		env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", req.GetGitProtocol()))
-
-		gitProtocolRequests.WithLabelValues(GitV2)
-	} else {
-		gitProtocolRequests.WithLabelValues(GitV1)
-	}
-
-	return env
-}
diff --git a/internal/git/protocol.go b/internal/git/protocol.go
index 0a2458080..83d6d3bef 100644
--- a/internal/git/protocol.go
+++ b/internal/git/protocol.go
@@ -1,24 +1,70 @@
 package git
 
 import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/grpc-ecosystem/go-grpc-middleware/tags"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-// RequestWithGitProtocol holds requests that respond to GitProtocol
-type RequestWithGitProtocol interface {
-	GetGitProtocol() string
-}
+const (
+	// ProtocolV2 is the special value used by Git clients to request protocol v2
+	ProtocolV2 = "version=2"
+)
 
 var (
 	gitProtocolRequests = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
-			Name: "gitaly_protocol_requests_total",
+			Name: "gitaly_git_protocol_requests_total",
 			Help: "Counter of Git protocol requests",
 		},
-		[]string{"protocol"},
+		[]string{"grpc_service", "grpc_method", "git_protocol"},
 	)
 )
 
 func init() {
 	prometheus.MustRegister(gitProtocolRequests)
 }
+
+// RequestWithGitProtocol holds requests that respond to GitProtocol
+type RequestWithGitProtocol interface {
+	GetGitProtocol() string
+}
+
+// AddGitProtocolEnv checks whether the request has Git protocol v2
+// and sets this in the environment.
+func AddGitProtocolEnv(ctx context.Context, req RequestWithGitProtocol, env []string) []string {
+	service, method := methodFromContext(ctx)
+
+	if req.GetGitProtocol() == ProtocolV2 {
+		env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", req.GetGitProtocol()))
+
+		gitProtocolRequests.WithLabelValues(service, method, "v2").Inc()
+	} else {
+		gitProtocolRequests.WithLabelValues(service, method, "v0").Inc()
+	}
+
+	return env
+}
+
+func methodFromContext(ctx context.Context) (service string, method string) {
+	tags := grpc_ctxtags.Extract(ctx)
+	ctxValue := tags.Values()["grpc.request.fullMethod"]
+	if ctxValue == nil {
+		return "", ""
+	}
+
+	if s, ok := ctxValue.(string); ok {
+		// Expect: "/foo.BarService/Qux"
+		split := strings.Split(s, "/")
+		if len(split) != 3 {
+			return "", ""
+		}
+
+		return split[1], split[2]
+	}
+
+	return "", ""
+}
diff --git a/internal/helper/fieldextractors/fieldextractor.go b/internal/helper/fieldextractors/fieldextractor.go
index 78b1db7dd..141a38067 100644
--- a/internal/helper/fieldextractors/fieldextractor.go
+++ b/internal/helper/fieldextractors/fieldextractor.go
@@ -77,17 +77,22 @@ func FieldExtractor(fullMethod string, req interface{}) map[string]interface{} {
 		return nil
 	}
 
+	var result map[string]interface{}
 	switch req.(type) {
 	case gitalypb.RenameNamespaceRequest:
-		return formatRenameNamespaceRequest(req.(gitalypb.RenameNamespaceRequest))
+		result = formatRenameNamespaceRequest(req.(gitalypb.RenameNamespaceRequest))
 	case repositoryBasedRequest:
-		return formatRepoRequest(req.(repositoryBasedRequest).GetRepository())
+		result = formatRepoRequest(req.(repositoryBasedRequest).GetRepository())
 	case namespaceBasedRequest:
-		return formatNamespaceRequest(req.(namespaceBasedRequest))
+		result = formatNamespaceRequest(req.(namespaceBasedRequest))
 	case storageBasedRequest:
-		return formatStorageRequest(req.(storageBasedRequest))
+		result = formatStorageRequest(req.(storageBasedRequest))
 	}
 
-	return nil
+	if result == nil {
+		result = make(map[string]interface{})
+	}
+	result["fullMethod"] = fullMethod
 
+	return result
 }
diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go
index 533a32d42..59e115d09 100644
--- a/internal/service/smarthttp/inforefs.go
+++ b/internal/service/smarthttp/inforefs.go
@@ -38,7 +38,7 @@ func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsR
 		"service": service,
 	}).Debug("handleInfoRefs")
 
-	env := git.AddGitProtocolEnv(req, []string{})
+	env := git.AddGitProtocolEnv(ctx, req, []string{})
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index 9c8683f99..7bde1fe9d 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -17,12 +17,13 @@ import (
 )
 
 func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePackServer) error {
+	ctx := stream.Context()
 	req, err := stream.Recv() // First request contains only Repository and GlId
 	if err != nil {
 		return err
 	}
 
-	grpc_logrus.Extract(stream.Context()).WithFields(log.Fields{
+	grpc_logrus.Extract(ctx).WithFields(log.Fields{
 		"GlID":             req.GlId,
 		"GlRepository":     req.GlRepository,
 		"GlUsername":       req.GlUsername,
@@ -51,7 +52,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 		env = append(env, fmt.Sprintf("GL_USERNAME=%s", req.GlUsername))
 	}
 
-	env = git.AddGitProtocolEnv(req, env)
+	env = git.AddGitProtocolEnv(ctx, req, env)
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -60,7 +61,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 
 	gitOptions := git.BuildGitOptions(req.GitConfigOptions, "receive-pack", "--stateless-rpc", repoPath)
 	osCommand := exec.Command(command.GitPath(), gitOptions...)
-	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, nil, env...)
+	cmd, err := command.New(ctx, osCommand, stdin, stdout, nil, env...)
 
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "PostReceivePack: %v", err)
diff --git a/internal/service/smarthttp/upload_pack.go b/internal/service/smarthttp/upload_pack.go
index 03fbc4d3e..01f74efc7 100644
--- a/internal/service/smarthttp/upload_pack.go
+++ b/internal/service/smarthttp/upload_pack.go
@@ -6,7 +6,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -32,7 +31,7 @@ func init() {
 }
 
 func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackServer) error {
-	grpc_logrus.Extract(stream.Context()).Debug("PostUploadPack")
+	ctx := stream.Context()
 
 	req, err := stream.Recv() // First request contains Repository only
 	if err != nil {
@@ -59,7 +58,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS
 		return stream.Send(&gitalypb.PostUploadPackResponse{Data: p})
 	})
 
-	env := git.AddGitProtocolEnv(req, []string{})
+	env := git.AddGitProtocolEnv(ctx, req, []string{})
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -74,7 +73,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS
 	args = append(args, "upload-pack", "--stateless-rpc", repoPath)
 
 	osCommand := exec.Command(command.GitPath(), args...)
-	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, nil, env...)
+	cmd, err := command.New(ctx, osCommand, stdin, stdout, nil, env...)
 
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "PostUploadPack: cmd: %v", err)
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index b291eb9a3..c49265a32 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -17,12 +17,13 @@ import (
 )
 
 func (s *server) SSHReceivePack(stream gitalypb.SSHService_SSHReceivePackServer) error {
+	ctx := stream.Context()
 	req, err := stream.Recv() // First request contains only Repository, GlId, and GlUsername
 	if err != nil {
 		return err
 	}
 
-	grpc_logrus.Extract(stream.Context()).WithFields(log.Fields{
+	grpc_logrus.Extract(ctx).WithFields(log.Fields{
 		"GlID":             req.GlId,
 		"GlRepository":     req.GlRepository,
 		"GlUsername":       req.GlUsername,
@@ -51,7 +52,7 @@ func (s *server) SSHReceivePack(stream gitalypb.SSHService_SSHReceivePackServer)
 	if req.GlRepository != "" {
 		env = append(env, fmt.Sprintf("GL_REPOSITORY=%s", req.GlRepository))
 	}
-	env = git.AddGitProtocolEnv(req, env)
+	env = git.AddGitProtocolEnv(ctx, req, env)
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -60,7 +61,7 @@ func (s *server) SSHReceivePack(stream gitalypb.SSHService_SSHReceivePackServer)
 
 	gitOptions := git.BuildGitOptions(req.GitConfigOptions, "receive-pack", repoPath)
 	osCommand := exec.Command(command.GitPath(), gitOptions...)
-	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, stderr, env...)
+	cmd, err := command.New(ctx, osCommand, stdin, stdout, stderr, env...)
 
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "SSHReceivePack: cmd: %v", err)
diff --git a/internal/service/ssh/upload_archive.go b/internal/service/ssh/upload_archive.go
index ee407ce48..682d87d6f 100644
--- a/internal/service/ssh/upload_archive.go
+++ b/internal/service/ssh/upload_archive.go
@@ -3,7 +3,6 @@ package ssh
 import (
 	"os/exec"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -13,8 +12,6 @@ import (
 )
 
 func (s *server) SSHUploadArchive(stream gitalypb.SSHService_SSHUploadArchiveServer) error {
-	grpc_logrus.Extract(stream.Context()).Debug("SSHUploadArchive")
-
 	req, err := stream.Recv() // First request contains Repository only
 	if err != nil {
 		return err
diff --git a/internal/service/ssh/upload_pack.go b/internal/service/ssh/upload_pack.go
index 28d3f5296..61d0fe618 100644
--- a/internal/service/ssh/upload_pack.go
+++ b/internal/service/ssh/upload_pack.go
@@ -5,7 +5,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -15,7 +14,7 @@ import (
 )
 
 func (s *server) SSHUploadPack(stream gitalypb.SSHService_SSHUploadPackServer) error {
-	grpc_logrus.Extract(stream.Context()).Debug("SSHUploadPack")
+	ctx := stream.Context()
 
 	req, err := stream.Recv() // First request contains Repository only
 	if err != nil {
@@ -37,7 +36,7 @@ func (s *server) SSHUploadPack(stream gitalypb.SSHService_SSHUploadPackServer) e
 		return stream.Send(&gitalypb.SSHUploadPackResponse{Stderr: p})
 	})
 
-	env := git.AddGitProtocolEnv(req, []string{})
+	env := git.AddGitProtocolEnv(ctx, req, []string{})
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -54,7 +53,7 @@ func (s *server) SSHUploadPack(stream gitalypb.SSHService_SSHUploadPackServer) e
 
 	osCommand := exec.Command(command.GitPath(), args...)
 
-	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, stderr, env...)
+	cmd, err := command.New(ctx, osCommand, stdin, stdout, stderr, env...)
 
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "SSHUploadPack: cmd: %v", err)