From 4f8c8845f38bf9f81579bb832b87ac9939100edd Mon Sep 17 00:00:00 2001 From: Tim Mwangi Date: Sun, 12 Feb 2023 12:14:55 -0800 Subject: [PATCH 1/2] fix: map HTTP response status code to equivalent GRPC Status code --- .../google.golang.org/grpc/server.go | 5 +-- .../google.golang.org/grpc/server_test.go | 16 +++++++- .../grpc/{status_text.go => status.go} | 41 +++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) rename sdk/instrumentation/google.golang.org/grpc/{status_text.go => status.go} (58%) diff --git a/sdk/instrumentation/google.golang.org/grpc/server.go b/sdk/instrumentation/google.golang.org/grpc/server.go index 8d8fc88..3c6b146 100644 --- a/sdk/instrumentation/google.golang.org/grpc/server.go +++ b/sdk/instrumentation/google.golang.org/grpc/server.go @@ -11,7 +11,6 @@ import ( internalconfig "github.com/hypertrace/goagent/sdk/internal/config" "github.com/hypertrace/goagent/sdk/internal/container" "google.golang.org/grpc" - "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" "google.golang.org/grpc/stats" @@ -106,7 +105,7 @@ func wrapHandler( } filterResult := filter.EvaluateBody(span, processingBody, md) if filterResult.Block { - return nil, status.Error(codes.Code(filterResult.ResponseStatusCode), StatusText(int(filterResult.ResponseStatusCode))) + return nil, status.Error(StatusCode(int(filterResult.ResponseStatusCode)), StatusText(int(filterResult.ResponseStatusCode))) } } } @@ -118,7 +117,7 @@ func wrapHandler( // TODO: decide what should be passed as URL in GRPC filterResult := filter.EvaluateURLAndHeaders(span, "", md) if filterResult.Block { - return nil, status.Error(codes.Code(filterResult.ResponseStatusCode), StatusText(int(filterResult.ResponseStatusCode))) + return nil, status.Error(StatusCode(int(filterResult.ResponseStatusCode)), StatusText(int(filterResult.ResponseStatusCode))) } } } diff --git a/sdk/instrumentation/google.golang.org/grpc/server_test.go b/sdk/instrumentation/google.golang.org/grpc/server_test.go index f87de6e..5707035 100644 --- a/sdk/instrumentation/google.golang.org/grpc/server_test.go +++ b/sdk/instrumentation/google.golang.org/grpc/server_test.go @@ -104,14 +104,17 @@ func TestServerInterceptorFilter(t *testing.T) { tCases := map[string]struct { expectedFilterResult bool + expectedStatusCode codes.Code multiFilter *filter.MultiFilter }{ "no filter": { expectedFilterResult: false, + expectedStatusCode: codes.OK, multiFilter: filter.NewMultiFilter(), }, "headers filter": { expectedFilterResult: true, + expectedStatusCode: codes.PermissionDenied, multiFilter: filter.NewMultiFilter(mock.Filter{ URLAndHeadersEvaluator: func(span sdk.Span, url string, headers map[string][]string) result.FilterResult { assert.Equal(t, []string{"test_value"}, headers["test_key"]) @@ -121,6 +124,7 @@ func TestServerInterceptorFilter(t *testing.T) { }, "body filter": { expectedFilterResult: true, + expectedStatusCode: codes.PermissionDenied, multiFilter: filter.NewMultiFilter(mock.Filter{ BodyEvaluator: func(span sdk.Span, body []byte, headers map[string][]string) result.FilterResult { assert.Equal(t, "{\"name\":\"Pupo\"}", string(body)) @@ -128,6 +132,16 @@ func TestServerInterceptorFilter(t *testing.T) { }, }), }, + "body filter return 412 Precondition failed": { + expectedFilterResult: true, + expectedStatusCode: codes.FailedPrecondition, + multiFilter: filter.NewMultiFilter(mock.Filter{ + BodyEvaluator: func(span sdk.Span, body []byte, headers map[string][]string) result.FilterResult { + assert.Equal(t, "{\"name\":\"Pupo\"}", string(body)) + return result.FilterResult{Block: true, ResponseStatusCode: 412} + }, + }), + }, } spans := []*mock.Span{} @@ -167,7 +181,7 @@ func TestServerInterceptorFilter(t *testing.T) { Name: "Pupo", }) if tCase.expectedFilterResult { - assert.Equal(t, codes.Code(403), status.Code(err)) + assert.Equal(t, tCase.expectedStatusCode, status.Code(err)) } else { assert.Nil(t, err) } diff --git a/sdk/instrumentation/google.golang.org/grpc/status_text.go b/sdk/instrumentation/google.golang.org/grpc/status.go similarity index 58% rename from sdk/instrumentation/google.golang.org/grpc/status_text.go rename to sdk/instrumentation/google.golang.org/grpc/status.go index 92f4266..eb16e8a 100644 --- a/sdk/instrumentation/google.golang.org/grpc/status_text.go +++ b/sdk/instrumentation/google.golang.org/grpc/status.go @@ -1,5 +1,9 @@ package grpc +import ( + "google.golang.org/grpc/codes" +) + func StatusText(code int) string { switch code { case 400: @@ -64,3 +68,40 @@ func StatusText(code int) string { return "Request Error" } } + +// StatusCode does a best effort mapping from HTTP Request Status code to GRPC Code. +func StatusCode(code int) codes.Code { + switch code { + case 400: + return codes.OutOfRange + case 401: + return codes.Unauthenticated + case 403: + return codes.PermissionDenied + case 404: + return codes.NotFound + case 407: + // "Proxy Authentication Required" + return codes.Unauthenticated + case 408: + // Request Timeout + return codes.DeadlineExceeded + case 412: + // "Precondition Failed" + return codes.FailedPrecondition + case 413: + // "Request Entity Too Large" + return codes.ResourceExhausted + case 414: + // "Request URI Too Long" + return codes.ResourceExhausted + case 429: + // "Too Many Requests" + return codes.ResourceExhausted + case 431: + // "Request Header Fields Too Large" + return codes.ResourceExhausted + default: + return codes.Unknown + } +} From 2404d24b5cef7c19954e1ba6974373f7a01f4bab Mon Sep 17 00:00:00 2001 From: Tim Mwangi Date: Mon, 13 Feb 2023 07:17:36 -0800 Subject: [PATCH 2/2] Add some unit tests --- .../google.golang.org/grpc/status.go | 17 ++++---------- .../google.golang.org/grpc/status_test.go | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 sdk/instrumentation/google.golang.org/grpc/status_test.go diff --git a/sdk/instrumentation/google.golang.org/grpc/status.go b/sdk/instrumentation/google.golang.org/grpc/status.go index eb16e8a..7069069 100644 --- a/sdk/instrumentation/google.golang.org/grpc/status.go +++ b/sdk/instrumentation/google.golang.org/grpc/status.go @@ -72,8 +72,6 @@ func StatusText(code int) string { // StatusCode does a best effort mapping from HTTP Request Status code to GRPC Code. func StatusCode(code int) codes.Code { switch code { - case 400: - return codes.OutOfRange case 401: return codes.Unauthenticated case 403: @@ -89,17 +87,10 @@ func StatusCode(code int) codes.Code { case 412: // "Precondition Failed" return codes.FailedPrecondition - case 413: - // "Request Entity Too Large" - return codes.ResourceExhausted - case 414: - // "Request URI Too Long" - return codes.ResourceExhausted - case 429: - // "Too Many Requests" - return codes.ResourceExhausted - case 431: - // "Request Header Fields Too Large" + case 413, // "Request Entity Too Large" + 414, // "Request URI Too Long" + 429, // "Too Many Requests" + 431: // "Request Header Fields Too Large" return codes.ResourceExhausted default: return codes.Unknown diff --git a/sdk/instrumentation/google.golang.org/grpc/status_test.go b/sdk/instrumentation/google.golang.org/grpc/status_test.go new file mode 100644 index 0000000..fe385cd --- /dev/null +++ b/sdk/instrumentation/google.golang.org/grpc/status_test.go @@ -0,0 +1,23 @@ +package grpc + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "google.golang.org/grpc/codes" +) + +func TestStatusCode(t *testing.T) { + assert.Equal(t, codes.Unauthenticated, StatusCode(401)) + assert.Equal(t, codes.PermissionDenied, StatusCode(403)) + assert.Equal(t, codes.NotFound, StatusCode(404)) + assert.Equal(t, codes.Unauthenticated, StatusCode(407)) + assert.Equal(t, codes.DeadlineExceeded, StatusCode(408)) + assert.Equal(t, codes.FailedPrecondition, StatusCode(412)) + assert.Equal(t, codes.ResourceExhausted, StatusCode(413)) + assert.Equal(t, codes.ResourceExhausted, StatusCode(414)) + assert.Equal(t, codes.ResourceExhausted, StatusCode(429)) + assert.Equal(t, codes.ResourceExhausted, StatusCode(431)) + assert.Equal(t, codes.Unknown, StatusCode(400)) + assert.Equal(t, codes.Unknown, StatusCode(500)) +}