Jaeger Exporter: Fix minor mapping discrepancies (#1626)

* Set span status code, message and ref types according to the spec * Serialize array attributes as string * Use correct lib name / version key * Add new and adjust existing tests * Update CHANGELOG Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
open-telemetry · Mar 8, 2021 · 05252f4 · 05252f4
1 parent 238e7c6
commit 05252f4
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -86,6 +86,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The sequential timing check of timestamps of go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue are now setup explicitly to be sequential (#1578). (#1579)
 - Validate tracestate header keys with vedors according to the W3C TraceContext specification (#1475). (#1581)
 - The OTLP exporter includes related labels for translations of a GaugeArray (#1563). (#1570)
+- Jaeger Exporter: Ensure mapping between OTEL and Jaeger span data complies with the specification. (#1626)
 
 ## [0.17.0] - 2020-02-12
 

diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go
@@ -17,6 +17,7 @@ package jaeger // import "go.opentelemetry.io/otel/exporters/trace/jaeger"
 import (
 	"context"
 	"encoding/binary"
+	"encoding/json"
 	"fmt"
 	"sync"
 
@@ -34,8 +35,8 @@ import (
 const (
 	defaultServiceName = "OpenTelemetry"
 
-	keyInstrumentationLibraryName    = "otel.instrumentation_library.name"
-	keyInstrumentationLibraryVersion = "otel.instrumentation_library.version"
+	keyInstrumentationLibraryName    = "otel.library.name"
+	keyInstrumentationLibraryVersion = "otel.library.version"
 )
 
 type Option func(*options)
@@ -287,16 +288,21 @@ func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span {
 		}
 	}
 
-	tags = append(tags,
-		getInt64Tag("status.code", int64(ss.StatusCode)),
-		getStringTag("status.message", ss.StatusMessage),
-		getStringTag("span.kind", ss.SpanKind.String()),
-	)
+	if ss.SpanKind != trace.SpanKindInternal {
+		tags = append(tags,
+			getStringTag("span.kind", ss.SpanKind.String()),
+		)
+	}
+
+	if ss.StatusCode != codes.Unset {
+		tags = append(tags,
+			getInt64Tag("status.code", int64(ss.StatusCode)),
+			getStringTag("status.message", ss.StatusMessage),
+		)
 
-	// Ensure that if Status.Code is not OK, that we set the "error" tag on the Jaeger span.
-	// See Issue https://github.com/census-instrumentation/opencensus-go/issues/1041
-	if ss.StatusCode != codes.Ok && ss.StatusCode != codes.Unset {
-		tags = append(tags, getBoolTag("error", true))
+		if ss.StatusCode == codes.Error {
+			tags = append(tags, getBoolTag("error", true))
+		}
 	}
 
 	var logs []*gen.Log
@@ -321,9 +327,7 @@ func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span {
 			TraceIdHigh: int64(binary.BigEndian.Uint64(link.TraceID[0:8])),
 			TraceIdLow:  int64(binary.BigEndian.Uint64(link.TraceID[8:16])),
 			SpanId:      int64(binary.BigEndian.Uint64(link.SpanID[:])),
-			// TODO(paivagustavo): properly set the reference type when specs are defined
-			//  see https://github.com/open-telemetry/opentelemetry-specification/issues/65
-			RefType: gen.SpanRefType_CHILD_OF,
+			RefType:     gen.SpanRefType_FOLLOWS_FROM,
 		})
 	}
 
@@ -373,6 +377,14 @@ func keyValueToTag(keyValue attribute.KeyValue) *gen.Tag {
 			VDouble: &f,
 			VType:   gen.TagType_DOUBLE,
 		}
+	case attribute.ARRAY:
+		json, _ := json.Marshal(keyValue.Value.AsArray())
+		a := (string)(json)
+		tag = &gen.Tag{
+			Key:   string(keyValue.Key),
+			VStr:  &a,
+			VType: gen.TagType_STRING,
+		}
 	}
 	return tag
 }

diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go
@@ -356,6 +356,7 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 	now := time.Now()
 	traceID, _ := trace.TraceIDFromHex("0102030405060708090a0b0c0d0e0f10")
 	spanID, _ := trace.SpanIDFromHex("0102030405060708")
+	parentSpanID, _ := trace.SpanIDFromHex("0807060504030201")
 
 	linkTraceID, _ := trace.TraceIDFromHex("0102030405060709090a0b0c0d0e0f11")
 	linkSpanID, _ := trace.SpanIDFromHex("0102030405060709")
@@ -366,6 +367,7 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 	doubleValue := 123.456
 	intValue := int64(123)
 	boolTrue := true
+	arrValue := "[0,1,2,3]"
 	statusMessage := "this is a problem"
 	spanKind := "client"
 	rv1 := "rv11"
@@ -425,8 +427,8 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 					{Key: "key", VType: gen.TagType_STRING, VStr: &keyValue},
 					{Key: "int", VType: gen.TagType_LONG, VLong: &intValue},
 					{Key: "error", VType: gen.TagType_BOOL, VBool: &boolTrue},
-					{Key: "otel.instrumentation_library.name", VType: gen.TagType_STRING, VStr: &instrLibName},
-					{Key: "otel.instrumentation_library.version", VType: gen.TagType_STRING, VStr: &instrLibVersion},
+					{Key: "otel.library.name", VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: "otel.library.version", VType: gen.TagType_STRING, VStr: &instrLibVersion},
 					{Key: "status.code", VType: gen.TagType_LONG, VLong: &statusCodeValue},
 					{Key: "status.message", VType: gen.TagType_STRING, VStr: &statusMessage},
 					{Key: "span.kind", VType: gen.TagType_STRING, VStr: &spanKind},
@@ -435,7 +437,7 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 				},
 				References: []*gen.SpanRef{
 					{
-						RefType:     gen.SpanRefType_CHILD_OF,
+						RefType:     gen.SpanRefType_FOLLOWS_FROM,
 						TraceIdHigh: int64(binary.BigEndian.Uint64(linkTraceID[0:8])),
 						TraceIdLow:  int64(binary.BigEndian.Uint64(linkTraceID[8:16])),
 						SpanId:      int64(binary.BigEndian.Uint64(linkSpanID[:])),
@@ -460,6 +462,60 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "with parent",
+			data: &export.SpanSnapshot{
+				ParentSpanID: parentSpanID,
+				SpanContext: trace.SpanContext{
+					TraceID: traceID,
+					SpanID:  spanID,
+				},
+				Links: []trace.Link{
+					{
+						SpanContext: trace.SpanContext{
+							TraceID: linkTraceID,
+							SpanID:  linkSpanID,
+						},
+					},
+				},
+				Name:      "/foo",
+				StartTime: now,
+				EndTime:   now,
+				Attributes: []attribute.KeyValue{
+					attribute.Array("arr", []int{0, 1, 2, 3}),
+				},
+				StatusCode:    codes.Unset,
+				StatusMessage: statusMessage,
+				SpanKind:      trace.SpanKindInternal,
+				InstrumentationLibrary: instrumentation.Library{
+					Name:    instrLibName,
+					Version: instrLibVersion,
+				},
+			},
+			want: &gen.Span{
+				TraceIdLow:    651345242494996240,
+				TraceIdHigh:   72623859790382856,
+				SpanId:        72623859790382856,
+				ParentSpanId:  578437695752307201,
+				OperationName: "/foo",
+				StartTime:     now.UnixNano() / 1000,
+				Duration:      0,
+				Tags: []*gen.Tag{
+					// status code, message and span kind should NOT be populated
+					{Key: "arr", VType: gen.TagType_STRING, VStr: &arrValue},
+					{Key: "otel.library.name", VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: "otel.library.version", VType: gen.TagType_STRING, VStr: &instrLibVersion},
+				},
+				References: []*gen.SpanRef{
+					{
+						RefType:     gen.SpanRefType_FOLLOWS_FROM,
+						TraceIdHigh: int64(binary.BigEndian.Uint64(linkTraceID[0:8])),
+						TraceIdLow:  int64(binary.BigEndian.Uint64(linkTraceID[8:16])),
+						SpanId:      int64(binary.BigEndian.Uint64(linkSpanID[:])),
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {