From 3ca58f38c4c1b89115f64aaf6c2f1a56b708abac Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Fri, 7 Apr 2023 11:38:36 +0300 Subject: [PATCH 01/13] implement based on new spec --- .../grpc/otelgrpc/interceptor.go | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go index b74d558e377..9ebdb387b54 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go @@ -57,6 +57,29 @@ var ( messageReceived = messageType(RPCMessageTypeReceived) ) +// serverStatus returns a span status code and message for a given gRPC +// status code. It maps specific gRPC status codes to a corresponding span +// status code and message. This function is intended for use on the server +// side of a gRPC connection. +// +// If the gRPC status code is Unknown, DeadlineExceeded, Unimplemented, +// Internal, Unavailable, or DataLoss, it returns a span status code of Error +// and the message from the gRPC status. Otherwise, it returns a span status +// code of Unset and an empty message. +func serverStatus(grpcStatus *status.Status) (grpc_codes.Code, string) { + switch grpcStatus.Code() { + case grpc_codes.Unknown, + grpc_codes.DeadlineExceeded, + grpc_codes.Unimplemented, + grpc_codes.Internal, + grpc_codes.Unavailable, + grpc_codes.DataLoss: + return codes.Error, grpcStatus.Message() + default: + return codes.Unset, "" + } +} + // UnaryClientInterceptor returns a grpc.UnaryClientInterceptor suitable // for use in a grpc.Dial call. func UnaryClientInterceptor(opts ...Option) grpc.UnaryClientInterceptor { @@ -342,8 +365,8 @@ func UnaryServerInterceptor(opts ...Option) grpc.UnaryServerInterceptor { resp, err := handler(ctx, req) if err != nil { s, _ := status.FromError(err) - statusCode = s.Code() - span.SetStatus(codes.Error, s.Message()) + statusCode, msg := serverStatus(s) + span.SetStatus(statusCode, msg) span.SetAttributes(statusCodeAttr(s.Code())) messageSent.Event(ctx, 1, s.Proto()) } else { @@ -435,7 +458,8 @@ func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor { err := handler(srv, wrapServerStream(ctx, ss)) if err != nil { s, _ := status.FromError(err) - span.SetStatus(codes.Error, s.Message()) + statusCode, msg := serverStatus(s) + span.SetStatus(statusCode, msg) span.SetAttributes(statusCodeAttr(s.Code())) } else { span.SetAttributes(statusCodeAttr(grpc_codes.OK)) From 9bce14e3980f88ec3f7f6b4ffdf3ca561232d36c Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Fri, 7 Apr 2023 12:18:26 +0300 Subject: [PATCH 02/13] fix return value of serverStatus --- instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go index 9ebdb387b54..45ba0ead97f 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go @@ -66,7 +66,7 @@ var ( // Internal, Unavailable, or DataLoss, it returns a span status code of Error // and the message from the gRPC status. Otherwise, it returns a span status // code of Unset and an empty message. -func serverStatus(grpcStatus *status.Status) (grpc_codes.Code, string) { +func serverStatus(grpcStatus *status.Status) (codes.Code, string) { switch grpcStatus.Code() { case grpc_codes.Unknown, grpc_codes.DeadlineExceeded, From 20ef010df529d164a26b7cfc9562d768470307ad Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 22:56:12 +0300 Subject: [PATCH 03/13] fix current tests --- .../google.golang.org/grpc/otelgrpc/test/interceptor_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 821b1c310bd..1701bacefdf 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -599,8 +599,8 @@ func TestServerInterceptorError(t *testing.T) { if !ok { t.Fatalf("failed to export error span") } - assert.Equal(t, codes.Error, span.Status().Code) - assert.Contains(t, deniedErr.Error(), span.Status().Description) + assert.Equal(t, codes.Unset, span.Status().Code) + assert.Empty(t, span.Status().Description) var codeAttr attribute.KeyValue for _, a := range span.Attributes() { if a.Key == otelgrpc.GRPCStatusCodeKey { From c4d7436415b56db9e546ff7ca3765ec5a06a530e Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 22:58:05 +0300 Subject: [PATCH 04/13] add internal error test --- .../grpc/otelgrpc/test/interceptor_test.go | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 1701bacefdf..f086565a111 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -583,7 +583,7 @@ func TestStreamClientInterceptorWithError(t *testing.T) { assert.Equal(t, codes.Error, span.Status().Code) } -func TestServerInterceptorError(t *testing.T) { +func TestServerInterceptorPermissionDeniedError(t *testing.T) { sr := tracetest.NewSpanRecorder() tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) @@ -618,6 +618,41 @@ func TestServerInterceptorError(t *testing.T) { }, span.Events()[1].Attributes) } +func TestServerInterceptorInternalError(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) + internalErr := status.Error(grpc_codes.Internal, "INTERNAL_TEXT") + handler := func(_ context.Context, _ interface{}) (interface{}, error) { + return nil, internalErr + } + _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{}, handler) + require.Error(t, err) + assert.Equal(t, err, internalErr) + + span, ok := getSpanFromRecorder(sr, "") + if !ok { + t.Fatalf("failed to export error span") + } + assert.Equal(t, codes.Error, span.Status().Code) + assert.Contains(t, internalErr.Error(), span.Status().Description) + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break + } + } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(grpc_codes.Internal)), codeAttr.Value) + } + assert.Len(t, span.Events(), 2) + assert.ElementsMatch(t, []attribute.KeyValue{ + attribute.Key("message.type").String("SENT"), + attribute.Key("message.id").Int(1), + }, span.Events()[1].Attributes) +} + func TestParseFullMethod(t *testing.T) { tests := []struct { fullMethod string From 34ec54533532022731c5a219f293ac53beb75e00 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 23:01:22 +0300 Subject: [PATCH 05/13] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73ba8d532ca..bc0acb4e92e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - AWS SDK add `rpc.system` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`. (#3582, #3617) - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) +### Changed + +- Update `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` to align gRPC server span status with the recent changes in the OpenTelemetry specification (see `https://github.com/open-telemetry/opentelemetry-specification/pull/3333`). + ### Fixed - Prevent taking from reservoir in AWS XRay Remote Sampler when there is zero capacity in `go.opentelemetry.io/contrib/samplers/aws/xray`. (#3684) From 042bddb1d1b002eb6b392d2a14dc1d8ababf6e39 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 23:14:23 +0300 Subject: [PATCH 06/13] move function to bottom --- .../grpc/otelgrpc/interceptor.go | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go index 45ba0ead97f..9601ee8d83f 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go @@ -57,29 +57,6 @@ var ( messageReceived = messageType(RPCMessageTypeReceived) ) -// serverStatus returns a span status code and message for a given gRPC -// status code. It maps specific gRPC status codes to a corresponding span -// status code and message. This function is intended for use on the server -// side of a gRPC connection. -// -// If the gRPC status code is Unknown, DeadlineExceeded, Unimplemented, -// Internal, Unavailable, or DataLoss, it returns a span status code of Error -// and the message from the gRPC status. Otherwise, it returns a span status -// code of Unset and an empty message. -func serverStatus(grpcStatus *status.Status) (codes.Code, string) { - switch grpcStatus.Code() { - case grpc_codes.Unknown, - grpc_codes.DeadlineExceeded, - grpc_codes.Unimplemented, - grpc_codes.Internal, - grpc_codes.Unavailable, - grpc_codes.DataLoss: - return codes.Error, grpcStatus.Message() - default: - return codes.Unset, "" - } -} - // UnaryClientInterceptor returns a grpc.UnaryClientInterceptor suitable // for use in a grpc.Dial call. func UnaryClientInterceptor(opts ...Option) grpc.UnaryClientInterceptor { @@ -523,3 +500,26 @@ func peerFromCtx(ctx context.Context) string { func statusCodeAttr(c grpc_codes.Code) attribute.KeyValue { return GRPCStatusCodeKey.Int64(int64(c)) } + +// serverStatus returns a span status code and message for a given gRPC +// status code. It maps specific gRPC status codes to a corresponding span +// status code and message. This function is intended for use on the server +// side of a gRPC connection. +// +// If the gRPC status code is Unknown, DeadlineExceeded, Unimplemented, +// Internal, Unavailable, or DataLoss, it returns a span status code of Error +// and the message from the gRPC status. Otherwise, it returns a span status +// code of Unset and an empty message. +func serverStatus(grpcStatus *status.Status) (codes.Code, string) { + switch grpcStatus.Code() { + case grpc_codes.Unknown, + grpc_codes.DeadlineExceeded, + grpc_codes.Unimplemented, + grpc_codes.Internal, + grpc_codes.Unavailable, + grpc_codes.DataLoss: + return codes.Error, grpcStatus.Message() + default: + return codes.Unset, "" + } +} From a4a0f20f812c31f08fa4a55438ba7ea3c83771df Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 23:14:30 +0300 Subject: [PATCH 07/13] add ok test --- .../grpc/otelgrpc/test/interceptor_test.go | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index f086565a111..9ff906ba653 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -583,6 +583,39 @@ func TestStreamClientInterceptorWithError(t *testing.T) { assert.Equal(t, codes.Error, span.Status().Code) } +func TestServerInterceptorOK(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) + handler := func(_ context.Context, _ interface{}) (interface{}, error) { + return nil, nil + } + _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{}, handler) + require.NoError(t, err) + + span, ok := getSpanFromRecorder(sr, "") + if !ok { + t.Fatalf("failed to export error span") + } + assert.Equal(t, codes.Unset, span.Status().Code) + assert.Empty(t, span.Status().Description) + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break + } + } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(grpc_codes.OK)), codeAttr.Value) + } + assert.Len(t, span.Events(), 2) + assert.ElementsMatch(t, []attribute.KeyValue{ + attribute.Key("message.type").String("SENT"), + attribute.Key("message.id").Int(1), + }, span.Events()[1].Attributes) +} + func TestServerInterceptorPermissionDeniedError(t *testing.T) { sr := tracetest.NewSpanRecorder() tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) From f11c9517b1bd74378f205ce698280682bc1831b7 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 8 Apr 2023 23:40:53 +0300 Subject: [PATCH 08/13] add stream server test --- .../grpc/otelgrpc/test/interceptor_test.go | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 9ff906ba653..24cf8f58397 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -686,6 +686,43 @@ func TestServerInterceptorInternalError(t *testing.T) { }, span.Events()[1].Attributes) } +type mockServerStream struct { + grpc.ServerStream +} + +func (m *mockServerStream) Context() context.Context { return context.Background() } + +func TestStreamServerInterceptorInternalError(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + usi := otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tp)) + internalErr := status.Error(grpc_codes.Internal, "INTERNAL_TEXT") + handler := func(_ interface{}, _ grpc.ServerStream) error { + return internalErr + } + + err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{}, handler) + require.Error(t, err) + assert.Equal(t, err, internalErr) + + span, ok := getSpanFromRecorder(sr, "") + if !ok { + t.Fatalf("failed to export error span") + } + assert.Equal(t, codes.Error, span.Status().Code) + assert.Contains(t, internalErr.Error(), span.Status().Description) + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break + } + } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(grpc_codes.Internal)), codeAttr.Value) + } +} + func TestParseFullMethod(t *testing.T) { tests := []struct { fullMethod string From 289dbd7905a4a62d14beb756ee022ccea13dd3a7 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sun, 9 Apr 2023 00:46:13 +0300 Subject: [PATCH 09/13] refactor tests as table-driven tests, and add test for StreamServerInterceptor --- .../grpc/otelgrpc/test/interceptor_test.go | 230 +++++++++--------- 1 file changed, 118 insertions(+), 112 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 24cf8f58397..85bb05e57dd 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -583,107 +583,98 @@ func TestStreamClientInterceptorWithError(t *testing.T) { assert.Equal(t, codes.Error, span.Status().Code) } -func TestServerInterceptorOK(t *testing.T) { - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, nil - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{}, handler) - require.NoError(t, err) - - span, ok := getSpanFromRecorder(sr, "") - if !ok { - t.Fatalf("failed to export error span") - } - assert.Equal(t, codes.Unset, span.Status().Code) - assert.Empty(t, span.Status().Description) - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break - } - } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(grpc_codes.OK)), codeAttr.Value) - } - assert.Len(t, span.Events(), 2) - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("SENT"), - attribute.Key("message.id").Int(1), - }, span.Events()[1].Attributes) +var serverChecks = []struct { + grpcCode grpc_codes.Code + spanCode codes.Code + name string +}{ + { + grpcCode: grpc_codes.OK, + spanCode: codes.Unset, + name: "ok", + }, + { + grpcCode: grpc_codes.Canceled, + spanCode: codes.Unset, + name: "cancelled", + }, + { + grpcCode: grpc_codes.NotFound, + spanCode: codes.Unset, + name: "not.found", + }, + { + grpcCode: grpc_codes.PermissionDenied, + spanCode: codes.Unset, + name: "permission.denied", + }, + { + grpcCode: grpc_codes.Unimplemented, + spanCode: codes.Error, + name: "unimplemented", + }, + { + grpcCode: grpc_codes.Internal, + spanCode: codes.Error, + name: "internal", + }, + { + grpcCode: grpc_codes.Unauthenticated, + spanCode: codes.Unset, + name: "unauthenticated", + }, } -func TestServerInterceptorPermissionDeniedError(t *testing.T) { +func UnaryServerInterceptor(t *testing.T) { sr := tracetest.NewSpanRecorder() tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) - deniedErr := status.Error(grpc_codes.PermissionDenied, "PERMISSION_DENIED_TEXT") - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, deniedErr - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{}, handler) - require.Error(t, err) - assert.Equal(t, err, deniedErr) + for _, check := range serverChecks { + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + handler := func(_ context.Context, _ interface{}) (interface{}, error) { + return nil, grpcErr + } + _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: check.name}, handler) + // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error + if check.grpcCode != grpc_codes.OK { + require.Error(t, err) + assert.Equal(t, err, grpcErr) + } else { + require.NoError(t, err) + } - span, ok := getSpanFromRecorder(sr, "") - if !ok { - t.Fatalf("failed to export error span") - } - assert.Equal(t, codes.Unset, span.Status().Code) - assert.Empty(t, span.Status().Description) - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break + span, ok := getSpanFromRecorder(sr, check.name) + if !assert.True(t, ok, "missing span %q", check.name) { + continue } - } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(grpc_codes.PermissionDenied)), codeAttr.Value) - } - assert.Len(t, span.Events(), 2) - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("SENT"), - attribute.Key("message.id").Int(1), - }, span.Events()[1].Attributes) -} -func TestServerInterceptorInternalError(t *testing.T) { - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) - internalErr := status.Error(grpc_codes.Internal, "INTERNAL_TEXT") - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, internalErr - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{}, handler) - require.Error(t, err) - assert.Equal(t, err, internalErr) + // validate span status + assert.Equal(t, check.spanCode, span.Status().Code) + if span.Status().Code == codes.Error { + assert.Contains(t, grpcErr.Error(), span.Status().Description) + } else { + assert.Empty(t, span.Status().Description) + } - span, ok := getSpanFromRecorder(sr, "") - if !ok { - t.Fatalf("failed to export error span") - } - assert.Equal(t, codes.Error, span.Status().Code) - assert.Contains(t, internalErr.Error(), span.Status().Description) - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break + // validate span attributes + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break + } } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(check.grpcCode)), codeAttr.Value) + } + + // validate events and their attributes + assert.Len(t, span.Events(), 2) + assert.ElementsMatch(t, []attribute.KeyValue{ + attribute.Key("message.type").String("SENT"), + attribute.Key("message.id").Int(1), + }, span.Events()[1].Attributes) } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(grpc_codes.Internal)), codeAttr.Value) - } - assert.Len(t, span.Events(), 2) - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("SENT"), - attribute.Key("message.id").Int(1), - }, span.Events()[1].Attributes) } type mockServerStream struct { @@ -692,34 +683,49 @@ type mockServerStream struct { func (m *mockServerStream) Context() context.Context { return context.Background() } -func TestStreamServerInterceptorInternalError(t *testing.T) { +func TestStreamServerInterceptor(t *testing.T) { sr := tracetest.NewSpanRecorder() tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tp)) - internalErr := status.Error(grpc_codes.Internal, "INTERNAL_TEXT") - handler := func(_ interface{}, _ grpc.ServerStream) error { - return internalErr - } + for _, check := range serverChecks { + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + handler := func(_ interface{}, _ grpc.ServerStream) error { + return grpcErr + } - err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{}, handler) - require.Error(t, err) - assert.Equal(t, err, internalErr) + err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: check.name}, handler) + // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error + if check.grpcCode != grpc_codes.OK { + require.Error(t, err) + assert.Equal(t, err, grpcErr) + } else { + require.NoError(t, err) + } - span, ok := getSpanFromRecorder(sr, "") - if !ok { - t.Fatalf("failed to export error span") - } - assert.Equal(t, codes.Error, span.Status().Code) - assert.Contains(t, internalErr.Error(), span.Status().Description) - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break + span, ok := getSpanFromRecorder(sr, check.name) + if !assert.True(t, ok, "missing span %q", check.name) { + continue + } + + // validate span status + assert.Equal(t, check.spanCode, span.Status().Code) + if span.Status().Code == codes.Error { + assert.Contains(t, grpcErr.Error(), span.Status().Description) + } else { + assert.Empty(t, span.Status().Description) + } + + // validate span attributes + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break + } + } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(check.grpcCode)), codeAttr.Value) } - } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(grpc_codes.Internal)), codeAttr.Value) } } From 2b36f98e4c35f477513d3ba684283bb8df7d5733 Mon Sep 17 00:00:00 2001 From: FaranIdo Date: Tue, 11 Apr 2023 16:56:44 +0300 Subject: [PATCH 10/13] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix CR note on the changelog Co-authored-by: Robert Pająk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc0acb4e92e..acab402d439 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed -- Update `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` to align gRPC server span status with the recent changes in the OpenTelemetry specification (see `https://github.com/open-telemetry/opentelemetry-specification/pull/3333`). +- Update `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` to align gRPC server span status with the changes in the OpenTelemetry specification. (#3685) ### Fixed From 5390053b099057f336518aff8916f5f85deceeb4 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Tue, 11 Apr 2023 18:17:58 +0300 Subject: [PATCH 11/13] refactor server tests to use shared assertion methods --- .../grpc/otelgrpc/test/interceptor_test.go | 195 ++++++++---------- 1 file changed, 91 insertions(+), 104 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 85bb05e57dd..ac6276b286e 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -584,96 +584,105 @@ func TestStreamClientInterceptorWithError(t *testing.T) { } var serverChecks = []struct { - grpcCode grpc_codes.Code - spanCode codes.Code - name string + grpcCode grpc_codes.Code + wantSpanCode codes.Code + name string }{ { - grpcCode: grpc_codes.OK, - spanCode: codes.Unset, - name: "ok", + grpcCode: grpc_codes.OK, + wantSpanCode: codes.Unset, + name: "ok", }, { - grpcCode: grpc_codes.Canceled, - spanCode: codes.Unset, - name: "cancelled", + grpcCode: grpc_codes.Canceled, + wantSpanCode: codes.Unset, + name: "cancelled", }, { - grpcCode: grpc_codes.NotFound, - spanCode: codes.Unset, - name: "not.found", + grpcCode: grpc_codes.NotFound, + wantSpanCode: codes.Unset, + name: "not.found", }, { - grpcCode: grpc_codes.PermissionDenied, - spanCode: codes.Unset, - name: "permission.denied", + grpcCode: grpc_codes.PermissionDenied, + wantSpanCode: codes.Unset, + name: "permission.denied", }, { - grpcCode: grpc_codes.Unimplemented, - spanCode: codes.Error, - name: "unimplemented", + grpcCode: grpc_codes.Unimplemented, + wantSpanCode: codes.Error, + name: "unimplemented", }, { - grpcCode: grpc_codes.Internal, - spanCode: codes.Error, - name: "internal", + grpcCode: grpc_codes.Internal, + wantSpanCode: codes.Error, + name: "internal", }, { - grpcCode: grpc_codes.Unauthenticated, - spanCode: codes.Unset, - name: "unauthenticated", + grpcCode: grpc_codes.Unauthenticated, + wantSpanCode: codes.Unset, + name: "unauthenticated", }, } -func UnaryServerInterceptor(t *testing.T) { - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) - for _, check := range serverChecks { - grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, grpcErr - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: check.name}, handler) - // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error - if check.grpcCode != grpc_codes.OK { - require.Error(t, err) - assert.Equal(t, err, grpcErr) - } else { - require.NoError(t, err) - } +func assertServerErr(t *testing.T, err error, grpcCode grpc_codes.Code) { + // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error + if grpcCode == grpc_codes.OK { + assert.NoError(t, err) + } else { + assert.Equal(t, grpcCode, status.Code(err)) + } +} - span, ok := getSpanFromRecorder(sr, check.name) - if !assert.True(t, ok, "missing span %q", check.name) { - continue - } +func assertServerSpan(t *testing.T, span trace.ReadOnlySpan, wantSpanCode codes.Code, grpcCode grpc_codes.Code, grpcErr error) { + // validate span status + assert.Equal(t, wantSpanCode, span.Status().Code) + if span.Status().Code == codes.Error { + assert.Contains(t, grpcErr.Error(), span.Status().Description) + } else { + assert.Empty(t, span.Status().Description) + } - // validate span status - assert.Equal(t, check.spanCode, span.Status().Code) - if span.Status().Code == codes.Error { - assert.Contains(t, grpcErr.Error(), span.Status().Description) - } else { - assert.Empty(t, span.Status().Description) + // validate grpc code span attribute + var codeAttr attribute.KeyValue + for _, a := range span.Attributes() { + if a.Key == otelgrpc.GRPCStatusCodeKey { + codeAttr = a + break } + } + if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { + assert.Equal(t, attribute.Int64Value(int64(grpcCode)), codeAttr.Value) + } +} - // validate span attributes - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break +// TestUnaryServerInterceptor tests the server interceptor for unary RPCs. +func TestUnaryServerInterceptor(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) + for _, check := range serverChecks { + t.Run(check.name, func(t *testing.T) { + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + handler := func(_ context.Context, _ interface{}) (interface{}, error) { + return nil, grpcErr } - } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(check.grpcCode)), codeAttr.Value) - } - - // validate events and their attributes - assert.Len(t, span.Events(), 2) - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("SENT"), - attribute.Key("message.id").Int(1), - }, span.Events()[1].Attributes) + // call the unary interceptor + _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: check.name}, handler) + assertServerErr(t, err, check.grpcCode) + + // validate span + span, ok := getSpanFromRecorder(sr, check.name) + require.True(t, ok, "missing span %s", check.name) + assertServerSpan(t, span, check.wantSpanCode, check.grpcCode, grpcErr) + + // validate events and their attributes + assert.Len(t, span.Events(), 2) + assert.ElementsMatch(t, []attribute.KeyValue{ + attribute.Key("message.type").String("SENT"), + attribute.Key("message.id").Int(1), + }, span.Events()[1].Attributes) + }) } } @@ -683,49 +692,27 @@ type mockServerStream struct { func (m *mockServerStream) Context() context.Context { return context.Background() } +// TestStreamServerInterceptor tests the server interceptor for streaming RPCs. func TestStreamServerInterceptor(t *testing.T) { sr := tracetest.NewSpanRecorder() tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tp)) for _, check := range serverChecks { - grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) - handler := func(_ interface{}, _ grpc.ServerStream) error { - return grpcErr - } - - err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: check.name}, handler) - // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error - if check.grpcCode != grpc_codes.OK { - require.Error(t, err) - assert.Equal(t, err, grpcErr) - } else { - require.NoError(t, err) - } - - span, ok := getSpanFromRecorder(sr, check.name) - if !assert.True(t, ok, "missing span %q", check.name) { - continue - } + t.Run(check.name, func(t *testing.T) { + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + handler := func(_ interface{}, _ grpc.ServerStream) error { + return grpcErr + } - // validate span status - assert.Equal(t, check.spanCode, span.Status().Code) - if span.Status().Code == codes.Error { - assert.Contains(t, grpcErr.Error(), span.Status().Description) - } else { - assert.Empty(t, span.Status().Description) - } + // call the stream interceptor + err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: check.name}, handler) + assertServerErr(t, err, check.grpcCode) - // validate span attributes - var codeAttr attribute.KeyValue - for _, a := range span.Attributes() { - if a.Key == otelgrpc.GRPCStatusCodeKey { - codeAttr = a - break - } - } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(check.grpcCode)), codeAttr.Value) - } + // validate span + span, ok := getSpanFromRecorder(sr, check.name) + require.True(t, ok, "missing span %s", check.name) + assertServerSpan(t, span, check.wantSpanCode, check.grpcCode, grpcErr) + }) } } From 8970bb83973fc686d6ac507b673550981b0915b8 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sat, 15 Apr 2023 15:58:41 +0300 Subject: [PATCH 12/13] fix more CR --- .../grpc/otelgrpc/test/interceptor_test.go | 106 +++++++++--------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index ac6276b286e..79b918e68e6 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -584,64 +584,67 @@ func TestStreamClientInterceptorWithError(t *testing.T) { } var serverChecks = []struct { - grpcCode grpc_codes.Code - wantSpanCode codes.Code - name string + name string + grpcCode grpc_codes.Code + grpcErr error + wantSpanCode codes.Code + wantSpanStatusDescription string }{ { - grpcCode: grpc_codes.OK, - wantSpanCode: codes.Unset, - name: "ok", + name: "ok", + grpcCode: grpc_codes.OK, + grpcErr: nil, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", }, { - grpcCode: grpc_codes.Canceled, - wantSpanCode: codes.Unset, - name: "cancelled", + name: "cancelled", + grpcCode: grpc_codes.Canceled, + grpcErr: status.Error(grpc_codes.Canceled, grpc_codes.Canceled.String()), + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", }, { - grpcCode: grpc_codes.NotFound, - wantSpanCode: codes.Unset, - name: "not.found", + name: "not.found", + grpcCode: grpc_codes.NotFound, + grpcErr: status.Error(grpc_codes.NotFound, grpc_codes.NotFound.String()), + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", }, { - grpcCode: grpc_codes.PermissionDenied, - wantSpanCode: codes.Unset, - name: "permission.denied", + name: "permission.denied", + grpcCode: grpc_codes.PermissionDenied, + grpcErr: status.Error(grpc_codes.PermissionDenied, grpc_codes.PermissionDenied.String()), + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", }, { - grpcCode: grpc_codes.Unimplemented, - wantSpanCode: codes.Error, - name: "unimplemented", + name: "unimplemented", + grpcCode: grpc_codes.Unimplemented, + grpcErr: status.Error(grpc_codes.Unimplemented, grpc_codes.Unimplemented.String()), + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.Unimplemented.String(), }, { - grpcCode: grpc_codes.Internal, - wantSpanCode: codes.Error, - name: "internal", + name: "internal", + grpcCode: grpc_codes.Internal, + grpcErr: status.Error(grpc_codes.Internal, grpc_codes.Internal.String()), + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.Internal.String(), }, { - grpcCode: grpc_codes.Unauthenticated, - wantSpanCode: codes.Unset, - name: "unauthenticated", + name: "unauthenticated", + grpcCode: grpc_codes.Unauthenticated, + grpcErr: status.Error(grpc_codes.Unauthenticated, grpc_codes.Unauthenticated.String()), + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", }, } -func assertServerErr(t *testing.T, err error, grpcCode grpc_codes.Code) { - // the error should be nil only if the gRPC code is OK, otherwise it should be the same as the gRPC error - if grpcCode == grpc_codes.OK { - assert.NoError(t, err) - } else { - assert.Equal(t, grpcCode, status.Code(err)) - } -} - -func assertServerSpan(t *testing.T, span trace.ReadOnlySpan, wantSpanCode codes.Code, grpcCode grpc_codes.Code, grpcErr error) { +func assertServerSpan(t *testing.T, wantSpanCode codes.Code, wantSpanStatusDescription string, wantGrpcCode grpc_codes.Code, span trace.ReadOnlySpan) { // validate span status assert.Equal(t, wantSpanCode, span.Status().Code) - if span.Status().Code == codes.Error { - assert.Contains(t, grpcErr.Error(), span.Status().Description) - } else { - assert.Empty(t, span.Status().Description) - } + assert.Equal(t, wantSpanStatusDescription, span.Status().Description) // validate grpc code span attribute var codeAttr attribute.KeyValue @@ -651,9 +654,9 @@ func assertServerSpan(t *testing.T, span trace.ReadOnlySpan, wantSpanCode codes. break } } - if assert.True(t, codeAttr.Valid(), "attributes contain gRPC status code") { - assert.Equal(t, attribute.Int64Value(int64(grpcCode)), codeAttr.Value) - } + + require.True(t, codeAttr.Valid(), "attributes contain gRPC status code") + assert.Equal(t, attribute.Int64Value(int64(wantGrpcCode)), codeAttr.Value) } // TestUnaryServerInterceptor tests the server interceptor for unary RPCs. @@ -663,18 +666,17 @@ func TestUnaryServerInterceptor(t *testing.T) { usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) for _, check := range serverChecks { t.Run(check.name, func(t *testing.T) { - grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + // call the unary interceptor handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, grpcErr + return nil, check.grpcErr } - // call the unary interceptor _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: check.name}, handler) - assertServerErr(t, err, check.grpcCode) + assert.Equal(t, check.grpcErr, err) // validate span span, ok := getSpanFromRecorder(sr, check.name) require.True(t, ok, "missing span %s", check.name) - assertServerSpan(t, span, check.wantSpanCode, check.grpcCode, grpcErr) + assertServerSpan(t, check.wantSpanCode, check.wantSpanStatusDescription, check.grpcCode, span) // validate events and their attributes assert.Len(t, span.Events(), 2) @@ -699,19 +701,17 @@ func TestStreamServerInterceptor(t *testing.T) { usi := otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tp)) for _, check := range serverChecks { t.Run(check.name, func(t *testing.T) { - grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) + // call the stream interceptor handler := func(_ interface{}, _ grpc.ServerStream) error { - return grpcErr + return check.grpcErr } - - // call the stream interceptor err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: check.name}, handler) - assertServerErr(t, err, check.grpcCode) + assert.Equal(t, check.grpcErr, err) // validate span span, ok := getSpanFromRecorder(sr, check.name) require.True(t, ok, "missing span %s", check.name) - assertServerSpan(t, span, check.wantSpanCode, check.grpcCode, grpcErr) + assertServerSpan(t, check.wantSpanCode, check.wantSpanStatusDescription, check.grpcCode, span) }) } } From 9bf16fb257e2e729aff95d8cedf014f673a1b8a0 Mon Sep 17 00:00:00 2001 From: "ido.faran" Date: Sun, 16 Apr 2023 21:47:17 +0300 Subject: [PATCH 13/13] add all gRPC status + remove name and grpcErr from vars --- .../grpc/otelgrpc/test/interceptor_test.go | 94 +++++++++++++------ 1 file changed, 66 insertions(+), 28 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 79b918e68e6..16a12771ed4 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -584,58 +584,92 @@ func TestStreamClientInterceptorWithError(t *testing.T) { } var serverChecks = []struct { - name string grpcCode grpc_codes.Code - grpcErr error wantSpanCode codes.Code wantSpanStatusDescription string }{ { - name: "ok", grpcCode: grpc_codes.OK, - grpcErr: nil, wantSpanCode: codes.Unset, wantSpanStatusDescription: "", }, { - name: "cancelled", grpcCode: grpc_codes.Canceled, - grpcErr: status.Error(grpc_codes.Canceled, grpc_codes.Canceled.String()), wantSpanCode: codes.Unset, wantSpanStatusDescription: "", }, { - name: "not.found", + grpcCode: grpc_codes.Unknown, + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.Unknown.String(), + }, + { + grpcCode: grpc_codes.InvalidArgument, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { + grpcCode: grpc_codes.DeadlineExceeded, + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.DeadlineExceeded.String(), + }, + { grpcCode: grpc_codes.NotFound, - grpcErr: status.Error(grpc_codes.NotFound, grpc_codes.NotFound.String()), wantSpanCode: codes.Unset, wantSpanStatusDescription: "", }, { - name: "permission.denied", + grpcCode: grpc_codes.AlreadyExists, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { grpcCode: grpc_codes.PermissionDenied, - grpcErr: status.Error(grpc_codes.PermissionDenied, grpc_codes.PermissionDenied.String()), wantSpanCode: codes.Unset, wantSpanStatusDescription: "", }, { - name: "unimplemented", + grpcCode: grpc_codes.ResourceExhausted, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { + grpcCode: grpc_codes.FailedPrecondition, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { + grpcCode: grpc_codes.Aborted, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { + grpcCode: grpc_codes.OutOfRange, + wantSpanCode: codes.Unset, + wantSpanStatusDescription: "", + }, + { grpcCode: grpc_codes.Unimplemented, - grpcErr: status.Error(grpc_codes.Unimplemented, grpc_codes.Unimplemented.String()), wantSpanCode: codes.Error, wantSpanStatusDescription: grpc_codes.Unimplemented.String(), }, { - name: "internal", grpcCode: grpc_codes.Internal, - grpcErr: status.Error(grpc_codes.Internal, grpc_codes.Internal.String()), wantSpanCode: codes.Error, wantSpanStatusDescription: grpc_codes.Internal.String(), }, { - name: "unauthenticated", + grpcCode: grpc_codes.Unavailable, + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.Unavailable.String(), + }, + { + grpcCode: grpc_codes.DataLoss, + wantSpanCode: codes.Error, + wantSpanStatusDescription: grpc_codes.DataLoss.String(), + }, + { grpcCode: grpc_codes.Unauthenticated, - grpcErr: status.Error(grpc_codes.Unauthenticated, grpc_codes.Unauthenticated.String()), wantSpanCode: codes.Unset, wantSpanStatusDescription: "", }, @@ -665,17 +699,19 @@ func TestUnaryServerInterceptor(t *testing.T) { tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tp)) for _, check := range serverChecks { - t.Run(check.name, func(t *testing.T) { + name := check.grpcCode.String() + t.Run(name, func(t *testing.T) { // call the unary interceptor + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, check.grpcErr + return nil, grpcErr } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: check.name}, handler) - assert.Equal(t, check.grpcErr, err) + _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: name}, handler) + assert.Equal(t, grpcErr, err) // validate span - span, ok := getSpanFromRecorder(sr, check.name) - require.True(t, ok, "missing span %s", check.name) + span, ok := getSpanFromRecorder(sr, name) + require.True(t, ok, "missing span %s", name) assertServerSpan(t, check.wantSpanCode, check.wantSpanStatusDescription, check.grpcCode, span) // validate events and their attributes @@ -700,17 +736,19 @@ func TestStreamServerInterceptor(t *testing.T) { tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) usi := otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tp)) for _, check := range serverChecks { - t.Run(check.name, func(t *testing.T) { + name := check.grpcCode.String() + t.Run(name, func(t *testing.T) { // call the stream interceptor + grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) handler := func(_ interface{}, _ grpc.ServerStream) error { - return check.grpcErr + return grpcErr } - err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: check.name}, handler) - assert.Equal(t, check.grpcErr, err) + err := usi(&grpc_testing.SimpleRequest{}, &mockServerStream{}, &grpc.StreamServerInfo{FullMethod: name}, handler) + assert.Equal(t, grpcErr, err) // validate span - span, ok := getSpanFromRecorder(sr, check.name) - require.True(t, ok, "missing span %s", check.name) + span, ok := getSpanFromRecorder(sr, name) + require.True(t, ok, "missing span %s", name) assertServerSpan(t, check.wantSpanCode, check.wantSpanStatusDescription, check.grpcCode, span) }) }