Merge branch '1072-all-arguments-for-namespaceservice-requests-should-be-explicitly-logged' into 'master'

Resolve "All arguments for NamespaceService requests should be explicitly logged"

Closes #1072

See merge request gitlab-org/gitaly!626

3 files changed, 34 insertions, 8 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 567a0bf8e..b4b0ab38c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Add param logging in NamespaceService RPCs
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/626
 - Sanitize URLs before sending gitaly-ruby exceptions to Sentry
   https://gitlab.com/gitlab-org/gitaly/merge_requests/625
 
diff --git a/internal/helper/fieldextractors/repository.go b/internal/helper/fieldextractors/fieldextractor.go
index 2b812d3d1..aaa47efe4 100644
--- a/internal/helper/fieldextractors/repository.go
+++ b/internal/helper/fieldextractors/fieldextractor.go
@@ -6,6 +6,15 @@ import (
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 )
 
+type repositoryBasedRequest interface {
+	GetRepository() *pb.Repository
+}
+
+type namespaceBasedRequest interface {
+	GetStorageName() string
+	GetName() string
+}
+
 func formatRepoRequest(repo *pb.Repository) map[string]interface{} {
 	if repo == nil {
 		// Signals that the client did not send a repo through, which
@@ -36,21 +45,36 @@ func getTopLevelGroupFromRepoPath(repoPath string) string {
 	return parts[0]
 }
 
-type repositoryBasedRequest interface {
-	GetRepository() *pb.Repository
+func formatNamespaceRequest(namespaceReq namespaceBasedRequest) map[string]interface{} {
+	return map[string]interface{}{
+		"StorageName": namespaceReq.GetStorageName(),
+		"Name":        namespaceReq.GetName(),
+	}
+}
+
+func formatRenameNamespaceRequest(renameReq pb.RenameNamespaceRequest) map[string]interface{} {
+	return map[string]interface{}{
+		"StorageName": renameReq.GetStorageName(),
+		"From":        renameReq.GetFrom(),
+		"To":          renameReq.GetTo(),
+	}
 }
 
-// RepositoryFieldExtractor will extract the repository fields from an incoming grpc request
-func RepositoryFieldExtractor(fullMethod string, req interface{}) map[string]interface{} {
+// FieldExtractor will extract the relevant fields from an incoming grpc request
+func FieldExtractor(fullMethod string, req interface{}) map[string]interface{} {
 	if req == nil {
 		return nil
 	}
 
-	if repoReq, ok := req.(repositoryBasedRequest); ok {
-		return formatRepoRequest(repoReq.GetRepository())
+	switch req.(type) {
+	case pb.RenameNamespaceRequest:
+		return formatRenameNamespaceRequest(req.(pb.RenameNamespaceRequest))
+	case repositoryBasedRequest:
+		return formatRepoRequest(req.(repositoryBasedRequest).GetRepository())
+	case namespaceBasedRequest:
+		return formatNamespaceRequest(req.(namespaceBasedRequest))
 	}
 
-	// Add other request handlers here in future
 	return nil
 
 }
diff --git a/internal/server/server.go b/internal/server/server.go
index eb5decb8d..2c9a4671a 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -39,7 +39,7 @@ func New(rubyServer *rubyserver.Server) *grpc.Server {
 	grpc_logrus.ReplaceGrpcLogger(logrusEntry)
 
 	ctxTagOpts := []grpc_ctxtags.Option{
-		grpc_ctxtags.WithFieldExtractorForInitialReq(fieldextractors.RepositoryFieldExtractor),
+		grpc_ctxtags.WithFieldExtractorForInitialReq(fieldextractors.FieldExtractor),
 	}
 
 	lh := limithandler.New()