From 8ff86dc7547497c53e590c2528125b3b16cbe064 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Fri, 28 Jun 2019 15:40:05 +0530 Subject: [PATCH] Improve handling of status code/message (#51) Improved how translators handling mapping status codes and messages to and from OC translator/trace/README.md documents the new behaviour --- go.mod | 2 + go.sum | 4 + .../tracesamplerprocessor.go | 2 +- .../tracesamplerprocessor_test.go | 2 +- receiver/jaegerreceiver/jaeger_agent_test.go | 12 - .../jaegerreceiver/trace_receiver_test.go | 12 - receiver/prometheusreceiver/config.go | 2 +- receiver/prometheusreceiver/config_test.go | 2 +- receiver/prometheusreceiver/factory.go | 2 +- receiver/prometheusreceiver/factory_test.go | 2 +- receiver/zipkinreceiver/config.go | 2 +- receiver/zipkinreceiver/config_test.go | 2 +- receiver/zipkinreceiver/factory.go | 2 +- receiver/zipkinreceiver/factory_test.go | 2 +- translator/trace/README.md | 74 ++++ translator/trace/grpc_http_mapper.go | 61 +++ .../trace/jaeger/jaegerthrift_to_protospan.go | 22 +- .../jaeger/jaegerthrift_to_protospan_test.go | 278 ++++++++++++- .../trace/jaeger/protospan_to_jaegerproto.go | 21 +- .../jaeger/protospan_to_jaegerproto_test.go | 217 ++++++++++ .../trace/jaeger/protospan_to_jaegerthrift.go | 31 ++ .../jaeger/protospan_to_jaegerthrift_test.go | 220 +++++++++- translator/trace/jaeger/status_code.go | 55 +++ .../jaeger/testdata/jaegerproto_batch_01.json | 202 +++++++++ .../jaeger/testdata/jaegerproto_batch_02.json | 80 ++++ translator/trace/protospan_translation.go | 7 + .../trace/spandata/protospan_to_spandata.go | 5 +- .../spandata/protospan_to_spandata_test.go | 3 +- translator/trace/zipkin/status_code.go | 110 +++++ .../zipkin/zipkinv1_thrift_to_protospan.go | 22 +- .../zipkinv1_thrift_to_protospan_test.go | 383 ++++++++++++++++++ .../trace/zipkin/zipkinv1_to_protospan.go | 24 +- .../zipkin/zipkinv1_to_protospan_test.go | 325 +++++++++++++++ 33 files changed, 2114 insertions(+), 76 deletions(-) create mode 100644 translator/trace/README.md create mode 100644 translator/trace/grpc_http_mapper.go create mode 100644 translator/trace/jaeger/status_code.go create mode 100644 translator/trace/jaeger/testdata/jaegerproto_batch_01.json create mode 100644 translator/trace/jaeger/testdata/jaegerproto_batch_02.json create mode 100644 translator/trace/zipkin/status_code.go diff --git a/go.mod b/go.mod index 7ee0c2ae0ad..0cdc1f26d0b 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/jaegertracing/jaeger v1.9.0 github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024 + github.com/mitchellh/mapstructure v1.1.2 // indirect github.com/omnition/scribe-go v0.0.0-20190131012523-9e3c68f31124 github.com/opentracing/opentracing-go v1.1.0 // indirect github.com/openzipkin/zipkin-go v0.1.6 @@ -37,6 +38,7 @@ require ( github.com/spf13/cobra v0.0.3 github.com/spf13/viper v1.2.1 github.com/streadway/quantile v0.0.0-20150917103942-b0c588724d25 // indirect + github.com/stretchr/objx v0.1.1 // indirect github.com/stretchr/testify v1.3.0 github.com/uber-go/atomic v1.3.2 // indirect github.com/uber/jaeger-client-go v2.16.0+incompatible // indirect diff --git a/go.sum b/go.sum index 5531be9ab8c..413ab84b408 100644 --- a/go.sum +++ b/go.sum @@ -216,6 +216,8 @@ github.com/mitchellh/go-testing-interface v1.0.0 h1:fzU/JVNcaqHQEcVFAKeR41fkiLdI github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eIhFbs2+BFgRtFPeD/KE+zxI= github.com/mitchellh/mapstructure v1.0.0 h1:vVpGvMXJPqSDh2VYHF7gsfQj8Ncx+Xw5Y1KHeTRY+7I= github.com/mitchellh/mapstructure v1.0.0/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE= +github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -323,6 +325,8 @@ github.com/streadway/quantile v0.0.0-20150917103942-b0c588724d25 h1:7z3LSn867ex6 github.com/streadway/quantile v0.0.0-20150917103942-b0c588724d25/go.mod h1:lbP8tGiBjZ5YWIc2fzuRpTaz0b/53vT6PEs3QuAWzuU= github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= +github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/processor/tracesamplerprocessor/tracesamplerprocessor.go b/processor/tracesamplerprocessor/tracesamplerprocessor.go index 8f5b5c43fe6..295f207e901 100644 --- a/processor/tracesamplerprocessor/tracesamplerprocessor.go +++ b/processor/tracesamplerprocessor/tracesamplerprocessor.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/processor/tracesamplerprocessor/tracesamplerprocessor_test.go b/processor/tracesamplerprocessor/tracesamplerprocessor_test.go index fd74365f7fe..3e7bdc1ec23 100644 --- a/processor/tracesamplerprocessor/tracesamplerprocessor_test.go +++ b/processor/tracesamplerprocessor/tracesamplerprocessor_test.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/jaegerreceiver/jaeger_agent_test.go b/receiver/jaegerreceiver/jaeger_agent_test.go index 629fc9a8d7e..c1f44a7c6f0 100644 --- a/receiver/jaegerreceiver/jaeger_agent_test.go +++ b/receiver/jaegerreceiver/jaeger_agent_test.go @@ -171,12 +171,6 @@ func testJaegerAgent(t *testing.T, agentEndpoint string, receiverConfig *Configu }, Attributes: &tracepb.Span_Attributes{ AttributeMap: map[string]*tracepb.AttributeValue{ - "status.code": { - Value: &tracepb.AttributeValue_IntValue{IntValue: trace.StatusCodeNotFound}, - }, - "status.message": { - Value: &tracepb.AttributeValue_StringValue{StringValue: &tracepb.TruncatableString{Value: "Stale indices"}}, - }, "error": { Value: &tracepb.AttributeValue_BoolValue{BoolValue: true}, }, @@ -204,12 +198,6 @@ func testJaegerAgent(t *testing.T, agentEndpoint string, receiverConfig *Configu }, Attributes: &tracepb.Span_Attributes{ AttributeMap: map[string]*tracepb.AttributeValue{ - "status.code": { - Value: &tracepb.AttributeValue_IntValue{IntValue: trace.StatusCodeInternal}, - }, - "status.message": { - Value: &tracepb.AttributeValue_StringValue{StringValue: &tracepb.TruncatableString{Value: "Frontend crash"}}, - }, "error": { Value: &tracepb.AttributeValue_BoolValue{BoolValue: true}, }, diff --git a/receiver/jaegerreceiver/trace_receiver_test.go b/receiver/jaegerreceiver/trace_receiver_test.go index a0454323a06..db5e63e3581 100644 --- a/receiver/jaegerreceiver/trace_receiver_test.go +++ b/receiver/jaegerreceiver/trace_receiver_test.go @@ -154,12 +154,6 @@ func TestReception(t *testing.T) { }, Attributes: &tracepb.Span_Attributes{ AttributeMap: map[string]*tracepb.AttributeValue{ - "status.code": { - Value: &tracepb.AttributeValue_IntValue{IntValue: trace.StatusCodeNotFound}, - }, - "status.message": { - Value: &tracepb.AttributeValue_StringValue{StringValue: &tracepb.TruncatableString{Value: "Stale indices"}}, - }, "error": { Value: &tracepb.AttributeValue_BoolValue{BoolValue: true}, }, @@ -187,12 +181,6 @@ func TestReception(t *testing.T) { }, Attributes: &tracepb.Span_Attributes{ AttributeMap: map[string]*tracepb.AttributeValue{ - "status.code": { - Value: &tracepb.AttributeValue_IntValue{IntValue: trace.StatusCodeInternal}, - }, - "status.message": { - Value: &tracepb.AttributeValue_StringValue{StringValue: &tracepb.TruncatableString{Value: "Frontend crash"}}, - }, "error": { Value: &tracepb.AttributeValue_BoolValue{BoolValue: true}, }, diff --git a/receiver/prometheusreceiver/config.go b/receiver/prometheusreceiver/config.go index eca2a108e9b..def7888303e 100644 --- a/receiver/prometheusreceiver/config.go +++ b/receiver/prometheusreceiver/config.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/prometheusreceiver/config_test.go b/receiver/prometheusreceiver/config_test.go index cf24cddb8c9..84a4f0b2514 100644 --- a/receiver/prometheusreceiver/config_test.go +++ b/receiver/prometheusreceiver/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/prometheusreceiver/factory.go b/receiver/prometheusreceiver/factory.go index 0858e1b6fbd..f7c66b783c1 100644 --- a/receiver/prometheusreceiver/factory.go +++ b/receiver/prometheusreceiver/factory.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/prometheusreceiver/factory_test.go b/receiver/prometheusreceiver/factory_test.go index 3089d2fbffe..63baaad1ea8 100644 --- a/receiver/prometheusreceiver/factory_test.go +++ b/receiver/prometheusreceiver/factory_test.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/zipkinreceiver/config.go b/receiver/zipkinreceiver/config.go index f31305a0a2b..73dbad0c46e 100644 --- a/receiver/zipkinreceiver/config.go +++ b/receiver/zipkinreceiver/config.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/zipkinreceiver/config_test.go b/receiver/zipkinreceiver/config_test.go index 7136262a6d8..7a757fed478 100644 --- a/receiver/zipkinreceiver/config_test.go +++ b/receiver/zipkinreceiver/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/zipkinreceiver/factory.go b/receiver/zipkinreceiver/factory.go index 36b82c974f2..67e738500a6 100644 --- a/receiver/zipkinreceiver/factory.go +++ b/receiver/zipkinreceiver/factory.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/receiver/zipkinreceiver/factory_test.go b/receiver/zipkinreceiver/factory_test.go index 37aabce9f85..661fd0dac3b 100644 --- a/receiver/zipkinreceiver/factory_test.go +++ b/receiver/zipkinreceiver/factory_test.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/translator/trace/README.md b/translator/trace/README.md new file mode 100644 index 00000000000..d7c2b560c6d --- /dev/null +++ b/translator/trace/README.md @@ -0,0 +1,74 @@ +# Overview + +This package implements a number of translators that help translate spans to and from OpenCensus format to a number of other supported formats such as Jaeger Proto, Jaeger Thrift, Zipkin Thrift, Zipkin JSON. This document mentions how certain non-obvious things should be handled. + +## Links: + +* [OpenTracing Semantic Conventions](https://github.com/opentracing/specification/blob/master/semantic_conventions.md) + +## Status Codes and Messages + +### OpenCensus + +OpenCensus protocol has a special field to represent the status of an operation. The status field has two fields, an int32 field called `code` and a string field called `message`. When converting from other formats, status field must be set from the relevant tags/attributes of the source format. When converting from OC to other formats, the status field must be translated to appropriate tags/attributes of the target format. + + +### Jaeger to OC + +Jaeger spans may contain two possible sets of tags that can possibly represent the status of an operation: + +- `status.code` and `status.message` +- `http.status_code` and `http.status_message` + +When converting from Jaeger to OC, + +1. OC status should be set from `status.code` and `status.message` tags if `status.code` tag is found on the Jaeger span. Since OC already has a special status field, these tags (`status.code` and `status.message`) are redundant and should be dropped from resultant OC span. +2. If the `status.code` tag is not present, status should be set from `http.status_code` and `http.status_message` if the `http.status_code` tag is found. HTTP status code should be mapped to the appropriate gRPC status code before using it in OC status. These tags should be preserved and added to the resultant OC span as attributes. +3. If none of the tags are found, OC status should not be set. + + +### Zipkin to OC + +In addition to the two sets of tags mentioned in the previous section, Zipkin spans can possibly contain a third set of tags to represent operation status resulting in the following sets of tags: + +- `census.status_code` and `census.status_description` +- `status.code` and `status.message` +- `http.status_code` and `http.status_message` + +When converting from Zipkin to OC, + +1. OC status should be set from `census.status_code` and `census.status_description` if `census.status_code` tag is found on the Zipkin span. These tags should be dropped from the resultant OC span. +2. If the `census.status_code` tag is not found in step 1, OC status should be set from `status.code` and `status.message` tags if the `status.code` tag is present. The tags should be dropped from the resultant OC span. +3. If no tags are found in step 1 and 2, OC status should be set from `http.status_code` and `http.status_message` if either `http.status_code` tag is found. These tags should be preserved and added to the resultant OC span as attributes. +4. If none of the tags are found, OC status should not be set. + + +Note that codes and messages from different sets of tags should not be mixed to form the status field. For example, OC status should not contain code from `http.status_code` but message from `status.message` and vice-versa. Both fields must be set from the same set of tags even if it means leaving one of the two fields empty. + + +### OC to Jaeger + +When converting from OC to Jaeger, if the OC span has a status field set, then + +* `code` should be added as a `status.code` tag. +* `message` should be added as a `status.message` tag. + +### OC to Zipkin + +When converting from OC to Zipkin, if the OC span has the status field set, then + +* `code` should be added as a `census.status_code` tag. +* `message` should be added as a `census.status_description` tag. + +In addition to this, if the OC status field represents a non-OK status, then a tag with the key `error` and value `true` should be added to the Zipkin span if one is not already present. + +### Note: + +If either target tags (`status.*` or `census.status_*`) are already present on the span, then they should be preserved and not overwritten from the status field. This is extremely unlikely to happen within the collector because of how things are implemented but any other implementations should still follow this rule. + + +## Converting HTTP status codes to OC codes + +The following guidelines should be followed for translating HTTP status codes to OC ones. https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto + +This is implemented by the `tracetranslator` package as `HTTPToOCCodeMapper`. \ No newline at end of file diff --git a/translator/trace/grpc_http_mapper.go b/translator/trace/grpc_http_mapper.go new file mode 100644 index 00000000000..1bb544f27a8 --- /dev/null +++ b/translator/trace/grpc_http_mapper.go @@ -0,0 +1,61 @@ +// Copyright 2019, OpenTelemetry 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 tracetranslator + +// https://github.com/googleapis/googleapis/blob/bee79fbe03254a35db125dc6d2f1e9b752b390fe/google/rpc/code.proto#L33-L186 +const ( + OCOK = 0 + OCCancelled = 1 + OCUnknown = 2 + OCInvalidArgument = 3 + OCDeadlineExceeded = 4 + OCNotFound = 5 + OCAlreadyExists = 6 + OCPermissionDenied = 7 + OCResourceExhausted = 8 + OCFailedPrecondition = 9 + OCAborted = 10 + OCOutOfRange = 11 + OCUnimplemented = 12 + OCInternal = 13 + OCUnavailable = 14 + OCDataLoss = 15 + OCUnauthenticated = 16 +) + +var httpToOCCodeMap = map[int32]int32{ + 400: OCInvalidArgument, + 401: OCUnauthenticated, + 403: OCPermissionDenied, + 404: OCNotFound, + 409: OCAborted, + 429: OCResourceExhausted, + 499: OCCancelled, + 500: OCInternal, + 501: OCUnimplemented, + 503: OCUnavailable, + 504: OCDeadlineExceeded, +} + +// OCStatusCodeFromHTTP takes an HTTP status code and return the appropriate OC status code +func OCStatusCodeFromHTTP(code int32) int32 { + if code >= 200 && code < 300 { + return OCOK + } + if code, ok := httpToOCCodeMap[code]; ok { + return code + } + return OCUnknown +} diff --git a/translator/trace/jaeger/jaegerthrift_to_protospan.go b/translator/trace/jaeger/jaegerthrift_to_protospan.go index 1999315f24e..10c1fc796c5 100644 --- a/translator/trace/jaeger/jaegerthrift_to_protospan.go +++ b/translator/trace/jaeger/jaegerthrift_to_protospan.go @@ -190,6 +190,8 @@ func jtagsToAttributes(tags []*jaeger.Tag) (string, tracepb.Span_SpanKind, *trac var statusCodePtr *int32 var statusMessage string + var httpStatusCodePtr *int32 + var httpStatusMessage string var message string sAttribs := make(map[string]*tracepb.AttributeValue) @@ -206,12 +208,19 @@ func jtagsToAttributes(tags []*jaeger.Tag) (string, tracepb.Span_SpanKind, *trac sKind = tracepb.Span_SERVER } - case "http.status_code", "status.code": // It is expected to be an int - statusCodePtr = new(int32) - *statusCodePtr = int32(tag.GetVLong()) + case tracetranslator.TagStatusCode: + statusCodePtr = statusCodeFromTag(tag) + continue - case "http.status_message", "status.message": + case tracetranslator.TagStatusMsg: statusMessage = tag.GetVStr() + continue + + case tracetranslator.TagHTTPStatusCode: + httpStatusCodePtr = statusCodeFromHTTPTag(tag) + + case tracetranslator.TagHTTPStatusMsg: + httpStatusMessage = tag.GetVStr() case "message": message = tag.GetVStr() @@ -248,6 +257,11 @@ func jtagsToAttributes(tags []*jaeger.Tag) (string, tracepb.Span_SpanKind, *trac sAttribs[tag.Key] = attrib } + if statusCodePtr == nil { + statusCodePtr = httpStatusCodePtr + statusMessage = httpStatusMessage + } + var sStatus *tracepb.Status if statusCodePtr != nil || statusMessage != "" { statusCode := int32(0) diff --git a/translator/trace/jaeger/jaegerthrift_to_protospan_test.go b/translator/trace/jaeger/jaegerthrift_to_protospan_test.go index 9d0a13fd2b7..6a2e0b65006 100644 --- a/translator/trace/jaeger/jaegerthrift_to_protospan_test.go +++ b/translator/trace/jaeger/jaegerthrift_to_protospan_test.go @@ -28,6 +28,7 @@ import ( tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/open-telemetry/opentelemetry-service/data" "github.com/open-telemetry/opentelemetry-service/internal/testutils" + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" ) func TestThriftBatchToOCProto_Roundtrip(t *testing.T) { @@ -285,7 +286,7 @@ func TestConservativeConversions(t *testing.T) { TraceId: []byte{0x00, 0x11, 0x12, 0x13, 0x14, 0x11, 0x11, 0x11, 0x01, 0x11, 0x11, 0x11, 0xFF, 0xFF, 0xFF, 0xFF}, // Ensure that the status code was properly translated Status: &tracepb.Status{ - Code: 403, + Code: 7, Message: "Forbidden", }, Attributes: &tracepb.Span_Attributes{ @@ -311,20 +312,7 @@ func TestConservativeConversions(t *testing.T) { Code: 13, Message: "proxy crashed", }, - Attributes: &tracepb.Span_Attributes{ - AttributeMap: map[string]*tracepb.AttributeValue{ - "status.code": { - Value: &tracepb.AttributeValue_IntValue{ - IntValue: 13, - }, - }, - "status.message": { - Value: &tracepb.AttributeValue_StringValue{ - StringValue: &tracepb.TruncatableString{Value: "proxy crashed"}, - }, - }, - }, - }, + Attributes: nil, }, }, }, @@ -354,6 +342,266 @@ func TestConservativeConversions(t *testing.T) { } } +func TestJaegerStatusTagsToOCStatus(t *testing.T) { + type test struct { + haveTags []*jaeger.Tag + wantAttributes *tracepb.Span_Attributes + wantStatus *tracepb.Status + } + + cases := []test{ + // only status.code tag + { + haveTags: []*jaeger.Tag{ + { + Key: "status.code", + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 13, + }, + }, + // only status.message tag + { + haveTags: []*jaeger.Tag{ + { + Key: "status.message", + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + wantAttributes: nil, + wantStatus: nil, + }, + // both status.code and status.message + { + haveTags: []*jaeger.Tag{ + { + Key: "status.code", + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "status.message", + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + + // http status.code + { + haveTags: []*jaeger.Tag{ + { + Key: "http.status_code", + VLong: func() *int64 { v := int64(404); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "http.status_message", + VStr: func() *string { v := "NotFound"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + + // http and oc + { + haveTags: []*jaeger.Tag{ + { + Key: "http.status_code", + VLong: func() *int64 { v := int64(404); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "http.status_message", + VStr: func() *string { v := "NotFound"; return &v }(), + VType: jaeger.TagType_STRING, + }, + { + Key: "status.code", + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "status.message", + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + // http and only oc code + { + haveTags: []*jaeger.Tag{ + { + Key: "http.status_code", + VLong: func() *int64 { v := int64(404); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "http.status_message", + VStr: func() *string { v := "NotFound"; return &v }(), + VType: jaeger.TagType_STRING, + }, + { + Key: "status.code", + VLong: func() *int64 { v := int64(14); return &v }(), + VType: jaeger.TagType_LONG, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 14, + }, + }, + // http and only oc message + { + haveTags: []*jaeger.Tag{ + { + Key: "http.status_code", + VLong: func() *int64 { v := int64(404); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: "http.status_message", + VStr: func() *string { v := "NotFound"; return &v }(), + VType: jaeger.TagType_STRING, + }, + { + Key: "status.message", + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + } + + for i, c := range cases { + gb, err := ThriftBatchToOCProto(&jaeger.Batch{ + Process: nil, + Spans: []*jaeger.Span{{ + TraceIdLow: 0x1001021314151617, + TraceIdHigh: 0x100102F3F4F5F6F7, + SpanId: 0x1011121314151617, + Tags: c.haveTags, + }}, + }) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb.Spans[0] + if !reflect.DeepEqual(gs.Attributes, c.wantAttributes) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Attributes, c.wantAttributes) + } + if !reflect.DeepEqual(gs.Status, c.wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, c.wantStatus) + } + } +} + +func TestHTTPToGRPCStatusCode(t *testing.T) { + for i := int64(100); i <= 600; i++ { + wantStatus := tracetranslator.OCStatusCodeFromHTTP(int32(i)) + gb, err := ThriftBatchToOCProto(&jaeger.Batch{ + Process: nil, + Spans: []*jaeger.Span{{ + TraceIdLow: 0x1001021314151617, + TraceIdHigh: 0x100102F3F4F5F6F7, + SpanId: 0x1011121314151617, + Tags: []*jaeger.Tag{{ + Key: "http.status_code", + VLong: &i, + VType: jaeger.TagType_LONG, + }}, + }}, + }) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb.Spans[0] + if !reflect.DeepEqual(gs.Status.Code, wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, wantStatus) + } + } +} + func jsonify(v interface{}) []byte { jb, _ := json.MarshalIndent(v, "", " ") return jb diff --git a/translator/trace/jaeger/protospan_to_jaegerproto.go b/translator/trace/jaeger/protospan_to_jaegerproto.go index 80a168d9dd4..f555db89b5b 100644 --- a/translator/trace/jaeger/protospan_to_jaegerproto.go +++ b/translator/trace/jaeger/protospan_to_jaegerproto.go @@ -387,12 +387,25 @@ func appendJaegerTagFromOCStatusProto(jTags []jaeger.KeyValue, ocStatus *tracepb return jTags } - jTag := jaeger.KeyValue{ - Key: "span.status", + for _, jt := range jTags { + if jt.Key == "status.code" || jt.Key == "status.message" { + return jTags + } + } + + jTags = append(jTags, jaeger.KeyValue{ + Key: tracetranslator.TagStatusCode, VInt64: int64(ocStatus.Code), - VStr: ocStatus.Message, + VType: jaeger.ValueType_INT64, + }) + + if ocStatus.Message != "" { + jTags = append(jTags, jaeger.KeyValue{ + Key: tracetranslator.TagStatusMsg, + VStr: ocStatus.Message, + VType: jaeger.ValueType_STRING, + }) } - jTags = append(jTags, jTag) return jTags } diff --git a/translator/trace/jaeger/protospan_to_jaegerproto_test.go b/translator/trace/jaeger/protospan_to_jaegerproto_test.go index c38f2eef331..7d86a716543 100644 --- a/translator/trace/jaeger/protospan_to_jaegerproto_test.go +++ b/translator/trace/jaeger/protospan_to_jaegerproto_test.go @@ -17,6 +17,7 @@ package jaeger import ( "encoding/json" "fmt" + "reflect" "sort" "testing" @@ -27,6 +28,7 @@ import ( "github.com/open-telemetry/opentelemetry-service/data" "github.com/open-telemetry/opentelemetry-service/internal/testutils" + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" ) func TestNilOCProtoNodeToJaegerProto(t *testing.T) { @@ -107,6 +109,221 @@ func TestOCProtoToJaegerProto(t *testing.T) { } } +func TestOCStatusToJaegerProtoTags(t *testing.T) { + + type test struct { + haveAttributes *tracepb.Span_Attributes + haveStatus *tracepb.Status + wantTags []jaeger.KeyValue + } + + cases := []test{ + // only status.code + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Code: 10, + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(10), + VType: jaeger.ValueType_INT64, + }, + }, + }, + // only status.message + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Message: "Forbidden", + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(0), + VType: jaeger.ValueType_INT64, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: "Forbidden", + VType: jaeger.ValueType_STRING, + }, + }, + }, + // both status.code and status.message + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Code: 12, + Message: "Forbidden", + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(12), + VType: jaeger.ValueType_INT64, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: "Forbidden", + VType: jaeger.ValueType_STRING, + }, + }, + }, + + // status and existing tags + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 13, + }, + }, + "status.message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "Error"}, + }, + }, + }, + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(13), + VType: jaeger.ValueType_INT64, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: "Error", + VType: jaeger.ValueType_STRING, + }, + }, + }, + + // partial existing tag + + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 13, + }, + }, + }, + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(13), + VType: jaeger.ValueType_INT64, + }, + }, + }, + + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "Error"}, + }, + }, + }, + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagStatusMsg, + VStr: "Error", + VType: jaeger.ValueType_STRING, + }, + }, + }, + // both status and tags + { + haveStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "http.status_code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + "http.status_message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantTags: []jaeger.KeyValue{ + { + Key: tracetranslator.TagHTTPStatusCode, + VInt64: int64(404), + VType: jaeger.ValueType_INT64, + }, + { + Key: tracetranslator.TagHTTPStatusMsg, + VStr: "NotFound", + VType: jaeger.ValueType_STRING, + }, + { + Key: tracetranslator.TagStatusCode, + VInt64: int64(13), + VType: jaeger.ValueType_INT64, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: "Forbidden", + VType: jaeger.ValueType_STRING, + }, + }, + }, + } + + fakeTraceID := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15} + fakeSpanID := []byte{0, 1, 2, 3, 4, 5, 6, 7} + for i, c := range cases { + gb, err := OCProtoToJaegerProto(data.TraceData{ + Spans: []*tracepb.Span{{ + TraceId: fakeTraceID, + SpanId: fakeSpanID, + Status: c.haveStatus, + Attributes: c.haveAttributes, + }}, + }) + + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb.Spans[0] + sort.Slice(gs.Tags, func(i, j int) bool { + return gs.Tags[i].Key < gs.Tags[j].Key + }) + if !reflect.DeepEqual(c.wantTags, gs.Tags) { + t.Fatalf("%d: Unsuccessful conversion\nGot:\n\t%v\nWant:\n\t%v", i, gs.Tags, c.wantTags) + } + } +} + // ocBatches has the OpenCensus proto batches used in the test. They are hard coded because // structs like tracepb.AttributeMap cannot be read from JSON. var ocBatches = []data.TraceData{ diff --git a/translator/trace/jaeger/protospan_to_jaegerthrift.go b/translator/trace/jaeger/protospan_to_jaegerthrift.go index d901a06085a..1b881ceb0be 100644 --- a/translator/trace/jaeger/protospan_to_jaegerthrift.go +++ b/translator/trace/jaeger/protospan_to_jaegerthrift.go @@ -202,6 +202,8 @@ func ocSpansToJaegerSpans(ocSpans []*tracepb.Span) ([]*jaeger.Span, error) { } } + jSpan.Tags = appendJaegerThriftTagFromOCStatus(jSpan.Tags, ocSpan.Status) + jSpans = append(jSpans, jSpan) } @@ -248,6 +250,35 @@ func ocLinksToJaegerReferences(ocSpanLinks *tracepb.Span_Links) ([]*jaeger.SpanR return jRefs, nil } +func appendJaegerThriftTagFromOCStatus(jTags []*jaeger.Tag, ocStatus *tracepb.Status) []*jaeger.Tag { + if ocStatus == nil { + return jTags + } + + for _, jt := range jTags { + if jt.Key == tracetranslator.TagStatusCode || jt.Key == tracetranslator.TagStatusMsg { + return jTags + } + } + + code := int64(ocStatus.Code) + jTags = append(jTags, &jaeger.Tag{ + Key: tracetranslator.TagStatusCode, + VLong: &code, + VType: jaeger.TagType_LONG, + }) + + if ocStatus.Message != "" { + jTags = append(jTags, &jaeger.Tag{ + Key: tracetranslator.TagStatusMsg, + VStr: &ocStatus.Message, + VType: jaeger.TagType_STRING, + }) + } + + return jTags +} + func appendJaegerTagFromOCSpanKind(jTags []*jaeger.Tag, ocSpanKind tracepb.Span_SpanKind) []*jaeger.Tag { // TODO: (@pjanotti): Replace any OpenTracing literals by importing github.com/opentracing/opentracing-go/ext? var tagValue string diff --git a/translator/trace/jaeger/protospan_to_jaegerthrift_test.go b/translator/trace/jaeger/protospan_to_jaegerthrift_test.go index 66b02e66210..c0a7f83d64d 100644 --- a/translator/trace/jaeger/protospan_to_jaegerthrift_test.go +++ b/translator/trace/jaeger/protospan_to_jaegerthrift_test.go @@ -17,12 +17,11 @@ package jaeger import ( "encoding/json" "fmt" + "reflect" "sort" "strings" "testing" - tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" - commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" @@ -30,9 +29,10 @@ import ( "github.com/open-telemetry/opentelemetry-service/data" "github.com/open-telemetry/opentelemetry-service/internal/testutils" + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" ) -func TestInvalidOCProtoIDs(t *testing.T) { +func TestThriftInvalidOCProtoIDs(t *testing.T) { fakeTraceID := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15} tests := []struct { name string @@ -166,6 +166,220 @@ func TestOCProtoToJaegerThrift(t *testing.T) { } } +func TestOCStatusToJaegerThriftTags(t *testing.T) { + + type test struct { + haveAttributes *tracepb.Span_Attributes + haveStatus *tracepb.Status + wantTags []*jaeger.Tag + } + + cases := []test{ + // only status.code + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Code: 10, + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(10); return &v }(), + VType: jaeger.TagType_LONG, + }, + }, + }, + // only status.message + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Message: "Message", + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(0); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: func() *string { v := "Message"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + }, + // both status.code and status.message + { + haveAttributes: nil, + haveStatus: &tracepb.Status{ + Code: 12, + Message: "Forbidden", + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(12); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + }, + + // status and existing tags + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 13, + }, + }, + "status.message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "Error"}, + }, + }, + }, + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: func() *string { v := "Error"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + }, + + // partial existing tag + + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 13, + }, + }, + }, + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + }, + }, + + { + haveStatus: &tracepb.Status{ + Code: 404, + Message: "NotFound", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "status.message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "Error"}, + }, + }, + }, + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagStatusMsg, + VStr: func() *string { v := "Error"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + }, + // both status and tags + { + haveStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + haveAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + "http.status_code": { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + "http.status_message": { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantTags: []*jaeger.Tag{ + { + Key: tracetranslator.TagHTTPStatusCode, + VLong: func() *int64 { v := int64(404); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: tracetranslator.TagHTTPStatusMsg, + VStr: func() *string { v := "NotFound"; return &v }(), + VType: jaeger.TagType_STRING, + }, + { + Key: tracetranslator.TagStatusCode, + VLong: func() *int64 { v := int64(13); return &v }(), + VType: jaeger.TagType_LONG, + }, + { + Key: tracetranslator.TagStatusMsg, + VStr: func() *string { v := "Forbidden"; return &v }(), + VType: jaeger.TagType_STRING, + }, + }, + }, + } + + fakeTraceID := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15} + fakeSpanID := []byte{0, 1, 2, 3, 4, 5, 6, 7} + for i, c := range cases { + gb, err := OCProtoToJaegerThrift(data.TraceData{ + Spans: []*tracepb.Span{{ + TraceId: fakeTraceID, + SpanId: fakeSpanID, + Status: c.haveStatus, + Attributes: c.haveAttributes, + }}, + }) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb.Spans[0] + sort.Slice(gs.Tags, func(i, j int) bool { + return gs.Tags[i].Key < gs.Tags[j].Key + }) + if !reflect.DeepEqual(c.wantTags, gs.Tags) { + t.Fatalf("%d: Unsuccessful conversion\nGot:\n\t%v\nWant:\n\t%v", i, gs.Tags, c.wantTags) + } + } +} + // tds has the TraceData proto used in the test. They are hard coded because // structs like tracepb.AttributeMap cannot be ready from JSON. var tds = []data.TraceData{ diff --git a/translator/trace/jaeger/status_code.go b/translator/trace/jaeger/status_code.go new file mode 100644 index 00000000000..f6127aca179 --- /dev/null +++ b/translator/trace/jaeger/status_code.go @@ -0,0 +1,55 @@ +// Copyright 2019, OpenTelemetry 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 jaeger + +import ( + "math" + "strconv" + + "github.com/jaegertracing/jaeger/thrift-gen/jaeger" + + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" +) + +func statusCodeFromHTTPTag(tag *jaeger.Tag) *int32 { + code := statusCodeFromTag(tag) + if code != nil { + *code = tracetranslator.OCStatusCodeFromHTTP(*code) + } + return code +} + +// statusCodeFromTag maps an integer attribute value to a status code (int32). +// The function return nil if the value is not an integer or an integer larger than what +// can fit in an int32 +func statusCodeFromTag(tag *jaeger.Tag) *int32 { + toInt32 := func(i int) *int32 { + if i <= math.MaxInt32 { + j := int32(i) + return &j + } + return nil + } + + switch tag.GetVType() { + case jaeger.TagType_LONG: + return toInt32(int(tag.GetVLong())) + case jaeger.TagType_STRING: + if i, err := strconv.Atoi(tag.GetVStr()); err == nil { + return toInt32(i) + } + } + return nil +} diff --git a/translator/trace/jaeger/testdata/jaegerproto_batch_01.json b/translator/trace/jaeger/testdata/jaegerproto_batch_01.json new file mode 100644 index 00000000000..4eb215d34a0 --- /dev/null +++ b/translator/trace/jaeger/testdata/jaegerproto_batch_01.json @@ -0,0 +1,202 @@ +{ + "process": { + "service_name": "api", + "tags": [ + { + "key": "hostname", + "v_type": 0, + "v_str": "api246-sjc1" + }, + { + "key": "pid", + "v_type": 2, + "v_int64": 13 + }, + { + "key": "start.time", + "v_type": 0, + "v_str": "2017-01-26T21:46:30.639875Z" + }, + { + "key": "ip", + "v_type": 0, + "v_str": "10.53.69.61" + }, + { + "key": "opencensus.exporterversion", + "v_type": 0, + "v_str": "someVersion" + }, + { + "key": "a.bool", + "v_type": 0, + "v_str": "true" + }, + { + "key": "a.double", + "v_type": 0, + "v_str": "1234.56789" + }, + { + "key": "a.long", + "v_type": 0, + "v_str": "123456789" + }, + { + "key": "a.binary", + "v_type": 0, + "v_str": "AQIDBAMCAQ==" + } + ] + }, + "spans": [ + { + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABkfZg=", + "parent_span_id": 6866147, + "operation_name": "get", + "start_time": "2017-01-26T21:46:31.639875Z", + "duration": 22938000, + "tags": [ + { + "key": "http.url", + "v_type": 0, + "v_str": "http://127.0.0.1:15598/client_transactions" + }, + { + "key": "span.kind", + "v_type": 0, + "v_str": "server" + }, + { + "key": "peer.port", + "v_type": 2, + "v_int64": 53931 + }, + { + "key": "someBool", + "v_type": 1, + "v_bool": true + }, + { + "key": "someDouble", + "v_type": 3, + "v_float64": 129.8 + }, + { + "key": "peer.service", + "v_type": 0, + "v_str": "rtapi" + }, + { + "key": "peer.ipv4", + "v_type": 2, + "v_int64": 3224716605 + } + ], + "logs": [ + { + "timestamp": "2017-01-26T21:46:31.639874Z", + "fields": [ + { + "key": "message.id", + "v_int64": 0, + "v_type": 2 + }, + { + "key": "message.type", + "v_str": "SENT", + "v_type": 0 + }, + { + "key": "message.compressed_size", + "v_int64": 512, + "v_type": 2 + }, + { + "key": "message.uncompressed_size", + "v_int64": 1024, + "v_type": 2 + } + ] + }, + { + "timestamp": "2017-01-26T21:46:31.639875Z", + "fields": [ + { + "key": "key1", + "v_type": 0, + "v_str": "value1" + } + ] + }, + { + "timestamp": "2017-01-26T21:46:31.639875Z", + "fields": [ + { + "key": "event", + "v_type": 0, + "v_str": "nothing" + }, + { + "key": "description", + "v_type": 0, + "v_str": "annotation description" + } + ] + } + ], + "process": { + "service_name": "api", + "tags": [ + + { + "key": "hostname", + "v_type": 0, + "v_str": "api246-sjc1" + }, + { + "key": "pid", + "v_type": 2, + "v_int64": 13 + }, + { + "key": "start.time", + "v_type": 0, + "v_str": "2017-01-26T21:46:30.639875Z" + }, + { + "key": "ip", + "v_type": 0, + "v_str": "10.53.69.61" + }, + { + "key": "opencensus.exporterversion", + "v_type": 0, + "v_str": "someVersion" + }, + { + "key": "a.bool", + "v_type": 0, + "v_str": "true" + }, + { + "key": "a.double", + "v_type": 0, + "v_str": "1234.56789" + }, + { + "key": "a.long", + "v_type": 0, + "v_str": "123456789" + }, + { + "key": "a.binary", + "v_type": 0, + "v_str": "AQIDBAMCAQ==" + } + ] + } + } + ] +} \ No newline at end of file diff --git a/translator/trace/jaeger/testdata/jaegerproto_batch_02.json b/translator/trace/jaeger/testdata/jaegerproto_batch_02.json new file mode 100644 index 00000000000..1c3e2270e89 --- /dev/null +++ b/translator/trace/jaeger/testdata/jaegerproto_batch_02.json @@ -0,0 +1,80 @@ +{ + "process": { + "service_name": "api", + "tags": null + }, + "spans": [ + { + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABkfZg=", + "parent_span_id": 0, + "operation_name": "get", + "start_time": "2017-01-26T21:46:31.639875Z", + "duration": 22938000, + "logs": null, + "process": { + "service_name": "api", + "tags": null + }, + "tags": [ + { + "key": "peer.service", + "v_type": 0, + "v_str": "AAAAAAAAMDk=" + }, + { + "key": "span.kind", + "v_type": 0, + "v_str": "server" + } + ] + }, + { + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABkfZk=", + "parent_span_id": 0, + "operation_name": "get", + "process": { + "service_name": "api", + "tags": null + }, + "logs": null, + "references": [ + { + "ref_type": 0, + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABkfZg=" + }, + { + "ref_type": 1, + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABoxOM=" + } + ], + "start_time": "2017-01-26T21:46:31.639875Z", + "duration": 22938000, + "tags": [ + { + "key": "span.kind", + "v_type": 0, + "v_str": "server" + } + ] + }, + { + "trace_id": "AAAAAAAAAABSlpqJVVcaPw==", + "span_id": "AAAAAABkfZg=", + "parent_span_id": 0, + "operation_name": "get2", + "start_time": "2017-01-26T21:46:32.639875Z", + "duration": 22938000, + "logs": null, + "tags": null, + "process": { + "service_name": "api", + "tags": null + } + } + ] + } + \ No newline at end of file diff --git a/translator/trace/protospan_translation.go b/translator/trace/protospan_translation.go index 1f32786ab94..9e60b00f5ac 100644 --- a/translator/trace/protospan_translation.go +++ b/translator/trace/protospan_translation.go @@ -22,4 +22,11 @@ const ( MessageEventTypeKey = "message.type" MessageEventCompressedSizeKey = "message.compressed_size" MessageEventUncompressedSizeKey = "message.uncompressed_size" + + TagStatusCode = "status.code" + TagStatusMsg = "status.message" + TagHTTPStatusCode = "http.status_code" + TagHTTPStatusMsg = "http.status_message" + TagZipkinCensusCode = "census.status_code" + TagZipkinCensusMsg = "census.status_description" ) diff --git a/translator/trace/spandata/protospan_to_spandata.go b/translator/trace/spandata/protospan_to_spandata.go index 5d756140729..34393a2f11d 100644 --- a/translator/trace/spandata/protospan_to_spandata.go +++ b/translator/trace/spandata/protospan_to_spandata.go @@ -19,12 +19,11 @@ import ( "errors" "time" - "go.opencensus.io/trace" - "go.opencensus.io/trace/tracestate" - tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" "github.com/golang/protobuf/ptypes/wrappers" + "go.opencensus.io/trace" + "go.opencensus.io/trace/tracestate" ) var errNilSpan = errors.New("expected a non-nil span") diff --git a/translator/trace/spandata/protospan_to_spandata_test.go b/translator/trace/spandata/protospan_to_spandata_test.go index 9a6b62832cc..50174d117df 100644 --- a/translator/trace/spandata/protospan_to_spandata_test.go +++ b/translator/trace/spandata/protospan_to_spandata_test.go @@ -21,12 +21,11 @@ import ( "testing" "time" + tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/wrappers" - "go.opencensus.io/trace" "go.opencensus.io/trace/tracestate" - tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/open-telemetry/opentelemetry-service/internal" ) diff --git a/translator/trace/zipkin/status_code.go b/translator/trace/zipkin/status_code.go new file mode 100644 index 00000000000..73dbec2afba --- /dev/null +++ b/translator/trace/zipkin/status_code.go @@ -0,0 +1,110 @@ +// Copyright 2019, OpenTelemetry 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 zipkin + +import ( + "math" + + tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" + + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" +) + +type status struct { + codePtr *int32 + message string +} + +// statusMapper contains codes translated from different sources to OC status codes +type statusMapper struct { + // oc status code extracted from "status.code" tags + fromStatus status + // oc status code extracted from "census.status_code" tags + fromCensus status + // oc status code extracted from "http.status_code" tags + fromHTTP status +} + +// ocStatus returns an OC status from the best possible extraction source. +// It'll first try to return status extracted from "census.status_code" to account for zipkin +// then fallback on code extracted from "status.code" tags +// and finally fallback on code extracted and translated from "http.status_code" +// ocStatus must be called after all tags/attributes are processed with the `fromAttribute` method. +func (m *statusMapper) ocStatus() *tracepb.Status { + var s status + switch { + case m.fromCensus.codePtr != nil: + s = m.fromCensus + case m.fromStatus.codePtr != nil: + s = m.fromStatus + default: + s = m.fromHTTP + } + + if s.codePtr != nil || s.message != "" { + code := int32(0) + if s.codePtr != nil { + code = *s.codePtr + } + return &tracepb.Status{ + Code: code, + Message: s.message, + } + } + return nil +} + +func (m *statusMapper) fromAttribute(key string, attrib *tracepb.AttributeValue) bool { + switch key { + case tracetranslator.TagZipkinCensusCode: + m.fromCensus.codePtr = attribToStatusCode(attrib) + return true + + case tracetranslator.TagZipkinCensusMsg: + m.fromCensus.message = attrib.GetStringValue().GetValue() + return true + + case tracetranslator.TagStatusCode: + m.fromStatus.codePtr = attribToStatusCode(attrib) + return true + + case tracetranslator.TagStatusMsg: + m.fromStatus.message = attrib.GetStringValue().GetValue() + return true + + case tracetranslator.TagHTTPStatusCode: + codePtr := attribToStatusCode(attrib) + if codePtr != nil { + *codePtr = tracetranslator.OCStatusCodeFromHTTP(*codePtr) + m.fromHTTP.codePtr = codePtr + } + + case tracetranslator.TagHTTPStatusMsg: + m.fromHTTP.message = attrib.GetStringValue().GetValue() + } + return false +} + +// attribToStatusCode maps an integer attribute value to a status code (int32). +// The function return nil if the value is not an integer or an integer larger than what +// can fit in an int32 +func attribToStatusCode(v *tracepb.AttributeValue) *int32 { + i := v.GetIntValue() + if i <= math.MaxInt32 { + j := int32(i) + return &j + } + return nil +} diff --git a/translator/trace/zipkin/zipkinv1_thrift_to_protospan.go b/translator/trace/zipkin/zipkinv1_thrift_to_protospan.go index 6c3a9049f93..55ae62513d0 100644 --- a/translator/trace/zipkin/zipkinv1_thrift_to_protospan.go +++ b/translator/trace/zipkin/zipkinv1_thrift_to_protospan.go @@ -56,7 +56,7 @@ func zipkinV1ThriftToOCSpan(zSpan *zipkincore.Span) (*tracepb.Span, *annotationP } // TODO: (@pjanotti) ideally we should error here instead of generating invalid OC proto - // however per https://github.com/census-instrumentation/opencensus-service/issues/349 + // however per https://github.com/open-telemetry/opentelemetry-service/issues/349 // failures on the receivers in general are silent at this moment, so letting them // proceed for now. We should validate the traceID, spanID and parentID are good with // OC proto requirements. @@ -68,7 +68,7 @@ func zipkinV1ThriftToOCSpan(zSpan *zipkincore.Span) (*tracepb.Span, *annotationP } parsedAnnotations := parseZipkinV1ThriftAnnotations(zSpan.Annotations) - attributes, localComponent := zipkinV1ThriftBinAnnotationsToOCAttributes(zSpan.BinaryAnnotations) + attributes, ocStatus, localComponent := zipkinV1ThriftBinAnnotationsToOCAttributes(zSpan.BinaryAnnotations) if parsedAnnotations.Endpoint.ServiceName == unknownServiceName && localComponent != "" { parsedAnnotations.Endpoint.ServiceName = localComponent } @@ -90,6 +90,7 @@ func zipkinV1ThriftToOCSpan(zSpan *zipkincore.Span) (*tracepb.Span, *annotationP TraceId: traceID, SpanId: spanID, ParentSpanId: parentID, + Status: ocStatus, Kind: parsedAnnotations.Kind, TimeEvents: parsedAnnotations.TimeEvents, StartTime: startTime, @@ -139,11 +140,12 @@ func toTranslatorEndpoint(e *zipkincore.Endpoint) *endpoint { var trueByteSlice = []byte{1} -func zipkinV1ThriftBinAnnotationsToOCAttributes(ztBinAnnotations []*zipkincore.BinaryAnnotation) (attributes *tracepb.Span_Attributes, fallbackServiceName string) { +func zipkinV1ThriftBinAnnotationsToOCAttributes(ztBinAnnotations []*zipkincore.BinaryAnnotation) (attributes *tracepb.Span_Attributes, status *tracepb.Status, fallbackServiceName string) { if len(ztBinAnnotations) == 0 { - return nil, "" + return nil, nil, "" } + sMapper := &statusMapper{} var localComponent string attributeMap := make(map[string]*tracepb.AttributeValue) for _, binaryAnnotation := range ztBinAnnotations { @@ -199,9 +201,19 @@ func zipkinV1ThriftBinAnnotationsToOCAttributes(ztBinAnnotations []*zipkincore.B localComponent = string(binaryAnnotation.Value) } + if drop := sMapper.fromAttribute(key, pbAttrib); drop { + continue + } + attributeMap[key] = pbAttrib } + status = sMapper.ocStatus() + + if len(attributeMap) == 0 { + return nil, status, "" + } + if fallbackServiceName == "" { fallbackServiceName = localComponent } @@ -209,7 +221,7 @@ func zipkinV1ThriftBinAnnotationsToOCAttributes(ztBinAnnotations []*zipkincore.B attributes = &tracepb.Span_Attributes{ AttributeMap: attributeMap, } - return attributes, fallbackServiceName + return attributes, status, fallbackServiceName } var errNotEnoughBytes = errors.New("not enough bytes representing the number") diff --git a/translator/trace/zipkin/zipkinv1_thrift_to_protospan_test.go b/translator/trace/zipkin/zipkinv1_thrift_to_protospan_test.go index 0bdecac1839..c187dcbdd36 100644 --- a/translator/trace/zipkin/zipkinv1_thrift_to_protospan_test.go +++ b/translator/trace/zipkin/zipkinv1_thrift_to_protospan_test.go @@ -15,13 +15,18 @@ package zipkin import ( + "encoding/binary" "encoding/json" "io/ioutil" + "math" "reflect" "sort" "testing" + tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/jaegertracing/jaeger/thrift-gen/zipkincore" + + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" ) func TestZipkinThriftFallbackToLocalComponent(t *testing.T) { @@ -107,6 +112,366 @@ func BenchmarkV1ThriftToOCProto(b *testing.B) { } } +func TestZipkinThriftAnnotationsToOCStatus(t *testing.T) { + type test struct { + haveTags []*zipkincore.BinaryAnnotation + wantAttributes *tracepb.Span_Attributes + wantStatus *tracepb.Status + } + + cases := []test{ + // too large code for OC + { + haveTags: []*zipkincore.BinaryAnnotation{{ + Key: "status.code", + Value: uint64ToBytes(math.MaxInt64), + AnnotationType: zipkincore.AnnotationType_I64, + }}, + wantAttributes: nil, + wantStatus: nil, + }, + // only status.code tag + { + haveTags: []*zipkincore.BinaryAnnotation{{ + Key: "status.code", + Value: uint64ToBytes(5), + AnnotationType: zipkincore.AnnotationType_I64, + }}, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 5, + }, + }, + { + haveTags: []*zipkincore.BinaryAnnotation{{ + Key: "status.code", + Value: uint32ToBytes(6), + AnnotationType: zipkincore.AnnotationType_I32, + }}, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 6, + }, + }, + { + haveTags: []*zipkincore.BinaryAnnotation{{ + Key: "status.code", + Value: uint16ToBytes(7), + AnnotationType: zipkincore.AnnotationType_I16, + }}, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 7, + }, + }, + // only status.message tag + { + haveTags: []*zipkincore.BinaryAnnotation{{ + Key: "status.message", + Value: []byte("Forbidden"), + AnnotationType: zipkincore.AnnotationType_STRING, + }}, + wantAttributes: nil, + wantStatus: nil, + }, + // both status.code and status.message + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "status.code", + Value: uint32ToBytes(13), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "status.message", + Value: []byte("Forbidden"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + + // http status.code + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "http.status_code", + Value: uint32ToBytes(404), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "http.status_message", + Value: []byte("NotFound"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + + // http and oc + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "http.status_code", + Value: uint32ToBytes(404), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "http.status_message", + Value: []byte("NotFound"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "status.code", + Value: uint32ToBytes(13), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "status.message", + Value: []byte("Forbidden"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + + // http and only oc code + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "http.status_code", + Value: uint32ToBytes(404), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "http.status_message", + Value: []byte("NotFound"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "status.code", + Value: uint32ToBytes(14), + AnnotationType: zipkincore.AnnotationType_I32, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 14, + }, + }, + // http and only oc message + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "http.status_code", + Value: uint32ToBytes(404), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "http.status_message", + Value: []byte("NotFound"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "status.message", + Value: []byte("Forbidden"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + + // census tags + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "census.status_code", + Value: uint32ToBytes(18), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "census.status_description", + Value: []byte("RPCError"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 18, + Message: "RPCError", + }, + }, + + // census tags priority over others + { + haveTags: []*zipkincore.BinaryAnnotation{ + { + Key: "census.status_code", + Value: uint32ToBytes(18), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "census.status_description", + Value: []byte("RPCError"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "http.status_code", + Value: uint32ToBytes(404), + AnnotationType: zipkincore.AnnotationType_I32, + }, + { + Key: "http.status_message", + Value: []byte("NotFound"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "status.message", + Value: []byte("Forbidden"), + AnnotationType: zipkincore.AnnotationType_STRING, + }, + { + Key: "status.code", + Value: uint32ToBytes(1), + AnnotationType: zipkincore.AnnotationType_I32, + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 18, + Message: "RPCError", + }, + }, + } + + for i, c := range cases { + zSpans := []*zipkincore.Span{{ + ID: 1, + TraceID: 1, + BinaryAnnotations: c.haveTags, + }} + gb, err := V1ThriftBatchToOCProto(zSpans) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb[0].Spans[0] + if !reflect.DeepEqual(gs.Attributes, c.wantAttributes) { + t.Fatalf("Unsuccessful conversion %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Attributes, c.wantAttributes) + } + + if !reflect.DeepEqual(gs.Status, c.wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, c.wantStatus) + } + } +} + +func TestThirftHTTPToGRPCStatusCode(t *testing.T) { + for i := int32(100); i <= 600; i++ { + wantStatus := tracetranslator.OCStatusCodeFromHTTP(i) + gb, err := V1ThriftBatchToOCProto([]*zipkincore.Span{{ + ID: 1, + TraceID: 1, + BinaryAnnotations: []*zipkincore.BinaryAnnotation{ + { + Key: "http.status_code", + Value: uint32ToBytes(uint32(i)), + AnnotationType: zipkincore.AnnotationType_I32, + }, + }, + }}) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb[0].Spans[0] + if !reflect.DeepEqual(gs.Status.Code, wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, wantStatus) + } + } +} + func Test_bytesInt16ToInt64(t *testing.T) { tests := []struct { name string @@ -266,3 +631,21 @@ func Test_bytesFloat64ToFloat64(t *testing.T) { }) } } + +func uint64ToBytes(i uint64) []byte { + b := make([]byte, 8) + binary.BigEndian.PutUint64(b, i) + return b +} + +func uint32ToBytes(i uint32) []byte { + b := make([]byte, 4) + binary.BigEndian.PutUint32(b, i) + return b +} + +func uint16ToBytes(i uint16) []byte { + b := make([]byte, 2) + binary.BigEndian.PutUint16(b, i) + return b +} diff --git a/translator/trace/zipkin/zipkinv1_to_protospan.go b/translator/trace/zipkin/zipkinv1_to_protospan.go index 0c8df2dcfb2..87e47b9fdd2 100644 --- a/translator/trace/zipkin/zipkinv1_to_protospan.go +++ b/translator/trace/zipkin/zipkinv1_to_protospan.go @@ -24,9 +24,10 @@ import ( tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" "github.com/jaegertracing/jaeger/thrift-gen/zipkincore" + "github.com/pkg/errors" + "github.com/open-telemetry/opentelemetry-service/data" tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" - "github.com/pkg/errors" ) var ( @@ -144,7 +145,7 @@ func zipkinV1ToOCSpan(zSpan *zipkinV1Span) (*tracepb.Span, *annotationParseResul } parsedAnnotations := parseZipkinV1Annotations(zSpan.Annotations) - attributes, localComponent := zipkinV1BinAnnotationsToOCAttributes(zSpan.BinaryAnnotations) + attributes, ocStatus, localComponent := zipkinV1BinAnnotationsToOCAttributes(zSpan.BinaryAnnotations) if parsedAnnotations.Endpoint.ServiceName == unknownServiceName && localComponent != "" { parsedAnnotations.Endpoint.ServiceName = localComponent } @@ -161,6 +162,7 @@ func zipkinV1ToOCSpan(zSpan *zipkinV1Span) (*tracepb.Span, *annotationParseResul TraceId: traceID, SpanId: spanID, ParentSpanId: parentID, + Status: ocStatus, Kind: parsedAnnotations.Kind, TimeEvents: parsedAnnotations.TimeEvents, StartTime: startTime, @@ -175,14 +177,16 @@ func zipkinV1ToOCSpan(zSpan *zipkinV1Span) (*tracepb.Span, *annotationParseResul return ocSpan, parsedAnnotations, nil } -func zipkinV1BinAnnotationsToOCAttributes(binAnnotations []*binaryAnnotation) (attributes *tracepb.Span_Attributes, fallbackServiceName string) { +func zipkinV1BinAnnotationsToOCAttributes(binAnnotations []*binaryAnnotation) (attributes *tracepb.Span_Attributes, status *tracepb.Status, fallbackServiceName string) { if len(binAnnotations) == 0 { - return nil, "" + return nil, nil, "" } + sMapper := &statusMapper{} var localComponent string attributeMap := make(map[string]*tracepb.AttributeValue) for _, binAnnotation := range binAnnotations { + if binAnnotation.Endpoint != nil && binAnnotation.Endpoint.ServiceName != "" { fallbackServiceName = binAnnotation.Endpoint.ServiceName } @@ -197,16 +201,24 @@ func zipkinV1BinAnnotationsToOCAttributes(binAnnotations []*binaryAnnotation) (a } key := binAnnotation.Key + if key == zipkincore.LOCAL_COMPONENT { // TODO: (@pjanotti) add reference to OpenTracing and change related tags to use them key = "component" localComponent = binAnnotation.Value } + + if drop := sMapper.fromAttribute(key, pbAttrib); drop { + continue + } + attributeMap[key] = pbAttrib } + status = sMapper.ocStatus() + if len(attributeMap) == 0 { - return nil, "" + return nil, status, "" } if fallbackServiceName == "" { @@ -217,7 +229,7 @@ func zipkinV1BinAnnotationsToOCAttributes(binAnnotations []*binaryAnnotation) (a AttributeMap: attributeMap, } - return attributes, fallbackServiceName + return attributes, status, fallbackServiceName } // annotationParseResult stores the results of examining the original annotations, diff --git a/translator/trace/zipkin/zipkinv1_to_protospan_test.go b/translator/trace/zipkin/zipkinv1_to_protospan_test.go index ce0f42b4e76..f77b631d0ef 100644 --- a/translator/trace/zipkin/zipkinv1_to_protospan_test.go +++ b/translator/trace/zipkin/zipkinv1_to_protospan_test.go @@ -19,12 +19,14 @@ import ( "io/ioutil" "reflect" "sort" + "strconv" "testing" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" "github.com/open-telemetry/opentelemetry-service/data" + tracetranslator "github.com/open-telemetry/opentelemetry-service/translator/trace" ) func Test_hexIDToOCID(t *testing.T) { @@ -239,6 +241,326 @@ func sortTraceByNodeName(trace []data.TraceData) { }) } +func TestZipkinAnnotationsToOCStatus(t *testing.T) { + type test struct { + haveTags []*binaryAnnotation + wantAttributes *tracepb.Span_Attributes + wantStatus *tracepb.Status + } + + cases := []test{ + // only status.code tag + { + haveTags: []*binaryAnnotation{{ + Key: "status.code", + Value: "13", + }}, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 13, + }, + }, + // only status.message tag + { + haveTags: []*binaryAnnotation{{ + Key: "status.message", + Value: "Forbidden", + }}, + wantAttributes: nil, + wantStatus: nil, + }, + // both status.code and status.message + { + haveTags: []*binaryAnnotation{ + { + Key: "status.code", + Value: "13", + }, + { + Key: "status.message", + Value: "Forbidden", + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + + // http status.code + { + haveTags: []*binaryAnnotation{ + { + Key: "http.status_code", + Value: "404", + }, + { + Key: "http.status_message", + Value: "NotFound", + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + + // http and oc + { + haveTags: []*binaryAnnotation{ + { + Key: "http.status_code", + Value: "404", + }, + { + Key: "http.status_message", + Value: "NotFound", + }, + { + Key: "status.code", + Value: "13", + }, + { + Key: "status.message", + Value: "Forbidden", + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 13, + Message: "Forbidden", + }, + }, + + // http and only oc code + { + haveTags: []*binaryAnnotation{ + { + Key: "http.status_code", + Value: "404", + }, + { + Key: "http.status_message", + Value: "NotFound", + }, + { + Key: "status.code", + Value: "14", + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 14, + }, + }, + // http and only oc message + { + haveTags: []*binaryAnnotation{ + { + Key: "http.status_code", + Value: "404", + }, + { + Key: "http.status_message", + Value: "NotFound", + }, + { + Key: "status.message", + Value: "Forbidden", + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 5, + Message: "NotFound", + }, + }, + + // census tags + { + haveTags: []*binaryAnnotation{ + { + Key: "census.status_code", + Value: "10", + }, + { + Key: "census.status_description", + Value: "RPCError", + }, + }, + wantAttributes: nil, + wantStatus: &tracepb.Status{ + Code: 10, + Message: "RPCError", + }, + }, + + // census tags priority over others + { + haveTags: []*binaryAnnotation{ + { + Key: "census.status_code", + Value: "10", + }, + { + Key: "census.status_description", + Value: "RPCError", + }, + { + Key: "http.status_code", + Value: "404", + }, + { + Key: "http.status_message", + Value: "NotFound", + }, + { + Key: "status.message", + Value: "Forbidden", + }, + { + Key: "status.code", + Value: "7", + }, + }, + wantAttributes: &tracepb.Span_Attributes{ + AttributeMap: map[string]*tracepb.AttributeValue{ + tracetranslator.TagHTTPStatusCode: { + Value: &tracepb.AttributeValue_IntValue{ + IntValue: 404, + }, + }, + tracetranslator.TagHTTPStatusMsg: { + Value: &tracepb.AttributeValue_StringValue{ + StringValue: &tracepb.TruncatableString{Value: "NotFound"}, + }, + }, + }, + }, + wantStatus: &tracepb.Status{ + Code: 10, + Message: "RPCError", + }, + }, + } + + fakeTraceID := "00000000000000010000000000000002" + fakeSpanID := "0000000000000001" + + for i, c := range cases { + zSpans := []*zipkinV1Span{{ + ID: fakeSpanID, + TraceID: fakeTraceID, + BinaryAnnotations: c.haveTags, + }} + zBytes, err := json.Marshal(zSpans) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gb, err := V1JSONBatchToOCProto(zBytes) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gs := gb[0].Spans[0] + + if !reflect.DeepEqual(gs.Attributes, c.wantAttributes) { + t.Fatalf("Unsuccessful conversion\nGot:\n\t%v\nWant:\n\t%v", gs.Attributes, c.wantAttributes) + } + + if !reflect.DeepEqual(gs.Status, c.wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, c.wantStatus) + } + } +} + +func TestJSONHTTPToGRPCStatusCode(t *testing.T) { + fakeTraceID := "00000000000000010000000000000002" + fakeSpanID := "0000000000000001" + for i := int32(100); i <= 600; i++ { + wantStatus := tracetranslator.OCStatusCodeFromHTTP(i) + zBytes, err := json.Marshal([]*zipkinV1Span{{ + ID: fakeSpanID, + TraceID: fakeTraceID, + BinaryAnnotations: []*binaryAnnotation{ + { + Key: "http.status_code", + Value: strconv.Itoa(int(i)), + }, + }, + }}) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + gb, err := V1JSONBatchToOCProto(zBytes) + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + continue + } + + gs := gb[0].Spans[0] + if !reflect.DeepEqual(gs.Status.Code, wantStatus) { + t.Fatalf("Unsuccessful conversion: %d\nGot:\n\t%v\nWant:\n\t%v", i, gs.Status, wantStatus) + } + } +} + // ocBatches has the OpenCensus proto batches used in the test. They are hard coded because // structs like tracepb.AttributeMap cannot be ready from JSON. var ocBatchesFromZipkinV1 = []data.TraceData{ @@ -395,6 +717,9 @@ var ocBatchesFromZipkinV1 = []data.TraceData{ Kind: tracepb.Span_SERVER, StartTime: ×tamp.Timestamp{Seconds: 1544805927, Nanos: 454487000}, EndTime: ×tamp.Timestamp{Seconds: 1544805927, Nanos: 457320000}, + Status: &tracepb.Status{ + Code: 0, + }, Attributes: &tracepb.Span_Attributes{ AttributeMap: map[string]*tracepb.AttributeValue{ "http.status_code": {