OptionalLogging interface with method ShouldLog

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
weaveworks · Jul 26, 2023 · 9927ce6 · 9927ce6
1 parent 70dc704
commit 9927ce6
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 11 deletions.
diff --git a/middleware/grpc_logging.go b/middleware/grpc_logging.go
@@ -17,9 +17,9 @@ const (
 	errorKey = "err"
 )
 
-// If an error implements Observe(), it will get called and GRPCServerLog will do nothing.
-type Observer interface {
-	Observe(ctx context.Context, _ logging.Interface, method string, duration time.Duration)
+// An error can implement ShouldLog() to control whether GRPCServerLog will log.
+type OptionalLogging interface {
+	ShouldLog(ctx context.Context, duration time.Duration) bool
 }
 
 // GRPCServerLog logs grpc requests, errors, and latency.
@@ -37,9 +37,8 @@ func (s GRPCServerLog) UnaryServerInterceptor(ctx context.Context, req interface
 	if err == nil && s.DisableRequestSuccessLog {
 		return resp, nil
 	}
-	var observer Observer
-	if errors.As(err, &observer) {
-		observer.Observe(ctx, s.Log, info.FullMethod, time.Since(begin))
+	var optional OptionalLogging
+	if errors.As(err, &optional) && !optional.ShouldLog(ctx, time.Since(begin)) {
 		return resp, err
 	}
 

diff --git a/middleware/grpc_logging_test.go b/middleware/grpc_logging_test.go
@@ -35,11 +35,9 @@ func BenchmarkGRPCServerLog_UnaryServerInterceptor_NoError(b *testing.B) {
 
 type doNotLogError struct{ Err error }
 
-func (i doNotLogError) Error() string { return i.Err.Error() }
-func (i doNotLogError) Unwrap() error { return i.Err }
-func (i doNotLogError) Observe(_ context.Context, _ logging.Interface, _ string, _ time.Duration) {
-	// no-op
-}
+func (i doNotLogError) Error() string                                     { return i.Err.Error() }
+func (i doNotLogError) Unwrap() error                                     { return i.Err }
+func (i doNotLogError) ShouldLog(_ context.Context, _ time.Duration) bool { return false }
 
 func TestGrpcLogging(t *testing.T) {
 	ctx := context.Background()