Fix Jaeger span status reporting and unify tag keys (#1761)

* Fix Jaeger span status reporting and unify tag keys Move all tag key strings to be consts defined in a unified location. Fix the status code and message tag keys to conform with the specification. Do not set the span status message if it is not set to conform with the specification. * Add changes to changelog * Update CHANGELOG.md Co-authored-by: Anthony Mirabella <a9@aneurysm9.com> * Fix misspell Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
open-telemetry · Apr 1, 2021 · 07a8d19 · 07a8d19
1 parent 4fa35c9
commit 07a8d19
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Fixed
 
 - The `Span.IsRecording` implementation from `go.opentelemetry.io/otel/sdk/trace` always returns false when not being sampled. (#1750)
+- The Jaeger exporter now correctly sets tags for the Span status code and message.
+  This means it uses the correct tag keys (`"otel.status_code"`, `"otel.status_description"`) and does not set the status message as a tag unless it is set on the span. (#1761)
 
 ### Changed
 

diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go
@@ -37,6 +37,10 @@ import (
 const (
 	keyInstrumentationLibraryName    = "otel.library.name"
 	keyInstrumentationLibraryVersion = "otel.library.version"
+	keyError                         = "error"
+	keySpanKind                      = "span.kind"
+	keyStatusCode                    = "otel.status_code"
+	keyStatusMessage                 = "otel.status_description"
 )
 
 type Option func(*options)
@@ -269,18 +273,18 @@ func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span {
 
 	if ss.SpanKind != trace.SpanKindInternal {
 		tags = append(tags,
-			getStringTag("span.kind", ss.SpanKind.String()),
+			getStringTag(keySpanKind, ss.SpanKind.String()),
 		)
 	}
 
 	if ss.StatusCode != codes.Unset {
-		tags = append(tags,
-			getInt64Tag("status.code", int64(ss.StatusCode)),
-			getStringTag("status.message", ss.StatusMessage),
-		)
+		tags = append(tags, getInt64Tag(keyStatusCode, int64(ss.StatusCode)))
+		if ss.StatusMessage != "" {
+			tags = append(tags, getStringTag(keyStatusMessage, ss.StatusMessage))
+		}
 
 		if ss.StatusCode == codes.Error {
-			tags = append(tags, getBoolTag("error", true))
+			tags = append(tags, getBoolTag(keyError, true))
 		}
 	}
 

diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go
@@ -363,6 +363,40 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 		data *export.SpanSnapshot
 		want *gen.Span
 	}{
+		{
+			name: "no status description",
+			data: &export.SpanSnapshot{
+				SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
+					TraceID: traceID,
+					SpanID:  spanID,
+				}),
+				Name:       "/foo",
+				StartTime:  now,
+				EndTime:    now,
+				StatusCode: codes.Error,
+				SpanKind:   trace.SpanKindClient,
+				InstrumentationLibrary: instrumentation.Library{
+					Name:    instrLibName,
+					Version: instrLibVersion,
+				},
+			},
+			want: &gen.Span{
+				TraceIdLow:    651345242494996240,
+				TraceIdHigh:   72623859790382856,
+				SpanId:        72623859790382856,
+				OperationName: "/foo",
+				StartTime:     now.UnixNano() / 1000,
+				Duration:      0,
+				Tags: []*gen.Tag{
+					{Key: keyError, VType: gen.TagType_BOOL, VBool: &boolTrue},
+					{Key: keyInstrumentationLibraryName, VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: keyInstrumentationLibraryVersion, VType: gen.TagType_STRING, VStr: &instrLibVersion},
+					{Key: keyStatusCode, VType: gen.TagType_LONG, VLong: &statusCodeValue},
+					// Should not have a status message because it was unset
+					{Key: keySpanKind, VType: gen.TagType_STRING, VStr: &spanKind},
+				},
+			},
+		},
 		{
 			name: "no parent",
 			data: &export.SpanSnapshot{
@@ -408,12 +442,12 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 					{Key: "double", VType: gen.TagType_DOUBLE, VDouble: &doubleValue},
 					{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.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},
+					{Key: keyError, VType: gen.TagType_BOOL, VBool: &boolTrue},
+					{Key: keyInstrumentationLibraryName, VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: keyInstrumentationLibraryVersion, VType: gen.TagType_STRING, VStr: &instrLibVersion},
+					{Key: keyStatusCode, VType: gen.TagType_LONG, VLong: &statusCodeValue},
+					{Key: keyStatusMessage, VType: gen.TagType_STRING, VStr: &statusMessage},
+					{Key: keySpanKind, VType: gen.TagType_STRING, VStr: &spanKind},
 				},
 				References: []*gen.SpanRef{
 					{
@@ -486,8 +520,8 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 				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},
+					{Key: keyInstrumentationLibraryName, VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: keyInstrumentationLibraryVersion, VType: gen.TagType_STRING, VStr: &instrLibVersion},
 				},
 				References: []*gen.SpanRef{
 					{
@@ -535,8 +569,8 @@ func Test_spanSnapshotToThrift(t *testing.T) {
 				StartTime:     now.UnixNano() / 1000,
 				Duration:      0,
 				Tags: []*gen.Tag{
-					{Key: "otel.library.name", VType: gen.TagType_STRING, VStr: &instrLibName},
-					{Key: "otel.library.version", VType: gen.TagType_STRING, VStr: &instrLibVersion},
+					{Key: keyInstrumentationLibraryName, VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: keyInstrumentationLibraryVersion, VType: gen.TagType_STRING, VStr: &instrLibVersion},
 				},
 			},
 		},
@@ -649,7 +683,7 @@ func TestJaegerBatchList(t *testing.T) {
 						{
 							OperationName: "s1",
 							Tags: []*gen.Tag{
-								{Key: "span.kind", VType: gen.TagType_STRING, VStr: &spanKind},
+								{Key: keySpanKind, VType: gen.TagType_STRING, VStr: &spanKind},
 							},
 							StartTime: now.UnixNano() / 1000,
 						},