From 3a8812f992b9d922566956ea035aaf91b4e1cdd2 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 5 Oct 2022 16:41:31 +0900 Subject: [PATCH 1/2] add test for attributesFromError method Signed-off-by: hlts2 --- .../grpc/interceptor/server/metric/metric.go | 26 +++--- .../interceptor/server/metric/metric_test.go | 82 +++++++++++++++++++ 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/internal/net/grpc/interceptor/server/metric/metric.go b/internal/net/grpc/interceptor/server/metric/metric.go index c1c8edfccb..f77817cdae 100644 --- a/internal/net/grpc/interceptor/server/metric/metric.go +++ b/internal/net/grpc/interceptor/server/metric/metric.go @@ -55,17 +55,7 @@ func MetricInterceptors() (grpc.UnaryServerInterceptor, grpc.StreamServerInterce } record := func(ctx context.Context, method string, err error, latency float64) { - code := codes.OK // default error is success when error is nil - if err != nil { - st, _ := status.FromError(err) - if st != nil { - code = st.Code() - } - } - attrs := []attribute.KeyValue{ - attribute.String(gRPCMethodKeyName, method), - attribute.String(gRPCStatus, code.String()), - } + attrs := attributesFromError(method, err) latencyHistgram.Record(ctx, latency, attrs...) completedRPCCnt.Add(ctx, 1, attrs...) } @@ -83,3 +73,17 @@ func MetricInterceptors() (grpc.UnaryServerInterceptor, grpc.StreamServerInterce return err }, nil } + +func attributesFromError(method string, err error) []attribute.KeyValue { + code := codes.OK // default error is success when error is nil + if err != nil { + st, _ := status.FromError(err) + if st != nil { + code = st.Code() + } + } + return []attribute.KeyValue{ + attribute.String(gRPCMethodKeyName, method), + attribute.String(gRPCStatus, code.String()), + } +} diff --git a/internal/net/grpc/interceptor/server/metric/metric_test.go b/internal/net/grpc/interceptor/server/metric/metric_test.go index f5538e439a..27c75c4a54 100644 --- a/internal/net/grpc/interceptor/server/metric/metric_test.go +++ b/internal/net/grpc/interceptor/server/metric/metric_test.go @@ -14,11 +14,14 @@ package metric import ( + "context" "reflect" "testing" "github.com/vdaas/vald/internal/errors" "github.com/vdaas/vald/internal/net/grpc" + "github.com/vdaas/vald/internal/net/grpc/codes" + "github.com/vdaas/vald/internal/observability/attribute" "github.com/vdaas/vald/internal/test/goleak" ) @@ -92,3 +95,82 @@ func TestMetricInterceptors(t *testing.T) { }) } } + +func Test_attributesFromError(t *testing.T) { + type T = []attribute.KeyValue + type args struct { + method string + err error + } + type want struct { + obj T + } + type test struct { + name string + args args + want want + checkFunc func(w want, obj T) error + beforeFunc func(*testing.T, args) + afterFunc func(*testing.T, args) + } + + defaultCheckFunc := func(w want, obj T) error { + if !reflect.DeepEqual(obj, w.obj) { + return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", obj, w.obj) + } + return nil + } + + tests := []test{ + { + name: "return []attribute.KeyValue when err is nil", + args: args{ + method: "InsertRPC", + err: nil, + }, + want: want{ + obj: []attribute.KeyValue{ + attribute.String(gRPCMethodKeyName, "InsertRPC"), + attribute.String(gRPCStatus, codes.OK.String()), + }, + }, + }, + { + name: "return []attribute.KeyValue when err is not nil", + args: args{ + method: "InsertRPC", + err: context.DeadlineExceeded, + }, + want: want{ + obj: []attribute.KeyValue{ + attribute.String(gRPCMethodKeyName, "InsertRPC"), + attribute.String(gRPCStatus, codes.DeadlineExceeded.String()), + }, + }, + }, + } + + for _, tc := range tests { + test := tc + t.Run(test.name, func(tt *testing.T) { + tt.Parallel() + defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) + if test.beforeFunc != nil { + test.beforeFunc(tt, test.args) + } + if test.afterFunc != nil { + defer test.afterFunc(tt, test.args) + } + + checkFunc := test.checkFunc + if test.checkFunc == nil { + checkFunc = defaultCheckFunc + } + + got := attributesFromError(test.args.method, test.args.err) + if err := checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) + } + }) + } +} From f3f578467444ec826bd95f4c9a7e62ea28310cce Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 5 Oct 2022 16:48:15 +0900 Subject: [PATCH 2/2] add new test to cover other error type Signed-off-by: hlts2 --- .../grpc/interceptor/server/metric/metric_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/net/grpc/interceptor/server/metric/metric_test.go b/internal/net/grpc/interceptor/server/metric/metric_test.go index 27c75c4a54..12f341d781 100644 --- a/internal/net/grpc/interceptor/server/metric/metric_test.go +++ b/internal/net/grpc/interceptor/server/metric/metric_test.go @@ -21,6 +21,7 @@ import ( "github.com/vdaas/vald/internal/errors" "github.com/vdaas/vald/internal/net/grpc" "github.com/vdaas/vald/internal/net/grpc/codes" + "github.com/vdaas/vald/internal/net/grpc/status" "github.com/vdaas/vald/internal/observability/attribute" "github.com/vdaas/vald/internal/test/goleak" ) @@ -148,6 +149,19 @@ func Test_attributesFromError(t *testing.T) { }, }, }, + { + name: "return []attribute.KeyValue when err type is wrapped error", + args: args{ + method: "InsertRPC", + err: status.WrapWithInvalidArgument("Insert API failed", errors.ErrIncompatibleDimensionSize(100, 940)), + }, + want: want{ + obj: []attribute.KeyValue{ + attribute.String(gRPCMethodKeyName, "InsertRPC"), + attribute.String(gRPCStatus, codes.InvalidArgument.String()), + }, + }, + }, } for _, tc := range tests {