From 6a0672b8016f23a4ca956b08c05104ca564785aa Mon Sep 17 00:00:00 2001
From: Nanosk07 <260391798@qq.com>
Date: Fri, 14 Feb 2025 22:56:25 +0800
Subject: [PATCH] fix: SlowThreshold configuration not taking effect (#4654)

---
 .../serverinterceptors/statinterceptor.go     |  7 ++-
 .../statinterceptor_test.go                   | 52 ++++++++-----------
 2 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/zrpc/internal/serverinterceptors/statinterceptor.go b/zrpc/internal/serverinterceptors/statinterceptor.go
index b31bf7ffa77a..874d84b098ab 100644
--- a/zrpc/internal/serverinterceptors/statinterceptor.go
+++ b/zrpc/internal/serverinterceptors/statinterceptor.go
@@ -25,7 +25,7 @@ var (
 
 // StatConf defines the static configuration for stat interceptor.
 type StatConf struct {
-	SlowThreshold        time.Duration `json:",default=500ms"`
+	SlowThreshold        time.Duration `json:",optional"`
 	IgnoreContentMethods []string      `json:",optional"`
 }
 
@@ -63,8 +63,7 @@ func UnaryStatInterceptor(metrics *stat.Metrics, conf StatConf) grpc.UnaryServer
 }
 
 func isSlow(duration, durationThreshold time.Duration) bool {
-	return duration > slowThreshold.Load() ||
-		(durationThreshold > 0 && duration > durationThreshold)
+	return durationThreshold > 0 && duration > durationThreshold
 }
 
 func logDuration(ctx context.Context, method string, req any, duration time.Duration,
@@ -84,7 +83,7 @@ func logDuration(ctx context.Context, method string, req any, duration time.Dura
 		content, err := json.Marshal(req)
 		if err != nil {
 			logx.WithContext(ctx).Errorf("%s - %s", addr, err.Error())
-		} else if duration > slowThreshold.Load() {
+		} else if isSlow(duration, durationThreshold) {
 			logger.Slowf("[RPC] slowcall - %s - %s - %s", addr, method, string(content))
 		} else {
 			logger.Infof("%s - %s - %s", addr, method, string(content))
diff --git a/zrpc/internal/serverinterceptors/statinterceptor_test.go b/zrpc/internal/serverinterceptors/statinterceptor_test.go
index d98178d38d9c..8205155daf99 100644
--- a/zrpc/internal/serverinterceptors/statinterceptor_test.go
+++ b/zrpc/internal/serverinterceptors/statinterceptor_test.go
@@ -43,10 +43,11 @@ func TestLogDuration(t *testing.T) {
 	assert.True(t, len(addrs) > 0)
 
 	tests := []struct {
-		name     string
-		ctx      context.Context
-		req      any
-		duration time.Duration
+		name              string
+		ctx               context.Context
+		req               any
+		duration          time.Duration
+		durationThreshold time.Duration
 	}{
 		{
 			name: "normal",
@@ -59,10 +60,11 @@ func TestLogDuration(t *testing.T) {
 			req:  make(chan lang.PlaceholderType), // not marshalable
 		},
 		{
-			name:     "timeout",
-			ctx:      context.Background(),
-			req:      "foo",
-			duration: time.Second,
+			name:              "timeout",
+			ctx:               context.Background(),
+			req:               "foo",
+			duration:          time.Second,
+			durationThreshold: time.Millisecond * 500,
 		},
 		{
 			name: "timeout",
@@ -86,7 +88,7 @@ func TestLogDuration(t *testing.T) {
 
 			assert.NotPanics(t, func() {
 				logDuration(test.ctx, "foo", test.req, test.duration,
-					collection.NewSet(), 0)
+					collection.NewSet(), test.durationThreshold)
 			})
 		})
 	}
@@ -98,10 +100,11 @@ func TestLogDurationWithoutContent(t *testing.T) {
 	assert.True(t, len(addrs) > 0)
 
 	tests := []struct {
-		name     string
-		ctx      context.Context
-		req      any
-		duration time.Duration
+		name              string
+		ctx               context.Context
+		req               any
+		duration          time.Duration
+		durationThreshold time.Duration
 	}{
 		{
 			name: "normal",
@@ -114,10 +117,11 @@ func TestLogDurationWithoutContent(t *testing.T) {
 			req:  make(chan lang.PlaceholderType), // not marshalable
 		},
 		{
-			name:     "timeout",
-			ctx:      context.Background(),
-			req:      "foo",
-			duration: time.Second,
+			name:              "timeout",
+			ctx:               context.Background(),
+			req:               "foo",
+			duration:          time.Second,
+			durationThreshold: time.Millisecond * 500,
 		},
 		{
 			name: "timeout",
@@ -146,7 +150,7 @@ func TestLogDurationWithoutContent(t *testing.T) {
 
 			assert.NotPanics(t, func() {
 				logDuration(test.ctx, "foo", test.req, test.duration,
-					collection.NewSet(), 0)
+					collection.NewSet(), test.durationThreshold)
 			})
 		})
 	}
@@ -225,7 +229,7 @@ func Test_isSlow(t *testing.T) {
 			args{
 				duration: time.Millisecond * 501,
 			},
-			true,
+			false,
 			nil,
 		},
 		{
@@ -237,16 +241,6 @@ func Test_isSlow(t *testing.T) {
 			true,
 			nil,
 		},
-		{
-			"dynamic",
-			args{
-				duration: time.Millisecond * 200,
-			},
-			true,
-			func() {
-				SetSlowThreshold(time.Millisecond * 100)
-			},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {