Repository: zeromicro/go-zero
Pull request #3107 (2023-04-08 14:33:42)

refactor: simplify zrpc stat config

codecov[bot]: ## [Codecov](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3107](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (3c09c61) into [master](https://codecov.io/gh/zeromicro/go-zero/commit/e13fd62d38fc12b04f6f598336a9abb626a34631?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (e13fd62) will **not change** coverage.
> The diff coverage is `100.00%`.

> :exclamation: Current head 3c09c61 differs from pull request most recent head bf558f9. Consider uploading reports for the commit bf558f9 to get more accurate results


| [Impacted Files](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [zrpc/config.go](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-enJwYy9jb25maWcuZ28=) | `100.00% <ø> (ø)` | |
| [zrpc/server.go](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-enJwYy9zZXJ2ZXIuZ28=) | `81.48% <ø> (ø)` | |
| [zrpc/internal/rpcserver.go](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-enJwYy9pbnRlcm5hbC9ycGNzZXJ2ZXIuZ28=) | `100.00% <100.00%> (ø)` | |
| [...rpc/internal/serverinterceptors/statinterceptor.go](https://codecov.io/gh/zeromicro/go-zero/pull/3107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-enJwYy9pbnRlcm5hbC9zZXJ2ZXJpbnRlcmNlcHRvcnMvc3RhdGludGVyY2VwdG9yLmdv) | `100.00% <100.00%> (ø)` | |

... and [2 files with indirect coverage changes](https://codecov.io/gh/zeromicro/go-zero/pull/3107/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)


Patch:
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/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)
 }


Test patch:
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...)