From 98c9b5928a4e4dabca85df46305330932a6f9730 Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Sat, 8 Apr 2023 22:45:05 +0800 Subject: [PATCH] refactor: simplify zrpc stat config (#3107) --- zrpc/config.go | 2 ++ zrpc/internal/config.go | 18 +++++----- zrpc/internal/rpcserver.go | 7 ++-- .../serverinterceptors/statinterceptor.go | 34 +++++++++---------- .../statinterceptor_test.go | 8 ++--- zrpc/server.go | 2 +- 6 files changed, 36 insertions(+), 35 deletions(-) diff --git a/zrpc/config.go b/zrpc/config.go index 646382fd..80f3bd2e 100644 --- a/zrpc/config.go +++ b/zrpc/config.go @@ -15,6 +15,8 @@ type ( ClientMiddlewaresConf = internal.ClientMiddlewaresConf // ServerMiddlewaresConf defines whether to use server middlewares. ServerMiddlewaresConf = internal.ServerMiddlewaresConf + // StatConf defines the stat config. + StatConf = internal.StatConf // A RpcClientConf is a rpc client config. RpcClientConf struct { diff --git a/zrpc/internal/config.go b/zrpc/internal/config.go index ddfb164e..b60e1f09 100644 --- a/zrpc/internal/config.go +++ b/zrpc/internal/config.go @@ -1,8 +1,11 @@ package internal -import "time" +import "github.com/zeromicro/go-zero/zrpc/internal/serverinterceptors" type ( + // StatConf defines the stat config. + StatConf = serverinterceptors.StatConf + // ClientMiddlewaresConf defines whether to use client middlewares. ClientMiddlewaresConf struct { Trace bool `json:",default=true"` @@ -14,12 +17,11 @@ type ( // ServerMiddlewaresConf defines whether to use server middlewares. ServerMiddlewaresConf struct { - Trace bool `json:",default=true"` - Recover bool `json:",default=true"` - Stat bool `json:",default=true"` - StatSlowThreshold time.Duration `json:",default=500ms"` - NotLoggingContentMethods []string `json:",optional"` - Prometheus bool `json:",default=true"` - Breaker bool `json:",default=true"` + Trace bool `json:",default=true"` + Recover bool `json:",default=true"` + Stat bool `json:",default=true"` + StatConf StatConf `json:",optional"` + Prometheus bool `json:",default=true"` + Breaker bool `json:",default=true"` } ) diff --git a/zrpc/internal/rpcserver.go b/zrpc/internal/rpcserver.go index 1efe35fd..f1b90e42 100644 --- a/zrpc/internal/rpcserver.go +++ b/zrpc/internal/rpcserver.go @@ -113,11 +113,8 @@ func (s *rpcServer) buildUnaryInterceptors() []grpc.UnaryServerInterceptor { interceptors = append(interceptors, serverinterceptors.UnaryRecoverInterceptor) } if s.middlewares.Stat { - interceptors = append(interceptors, serverinterceptors.UnaryStatInterceptor(s.metrics, - serverinterceptors.StatConf{ - SlowThreshold: s.middlewares.StatSlowThreshold, - NotLoggingContentMethods: s.middlewares.NotLoggingContentMethods, - })) + interceptors = append(interceptors, + serverinterceptors.UnaryStatInterceptor(s.metrics, s.middlewares.StatConf)) } if s.middlewares.Prometheus { interceptors = append(interceptors, serverinterceptors.UnaryPrometheusInterceptor) diff --git a/zrpc/internal/serverinterceptors/statinterceptor.go b/zrpc/internal/serverinterceptors/statinterceptor.go index 76812ae5..b31bf7ff 100644 --- a/zrpc/internal/serverinterceptors/statinterceptor.go +++ b/zrpc/internal/serverinterceptors/statinterceptor.go @@ -19,20 +19,20 @@ import ( const defaultSlowThreshold = time.Millisecond * 500 var ( - notLoggingContentMethods sync.Map - slowThreshold = syncx.ForAtomicDuration(defaultSlowThreshold) + ignoreContentMethods sync.Map + slowThreshold = syncx.ForAtomicDuration(defaultSlowThreshold) ) // StatConf defines the static configuration for stat interceptor. type StatConf struct { - SlowThreshold time.Duration - NotLoggingContentMethods []string + SlowThreshold time.Duration `json:",default=500ms"` + IgnoreContentMethods []string `json:",optional"` } // DontLogContentForMethod disable logging content for given method. // Deprecated: use StatConf instead. func DontLogContentForMethod(method string) { - notLoggingContentMethods.Store(method, lang.Placeholder) + ignoreContentMethods.Store(method, lang.Placeholder) } // SetSlowThreshold sets the slow threshold. @@ -44,7 +44,7 @@ func SetSlowThreshold(threshold time.Duration) { // UnaryStatInterceptor returns a func that uses given metrics to report stats. func UnaryStatInterceptor(metrics *stat.Metrics, conf StatConf) grpc.UnaryServerInterceptor { staticNotLoggingContentMethods := collection.NewSet() - staticNotLoggingContentMethods.AddStr(conf.NotLoggingContentMethods...) + staticNotLoggingContentMethods.AddStr(conf.IgnoreContentMethods...) return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) { @@ -62,8 +62,13 @@ func UnaryStatInterceptor(metrics *stat.Metrics, conf StatConf) grpc.UnaryServer } } +func isSlow(duration, durationThreshold time.Duration) bool { + return duration > slowThreshold.Load() || + (durationThreshold > 0 && duration > durationThreshold) +} + func logDuration(ctx context.Context, method string, req any, duration time.Duration, - staticNotLoggingContentMethods *collection.Set, staticSlowThreshold time.Duration) { + ignoreMethods *collection.Set, durationThreshold time.Duration) { var addr string client, ok := peer.FromContext(ctx) if ok { @@ -71,8 +76,8 @@ func logDuration(ctx context.Context, method string, req any, duration time.Dura } logger := logx.WithContext(ctx).WithDuration(duration) - if !shouldLogContent(method, staticNotLoggingContentMethods) { - if isSlow(duration, staticSlowThreshold) { + if !shouldLogContent(method, ignoreMethods) { + if isSlow(duration, durationThreshold) { logger.Slowf("[RPC] slowcall - %s - %s", addr, method) } } else { @@ -87,12 +92,7 @@ func logDuration(ctx context.Context, method string, req any, duration time.Dura } } -func shouldLogContent(method string, staticNotLoggingContentMethods *collection.Set) bool { - _, ok := notLoggingContentMethods.Load(method) - return !ok && !staticNotLoggingContentMethods.Contains(method) -} - -func isSlow(duration time.Duration, staticSlowThreshold time.Duration) bool { - return duration > slowThreshold.Load() || - (staticSlowThreshold > 0 && duration > staticSlowThreshold) +func shouldLogContent(method string, ignoreMethods *collection.Set) bool { + _, ok := ignoreContentMethods.Load(method) + return !ok && !ignoreMethods.Contains(method) } diff --git a/zrpc/internal/serverinterceptors/statinterceptor_test.go b/zrpc/internal/serverinterceptors/statinterceptor_test.go index d3cbfaed..d98178d3 100644 --- a/zrpc/internal/serverinterceptors/statinterceptor_test.go +++ b/zrpc/internal/serverinterceptors/statinterceptor_test.go @@ -135,9 +135,9 @@ func TestLogDurationWithoutContent(t *testing.T) { } DontLogContentForMethod("foo") - // reset notLoggingContentMethods + // reset ignoreContentMethods t.Cleanup(func() { - notLoggingContentMethods = sync.Map{} + ignoreContentMethods = sync.Map{} }) for _, test := range tests { test := test @@ -198,9 +198,9 @@ func Test_shouldLogContent(t *testing.T) { if tt.setup != nil { tt.setup() } - // reset notLoggingContentMethods + // reset ignoreContentMethods t.Cleanup(func() { - notLoggingContentMethods = sync.Map{} + ignoreContentMethods = sync.Map{} }) set := collection.NewSet() set.AddStr(tt.args.staticNotLoggingContentMethods...) diff --git a/zrpc/server.go b/zrpc/server.go index 4b9963d6..f87e52d5 100644 --- a/zrpc/server.go +++ b/zrpc/server.go @@ -100,7 +100,7 @@ func (rs *RpcServer) Stop() { } // DontLogContentForMethod disable logging content for given method. -// Deprecated: use ServerMiddlewaresConf.NotLoggingContentMethods instead. +// Deprecated: use ServerMiddlewaresConf.IgnoreContentMethods instead. func DontLogContentForMethod(method string) { serverinterceptors.DontLogContentForMethod(method) }