fix: the counter was not decremented when an error occurred (#3013)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: yizhenqiang <yizhenqiang@apache.org>
Fixes: #3012
diff --git a/filter/active/filter.go b/filter/active/filter.go
index ce72025..ab070fb 100644
--- a/filter/active/filter.go
+++ b/filter/active/filter.go
@@ -74,6 +74,8 @@
if err != nil {
result.SetError(err)
logger.Errorf("parse dubbo_invoke_start_time to int64 failed")
+ // When err is not nil, use default elapsed value of 1
+ base.EndCount(invoker.GetURL(), inv.MethodName(), 1, false)
return result
}
elapsed := base.CurrentTimeMillis() - startTime
diff --git a/filter/active/filter_test.go b/filter/active/filter_test.go
index 7c4a692..a864d27 100644
--- a/filter/active/filter_test.go
+++ b/filter/active/filter_test.go
@@ -20,6 +20,7 @@
import (
"context"
"errors"
+ "fmt"
"strconv"
"testing"
)
@@ -32,6 +33,7 @@
import (
"dubbo.apache.org/dubbo-go/v3/common"
+ "dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/protocol/base"
"dubbo.apache.org/dubbo-go/v3/protocol/invocation"
"dubbo.apache.org/dubbo-go/v3/protocol/mock"
@@ -40,7 +42,7 @@
func TestFilterInvoke(t *testing.T) {
invoc := invocation.NewRPCInvocation("test", []any{"OK"}, make(map[string]any))
- url, _ := common.NewURL("dubbo://192.168.10.10:20000/com.ikurento.user.UserProvider")
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
filter := activeFilter{}
ctrl := gomock.NewController(t)
defer ctrl.Finish()
@@ -57,7 +59,7 @@
invoc := invocation.NewRPCInvocation("test", []any{"OK"}, map[string]any{
dubboInvokeStartTime: strconv.FormatInt(c-int64(elapsed), 10),
})
- url, _ := common.NewURL("dubbo://192.168.10.10:20000/com.ikurento.user.UserProvider")
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
filter := activeFilter{}
ctrl := gomock.NewController(t)
defer ctrl.Finish()
@@ -81,3 +83,133 @@
assert.True(t, urlStatus.GetLastRequestFailedTimestamp() != int64(0))
assert.True(t, methodStatus.GetLastRequestFailedTimestamp() != int64(0))
}
+
+func TestFilterOnResponseWithDefer(t *testing.T) {
+ base.CleanAllStatus()
+
+ // Test scenario 1: dubboInvokeStartTime is parsed successfully and the result is correct.
+ t.Run("ParseSuccessAndResultSuccess", func(t *testing.T) {
+ defer base.CleanAllStatus()
+
+ c := base.CurrentTimeMillis()
+ invoc := invocation.NewRPCInvocation("test1", []any{"OK"}, map[string]any{
+ dubboInvokeStartTime: strconv.FormatInt(c, 10),
+ })
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
+ filter := activeFilter{}
+ ctrl := gomock.NewController(t)
+ defer ctrl.Finish()
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().GetURL().Return(url).Times(1)
+ rpcResult := &result.RPCResult{}
+
+ filter.OnResponse(context.TODO(), rpcResult, invoker, invoc)
+
+ methodStatus := base.GetMethodStatus(url, "test1")
+ urlStatus := base.GetURLStatus(url)
+
+ assert.Equal(t, int32(1), methodStatus.GetTotal())
+ assert.Equal(t, int32(1), urlStatus.GetTotal())
+ assert.Equal(t, int32(0), methodStatus.GetFailed())
+ assert.Equal(t, int32(0), urlStatus.GetFailed())
+ assert.Equal(t, int32(0), methodStatus.GetSuccessiveRequestFailureCount())
+ assert.Equal(t, int32(0), urlStatus.GetSuccessiveRequestFailureCount())
+ assert.True(t, methodStatus.GetTotalElapsed() >= int64(0))
+ assert.True(t, urlStatus.GetTotalElapsed() >= int64(0))
+ })
+
+ // Test scenario 2: dubboInvokeStartTime is parsed successfully, but the result is incorrect
+ t.Run("ParseSuccessAndResultFailed", func(t *testing.T) {
+ defer base.CleanAllStatus()
+
+ c := base.CurrentTimeMillis()
+ invoc := invocation.NewRPCInvocation("test2", []any{"OK"}, map[string]any{
+ dubboInvokeStartTime: strconv.FormatInt(c, 10),
+ })
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
+ filter := activeFilter{}
+ ctrl := gomock.NewController(t)
+ defer ctrl.Finish()
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().GetURL().Return(url).Times(1)
+ rpcResult := &result.RPCResult{
+ Err: errors.New("test error"),
+ }
+
+ filter.OnResponse(context.TODO(), rpcResult, invoker, invoc)
+
+ methodStatus := base.GetMethodStatus(url, "test2")
+ urlStatus := base.GetURLStatus(url)
+
+ assert.Equal(t, int32(1), methodStatus.GetTotal())
+ assert.Equal(t, int32(1), urlStatus.GetTotal())
+ assert.Equal(t, int32(1), methodStatus.GetFailed())
+ assert.Equal(t, int32(1), urlStatus.GetFailed())
+ assert.Equal(t, int32(1), methodStatus.GetSuccessiveRequestFailureCount())
+ assert.Equal(t, int32(1), urlStatus.GetSuccessiveRequestFailureCount())
+ assert.True(t, methodStatus.GetFailedElapsed() >= int64(0))
+ assert.True(t, urlStatus.GetFailedElapsed() >= int64(0))
+ assert.True(t, urlStatus.GetLastRequestFailedTimestamp() != int64(0))
+ assert.True(t, methodStatus.GetLastRequestFailedTimestamp() != int64(0))
+ })
+
+ // Test Scenario 3: dubboInvokeStartTime parsing failed (non-numeric string)
+ t.Run("ParseFailedWithInvalidString", func(t *testing.T) {
+ defer base.CleanAllStatus()
+
+ invoc := invocation.NewRPCInvocation("test3", []any{"OK"}, map[string]any{
+ dubboInvokeStartTime: "invalid-time",
+ })
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
+ filter := activeFilter{}
+ ctrl := gomock.NewController(t)
+ defer ctrl.Finish()
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().GetURL().Return(url).Times(1)
+ rpcResult := &result.RPCResult{}
+
+ result := filter.OnResponse(context.TODO(), rpcResult, invoker, invoc)
+ assert.NotNil(t, result.Error())
+
+ methodStatus := base.GetMethodStatus(url, "test3")
+ urlStatus := base.GetURLStatus(url)
+
+ // Verification count and status - should use the default duration of 1 and be marked as failed
+ assert.Equal(t, int32(1), methodStatus.GetTotal())
+ assert.Equal(t, int32(1), urlStatus.GetTotal())
+ assert.Equal(t, int32(1), methodStatus.GetFailed())
+ assert.Equal(t, int32(1), urlStatus.GetFailed())
+ assert.Equal(t, int32(1), methodStatus.GetSuccessiveRequestFailureCount())
+ assert.Equal(t, int32(1), urlStatus.GetSuccessiveRequestFailureCount())
+ assert.True(t, methodStatus.GetFailedElapsed() >= int64(1))
+ assert.True(t, urlStatus.GetFailedElapsed() >= int64(1))
+ })
+
+ // Test scenario 4: dubboInvokeStartTime does not exist (use the default value 0)
+ t.Run("ParseFailedWithDefaultValue", func(t *testing.T) {
+ defer base.CleanAllStatus()
+
+ invoc := invocation.NewRPCInvocation("test4", []any{"OK"}, make(map[string]any))
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://%s:%d/com.ikurento.user.UserProvider", constant.LocalHostValue, constant.DefaultPort))
+ filter := activeFilter{}
+ ctrl := gomock.NewController(t)
+ defer ctrl.Finish()
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().GetURL().Return(url).Times(1)
+ rpcResult := &result.RPCResult{}
+
+ filter.OnResponse(context.TODO(), rpcResult, invoker, invoc)
+
+ methodStatus := base.GetMethodStatus(url, "test4")
+ urlStatus := base.GetURLStatus(url)
+
+ assert.Equal(t, int32(1), methodStatus.GetTotal())
+ assert.Equal(t, int32(1), urlStatus.GetTotal())
+ assert.Equal(t, int32(0), methodStatus.GetFailed())
+ assert.Equal(t, int32(0), urlStatus.GetFailed())
+ assert.Equal(t, int32(0), methodStatus.GetSuccessiveRequestFailureCount())
+ assert.Equal(t, int32(0), urlStatus.GetSuccessiveRequestFailureCount())
+ assert.True(t, methodStatus.GetTotalElapsed() > int64(0))
+ assert.True(t, urlStatus.GetTotalElapsed() > int64(0))
+ })
+}
