From 74509731c92f1003b9efa54ceab172bf23baaadc Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sat, 1 May 2021 21:19:03 +0800 Subject: [PATCH 1/8] Add tracing interceptor --- interceptors/tracing/interceptors.go | 59 ++++ interceptors/tracing/interceptors_test.go | 317 ++++++++++++++++++++++ interceptors/tracing/kv/value.go | 171 ++++++++++++ interceptors/tracing/kv/value_test.go | 56 ++++ interceptors/tracing/reporter.go | 65 +++++ interceptors/tracing/tracing.go | 57 ++++ 6 files changed, 725 insertions(+) create mode 100644 interceptors/tracing/interceptors.go create mode 100644 interceptors/tracing/interceptors_test.go create mode 100644 interceptors/tracing/kv/value.go create mode 100644 interceptors/tracing/kv/value_test.go create mode 100644 interceptors/tracing/reporter.go create mode 100644 interceptors/tracing/tracing.go diff --git a/interceptors/tracing/interceptors.go b/interceptors/tracing/interceptors.go new file mode 100644 index 000000000..681d581ae --- /dev/null +++ b/interceptors/tracing/interceptors.go @@ -0,0 +1,59 @@ +package tracing + +import ( + "context" + + "google.golang.org/grpc" + + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors" +) + +type SpanKind string + +const ( + SpanKindServer SpanKind = "server" + SpanKindClient SpanKind = "client" +) + +type reportable struct { + tracer Tracer +} + +func (r *reportable) ServerReporter(ctx context.Context, _ interface{}, typ interceptors.GRPCType, service string, method string) (interceptors.Reporter, context.Context) { + return r.reporter(ctx, service, method, SpanKindServer) +} + +func (r *reportable) ClientReporter(ctx context.Context, _ interface{}, typ interceptors.GRPCType, service string, method string) (interceptors.Reporter, context.Context) { + return r.reporter(ctx, service, method, SpanKindClient) +} + +func (r *reportable) reporter(ctx context.Context, service string, method string, kind SpanKind) (interceptors.Reporter, context.Context) { + newCtx, span := r.tracer.Start(ctx, interceptors.FullMethod(service, method), kind) + reporter := reporter{ctx: newCtx, span: span} + + return &reporter, newCtx +} + +// UnaryClientInterceptor returns a new unary client interceptor that optionally traces the execution of external gRPC calls. +// Tracer will use tags (from tags package) available in current context as fields. +func UnaryClientInterceptor(tracer Tracer) grpc.UnaryClientInterceptor { + return interceptors.UnaryClientInterceptor(&reportable{tracer: tracer}) +} + +// StreamClientInterceptor returns a new streaming client interceptor that optionally traces the execution of external gRPC calls. +// Tracer will use tags (from tags package) available in current context as fields. +func StreamClientInterceptor(tracer Tracer) grpc.StreamClientInterceptor { + return interceptors.StreamClientInterceptor(&reportable{tracer: tracer}) +} + +// UnaryServerInterceptor returns a new unary server interceptors that optionally traces endpoint handling. +// Tracer will use tags (from tags package) available in current context as fields. +func UnaryServerInterceptor(tracer Tracer) grpc.UnaryServerInterceptor { + return interceptors.UnaryServerInterceptor(&reportable{tracer: tracer}) +} + +// StreamServerInterceptor returns a new stream server interceptors that optionally traces endpoint handling. +// Tracer will use tags (from tags package) available in current context as fields. +func StreamServerInterceptor(tracer Tracer) grpc.StreamServerInterceptor { + return interceptors.StreamServerInterceptor(&reportable{tracer: tracer}) +} diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go new file mode 100644 index 000000000..99d592964 --- /dev/null +++ b/interceptors/tracing/interceptors_test.go @@ -0,0 +1,317 @@ +package tracing_test + +import ( + "context" + "io" + "strconv" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tags" + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing" + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" + "github.com/grpc-ecosystem/go-grpc-middleware/v2/testing/testpb" +) + +var ( + id int64 = 0 + traceIDHeaderKey = "traceid" + spanIDHeaderKey = "spanid" +) + +func extractFromContext(ctx context.Context, kind tracing.SpanKind) *mockSpan { + var m metadata.MD + if kind == tracing.SpanKindClient { + m, _ = metadata.FromOutgoingContext(ctx) + } else { + m, _ = metadata.FromIncomingContext(ctx) + } + + traceIDValues := m.Get(traceIDHeaderKey) + if len(traceIDValues) == 0 { + return nil + } + spanIDValues := m.Get(spanIDHeaderKey) + if len(spanIDValues) == 0 { + return nil + } + + return &mockSpan{ + traceID: traceIDValues[0], + spanID: spanIDValues[0], + } +} + +func injectWithContext(ctx context.Context, span *mockSpan, kind tracing.SpanKind) context.Context { + var m metadata.MD + if kind == tracing.SpanKindClient { + m, _ = metadata.FromOutgoingContext(ctx) + } else { + m, _ = metadata.FromIncomingContext(ctx) + } + m = m.Copy() + + m.Set(traceIDHeaderKey, span.traceID) + m.Set(spanIDHeaderKey, span.spanID) + + ctx = metadata.NewOutgoingContext(ctx, m) + return ctx +} + +func genID() string { + return strconv.FormatInt(atomic.AddInt64(&id, 1), 10) +} + +// Implements Tracker +type mockTracer struct { + spanStore map[string]*mockSpan +} + +func (t *mockTracer) ListSpan(kind tracing.SpanKind) []*mockSpan { + var spans []*mockSpan + for _, v := range t.spanStore { + if v.kind == kind { + spans = append(spans, v) + } + } + return spans +} + +func (t *mockTracer) Reset() { + t.spanStore = make(map[string]*mockSpan) +} + +func newMockTracer() *mockTracer { + return &mockTracer{ + spanStore: make(map[string]*mockSpan), + } +} + +func (t *mockTracer) Start(ctx context.Context, spanName string, kind tracing.SpanKind) (context.Context, tracing.Span) { + span := mockSpan{ + spanID: genID(), + name: spanName, + kind: kind, + statusCode: codes.OK, + } + + // parentSpan := spanFromContext(ctx) + parentSpan := extractFromContext(ctx, kind) + if parentSpan != nil { + // Fetch span from context as parent span + span.traceID = parentSpan.traceID + span.parentSpanID = parentSpan.spanID + } else { + span.traceID = genID() + } + + t.spanStore[span.spanID] = &span + + // ctx = contextWithSpan(ctx, &span) + if kind == tracing.SpanKindClient { + ctx = injectWithContext(ctx, &span, kind) + } + return ctx, &span +} + +// Implements Span +type mockSpan struct { + traceID string + spanID string + parentSpanID string + + name string + kind tracing.SpanKind + end bool + + statusCode codes.Code + statusMessage string + + msgSendCounter int + msgReceivedCounter int + eventNameList []string + attributesList [][]kv.KeyValue +} + +func (s *mockSpan) SetAttributes(attrs ...kv.KeyValue) { + s.attributesList = append(s.attributesList, attrs) +} + +func (s *mockSpan) End() { + s.end = true +} + +func (s *mockSpan) SetStatus(code codes.Code, message string) { + s.statusCode = code + s.statusMessage = message +} + +func (s *mockSpan) AddEvent(name string, attrs ...kv.KeyValue) { + s.eventNameList = append(s.eventNameList, name) + + for _, v := range attrs { + switch v { + case tracing.RPCMessageTypeSent: + s.msgSendCounter++ + case tracing.RPCMessageTypeReceived: + s.msgReceivedCounter++ + } + } +} + +type tracingSuite struct { + *testpb.InterceptorTestSuite + tracer *mockTracer +} + +func (s *tracingSuite) BeforeTest(suiteName, testName string) { + s.tracer.Reset() +} + +func (s *tracingSuite) TestPing() { + method := "/testing.testpb.v1.TestService/Ping" + errorMethod := "/testing.testpb.v1.TestService/PingError" + t := s.T() + + testCases := []struct { + name string + error bool + errorMessage string + }{ + { + name: "OK", + error: false, + }, + { + name: "invalid argument error", + error: true, + errorMessage: "Userspace error.", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s.tracer.Reset() + + var err error + if tc.error { + req := &testpb.PingErrorRequest{ErrorCodeReturned: uint32(codes.InvalidArgument)} + _, err = s.Client.PingError(s.SimpleCtx(), req) + } else { + req := &testpb.PingRequest{Value: "something"} + _, err = s.Client.Ping(s.SimpleCtx(), req) + } + if tc.error { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + clientSpans := s.tracer.ListSpan(tracing.SpanKindClient) + serverSpans := s.tracer.ListSpan(tracing.SpanKindServer) + require.Len(t, clientSpans, 1) + require.Len(t, serverSpans, 1) + + clientSpan := clientSpans[0] + assert.True(t, clientSpan.end) + assert.Equal(t, 1, clientSpan.msgSendCounter) + assert.Equal(t, 1, clientSpan.msgReceivedCounter) + assert.Equal(t, []string{"message", "message"}, clientSpan.eventNameList) + + serverSpan := serverSpans[0] + assert.True(t, serverSpan.end) + assert.Equal(t, 1, serverSpan.msgSendCounter) + assert.Equal(t, 1, serverSpan.msgReceivedCounter) + assert.Equal(t, []string{"message", "message"}, serverSpan.eventNameList) + + assert.Equal(t, clientSpan.traceID, serverSpan.traceID) + assert.Equal(t, clientSpan.spanID, serverSpan.parentSpanID) + + if tc.error { + assert.Equal(t, codes.InvalidArgument, clientSpan.statusCode) + assert.Equal(t, tc.errorMessage, clientSpan.statusMessage) + assert.Equal(t, errorMethod, clientSpan.name) + assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, clientSpan.attributesList) + + assert.Equal(t, errorMethod, serverSpan.name) + assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, serverSpan.attributesList) + } else { + assert.Equal(t, codes.OK, clientSpan.statusCode) + assert.Equal(t, method, clientSpan.name) + assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, clientSpan.attributesList) + + assert.Equal(t, method, serverSpan.name) + assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, serverSpan.attributesList) + } + }) + } +} + +func (s *tracingSuite) TestPingList() { + t := s.T() + method := "/testing.testpb.v1.TestService/PingList" + + stream, err := s.Client.PingList(s.SimpleCtx(), &testpb.PingListRequest{Value: "something"}) + require.NoError(t, err) + + for { + _, err := stream.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + } + + clientSpans := s.tracer.ListSpan(tracing.SpanKindClient) + serverSpans := s.tracer.ListSpan(tracing.SpanKindServer) + require.Len(t, clientSpans, 1) + require.Len(t, serverSpans, 1) + + clientSpan := clientSpans[0] + assert.True(t, clientSpan.end) + assert.Equal(t, 1, clientSpan.msgSendCounter) + assert.Equal(t, testpb.ListResponseCount+1, clientSpan.msgReceivedCounter) + assert.Equal(t, codes.OK, clientSpan.statusCode) + assert.Equal(t, method, clientSpan.name) + + serverSpan := serverSpans[0] + assert.True(t, serverSpan.end) + assert.Equal(t, testpb.ListResponseCount, serverSpan.msgSendCounter) + assert.Equal(t, 1, serverSpan.msgReceivedCounter) + assert.Equal(t, codes.OK, serverSpan.statusCode) + assert.Equal(t, method, serverSpan.name) +} + +func TestSuite(t *testing.T) { + tracer := newMockTracer() + + s := tracingSuite{ + InterceptorTestSuite: &testpb.InterceptorTestSuite{ + TestService: &testpb.TestPingService{T: t}, + }, + tracer: tracer, + } + s.InterceptorTestSuite.ClientOpts = []grpc.DialOption{ + grpc.WithUnaryInterceptor(tracing.UnaryClientInterceptor(tracer)), + grpc.WithStreamInterceptor(tracing.StreamClientInterceptor(tracer)), + } + s.InterceptorTestSuite.ServerOpts = []grpc.ServerOption{ + grpc.ChainUnaryInterceptor( + tags.UnaryServerInterceptor(tags.WithFieldExtractor(tags.CodeGenRequestFieldExtractor)), + tracing.UnaryServerInterceptor(tracer), + ), + grpc.ChainStreamInterceptor( + tags.StreamServerInterceptor(tags.WithFieldExtractor(tags.CodeGenRequestFieldExtractor)), + tracing.StreamServerInterceptor(tracer), + ), + } + + suite.Run(t, &s) +} diff --git a/interceptors/tracing/kv/value.go b/interceptors/tracing/kv/value.go new file mode 100644 index 000000000..224f95070 --- /dev/null +++ b/interceptors/tracing/kv/value.go @@ -0,0 +1,171 @@ +package kv + +import ( + "math" +) + +// KeyValue holds a key and value pair. +type KeyValue struct { + Key Key + Value Value +} + +type Key string + +// Bool creates a KeyValue instance with a BOOL Value. +// +// If creating both key and a bool value at the same time, then +// instead of calling kv.Key(name).Bool(value) consider using a +// convenience function provided by the api/key package - +// key.Bool(name, value). +func (k Key) Bool(v bool) KeyValue { + return KeyValue{ + Key: k, + Value: boolValue(v), + } +} + +// Int64 creates a KeyValue instance with an INT64 Value. +// +// If creating both key and an int64 value at the same time, then +// instead of calling kv.Key(name).Int64(value) consider using a +// convenience function provided by the api/key package - +// key.Int64(name, value). +func (k Key) Int64(v int64) KeyValue { + return KeyValue{ + Key: k, + Value: int64Value(v), + } +} + +// Float64 creates a KeyValue instance with a FLOAT64 Value. +// +// If creating both key and a float64 value at the same time, then +// instead of calling kv.Key(name).Float64(value) consider using a +// convenience function provided by the api/key package - +// key.Float64(name, value). +func (k Key) Float64(v float64) KeyValue { + return KeyValue{ + Key: k, + Value: float64Value(v), + } +} + +// String creates a KeyValue instance with a STRING Value. +// +// If creating both key and a string value at the same time, then +// instead of calling kv.Key(name).String(value) consider using a +// convenience function provided by the api/key package - +// key.String(name, value). +func (k Key) String(v string) KeyValue { + return KeyValue{ + Key: k, + Value: stringValue(v), + } +} + +// Int creates a KeyValue instance with either an INT32 or an INT64 +// Value, depending on whether the int type is 32 or 64 bits wide. +// +// If creating both key and an int value at the same time, then +// instead of calling kv.Key(name).Int(value) consider using a +// convenience function provided by the api/key package - +// key.Int(name, value). +func (k Key) Int(v int) KeyValue { + return KeyValue{ + Key: k, + Value: intValue(v), + } +} + +// ValueType describes the type of the data Value holds. +type ValueType int + +const ( + INVALID ValueType = iota // No value. + // BOOL is a boolean Type Value. + BOOL + // INT64 is a 64-bit signed integral Type Value. + INT64 + // FLOAT64 is a 64-bit floating point Type Value. + FLOAT64 + // STRING is a string Type Value. + STRING +) + +type Value struct { + vtype ValueType + numeric uint64 + stringly string +} + +func boolTowRaw(b bool) uint64 { + if b { + return 1 + } + return 0 +} + +func rawToBool(r uint64) bool { + return r != 0 +} + +func boolValue(v bool) Value { + return Value{ + vtype: BOOL, + numeric: boolTowRaw(v), + } +} + +func int64Value(v int64) Value { + return Value{ + vtype: INT64, + numeric: uint64(v), + } +} + +func float64Value(v float64) Value { + return Value{ + vtype: FLOAT64, + numeric: math.Float64bits(v), + } +} + +func stringValue(v string) Value { + return Value{ + vtype: STRING, + stringly: v, + } +} + +// intValue creates an INT64 Value. +func intValue(v int) Value { + return int64Value(int64(v)) +} + +func (v Value) AsBool() bool { + return rawToBool(v.numeric) +} + +// AsInt64 returns the int64 value. Make sure that the Value's type is +// INT64. +func (v Value) AsInt64() int64 { + return int64(v.numeric) +} + +// AsFloat64 returns the float64 value. Make sure that the Value's +// type is FLOAT64. +func (v Value) AsFloat64() float64 { + return math.Float64frombits(v.numeric) +} + +// AsString returns the string value. Make sure that the Value's type +// is STRING. +func (v Value) AsString() string { + return v.stringly +} + +// Type returns a type of the Value. +func (v Value) Type() ValueType { + return v.vtype +} diff --git a/interceptors/tracing/kv/value_test.go b/interceptors/tracing/kv/value_test.go new file mode 100644 index 000000000..34c48687b --- /dev/null +++ b/interceptors/tracing/kv/value_test.go @@ -0,0 +1,56 @@ +package kv + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestKey(t *testing.T) { + testCases := []struct { + name string + keyValue KeyValue + expectedValue interface{} + }{ + { + name: "true", + keyValue: Key("bool").Bool(true), + expectedValue: true, + }, + { + name: "false", + keyValue: Key("bool").Bool(false), + expectedValue: false, + }, + { + name: "int64", + keyValue: Key("int64").Int64(43), + expectedValue: int64(43), + }, + { + name: "float64", + keyValue: Key("float64").Float64(43), + expectedValue: float64(43), + }, + { + name: "string", + keyValue: Key("string").String("foo"), + expectedValue: "foo", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + switch tc.keyValue.Value.Type() { + case BOOL: + assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsBool()) + case INT64: + assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsInt64()) + case FLOAT64: + assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsFloat64()) + case STRING: + assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsString()) + } + }) + } +} diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go new file mode 100644 index 000000000..d8b5df61d --- /dev/null +++ b/interceptors/tracing/reporter.go @@ -0,0 +1,65 @@ +package tracing + +import ( + "context" + "io" + "time" + + "github.com/golang/protobuf/proto" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" +) + +type reporter struct { + ctx context.Context + span Span + + receivedMessageID int + sentMessageID int +} + +func (o *reporter) PostCall(err error, _ time.Duration) { + // Finish span. + if err != nil && err != io.EOF { + s, _ := status.FromError(err) + o.span.SetStatus(s.Code(), s.Message()) + o.span.SetAttributes(statusCodeAttr(s.Code())) + } else { + o.span.SetAttributes(statusCodeAttr(codes.OK)) + } + o.span.End() +} + +func (o *reporter) PostMsgSend(payload interface{}, err error, d time.Duration) { + o.sentMessageID++ + + addEvent(o.span, RPCMessageTypeSent, o.sentMessageID, payload) +} + +func (o *reporter) PostMsgReceive(payload interface{}, err error, d time.Duration) { + o.receivedMessageID++ + + addEvent(o.span, RPCMessageTypeReceived, o.receivedMessageID, payload) +} + +func addEvent(span Span, messageType kv.KeyValue, messageID int, payload interface{}) { + if p, ok := payload.(proto.Message); ok { + span.AddEvent("message", + messageType, + rpcMessageIDKey.Int(messageID), + rpcMessageUncompressedSizeKey.Int(proto.Size(p)), + ) + } else { + span.AddEvent("message", + messageType, + rpcMessageIDKey.Int(messageID), + ) + } +} + +// statusCodeAttr returns status code attribute based on given gRPC code +func statusCodeAttr(c codes.Code) kv.KeyValue { + return grpcStatusCodeKey.Int64(int64(c)) +} \ No newline at end of file diff --git a/interceptors/tracing/tracing.go b/interceptors/tracing/tracing.go new file mode 100644 index 000000000..3415ecce3 --- /dev/null +++ b/interceptors/tracing/tracing.go @@ -0,0 +1,57 @@ +package tracing + +import ( + "context" + + "google.golang.org/grpc/codes" + + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" +) + +const ( + // Type of message transmitted or received. + rpcMessageTypeKey = kv.Key("message.type") + + // Identifier of message transmitted or received. + rpcMessageIDKey = kv.Key("message.id") + + // The uncompressed size of the message transmitted or received in + // bytes. + rpcMessageUncompressedSizeKey = kv.Key("message.uncompressed_size") + + // grpcStatusCodeKey is convention for numeric status code of a gRPC request. + grpcStatusCodeKey = kv.Key("rpc.grpc.status_code") +) + +var ( + RPCMessageTypeSent = rpcMessageTypeKey.String("SENT") + RPCMessageTypeReceived = rpcMessageTypeKey.String("RECEIVED") +) + +type Tracer interface { + Start(ctx context.Context, spanName string, kind SpanKind) (context.Context, Span) +} + +type Span interface { + // End completes the span. No updates are allowed to span after it + // ends. The only exception is setting status of the span. + End() + + // SetStatus sets the status of the span in the form of a code + // and a message. SetStatus overrides the value of previous + // calls to SetStatus on the Span. + // + // The default span status is OK, so it is not necessary to + // explicitly set an OK status on successful Spans unless it + // is to add an OK message or to override a previous status on the Span. + SetStatus(code codes.Code, msg string) + + // AddEvent adds an event to the span. + // Middleware will call it while receiving or sending messages. + AddEvent(name string, attrs ...kv.KeyValue) + + // SetAttributes sets kv as attributes of the Span. If a key from kv + // already exists for an attribute of the Span it should be overwritten with + // the value contained in kv. + SetAttributes(attrs ...kv.KeyValue) +} From be1e9916b6a7a37a94368d9263909652e1d56b9b Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 8 Aug 2021 23:46:40 +0800 Subject: [PATCH 2/8] Fix CI --- interceptors/tracing/interceptors.go | 3 ++ interceptors/tracing/interceptors_test.go | 63 ++++++++++++----------- interceptors/tracing/kv/value.go | 3 ++ interceptors/tracing/kv/value_test.go | 3 ++ interceptors/tracing/reporter.go | 7 ++- interceptors/tracing/tracing.go | 19 ++++--- 6 files changed, 58 insertions(+), 40 deletions(-) diff --git a/interceptors/tracing/interceptors.go b/interceptors/tracing/interceptors.go index 681d581ae..a30fd3d27 100644 --- a/interceptors/tracing/interceptors.go +++ b/interceptors/tracing/interceptors.go @@ -1,3 +1,6 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package tracing import ( diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go index 99d592964..e09a499cb 100644 --- a/interceptors/tracing/interceptors_test.go +++ b/interceptors/tracing/interceptors_test.go @@ -1,3 +1,6 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package tracing_test import ( @@ -6,14 +9,14 @@ import ( "strconv" "sync/atomic" "testing" - + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" - + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tags" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" @@ -33,7 +36,7 @@ func extractFromContext(ctx context.Context, kind tracing.SpanKind) *mockSpan { } else { m, _ = metadata.FromIncomingContext(ctx) } - + traceIDValues := m.Get(traceIDHeaderKey) if len(traceIDValues) == 0 { return nil @@ -42,7 +45,7 @@ func extractFromContext(ctx context.Context, kind tracing.SpanKind) *mockSpan { if len(spanIDValues) == 0 { return nil } - + return &mockSpan{ traceID: traceIDValues[0], spanID: spanIDValues[0], @@ -57,10 +60,10 @@ func injectWithContext(ctx context.Context, span *mockSpan, kind tracing.SpanKin m, _ = metadata.FromIncomingContext(ctx) } m = m.Copy() - + m.Set(traceIDHeaderKey, span.traceID) m.Set(spanIDHeaderKey, span.spanID) - + ctx = metadata.NewOutgoingContext(ctx, m) return ctx } @@ -101,7 +104,7 @@ func (t *mockTracer) Start(ctx context.Context, spanName string, kind tracing.Sp kind: kind, statusCode: codes.OK, } - + // parentSpan := spanFromContext(ctx) parentSpan := extractFromContext(ctx, kind) if parentSpan != nil { @@ -111,9 +114,9 @@ func (t *mockTracer) Start(ctx context.Context, spanName string, kind tracing.Sp } else { span.traceID = genID() } - + t.spanStore[span.spanID] = &span - + // ctx = contextWithSpan(ctx, &span) if kind == tracing.SpanKindClient { ctx = injectWithContext(ctx, &span, kind) @@ -126,14 +129,14 @@ type mockSpan struct { traceID string spanID string parentSpanID string - + name string kind tracing.SpanKind end bool - + statusCode codes.Code statusMessage string - + msgSendCounter int msgReceivedCounter int eventNameList []string @@ -155,7 +158,7 @@ func (s *mockSpan) SetStatus(code codes.Code, message string) { func (s *mockSpan) AddEvent(name string, attrs ...kv.KeyValue) { s.eventNameList = append(s.eventNameList, name) - + for _, v := range attrs { switch v { case tracing.RPCMessageTypeSent: @@ -179,7 +182,7 @@ func (s *tracingSuite) TestPing() { method := "/testing.testpb.v1.TestService/Ping" errorMethod := "/testing.testpb.v1.TestService/PingError" t := s.T() - + testCases := []struct { name string error bool @@ -195,11 +198,11 @@ func (s *tracingSuite) TestPing() { errorMessage: "Userspace error.", }, } - + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { s.tracer.Reset() - + var err error if tc.error { req := &testpb.PingErrorRequest{ErrorCodeReturned: uint32(codes.InvalidArgument)} @@ -213,40 +216,40 @@ func (s *tracingSuite) TestPing() { } else { require.NoError(t, err) } - + clientSpans := s.tracer.ListSpan(tracing.SpanKindClient) serverSpans := s.tracer.ListSpan(tracing.SpanKindServer) require.Len(t, clientSpans, 1) require.Len(t, serverSpans, 1) - + clientSpan := clientSpans[0] assert.True(t, clientSpan.end) assert.Equal(t, 1, clientSpan.msgSendCounter) assert.Equal(t, 1, clientSpan.msgReceivedCounter) assert.Equal(t, []string{"message", "message"}, clientSpan.eventNameList) - + serverSpan := serverSpans[0] assert.True(t, serverSpan.end) assert.Equal(t, 1, serverSpan.msgSendCounter) assert.Equal(t, 1, serverSpan.msgReceivedCounter) assert.Equal(t, []string{"message", "message"}, serverSpan.eventNameList) - + assert.Equal(t, clientSpan.traceID, serverSpan.traceID) assert.Equal(t, clientSpan.spanID, serverSpan.parentSpanID) - + if tc.error { assert.Equal(t, codes.InvalidArgument, clientSpan.statusCode) assert.Equal(t, tc.errorMessage, clientSpan.statusMessage) assert.Equal(t, errorMethod, clientSpan.name) assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, clientSpan.attributesList) - + assert.Equal(t, errorMethod, serverSpan.name) assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, serverSpan.attributesList) } else { assert.Equal(t, codes.OK, clientSpan.statusCode) assert.Equal(t, method, clientSpan.name) assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, clientSpan.attributesList) - + assert.Equal(t, method, serverSpan.name) assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, serverSpan.attributesList) } @@ -257,10 +260,10 @@ func (s *tracingSuite) TestPing() { func (s *tracingSuite) TestPingList() { t := s.T() method := "/testing.testpb.v1.TestService/PingList" - + stream, err := s.Client.PingList(s.SimpleCtx(), &testpb.PingListRequest{Value: "something"}) require.NoError(t, err) - + for { _, err := stream.Recv() if err == io.EOF { @@ -268,19 +271,19 @@ func (s *tracingSuite) TestPingList() { } require.NoError(t, err) } - + clientSpans := s.tracer.ListSpan(tracing.SpanKindClient) serverSpans := s.tracer.ListSpan(tracing.SpanKindServer) require.Len(t, clientSpans, 1) require.Len(t, serverSpans, 1) - + clientSpan := clientSpans[0] assert.True(t, clientSpan.end) assert.Equal(t, 1, clientSpan.msgSendCounter) assert.Equal(t, testpb.ListResponseCount+1, clientSpan.msgReceivedCounter) assert.Equal(t, codes.OK, clientSpan.statusCode) assert.Equal(t, method, clientSpan.name) - + serverSpan := serverSpans[0] assert.True(t, serverSpan.end) assert.Equal(t, testpb.ListResponseCount, serverSpan.msgSendCounter) @@ -291,7 +294,7 @@ func (s *tracingSuite) TestPingList() { func TestSuite(t *testing.T) { tracer := newMockTracer() - + s := tracingSuite{ InterceptorTestSuite: &testpb.InterceptorTestSuite{ TestService: &testpb.TestPingService{T: t}, @@ -312,6 +315,6 @@ func TestSuite(t *testing.T) { tracing.StreamServerInterceptor(tracer), ), } - + suite.Run(t, &s) } diff --git a/interceptors/tracing/kv/value.go b/interceptors/tracing/kv/value.go index 224f95070..82e37caca 100644 --- a/interceptors/tracing/kv/value.go +++ b/interceptors/tracing/kv/value.go @@ -1,3 +1,6 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package kv import ( diff --git a/interceptors/tracing/kv/value_test.go b/interceptors/tracing/kv/value_test.go index 34c48687b..29edfc657 100644 --- a/interceptors/tracing/kv/value_test.go +++ b/interceptors/tracing/kv/value_test.go @@ -1,3 +1,6 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package kv import ( diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go index d8b5df61d..231e8ff9a 100644 --- a/interceptors/tracing/reporter.go +++ b/interceptors/tracing/reporter.go @@ -1,3 +1,6 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package tracing import ( @@ -5,9 +8,9 @@ import ( "io" "time" - "github.com/golang/protobuf/proto" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" ) @@ -62,4 +65,4 @@ func addEvent(span Span, messageType kv.KeyValue, messageID int, payload interfa // statusCodeAttr returns status code attribute based on given gRPC code func statusCodeAttr(c codes.Code) kv.KeyValue { return grpcStatusCodeKey.Int64(int64(c)) -} \ No newline at end of file +} diff --git a/interceptors/tracing/tracing.go b/interceptors/tracing/tracing.go index 3415ecce3..84f71c46a 100644 --- a/interceptors/tracing/tracing.go +++ b/interceptors/tracing/tracing.go @@ -1,24 +1,27 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + package tracing import ( "context" - + "google.golang.org/grpc/codes" - + "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" ) const ( // Type of message transmitted or received. rpcMessageTypeKey = kv.Key("message.type") - + // Identifier of message transmitted or received. rpcMessageIDKey = kv.Key("message.id") - + // The uncompressed size of the message transmitted or received in // bytes. rpcMessageUncompressedSizeKey = kv.Key("message.uncompressed_size") - + // grpcStatusCodeKey is convention for numeric status code of a gRPC request. grpcStatusCodeKey = kv.Key("rpc.grpc.status_code") ) @@ -36,7 +39,7 @@ type Span interface { // End completes the span. No updates are allowed to span after it // ends. The only exception is setting status of the span. End() - + // SetStatus sets the status of the span in the form of a code // and a message. SetStatus overrides the value of previous // calls to SetStatus on the Span. @@ -45,11 +48,11 @@ type Span interface { // explicitly set an OK status on successful Spans unless it // is to add an OK message or to override a previous status on the Span. SetStatus(code codes.Code, msg string) - + // AddEvent adds an event to the span. // Middleware will call it while receiving or sending messages. AddEvent(name string, attrs ...kv.KeyValue) - + // SetAttributes sets kv as attributes of the Span. If a key from kv // already exists for an attribute of the Span it should be overwritten with // the value contained in kv. From d0650ba634aa1f36f500077dfdd443280f90f957 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 25 Aug 2021 17:24:44 +0800 Subject: [PATCH 3/8] Follow dependencies changes --- interceptors/tracing/interceptors.go | 33 +++++++++-------------- interceptors/tracing/interceptors_test.go | 3 --- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/interceptors/tracing/interceptors.go b/interceptors/tracing/interceptors.go index a30fd3d27..e2f91d768 100644 --- a/interceptors/tracing/interceptors.go +++ b/interceptors/tracing/interceptors.go @@ -18,45 +18,38 @@ const ( SpanKindClient SpanKind = "client" ) -type reportable struct { - tracer Tracer -} - -func (r *reportable) ServerReporter(ctx context.Context, _ interface{}, typ interceptors.GRPCType, service string, method string) (interceptors.Reporter, context.Context) { - return r.reporter(ctx, service, method, SpanKindServer) -} - -func (r *reportable) ClientReporter(ctx context.Context, _ interface{}, typ interceptors.GRPCType, service string, method string) (interceptors.Reporter, context.Context) { - return r.reporter(ctx, service, method, SpanKindClient) -} - -func (r *reportable) reporter(ctx context.Context, service string, method string, kind SpanKind) (interceptors.Reporter, context.Context) { - newCtx, span := r.tracer.Start(ctx, interceptors.FullMethod(service, method), kind) - reporter := reporter{ctx: newCtx, span: span} +func reportable(tracer Tracer) interceptors.CommonReportableFunc { + return func(ctx context.Context, c interceptors.CallMeta, isClient bool) (interceptors.Reporter, context.Context) { + kind := SpanKindServer + if isClient { + kind = SpanKindClient + } - return &reporter, newCtx + newCtx, span := tracer.Start(ctx, c.FullMethod(), kind) + return &reporter{ctx: newCtx, span: span}, newCtx + } } // UnaryClientInterceptor returns a new unary client interceptor that optionally traces the execution of external gRPC calls. // Tracer will use tags (from tags package) available in current context as fields. func UnaryClientInterceptor(tracer Tracer) grpc.UnaryClientInterceptor { - return interceptors.UnaryClientInterceptor(&reportable{tracer: tracer}) + return interceptors.UnaryClientInterceptor(reportable(tracer)) } // StreamClientInterceptor returns a new streaming client interceptor that optionally traces the execution of external gRPC calls. // Tracer will use tags (from tags package) available in current context as fields. func StreamClientInterceptor(tracer Tracer) grpc.StreamClientInterceptor { - return interceptors.StreamClientInterceptor(&reportable{tracer: tracer}) + return interceptors.StreamClientInterceptor(reportable(tracer)) } // UnaryServerInterceptor returns a new unary server interceptors that optionally traces endpoint handling. // Tracer will use tags (from tags package) available in current context as fields. func UnaryServerInterceptor(tracer Tracer) grpc.UnaryServerInterceptor { - return interceptors.UnaryServerInterceptor(&reportable{tracer: tracer}) + return interceptors.UnaryServerInterceptor(reportable(tracer)) } // StreamServerInterceptor returns a new stream server interceptors that optionally traces endpoint handling. // Tracer will use tags (from tags package) available in current context as fields. func StreamServerInterceptor(tracer Tracer) grpc.StreamServerInterceptor { - return interceptors.StreamServerInterceptor(&reportable{tracer: tracer}) + return interceptors.StreamServerInterceptor(reportable(tracer)) } diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go index e09a499cb..3f25e4eac 100644 --- a/interceptors/tracing/interceptors_test.go +++ b/interceptors/tracing/interceptors_test.go @@ -17,7 +17,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" - "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tags" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" "github.com/grpc-ecosystem/go-grpc-middleware/v2/testing/testpb" @@ -307,11 +306,9 @@ func TestSuite(t *testing.T) { } s.InterceptorTestSuite.ServerOpts = []grpc.ServerOption{ grpc.ChainUnaryInterceptor( - tags.UnaryServerInterceptor(tags.WithFieldExtractor(tags.CodeGenRequestFieldExtractor)), tracing.UnaryServerInterceptor(tracer), ), grpc.ChainStreamInterceptor( - tags.StreamServerInterceptor(tags.WithFieldExtractor(tags.CodeGenRequestFieldExtractor)), tracing.StreamServerInterceptor(tracer), ), } From 822802af57d7df9747f123c7df09ae91c9b6ec50 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 25 Aug 2021 17:31:35 +0800 Subject: [PATCH 4/8] Fix comments --- interceptors/tracing/reporter.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go index 231e8ff9a..a80298431 100644 --- a/interceptors/tracing/reporter.go +++ b/interceptors/tracing/reporter.go @@ -37,13 +37,11 @@ func (o *reporter) PostCall(err error, _ time.Duration) { func (o *reporter) PostMsgSend(payload interface{}, err error, d time.Duration) { o.sentMessageID++ - addEvent(o.span, RPCMessageTypeSent, o.sentMessageID, payload) } func (o *reporter) PostMsgReceive(payload interface{}, err error, d time.Duration) { o.receivedMessageID++ - addEvent(o.span, RPCMessageTypeReceived, o.receivedMessageID, payload) } From 7854af0f0b454e1a3c4de17cda70a15d47e023e5 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 31 Aug 2021 11:20:38 +0800 Subject: [PATCH 5/8] Avoid redundant else and return early --- interceptors/tracing/reporter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go index a80298431..b54fc45eb 100644 --- a/interceptors/tracing/reporter.go +++ b/interceptors/tracing/reporter.go @@ -52,12 +52,12 @@ func addEvent(span Span, messageType kv.KeyValue, messageID int, payload interfa rpcMessageIDKey.Int(messageID), rpcMessageUncompressedSizeKey.Int(proto.Size(p)), ) - } else { - span.AddEvent("message", - messageType, - rpcMessageIDKey.Int(messageID), - ) + return } + span.AddEvent("message", + messageType, + rpcMessageIDKey.Int(messageID), + ) } // statusCodeAttr returns status code attribute based on given gRPC code From 135fdbd2fa40f425dfae57269632022a2e138918 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 1 Sep 2021 11:01:32 +0800 Subject: [PATCH 6/8] Update interceptors/tracing/interceptors_test.go Co-authored-by: Yash Sharma --- interceptors/tracing/interceptors_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go index 3f25e4eac..9989f34c8 100644 --- a/interceptors/tracing/interceptors_test.go +++ b/interceptors/tracing/interceptors_test.go @@ -104,7 +104,6 @@ func (t *mockTracer) Start(ctx context.Context, spanName string, kind tracing.Sp statusCode: codes.OK, } - // parentSpan := spanFromContext(ctx) parentSpan := extractFromContext(ctx, kind) if parentSpan != nil { // Fetch span from context as parent span @@ -115,8 +114,6 @@ func (t *mockTracer) Start(ctx context.Context, spanName string, kind tracing.Sp } t.spanStore[span.spanID] = &span - - // ctx = contextWithSpan(ctx, &span) if kind == tracing.SpanKindClient { ctx = injectWithContext(ctx, &span, kind) } From 9202a1567c91319d19b44adbf2814b8d80914baa Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 14 Apr 2022 22:30:50 +0800 Subject: [PATCH 7/8] Remove kv package --- interceptors/tracing/interceptors_test.go | 15 +- interceptors/tracing/kv.go | 21 +++ interceptors/tracing/kv/value.go | 174 ---------------------- interceptors/tracing/kv/value_test.go | 59 -------- interceptors/tracing/reporter.go | 14 +- interceptors/tracing/tracing.go | 18 +-- 6 files changed, 42 insertions(+), 259 deletions(-) create mode 100644 interceptors/tracing/kv.go delete mode 100644 interceptors/tracing/kv/value.go delete mode 100644 interceptors/tracing/kv/value_test.go diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go index 9989f34c8..f4e91cf52 100644 --- a/interceptors/tracing/interceptors_test.go +++ b/interceptors/tracing/interceptors_test.go @@ -18,7 +18,6 @@ import ( "google.golang.org/grpc/metadata" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing" - "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" "github.com/grpc-ecosystem/go-grpc-middleware/v2/testing/testpb" ) @@ -136,10 +135,10 @@ type mockSpan struct { msgSendCounter int msgReceivedCounter int eventNameList []string - attributesList [][]kv.KeyValue + attributesList [][]tracing.KeyValue } -func (s *mockSpan) SetAttributes(attrs ...kv.KeyValue) { +func (s *mockSpan) SetAttributes(attrs ...tracing.KeyValue) { s.attributesList = append(s.attributesList, attrs) } @@ -152,7 +151,7 @@ func (s *mockSpan) SetStatus(code codes.Code, message string) { s.statusMessage = message } -func (s *mockSpan) AddEvent(name string, attrs ...kv.KeyValue) { +func (s *mockSpan) AddEvent(name string, attrs ...tracing.KeyValue) { s.eventNameList = append(s.eventNameList, name) for _, v := range attrs { @@ -237,17 +236,17 @@ func (s *tracingSuite) TestPing() { assert.Equal(t, codes.InvalidArgument, clientSpan.statusCode) assert.Equal(t, tc.errorMessage, clientSpan.statusMessage) assert.Equal(t, errorMethod, clientSpan.name) - assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, clientSpan.attributesList) + assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(3))}}, clientSpan.attributesList) assert.Equal(t, errorMethod, serverSpan.name) - assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(3)}}, serverSpan.attributesList) + assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(3))}}, serverSpan.attributesList) } else { assert.Equal(t, codes.OK, clientSpan.statusCode) assert.Equal(t, method, clientSpan.name) - assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, clientSpan.attributesList) + assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(0))}}, clientSpan.attributesList) assert.Equal(t, method, serverSpan.name) - assert.Equal(t, [][]kv.KeyValue{{kv.Key("rpc.grpc.status_code").Int64(0)}}, serverSpan.attributesList) + assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(0))}}, serverSpan.attributesList) } }) } diff --git a/interceptors/tracing/kv.go b/interceptors/tracing/kv.go new file mode 100644 index 000000000..30e59527d --- /dev/null +++ b/interceptors/tracing/kv.go @@ -0,0 +1,21 @@ +// Copyright (c) The go-grpc-middleware Authors. +// Licensed under the Apache License 2.0. + +package tracing + +type Key string + +// KeyValue holds a key and value pair. +type KeyValue struct { + Key Key + Value interface{} +} + +// Value creates a KeyValue instance with a Value. +// It supports string, bool, int, int64, float64, string. +func (k Key) Value(v interface{}) KeyValue { + return KeyValue{ + Key: k, + Value: v, + } +} diff --git a/interceptors/tracing/kv/value.go b/interceptors/tracing/kv/value.go deleted file mode 100644 index 82e37caca..000000000 --- a/interceptors/tracing/kv/value.go +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright (c) The go-grpc-middleware Authors. -// Licensed under the Apache License 2.0. - -package kv - -import ( - "math" -) - -// KeyValue holds a key and value pair. -type KeyValue struct { - Key Key - Value Value -} - -type Key string - -// Bool creates a KeyValue instance with a BOOL Value. -// -// If creating both key and a bool value at the same time, then -// instead of calling kv.Key(name).Bool(value) consider using a -// convenience function provided by the api/key package - -// key.Bool(name, value). -func (k Key) Bool(v bool) KeyValue { - return KeyValue{ - Key: k, - Value: boolValue(v), - } -} - -// Int64 creates a KeyValue instance with an INT64 Value. -// -// If creating both key and an int64 value at the same time, then -// instead of calling kv.Key(name).Int64(value) consider using a -// convenience function provided by the api/key package - -// key.Int64(name, value). -func (k Key) Int64(v int64) KeyValue { - return KeyValue{ - Key: k, - Value: int64Value(v), - } -} - -// Float64 creates a KeyValue instance with a FLOAT64 Value. -// -// If creating both key and a float64 value at the same time, then -// instead of calling kv.Key(name).Float64(value) consider using a -// convenience function provided by the api/key package - -// key.Float64(name, value). -func (k Key) Float64(v float64) KeyValue { - return KeyValue{ - Key: k, - Value: float64Value(v), - } -} - -// String creates a KeyValue instance with a STRING Value. -// -// If creating both key and a string value at the same time, then -// instead of calling kv.Key(name).String(value) consider using a -// convenience function provided by the api/key package - -// key.String(name, value). -func (k Key) String(v string) KeyValue { - return KeyValue{ - Key: k, - Value: stringValue(v), - } -} - -// Int creates a KeyValue instance with either an INT32 or an INT64 -// Value, depending on whether the int type is 32 or 64 bits wide. -// -// If creating both key and an int value at the same time, then -// instead of calling kv.Key(name).Int(value) consider using a -// convenience function provided by the api/key package - -// key.Int(name, value). -func (k Key) Int(v int) KeyValue { - return KeyValue{ - Key: k, - Value: intValue(v), - } -} - -// ValueType describes the type of the data Value holds. -type ValueType int - -const ( - INVALID ValueType = iota // No value. - // BOOL is a boolean Type Value. - BOOL - // INT64 is a 64-bit signed integral Type Value. - INT64 - // FLOAT64 is a 64-bit floating point Type Value. - FLOAT64 - // STRING is a string Type Value. - STRING -) - -type Value struct { - vtype ValueType - numeric uint64 - stringly string -} - -func boolTowRaw(b bool) uint64 { - if b { - return 1 - } - return 0 -} - -func rawToBool(r uint64) bool { - return r != 0 -} - -func boolValue(v bool) Value { - return Value{ - vtype: BOOL, - numeric: boolTowRaw(v), - } -} - -func int64Value(v int64) Value { - return Value{ - vtype: INT64, - numeric: uint64(v), - } -} - -func float64Value(v float64) Value { - return Value{ - vtype: FLOAT64, - numeric: math.Float64bits(v), - } -} - -func stringValue(v string) Value { - return Value{ - vtype: STRING, - stringly: v, - } -} - -// intValue creates an INT64 Value. -func intValue(v int) Value { - return int64Value(int64(v)) -} - -func (v Value) AsBool() bool { - return rawToBool(v.numeric) -} - -// AsInt64 returns the int64 value. Make sure that the Value's type is -// INT64. -func (v Value) AsInt64() int64 { - return int64(v.numeric) -} - -// AsFloat64 returns the float64 value. Make sure that the Value's -// type is FLOAT64. -func (v Value) AsFloat64() float64 { - return math.Float64frombits(v.numeric) -} - -// AsString returns the string value. Make sure that the Value's type -// is STRING. -func (v Value) AsString() string { - return v.stringly -} - -// Type returns a type of the Value. -func (v Value) Type() ValueType { - return v.vtype -} diff --git a/interceptors/tracing/kv/value_test.go b/interceptors/tracing/kv/value_test.go deleted file mode 100644 index 29edfc657..000000000 --- a/interceptors/tracing/kv/value_test.go +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) The go-grpc-middleware Authors. -// Licensed under the Apache License 2.0. - -package kv - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestKey(t *testing.T) { - testCases := []struct { - name string - keyValue KeyValue - expectedValue interface{} - }{ - { - name: "true", - keyValue: Key("bool").Bool(true), - expectedValue: true, - }, - { - name: "false", - keyValue: Key("bool").Bool(false), - expectedValue: false, - }, - { - name: "int64", - keyValue: Key("int64").Int64(43), - expectedValue: int64(43), - }, - { - name: "float64", - keyValue: Key("float64").Float64(43), - expectedValue: float64(43), - }, - { - name: "string", - keyValue: Key("string").String("foo"), - expectedValue: "foo", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - switch tc.keyValue.Value.Type() { - case BOOL: - assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsBool()) - case INT64: - assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsInt64()) - case FLOAT64: - assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsFloat64()) - case STRING: - assert.Equal(t, tc.expectedValue, tc.keyValue.Value.AsString()) - } - }) - } -} diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go index b54fc45eb..e38b31180 100644 --- a/interceptors/tracing/reporter.go +++ b/interceptors/tracing/reporter.go @@ -11,8 +11,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" - - "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" ) type reporter struct { @@ -45,22 +43,22 @@ func (o *reporter) PostMsgReceive(payload interface{}, err error, d time.Duratio addEvent(o.span, RPCMessageTypeReceived, o.receivedMessageID, payload) } -func addEvent(span Span, messageType kv.KeyValue, messageID int, payload interface{}) { +func addEvent(span Span, messageType KeyValue, messageID int, payload interface{}) { if p, ok := payload.(proto.Message); ok { span.AddEvent("message", messageType, - rpcMessageIDKey.Int(messageID), - rpcMessageUncompressedSizeKey.Int(proto.Size(p)), + rpcMessageIDKey.Value(messageID), + rpcMessageUncompressedSizeKey.Value(proto.Size(p)), ) return } span.AddEvent("message", messageType, - rpcMessageIDKey.Int(messageID), + rpcMessageIDKey.Value(messageID), ) } // statusCodeAttr returns status code attribute based on given gRPC code -func statusCodeAttr(c codes.Code) kv.KeyValue { - return grpcStatusCodeKey.Int64(int64(c)) +func statusCodeAttr(c codes.Code) KeyValue { + return grpcStatusCodeKey.Value(int64(c)) } diff --git a/interceptors/tracing/tracing.go b/interceptors/tracing/tracing.go index 84f71c46a..08299ce1d 100644 --- a/interceptors/tracing/tracing.go +++ b/interceptors/tracing/tracing.go @@ -7,28 +7,26 @@ import ( "context" "google.golang.org/grpc/codes" - - "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tracing/kv" ) const ( // Type of message transmitted or received. - rpcMessageTypeKey = kv.Key("message.type") + rpcMessageTypeKey = Key("message.type") // Identifier of message transmitted or received. - rpcMessageIDKey = kv.Key("message.id") + rpcMessageIDKey = Key("message.id") // The uncompressed size of the message transmitted or received in // bytes. - rpcMessageUncompressedSizeKey = kv.Key("message.uncompressed_size") + rpcMessageUncompressedSizeKey = Key("message.uncompressed_size") // grpcStatusCodeKey is convention for numeric status code of a gRPC request. - grpcStatusCodeKey = kv.Key("rpc.grpc.status_code") + grpcStatusCodeKey = Key("rpc.grpc.status_code") ) var ( - RPCMessageTypeSent = rpcMessageTypeKey.String("SENT") - RPCMessageTypeReceived = rpcMessageTypeKey.String("RECEIVED") + RPCMessageTypeSent = rpcMessageTypeKey.Value("SENT") + RPCMessageTypeReceived = rpcMessageTypeKey.Value("RECEIVED") ) type Tracer interface { @@ -51,10 +49,10 @@ type Span interface { // AddEvent adds an event to the span. // Middleware will call it while receiving or sending messages. - AddEvent(name string, attrs ...kv.KeyValue) + AddEvent(name string, attrs ...KeyValue) // SetAttributes sets kv as attributes of the Span. If a key from kv // already exists for an attribute of the Span it should be overwritten with // the value contained in kv. - SetAttributes(attrs ...kv.KeyValue) + SetAttributes(attrs ...KeyValue) } From 545d37aa07ed8705b1e413eb0a7acb8356c8ec0e Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Fri, 22 Apr 2022 00:14:49 +0800 Subject: [PATCH 8/8] Remove keyvalue --- interceptors/tracing/interceptors_test.go | 37 ++++++++++++++--------- interceptors/tracing/kv.go | 21 ------------- interceptors/tracing/reporter.go | 16 +++++----- interceptors/tracing/tracing.go | 24 +++++++-------- 4 files changed, 43 insertions(+), 55 deletions(-) delete mode 100644 interceptors/tracing/kv.go diff --git a/interceptors/tracing/interceptors_test.go b/interceptors/tracing/interceptors_test.go index f4e91cf52..f74854ca4 100644 --- a/interceptors/tracing/interceptors_test.go +++ b/interceptors/tracing/interceptors_test.go @@ -135,11 +135,11 @@ type mockSpan struct { msgSendCounter int msgReceivedCounter int eventNameList []string - attributesList [][]tracing.KeyValue + attributesList [][]interface{} } -func (s *mockSpan) SetAttributes(attrs ...tracing.KeyValue) { - s.attributesList = append(s.attributesList, attrs) +func (s *mockSpan) SetAttributes(keyvals ...interface{}) { + s.attributesList = append(s.attributesList, keyvals) } func (s *mockSpan) End() { @@ -151,15 +151,24 @@ func (s *mockSpan) SetStatus(code codes.Code, message string) { s.statusMessage = message } -func (s *mockSpan) AddEvent(name string, attrs ...tracing.KeyValue) { +func (s *mockSpan) AddEvent(name string, keyvals ...interface{}) { s.eventNameList = append(s.eventNameList, name) - for _, v := range attrs { - switch v { - case tracing.RPCMessageTypeSent: - s.msgSendCounter++ - case tracing.RPCMessageTypeReceived: - s.msgReceivedCounter++ + if len(keyvals)%2 == 1 { + keyvals = append(keyvals, nil) + } + + for i := 0; i < len(keyvals); i += 2 { + k, keyOK := keyvals[i].(string) + v, valueOK := keyvals[i+1].(string) + + if keyOK && valueOK && k == "message.type" { + switch v { + case tracing.RPCMessageTypeSent: + s.msgSendCounter++ + case tracing.RPCMessageTypeReceived: + s.msgReceivedCounter++ + } } } } @@ -236,17 +245,17 @@ func (s *tracingSuite) TestPing() { assert.Equal(t, codes.InvalidArgument, clientSpan.statusCode) assert.Equal(t, tc.errorMessage, clientSpan.statusMessage) assert.Equal(t, errorMethod, clientSpan.name) - assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(3))}}, clientSpan.attributesList) + assert.Equal(t, [][]interface{}{{[]interface{}{"rpc.grpc.status_code", int64(3)}}}, clientSpan.attributesList) assert.Equal(t, errorMethod, serverSpan.name) - assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(3))}}, serverSpan.attributesList) + assert.Equal(t, [][]interface{}{{[]interface{}{"rpc.grpc.status_code", int64(3)}}}, serverSpan.attributesList) } else { assert.Equal(t, codes.OK, clientSpan.statusCode) assert.Equal(t, method, clientSpan.name) - assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(0))}}, clientSpan.attributesList) + assert.Equal(t, [][]interface{}{{[]interface{}{"rpc.grpc.status_code", int64(0)}}}, clientSpan.attributesList) assert.Equal(t, method, serverSpan.name) - assert.Equal(t, [][]tracing.KeyValue{{tracing.Key("rpc.grpc.status_code").Value(int64(0))}}, serverSpan.attributesList) + assert.Equal(t, [][]interface{}{{[]interface{}{"rpc.grpc.status_code", int64(0)}}}, serverSpan.attributesList) } }) } diff --git a/interceptors/tracing/kv.go b/interceptors/tracing/kv.go deleted file mode 100644 index 30e59527d..000000000 --- a/interceptors/tracing/kv.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) The go-grpc-middleware Authors. -// Licensed under the Apache License 2.0. - -package tracing - -type Key string - -// KeyValue holds a key and value pair. -type KeyValue struct { - Key Key - Value interface{} -} - -// Value creates a KeyValue instance with a Value. -// It supports string, bool, int, int64, float64, string. -func (k Key) Value(v interface{}) KeyValue { - return KeyValue{ - Key: k, - Value: v, - } -} diff --git a/interceptors/tracing/reporter.go b/interceptors/tracing/reporter.go index e38b31180..4a0e40ebd 100644 --- a/interceptors/tracing/reporter.go +++ b/interceptors/tracing/reporter.go @@ -43,22 +43,22 @@ func (o *reporter) PostMsgReceive(payload interface{}, err error, d time.Duratio addEvent(o.span, RPCMessageTypeReceived, o.receivedMessageID, payload) } -func addEvent(span Span, messageType KeyValue, messageID int, payload interface{}) { +func addEvent(span Span, messageType string, messageID int, payload interface{}) { if p, ok := payload.(proto.Message); ok { span.AddEvent("message", - messageType, - rpcMessageIDKey.Value(messageID), - rpcMessageUncompressedSizeKey.Value(proto.Size(p)), + rpcMessageTypeKey, messageType, + rpcMessageIDKey, messageID, + rpcMessageUncompressedSizeKey, proto.Size(p), ) return } span.AddEvent("message", - messageType, - rpcMessageIDKey.Value(messageID), + rpcMessageTypeKey, messageType, + rpcMessageIDKey, messageID, ) } // statusCodeAttr returns status code attribute based on given gRPC code -func statusCodeAttr(c codes.Code) KeyValue { - return grpcStatusCodeKey.Value(int64(c)) +func statusCodeAttr(c codes.Code) []interface{} { + return []interface{}{grpcStatusCodeKey, int64(c)} } diff --git a/interceptors/tracing/tracing.go b/interceptors/tracing/tracing.go index 08299ce1d..bd47d20db 100644 --- a/interceptors/tracing/tracing.go +++ b/interceptors/tracing/tracing.go @@ -11,22 +11,22 @@ import ( const ( // Type of message transmitted or received. - rpcMessageTypeKey = Key("message.type") + rpcMessageTypeKey = "message.type" // Identifier of message transmitted or received. - rpcMessageIDKey = Key("message.id") + rpcMessageIDKey = "message.id" // The uncompressed size of the message transmitted or received in // bytes. - rpcMessageUncompressedSizeKey = Key("message.uncompressed_size") + rpcMessageUncompressedSizeKey = "message.uncompressed_size" // grpcStatusCodeKey is convention for numeric status code of a gRPC request. - grpcStatusCodeKey = Key("rpc.grpc.status_code") + grpcStatusCodeKey = "rpc.grpc.status_code" ) -var ( - RPCMessageTypeSent = rpcMessageTypeKey.Value("SENT") - RPCMessageTypeReceived = rpcMessageTypeKey.Value("RECEIVED") +const ( + RPCMessageTypeSent = "SENT" + RPCMessageTypeReceived = "RECEIVED" ) type Tracer interface { @@ -47,12 +47,12 @@ type Span interface { // is to add an OK message or to override a previous status on the Span. SetStatus(code codes.Code, msg string) - // AddEvent adds an event to the span. + // AddEvent adds an event to the span with key/value pairs as attributes. // Middleware will call it while receiving or sending messages. - AddEvent(name string, attrs ...KeyValue) + AddEvent(name string, keyvals ...interface{}) - // SetAttributes sets kv as attributes of the Span. If a key from kv + // SetAttributes sets key/value pairs as attributes. If a key // already exists for an attribute of the Span it should be overwritten with - // the value contained in kv. - SetAttributes(attrs ...KeyValue) + // the value. + SetAttributes(keyvals ...interface{}) }