Merge branch 'feature/skipLogByRequestFullMethod' into 'master'

Add ENV to select or skip gRPC log

See merge request gitlab-org/gitaly!4168

3 files changed, 116 insertions, 0 deletions

diff --git a/internal/gitaly/server/server.go b/internal/gitaly/server/server.go
index bc34b49d8..2304c0b8f 100644
--- a/internal/gitaly/server/server.go
+++ b/internal/gitaly/server/server.go
@@ -110,6 +110,7 @@ func New(
 			grpcmwlogrus.StreamServerInterceptor(logrusEntry,
 				grpcmwlogrus.WithTimestampFormat(gitalylog.LogTimestampFormat),
 				logMsgProducer,
+				gitalylog.DeciderOption(),
 			),
 			gitalylog.StreamLogDataCatcherServerInterceptor(),
 			sentryhandler.StreamLogHandler,
@@ -131,6 +132,7 @@ func New(
 			grpcmwlogrus.UnaryServerInterceptor(logrusEntry,
 				grpcmwlogrus.WithTimestampFormat(gitalylog.LogTimestampFormat),
 				logMsgProducer,
+				gitalylog.DeciderOption(),
 			),
 			gitalylog.UnaryLogDataCatcherServerInterceptor(),
 			sentryhandler.UnaryLogHandler,
diff --git a/internal/log/log.go b/internal/log/log.go
index 5c4e5f6db..b987283b2 100644
--- a/internal/log/log.go
+++ b/internal/log/log.go
@@ -3,7 +3,9 @@ package log
 import (
 	"context"
 	"os"
+	"regexp"
 
+	grpcmwlogging "github.com/grpc-ecosystem/go-grpc-middleware/logging"
 	grpcmwlogrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
@@ -91,6 +93,48 @@ func Configure(loggers []*logrus.Logger, format string, level string) {
 	}
 }
 
+// DeciderOption returns a Option to support log filtering.
+// If "GITALY_LOG_REQUEST_METHOD_DENY_PATTERN" ENV variable is set, logger will filter out the log whose "fullMethodName" matches it;
+// If "GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN" ENV variable is set, logger will only keep the log whose "fullMethodName" matches it;
+// Under any conditions, the error log will not be filtered out;
+// If the ENV variables are not set, there will be no additional effects.
+func DeciderOption() grpcmwlogrus.Option {
+	matcher := methodNameMatcherFromEnv()
+
+	if matcher == nil {
+		return grpcmwlogrus.WithDecider(grpcmwlogging.DefaultDeciderMethod)
+	}
+
+	decider := func(fullMethodName string, err error) bool {
+		if err != nil {
+			return true
+		}
+		return matcher(fullMethodName)
+	}
+
+	return grpcmwlogrus.WithDecider(decider)
+}
+
+func methodNameMatcherFromEnv() func(string) bool {
+	if pattern := os.Getenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN"); pattern != "" {
+		methodRegex := regexp.MustCompile(pattern)
+
+		return func(fullMethodName string) bool {
+			return methodRegex.MatchString(fullMethodName)
+		}
+	}
+
+	if pattern := os.Getenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN"); pattern != "" {
+		methodRegex := regexp.MustCompile(pattern)
+
+		return func(fullMethodName string) bool {
+			return !methodRegex.MatchString(fullMethodName)
+		}
+	}
+
+	return nil
+}
+
 func mapGrpcLogLevel(level logrus.Level) logrus.Level {
 	// Honor grpc-go's debug settings: https://github.com/grpc/grpc-go#how-to-turn-on-logging
 	logLevel := os.Getenv("GRPC_GO_LOG_SEVERITY_LEVEL")
diff --git a/internal/log/log_test.go b/internal/log/log_test.go
index 93f4b1325..a6b32b93b 100644
--- a/internal/log/log_test.go
+++ b/internal/log/log_test.go
@@ -445,3 +445,73 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 func createContext() context.Context {
 	return context.Background()
 }
+
+func TestLogDeciderOption_logByRegexpMatch(t *testing.T) {
+	methodNames := []string{
+		"/grpc.health.v1.Health/Check",
+		"/gitaly.SmartHTTPService/InfoRefsUploadPack",
+		"/gitaly.SmartHTTPService/PostUploadPackWithSidechannel",
+	}
+	for _, tc := range []struct {
+		desc             string
+		skip             string
+		only             string
+		shouldLogMethods []string
+	}{
+		{
+			desc:             "default setting",
+			skip:             "",
+			only:             "",
+			shouldLogMethods: []string{"Check", "InfoRefsUploadPack", "PostUploadPackWithSidechannel"},
+		},
+		{
+			desc:             "only log Check",
+			skip:             "",
+			only:             "^/grpc.health.v1.Health/Check$",
+			shouldLogMethods: []string{"Check"},
+		},
+		{
+			desc:             "skip log Check",
+			skip:             "^/grpc.health.v1.Health/Check$",
+			only:             "",
+			shouldLogMethods: []string{"InfoRefsUploadPack", "PostUploadPackWithSidechannel"},
+		},
+		{
+			// If condition 'only' exists, ignore condition 'skip'
+			desc:             "only log Check and ignore skip setting",
+			skip:             "^/grpc.health.v1.Health/Check$",
+			only:             "^/grpc.health.v1.Health/Check$",
+			shouldLogMethods: []string{"Check"},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			require.NoError(t, os.Setenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN", tc.skip))
+			defer func() { require.NoError(t, os.Unsetenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN")) }()
+			require.NoError(t, os.Setenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN", tc.only))
+			defer func() { require.NoError(t, os.Unsetenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN")) }()
+
+			logger, hook := test.NewNullLogger()
+			interceptor := grpcmwlogrus.UnaryServerInterceptor(logrus.NewEntry(logger), DeciderOption())
+
+			ctx := createContext()
+			for _, methodName := range methodNames {
+				_, err := interceptor(
+					ctx,
+					nil,
+					&grpc.UnaryServerInfo{FullMethod: methodName},
+					func(ctx context.Context, req interface{}) (interface{}, error) {
+						return nil, nil
+					},
+				)
+				require.NoError(t, err)
+			}
+
+			entries := hook.AllEntries()
+			require.Len(t, entries, len(tc.shouldLogMethods))
+			for idx, entry := range entries {
+				require.Equal(t, entry.Message, "finished unary call with code OK")
+				require.Equal(t, entry.Data["grpc.method"], tc.shouldLogMethods[idx])
+			}
+		})
+	}
+}