From 10cec2c05ea2cfb8b0d856711daedc49d8a45c56 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Mon, 30 Apr 2018 11:08:19 -0700 Subject: [PATCH] Export constants for trace.Status codes (#734) --- exporter/jaeger/jaeger_test.go | 2 +- exporter/stackdriver/stats.go | 4 +- exporter/stackdriver/trace.go | 2 +- plugin/ochttp/server.go | 4 +- plugin/ochttp/server_test.go | 10 ++-- plugin/ochttp/trace.go | 89 +++++++++++++--------------------- plugin/ochttp/trace_test.go | 21 ++++---- trace/status_codes.go | 37 ++++++++++++++ 8 files changed, 95 insertions(+), 74 deletions(-) create mode 100644 trace/status_codes.go diff --git a/exporter/jaeger/jaeger_test.go b/exporter/jaeger/jaeger_test.go index 977ff854c..5d5431da0 100644 --- a/exporter/jaeger/jaeger_test.go +++ b/exporter/jaeger/jaeger_test.go @@ -98,7 +98,7 @@ func Test_spanDataToThrift(t *testing.T) { }, }, }, - Status: trace.Status{Code: 2, Message: "error"}, + Status: trace.Status{Code: trace.StatusCodeUnknown, Message: "error"}, }, want: &gen.Span{ TraceIdLow: 651345242494996240, diff --git a/exporter/stackdriver/stats.go b/exporter/stackdriver/stats.go index c8cfb90b9..93635dac8 100644 --- a/exporter/stackdriver/stats.go +++ b/exporter/stackdriver/stats.go @@ -164,13 +164,13 @@ func (e *statsExporter) uploadStats(vds []*view.Data) error { for _, vd := range vds { if err := e.createMeasure(ctx, vd); err != nil { - span.SetStatus(trace.Status{Code: 2, Message: err.Error()}) + span.SetStatus(trace.Status{Code: trace.StatusCodeUnknown, Message: err.Error()}) return err } } for _, req := range e.makeReq(vds, maxTimeSeriesPerUpload) { if err := e.c.CreateTimeSeries(ctx, req); err != nil { - span.SetStatus(trace.Status{Code: 2, Message: err.Error()}) + span.SetStatus(trace.Status{Code: trace.StatusCodeUnknown, Message: err.Error()}) // TODO(jbd): Don't fail fast here, batch errors? return err } diff --git a/exporter/stackdriver/trace.go b/exporter/stackdriver/trace.go index bba3a5651..e3fd6bab7 100644 --- a/exporter/stackdriver/trace.go +++ b/exporter/stackdriver/trace.go @@ -127,7 +127,7 @@ func (e *traceExporter) uploadSpans(spans []*trace.SpanData) { err := e.client.BatchWriteSpans(ctx, &req) if err != nil { - span.SetStatus(trace.Status{Code: 2, Message: err.Error()}) + span.SetStatus(trace.Status{Code: trace.StatusCodeUnknown, Message: err.Error()}) e.o.handleError(err) } } diff --git a/plugin/ochttp/server.go b/plugin/ochttp/server.go index 7953f4bf1..554e05ffa 100644 --- a/plugin/ochttp/server.go +++ b/plugin/ochttp/server.go @@ -135,6 +135,7 @@ type trackingResponseWriter struct { respSize int64 start time.Time statusCode int + statusLine string endOnce sync.Once writer http.ResponseWriter } @@ -159,7 +160,7 @@ func (t *trackingResponseWriter) end() { } span := trace.FromContext(t.ctx) - span.SetStatus(status(t.statusCode)) + span.SetStatus(TraceStatus(t.statusCode, t.statusLine)) m := []stats.Measurement{ ServerLatency.M(float64(time.Since(t.start)) / float64(time.Millisecond)), @@ -186,6 +187,7 @@ func (t *trackingResponseWriter) Write(data []byte) (int, error) { func (t *trackingResponseWriter) WriteHeader(statusCode int) { t.writer.WriteHeader(statusCode) t.statusCode = statusCode + t.statusLine = http.StatusText(t.statusCode) } func (t *trackingResponseWriter) Flush() { diff --git a/plugin/ochttp/server_test.go b/plugin/ochttp/server_test.go index b1538e76b..1828f8608 100644 --- a/plugin/ochttp/server_test.go +++ b/plugin/ochttp/server_test.go @@ -290,11 +290,11 @@ func TestEnsureTrackingResponseWriterSetsStatusCode(t *testing.T) { res *http.Response want trace.Status }{ - {res: &http.Response{StatusCode: 200}, want: trace.Status{Code: 0, Message: `"OK"`}}, - {res: &http.Response{StatusCode: 500}, want: trace.Status{Code: 2, Message: `"UNKNOWN"`}}, - {res: &http.Response{StatusCode: 403}, want: trace.Status{Code: 7, Message: `"PERMISSION_DENIED"`}}, - {res: &http.Response{StatusCode: 401}, want: trace.Status{Code: 16, Message: `"UNAUTHENTICATED"`}}, - {res: &http.Response{StatusCode: 429}, want: trace.Status{Code: 8, Message: `"RESOURCE_EXHAUSTED"`}}, + {res: &http.Response{StatusCode: 200}, want: trace.Status{Code: trace.StatusCodeOK, Message: `"OK"`}}, + {res: &http.Response{StatusCode: 500}, want: trace.Status{Code: trace.StatusCodeUnknown, Message: `"UNKNOWN"`}}, + {res: &http.Response{StatusCode: 403}, want: trace.Status{Code: trace.StatusCodePermissionDenied, Message: `"PERMISSION_DENIED"`}}, + {res: &http.Response{StatusCode: 401}, want: trace.Status{Code: trace.StatusCodeUnauthenticated, Message: `"UNAUTHENTICATED"`}}, + {res: &http.Response{StatusCode: 429}, want: trace.Status{Code: trace.StatusCodeResourceExhausted, Message: `"RESOURCE_EXHAUSTED"`}}, } for _, tt := range tests { diff --git a/plugin/ochttp/trace.go b/plugin/ochttp/trace.go index 9bf310cc9..80ee86c7a 100644 --- a/plugin/ochttp/trace.go +++ b/plugin/ochttp/trace.go @@ -65,13 +65,13 @@ func (t *traceTransport) RoundTrip(req *http.Request) (*http.Response, error) { span.AddAttributes(requestAttrs(req)...) resp, err := t.base.RoundTrip(req) if err != nil { - span.SetStatus(trace.Status{Code: 2, Message: err.Error()}) + span.SetStatus(trace.Status{Code: trace.StatusCodeUnknown, Message: err.Error()}) span.End() return resp, err } span.AddAttributes(responseAttrs(resp)...) - span.SetStatus(status(resp.StatusCode)) + span.SetStatus(TraceStatus(resp.StatusCode, resp.Status)) // span.End() will be invoked after // a read from resp.Body returns io.EOF or when @@ -146,73 +146,54 @@ func responseAttrs(resp *http.Response) []trace.Attribute { } } -func status(statusCode int) trace.Status { +// HTTPStatusToTraceStatus converts the HTTP status code to a trace.Status that +// represents the outcome as closely as possible. +func TraceStatus(httpStatusCode int, statusLine string) trace.Status { var code int32 - if statusCode < 200 || statusCode >= 400 { - code = codeUnknown + if httpStatusCode < 200 || httpStatusCode >= 400 { + code = trace.StatusCodeUnknown } - switch statusCode { + switch httpStatusCode { case 499: - code = codeCancelled + code = trace.StatusCodeCancelled case http.StatusBadRequest: - code = codeInvalidArgument + code = trace.StatusCodeInvalidArgument case http.StatusGatewayTimeout: - code = codeDeadlineExceeded + code = trace.StatusCodeDeadlineExceeded case http.StatusNotFound: - code = codeNotFound + code = trace.StatusCodeNotFound case http.StatusForbidden: - code = codePermissionDenied + code = trace.StatusCodePermissionDenied case http.StatusUnauthorized: // 401 is actually unauthenticated. - code = codeUnathenticated + code = trace.StatusCodeUnauthenticated case http.StatusTooManyRequests: - code = codeResourceExhausted + code = trace.StatusCodeResourceExhausted case http.StatusNotImplemented: - code = codeUnimplemented + code = trace.StatusCodeUnimplemented case http.StatusServiceUnavailable: - code = codeUnavailable + code = trace.StatusCodeUnavailable case http.StatusOK: - code = codeOK + code = trace.StatusCodeOK } return trace.Status{Code: code, Message: codeToStr[code]} } -// TODO(jbd): Provide status codes from trace package. -const ( - codeOK = 0 - codeCancelled = 1 - codeUnknown = 2 - codeInvalidArgument = 3 - codeDeadlineExceeded = 4 - codeNotFound = 5 - codeAlreadyExists = 6 - codePermissionDenied = 7 - codeResourceExhausted = 8 - codeFailedPrecondition = 9 - codeAborted = 10 - codeOutOfRange = 11 - codeUnimplemented = 12 - codeInternal = 13 - codeUnavailable = 14 - codeDataLoss = 15 - codeUnathenticated = 16 -) - var codeToStr = map[int32]string{ - codeOK: `"OK"`, - codeCancelled: `"CANCELLED"`, - codeUnknown: `"UNKNOWN"`, - codeInvalidArgument: `"INVALID_ARGUMENT"`, - codeDeadlineExceeded: `"DEADLINE_EXCEEDED"`, - codeNotFound: `"NOT_FOUND"`, - codeAlreadyExists: `"ALREADY_EXISTS"`, - codePermissionDenied: `"PERMISSION_DENIED"`, - codeResourceExhausted: `"RESOURCE_EXHAUSTED"`, - codeFailedPrecondition: `"FAILED_PRECONDITION"`, - codeAborted: `"ABORTED"`, - codeOutOfRange: `"OUT_OF_RANGE"`, - codeUnimplemented: `"UNIMPLEMENTED"`, - codeInternal: `"INTERNAL"`, - codeUnavailable: `"UNAVAILABLE"`, - codeDataLoss: `"DATA_LOSS"`, - codeUnathenticated: `"UNAUTHENTICATED"`, + trace.StatusCodeOK: `"OK"`, + trace.StatusCodeCancelled: `"CANCELLED"`, + trace.StatusCodeUnknown: `"UNKNOWN"`, + trace.StatusCodeInvalidArgument: `"INVALID_ARGUMENT"`, + trace.StatusCodeDeadlineExceeded: `"DEADLINE_EXCEEDED"`, + trace.StatusCodeNotFound: `"NOT_FOUND"`, + trace.StatusCodeAlreadyExists: `"ALREADY_EXISTS"`, + trace.StatusCodePermissionDenied: `"PERMISSION_DENIED"`, + trace.StatusCodeResourceExhausted: `"RESOURCE_EXHAUSTED"`, + trace.StatusCodeFailedPrecondition: `"FAILED_PRECONDITION"`, + trace.StatusCodeAborted: `"ABORTED"`, + trace.StatusCodeOutOfRange: `"OUT_OF_RANGE"`, + trace.StatusCodeUnimplemented: `"UNIMPLEMENTED"`, + trace.StatusCodeInternal: `"INTERNAL"`, + trace.StatusCodeUnavailable: `"UNAVAILABLE"`, + trace.StatusCodeDataLoss: `"DATA_LOSS"`, + trace.StatusCodeUnauthenticated: `"UNAUTHENTICATED"`, } diff --git a/plugin/ochttp/trace_test.go b/plugin/ochttp/trace_test.go index 513fb7d7b..be46cc955 100644 --- a/plugin/ochttp/trace_test.go +++ b/plugin/ochttp/trace_test.go @@ -437,19 +437,20 @@ func TestStatusUnitTest(t *testing.T) { in int want trace.Status }{ - {200, trace.Status{Code: 0, Message: `"OK"`}}, - {100, trace.Status{Code: 2, Message: `"UNKNOWN"`}}, - {500, trace.Status{Code: 2, Message: `"UNKNOWN"`}}, - {404, trace.Status{Code: 5, Message: `"NOT_FOUND"`}}, - {600, trace.Status{Code: 2, Message: `"UNKNOWN"`}}, - {401, trace.Status{Code: 16, Message: `"UNAUTHENTICATED"`}}, - {403, trace.Status{Code: 7, Message: `"PERMISSION_DENIED"`}}, - {301, trace.Status{Code: 0, Message: `"OK"`}}, - {501, trace.Status{Code: 12, Message: `"UNIMPLEMENTED"`}}, + {200, trace.Status{Code: trace.StatusCodeOK, Message: `"OK"`}}, + {204, trace.Status{Code: trace.StatusCodeOK, Message: `"OK"`}}, + {100, trace.Status{Code: trace.StatusCodeUnknown, Message: `"UNKNOWN"`}}, + {500, trace.Status{Code: trace.StatusCodeUnknown, Message: `"UNKNOWN"`}}, + {404, trace.Status{Code: trace.StatusCodeNotFound, Message: `"NOT_FOUND"`}}, + {600, trace.Status{Code: trace.StatusCodeUnknown, Message: `"UNKNOWN"`}}, + {401, trace.Status{Code: trace.StatusCodeUnauthenticated, Message: `"UNAUTHENTICATED"`}}, + {403, trace.Status{Code: trace.StatusCodePermissionDenied, Message: `"PERMISSION_DENIED"`}}, + {301, trace.Status{Code: trace.StatusCodeOK, Message: `"OK"`}}, + {501, trace.Status{Code: trace.StatusCodeUnimplemented, Message: `"UNIMPLEMENTED"`}}, } for _, tt := range tests { - got, want := status(tt.in), tt.want + got, want := TraceStatus(tt.in, ""), tt.want if got != want { t.Errorf("status(%d) got = (%#v) want = (%#v)", tt.in, got, want) } diff --git a/trace/status_codes.go b/trace/status_codes.go new file mode 100644 index 000000000..ec60effd1 --- /dev/null +++ b/trace/status_codes.go @@ -0,0 +1,37 @@ +// Copyright 2018, OpenCensus Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trace + +// Status codes for use with Span.SetStatus. These correspond to the status +// codes used by gRPC defined here: https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto +const ( + StatusCodeOK = 0 + StatusCodeCancelled = 1 + StatusCodeUnknown = 2 + StatusCodeInvalidArgument = 3 + StatusCodeDeadlineExceeded = 4 + StatusCodeNotFound = 5 + StatusCodeAlreadyExists = 6 + StatusCodePermissionDenied = 7 + StatusCodeResourceExhausted = 8 + StatusCodeFailedPrecondition = 9 + StatusCodeAborted = 10 + StatusCodeOutOfRange = 11 + StatusCodeUnimplemented = 12 + StatusCodeInternal = 13 + StatusCodeUnavailable = 14 + StatusCodeDataLoss = 15 + StatusCodeUnauthenticated = 16 +)