Add InstrumentationLibrary info to Zipkin/Jaeger exporters (#1119)

* Add InstrumentationLibrary info to Zipkin/Jaeger exporters This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues #1086 #1087 * Reflect change in CHANGELOG
open-telemetry · Sep 3, 2020 · f38e190 · f38e190
1 parent 440c4bd
commit f38e190
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,13 +12,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - In the `go.opentelemetry.io/otel/api/trace` package a new `TracerConfigure` function was added to configure a new `TracerConfig`.
    This addition was made to conform with our project option conventions. (#1109)
+- Instrumentation library information was added to the Zipkin exporter. (#1119)
 
 ### Changed
 
 - Add reconnecting udp connection type to Jaeger exporter.
    This change adds a new optional implementation of the udp conn interface used to detect changes to an agent's host dns record.
    It then adopts the new destination address to ensure the exporter doesn't get stuck. This change was ported from jaegertracing/jaeger-client-go#520. (#1063)
 - The `go.opentelemetry.io/otel/api/trace` `TracerOption` was changed to an interface to conform to project option conventions. (#1109)
+- Rename Jaeger tags used for instrumentation library information to reflect changes in OpenTelemetry specification. (#1119)
 
 ## [0.11.0] - 2020-08-24
 

diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go
@@ -29,7 +29,12 @@ import (
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
-const defaultServiceName = "OpenTelemetry"
+const (
+	defaultServiceName = "OpenTelemetry"
+
+	keyInstrumentationLibraryName    = "otel.instrumentation_library.name"
+	keyInstrumentationLibraryVersion = "otel.instrumentation_library.version"
+)
 
 type Option func(*options)
 
@@ -228,11 +233,10 @@ func spanDataToThrift(data *export.SpanData) *gen.Span {
 			}
 		}
 	}
-
 	if il := data.InstrumentationLibrary; il.Name != "" {
-		tags = append(tags, getStringTag("instrumentation.name", il.Name))
+		tags = append(tags, getStringTag(keyInstrumentationLibraryName, il.Name))
 		if il.Version != "" {
-			tags = append(tags, getStringTag("instrumentation.version", il.Version))
+			tags = append(tags, getStringTag(keyInstrumentationLibraryVersion, il.Version))
 		}
 	}
 

diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go
@@ -430,8 +430,8 @@ func Test_spanDataToThrift(t *testing.T) {
 					{Key: "key", VType: gen.TagType_STRING, VStr: &keyValue},
 					{Key: "uint", VType: gen.TagType_LONG, VLong: &uintValue},
 					{Key: "error", VType: gen.TagType_BOOL, VBool: &boolTrue},
-					{Key: "instrumentation.name", VType: gen.TagType_STRING, VStr: &instrLibName},
-					{Key: "instrumentation.version", VType: gen.TagType_STRING, VStr: &instrLibVersion},
+					{Key: "otel.instrumentation_library.name", VType: gen.TagType_STRING, VStr: &instrLibName},
+					{Key: "otel.instrumentation_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},

diff --git a/exporters/trace/zipkin/go.mod b/exporters/trace/zipkin/go.mod
@@ -8,6 +8,7 @@ replace (
 )
 
 require (
+	github.com/google/go-cmp v0.5.2
 	github.com/openzipkin/zipkin-go v0.2.3
 	github.com/stretchr/testify v1.6.1
 	go.opentelemetry.io/otel v0.11.0

diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go
@@ -26,6 +26,11 @@ import (
 	export "go.opentelemetry.io/otel/sdk/export/trace"
 )
 
+const (
+	keyInstrumentationLibraryName    = "otel.instrumentation_library.name"
+	keyInstrumentationLibraryVersion = "otel.instrumentation_library.version"
+)
+
 func toZipkinSpanModels(batch []*export.SpanData, serviceName string) []zkmodel.SpanModel {
 	models := make([]zkmodel.SpanModel, 0, len(batch))
 	for _, data := range batch {
@@ -132,9 +137,16 @@ func attributesToJSONMapString(attributes []label.KeyValue) string {
 	return (string)(jsonBytes)
 }
 
+// extraZipkinTags are those that may be added to every outgoing span
+var extraZipkinTags = []string{
+	"ot.status_code",
+	"ot.status_description",
+	keyInstrumentationLibraryName,
+	keyInstrumentationLibraryVersion,
+}
+
 func toZipkinTags(data *export.SpanData) map[string]string {
-	// +2 for status code and for status message
-	m := make(map[string]string, len(data.Attributes)+2)
+	m := make(map[string]string, len(data.Attributes)+len(extraZipkinTags))
 	for _, kv := range data.Attributes {
 		m[(string)(kv.Key)] = kv.Value.Emit()
 	}
@@ -143,5 +155,12 @@ func toZipkinTags(data *export.SpanData) map[string]string {
 	}
 	m["ot.status_code"] = data.StatusCode.String()
 	m["ot.status_description"] = data.StatusMessage
+
+	if il := data.InstrumentationLibrary; il.Name != "" {
+		m[keyInstrumentationLibraryName] = il.Name
+		if il.Version != "" {
+			m[keyInstrumentationLibraryVersion] = il.Version
+		}
+	}
 	return m
 }
diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go
@@ -15,16 +15,20 @@
 package zipkin
 
 import (
+	"fmt"
+	"strconv"
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	zkmodel "github.com/openzipkin/zipkin-go/model"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
 
 	"go.opentelemetry.io/otel/api/trace"
 	"go.opentelemetry.io/otel/label"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/sdk/instrumentation"
 )
 
 func TestModelConversion(t *testing.T) {
@@ -656,3 +660,123 @@ func zkmodelIDPtr(n uint64) *zkmodel.ID {
 	id := zkmodel.ID(n)
 	return &id
 }
+
+func Test_toZipkinTags(t *testing.T) {
+	keyValue := "value"
+	doubleValue := 123.456
+	uintValue := int64(123)
+	statusMessage := "this is a problem"
+	instrLibName := "instrumentation-library"
+	instrLibVersion := "semver:1.0.0"
+
+	tests := []struct {
+		name string
+		data *export.SpanData
+		want map[string]string
+	}{
+		{
+			name: "attributes",
+			data: &export.SpanData{
+				Attributes: []label.KeyValue{
+					label.String("key", keyValue),
+					label.Float64("double", doubleValue),
+					label.Uint64("uint", uint64(uintValue)),
+					label.Bool("ok", true),
+				},
+			},
+			want: map[string]string{
+				"double":                fmt.Sprint(doubleValue),
+				"key":                   keyValue,
+				"ok":                    "true",
+				"uint":                  strconv.FormatInt(uintValue, 10),
+				"ot.status_code":        codes.OK.String(),
+				"ot.status_description": "",
+			},
+		},
+		{
+			name: "no attributes",
+			data: &export.SpanData{},
+			want: map[string]string{
+				"ot.status_code":        codes.OK.String(),
+				"ot.status_description": "",
+			},
+		},
+		{
+			name: "omit-noerror",
+			data: &export.SpanData{
+				Attributes: []label.KeyValue{
+					label.Bool("error", false),
+				},
+			},
+			want: map[string]string{
+				"ot.status_code":        codes.OK.String(),
+				"ot.status_description": "",
+			},
+		},
+		{
+			name: "statusCode",
+			data: &export.SpanData{
+				Attributes: []label.KeyValue{
+					label.String("key", keyValue),
+					label.Bool("error", true),
+				},
+				StatusCode:    codes.Unknown,
+				StatusMessage: statusMessage,
+			},
+			want: map[string]string{
+				"error":                 "true",
+				"key":                   keyValue,
+				"ot.status_code":        codes.Unknown.String(),
+				"ot.status_description": statusMessage,
+			},
+		},
+		{
+			name: "instrLib-empty",
+			data: &export.SpanData{
+				InstrumentationLibrary: instrumentation.Library{},
+			},
+			want: map[string]string{
+				"ot.status_code":        codes.OK.String(),
+				"ot.status_description": "",
+			},
+		},
+		{
+			name: "instrLib-noversion",
+			data: &export.SpanData{
+				Attributes: []label.KeyValue{},
+				InstrumentationLibrary: instrumentation.Library{
+					Name: instrLibName,
+				},
+			},
+			want: map[string]string{
+				"otel.instrumentation_library.name": instrLibName,
+				"ot.status_code":                    codes.OK.String(),
+				"ot.status_description":             "",
+			},
+		},
+		{
+			name: "instrLib-with-version",
+			data: &export.SpanData{
+				Attributes: []label.KeyValue{},
+				InstrumentationLibrary: instrumentation.Library{
+					Name:    instrLibName,
+					Version: instrLibVersion,
+				},
+			},
+			want: map[string]string{
+				"otel.instrumentation_library.name":    instrLibName,
+				"otel.instrumentation_library.version": instrLibVersion,
+				"ot.status_code":                       codes.OK.String(),
+				"ot.status_description":                "",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := toZipkinTags(tt.data)
+			if diff := cmp.Diff(got, tt.want); diff != "" {
+				t.Errorf("Diff%v", diff)
+			}
+		})
+	}
+}