From 05252f40d807999388813afb2802918a38cc0e8a Mon Sep 17 00:00:00 2001 From: Matej Gera <38492574+matej-g@users.noreply.github.com> Date: Mon, 8 Mar 2021 18:32:02 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + exporters/trace/jaeger/jaeger.go | 40 +++++++++++------ exporters/trace/jaeger/jaeger_test.go | 62 +++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e13a862638b..2a4ea6e3417 100644 --- 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 index dcd43c83fee..3203258744d 100644 --- 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 index 503b9d9668e..6a52f973886 100644 --- 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) {