When converting to/from Jaeger set error tag if status is not ok (ope…

…n-telemetry#833) * When converting to/from Jaeger set error tag if status is not ok Signed-off-by: Bogdan Drutu <[email protected]> * Do not set error=false Signed-off-by: Bogdan Drutu <[email protected]>
dmitryax · Apr 14, 2020 · 75ae919 · 75ae919
1 parent 380a0e7
commit 75ae919
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 1 deletion.
diff --git a/translator/trace/jaeger/jaegerproto_to_protospan.go b/translator/trace/jaeger/jaegerproto_to_protospan.go
@@ -24,6 +24,7 @@ import (
 	tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1"
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/thrift-gen/jaeger"
+	"go.opencensus.io/trace"
 
 	"github.com/open-telemetry/opentelemetry-collector/consumer/consumerdata"
 	"github.com/open-telemetry/opentelemetry-collector/internal"
@@ -203,6 +204,14 @@ func jProtoTagsToAttributes(tags []model.KeyValue) (string, tracepb.Span_SpanKin
 				sKind = tracepb.Span_SERVER
 			}
 
+		case tracetranslator.TagError:
+			if statusCodePtr == nil {
+				// Only set this if tracetranslator.TagStatusCode does not exists.
+				// The tracetranslator.TagStatusCode branch will overwrite the value if exists.
+				code := int32(trace.StatusCodeUnknown)
+				statusCodePtr = &code
+			}
+
 		case tracetranslator.TagStatusCode:
 			statusCodePtr = statusCodeFromProtoTag(tag)
 			continue

diff --git a/translator/trace/jaeger/jaegerproto_to_protospan_test.go b/translator/trace/jaeger/jaegerproto_to_protospan_test.go
@@ -161,6 +161,29 @@ func expectedTraceData(t1, t2, t3 time.Time) consumerdata.TraceData {
 					},
 				},
 			},
+			{
+				TraceId:   traceID,
+				SpanId:    parentSpanID,
+				Name:      &tracepb.TruncatableString{Value: "ProxyFetch"},
+				StartTime: internal.TimeToTimestamp(t2),
+				EndTime:   internal.TimeToTimestamp(t3),
+				Kind:      tracepb.Span_CLIENT,
+				Status: &tracepb.Status{
+					Code: trace.StatusCodeUnknown,
+				},
+				Attributes: &tracepb.Span_Attributes{
+					AttributeMap: map[string]*tracepb.AttributeValue{
+						"error": {
+							Value: &tracepb.AttributeValue_BoolValue{BoolValue: true},
+						},
+						"span.kind": {
+							Value: &tracepb.AttributeValue_StringValue{
+								StringValue: &tracepb.TruncatableString{Value: "client"},
+							},
+						},
+					},
+				},
+			},
 		},
 	}
 
@@ -226,7 +249,18 @@ func grpcFixture(t1 time.Time, d1, d2 time.Duration) model.Batch {
 					model.String(tracetranslator.TagStatusMsg, "Frontend crash"),
 					model.Int64(tracetranslator.TagStatusCode, trace.StatusCodeInternal),
 					model.String(tracetranslator.TagHTTPStatusMsg, "Internal server error"),
-					model.Int64("http.status_code", 500),
+					model.Int64(tracetranslator.TagHTTPStatusCode, 500),
+					model.Bool("error", true),
+					model.String("span.kind", "client"),
+				},
+			},
+			{
+				TraceID:       traceID,
+				SpanID:        parentSpanID,
+				OperationName: "ProxyFetch",
+				StartTime:     t1.Add(d1),
+				Duration:      d2,
+				Tags: []model.KeyValue{
 					model.Bool("error", true),
 					model.String("span.kind", "client"),
 				},

diff --git a/translator/trace/jaeger/protospan_to_jaegerproto.go b/translator/trace/jaeger/protospan_to_jaegerproto.go
@@ -412,6 +412,14 @@ func appendJaegerTagFromOCStatusProto(jTags []jaeger.KeyValue, ocStatus *tracepb
 		VType:  jaeger.ValueType_INT64,
 	})
 
+	if ocStatus.Code != 0 {
+		jTags = append(jTags, jaeger.KeyValue{
+			Key:   tracetranslator.TagError,
+			VBool: ocStatus.Code != 0,
+			VType: jaeger.ValueType_BOOL,
+		})
+	}
+
 	if ocStatus.Message != "" {
 		jTags = append(jTags, jaeger.KeyValue{
 			Key:   tracetranslator.TagStatusMsg,

diff --git a/translator/trace/jaeger/protospan_to_jaegerproto_test.go b/translator/trace/jaeger/protospan_to_jaegerproto_test.go
@@ -134,6 +134,11 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 					VInt64: int64(10),
 					VType:  jaeger.ValueType_INT64,
 				},
+				{
+					Key:   tracetranslator.TagError,
+					VBool: true,
+					VType: jaeger.ValueType_BOOL,
+				},
 			},
 		},
 		// only status.message
@@ -173,6 +178,11 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 					VStr:  "Forbidden",
 					VType: jaeger.ValueType_STRING,
 				},
+				{
+					Key:   tracetranslator.TagError,
+					VBool: true,
+					VType: jaeger.ValueType_BOOL,
+				},
 			},
 		},
 
@@ -194,6 +204,11 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 							StringValue: &tracepb.TruncatableString{Value: "Error"},
 						},
 					},
+					"error": {
+						Value: &tracepb.AttributeValue_BoolValue{
+							BoolValue: true,
+						},
+					},
 				},
 			},
 			wantTags: []jaeger.KeyValue{
@@ -207,6 +222,11 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 					VStr:  "Error",
 					VType: jaeger.ValueType_STRING,
 				},
+				{
+					Key:   tracetranslator.TagError,
+					VBool: true,
+					VType: jaeger.ValueType_BOOL,
+				},
 			},
 		},
 
@@ -298,6 +318,11 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 					VStr:  "Forbidden",
 					VType: jaeger.ValueType_STRING,
 				},
+				{
+					Key:   tracetranslator.TagError,
+					VBool: true,
+					VType: jaeger.ValueType_BOOL,
+				},
 			},
 		},
 	}
@@ -322,6 +347,9 @@ func TestOCStatusToJaegerProtoTags(t *testing.T) {
 		sort.Slice(gs.Tags, func(i, j int) bool {
 			return gs.Tags[i].Key < gs.Tags[j].Key
 		})
+		sort.Slice(c.wantTags, func(i, j int) bool {
+			return c.wantTags[i].Key < c.wantTags[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)
 		}

diff --git a/translator/trace/protospan_translation.go b/translator/trace/protospan_translation.go
@@ -27,6 +27,7 @@ const (
 
 	TagStatusCode       = "status.code"
 	TagStatusMsg        = "status.message"
+	TagError            = "error"
 	TagHTTPStatusCode   = "http.status_code"
 	TagHTTPStatusMsg    = "http.status_message"
 	TagZipkinCensusCode = "census.status_code"