From df86b7cd2d0acb06358a54b3fa211523fdc51ca8 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Wed, 2 Sep 2020 13:43:10 -0700 Subject: [PATCH] Add InstrumentationLibrary info to Zipkin/Jaeger exporters This addresses spec issues https://github.com/open-telemetry/opentelemetry-specification/pull/800 https://github.com/open-telemetry/opentelemetry-specification/pull/801 and opentelemetry-go issues https://github.com/open-telemetry/opentelemetry-go/issues/1086 https://github.com/open-telemetry/opentelemetry-go/issues/1087 --- exporters/trace/jaeger/jaeger.go | 12 ++- exporters/trace/jaeger/jaeger_test.go | 4 +- exporters/trace/zipkin/go.mod | 1 + exporters/trace/zipkin/model.go | 23 ++++- exporters/trace/zipkin/model_test.go | 124 ++++++++++++++++++++++++++ 5 files changed, 156 insertions(+), 8 deletions(-) diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index 29ba1db91ad..001c269ae1a 100644 --- 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 index f4dfcb592fd..a25e08c977b 100644 --- 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 index b99972d4e7e..284c42f3a8b 100644 --- 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 index b8e78b3dd03..ebefcb93242 100644 --- 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 index a2ea9db2d51..d7219705008 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -15,9 +15,12 @@ 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" @@ -25,6 +28,7 @@ import ( "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) + } + }) + } +}