diff --git a/CHANGELOG.md b/CHANGELOG.md index 12817dafabc..89f53c89f61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm 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) - The Jaeger exporter now correctly records Span event's names using the `"event"` key for a tag. Additionally, this tag is overridden, as specified in the OTel specification, if the event contains an attribute with that key. (#1768) +- Zipkin Exporter: Ensure mapping between OTel and Zipkin span data complies with the specification. (#1688) ### Changed diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index 5404613cbb0..f3a2dde1762 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -18,18 +18,25 @@ import ( "encoding/binary" "encoding/json" "fmt" + "net" + "strconv" zkmodel "github.com/openzipkin/zipkin-go/model" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + sdktrace "go.opentelemetry.io/otel/sdk/trace" tracesdk "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/trace" ) const ( - keyInstrumentationLibraryName = "otel.instrumentation_library.name" - keyInstrumentationLibraryVersion = "otel.instrumentation_library.version" + keyInstrumentationLibraryName = "otel.library.name" + keyInstrumentationLibraryVersion = "otel.library.version" + + keyPeerHostname attribute.Key = "peer.hostname" + keyPeerAddress attribute.Key = "peer.address" ) func toZipkinSpanModels(batch []*tracesdk.SpanSnapshot) []zkmodel.SpanModel { @@ -62,7 +69,7 @@ func toZipkinSpanModel(data *tracesdk.SpanSnapshot) zkmodel.SpanModel { LocalEndpoint: &zkmodel.Endpoint{ ServiceName: getServiceName(data.Resource.Attributes()), }, - RemoteEndpoint: nil, // *Endpoint + RemoteEndpoint: toZipkinRemoteEndpoint(data), Annotations: toZipkinAnnotations(data.MessageEvents), Tags: toZipkinTags(data), } @@ -152,7 +159,6 @@ func attributesToJSONMapString(attributes []attribute.KeyValue) string { // extraZipkinTags are those that may be added to every outgoing span var extraZipkinTags = []string{ "otel.status_code", - "otel.status_description", keyInstrumentationLibraryName, keyInstrumentationLibraryVersion, } @@ -160,13 +166,25 @@ var extraZipkinTags = []string{ func toZipkinTags(data *tracesdk.SpanSnapshot) map[string]string { m := make(map[string]string, len(data.Attributes)+len(extraZipkinTags)) for _, kv := range data.Attributes { - m[(string)(kv.Key)] = kv.Value.Emit() + switch kv.Value.Type() { + // For array attributes, serialize as JSON list string. + case attribute.ARRAY: + json, _ := json.Marshal(kv.Value.AsArray()) + m[(string)(kv.Key)] = (string)(json) + default: + m[(string)(kv.Key)] = kv.Value.Emit() + } } - if v, ok := m["error"]; ok && v == "false" { + + if data.StatusCode != codes.Unset { + m["otel.status_code"] = data.StatusCode.String() + } + + if data.StatusCode == codes.Error { + m["error"] = data.StatusMessage + } else { delete(m, "error") } - m["otel.status_code"] = data.StatusCode.String() - m["otel.status_description"] = data.StatusMessage if il := data.InstrumentationLibrary; il.Name != "" { m[keyInstrumentationLibraryName] = il.Name @@ -174,5 +192,85 @@ func toZipkinTags(data *tracesdk.SpanSnapshot) map[string]string { m[keyInstrumentationLibraryVersion] = il.Version } } + + if len(m) == 0 { + return nil + } + return m } + +// Rank determines selection order for remote endpoint. See the specification +// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.0.1/specification/trace/sdk_exporters/zipkin.md#otlp---zipkin +var remoteEndpointKeyRank = map[attribute.Key]int{ + semconv.PeerServiceKey: 0, + semconv.NetPeerNameKey: 1, + semconv.NetPeerIPKey: 2, + keyPeerHostname: 3, + keyPeerAddress: 4, + semconv.HTTPHostKey: 5, + semconv.DBNameKey: 6, +} + +func toZipkinRemoteEndpoint(data *sdktrace.SpanSnapshot) *zkmodel.Endpoint { + // Should be set only for client or producer kind + if data.SpanKind != trace.SpanKindClient && + data.SpanKind != trace.SpanKindProducer { + return nil + } + + var endpointAttr attribute.KeyValue + for _, kv := range data.Attributes { + rank, ok := remoteEndpointKeyRank[kv.Key] + if !ok { + continue + } + + currentKeyRank, ok := remoteEndpointKeyRank[endpointAttr.Key] + if ok && rank < currentKeyRank { + endpointAttr = kv + } else if !ok { + endpointAttr = kv + } + } + + if endpointAttr.Key == "" { + return nil + } + + if endpointAttr.Key != semconv.NetPeerIPKey && + endpointAttr.Value.Type() == attribute.STRING { + return &zkmodel.Endpoint{ + ServiceName: endpointAttr.Value.AsString(), + } + } + + return remoteEndpointPeerIPWithPort(endpointAttr.Value.AsString(), data.Attributes) +} + +// Handles `net.peer.ip` remote endpoint separately (should include `net.peer.ip` +// as well, if available). +func remoteEndpointPeerIPWithPort(peerIP string, attrs []attribute.KeyValue) *zkmodel.Endpoint { + ip := net.ParseIP(peerIP) + if ip == nil { + return nil + } + + endpoint := &zkmodel.Endpoint{} + // Determine if IPv4 or IPv6 + if ip.To4() != nil { + endpoint.IPv4 = ip + } else { + endpoint.IPv6 = ip + } + + for _, kv := range attrs { + if kv.Key == semconv.NetPeerPortKey { + port, _ := strconv.ParseUint(kv.Value.Emit(), 10, 16) + endpoint.Port = uint16(port) + return endpoint + } + } + + return endpoint +} diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index 89f9c532f6a..fffe1fb5d7f 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -16,6 +16,7 @@ package zipkin import ( "fmt" + "net" "strconv" "testing" "time" @@ -57,6 +58,7 @@ func TestModelConversion(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), + attribute.Array("attr3", []int{0, 1, 2}), }, MessageEvents: []trace.Event{ { @@ -198,6 +200,9 @@ func TestModelConversion(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), + attribute.String("peer.hostname", "test-peer-hostname"), + attribute.String("net.peer.ip", "1.2.3.4"), + attribute.Int64("net.peer.port", 9876), }, MessageEvents: []trace.Event{ { @@ -343,9 +348,8 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + StatusCode: codes.Unset, + Resource: resource, }, } @@ -383,10 +387,11 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "attr3": "[0,1,2]", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data with no parent @@ -422,10 +427,10 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data of unspecified kind @@ -461,10 +466,10 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data of internal kind @@ -500,10 +505,10 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data of client kind @@ -527,7 +532,10 @@ func TestModelConversion(t *testing.T) { LocalEndpoint: &zkmodel.Endpoint{ ServiceName: "model-test", }, - RemoteEndpoint: nil, + RemoteEndpoint: &zkmodel.Endpoint{ + IPv4: net.ParseIP("1.2.3.4"), + Port: 9876, + }, Annotations: []zkmodel.Annotation{ { Timestamp: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), @@ -539,10 +547,13 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "net.peer.ip": "1.2.3.4", + "net.peer.port": "9876", + "peer.hostname": "test-peer-hostname", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data of producer kind @@ -578,10 +589,10 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data of consumer kind @@ -617,10 +628,10 @@ func TestModelConversion(t *testing.T) { }, }, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data with no events @@ -647,10 +658,10 @@ func TestModelConversion(t *testing.T) { RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ - "attr1": "42", - "attr2": "bar", - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "attr1": "42", + "attr2": "bar", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model for span data with an "error" attribute set to "false" @@ -685,10 +696,7 @@ func TestModelConversion(t *testing.T) { Value: "ev2", }, }, - Tags: map[string]string{ - "otel.status_code": "Error", - "otel.status_description": "404, file not found", - }, + Tags: nil, // should be omitted }, } gottenOutputBatch := toZipkinSpanModels(inputBatch) @@ -700,7 +708,7 @@ func zkmodelIDPtr(n uint64) *zkmodel.ID { return &id } -func Test_toZipkinTags(t *testing.T) { +func TestTagsTransformation(t *testing.T) { keyValue := "value" doubleValue := 123.456 uintValue := int64(123) @@ -724,21 +732,16 @@ func Test_toZipkinTags(t *testing.T) { }, }, want: map[string]string{ - "double": fmt.Sprint(doubleValue), - "key": keyValue, - "ok": "true", - "uint": strconv.FormatInt(uintValue, 10), - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", + "double": fmt.Sprint(doubleValue), + "key": keyValue, + "ok": "true", + "uint": strconv.FormatInt(uintValue, 10), }, }, { name: "no attributes", data: &tracesdk.SpanSnapshot{}, - want: map[string]string{ - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", - }, + want: nil, }, { name: "omit-noerror", @@ -747,10 +750,7 @@ func Test_toZipkinTags(t *testing.T) { attribute.Bool("error", false), }, }, - want: map[string]string{ - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", - }, + want: nil, }, { name: "statusCode", @@ -763,10 +763,9 @@ func Test_toZipkinTags(t *testing.T) { StatusMessage: statusMessage, }, want: map[string]string{ - "error": "true", - "key": keyValue, - "otel.status_code": codes.Error.String(), - "otel.status_description": statusMessage, + "error": statusMessage, + "key": keyValue, + "otel.status_code": codes.Error.String(), }, }, { @@ -774,10 +773,7 @@ func Test_toZipkinTags(t *testing.T) { data: &tracesdk.SpanSnapshot{ InstrumentationLibrary: instrumentation.Library{}, }, - want: map[string]string{ - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", - }, + want: nil, }, { name: "instrLib-noversion", @@ -788,9 +784,7 @@ func Test_toZipkinTags(t *testing.T) { }, }, want: map[string]string{ - "otel.instrumentation_library.name": instrLibName, - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", + "otel.library.name": instrLibName, }, }, { @@ -803,10 +797,8 @@ func Test_toZipkinTags(t *testing.T) { }, }, want: map[string]string{ - "otel.instrumentation_library.name": instrLibName, - "otel.instrumentation_library.version": instrLibVersion, - "otel.status_code": codes.Unset.String(), - "otel.status_description": "", + "otel.library.name": instrLibName, + "otel.library.version": instrLibVersion, }, }, } @@ -820,6 +812,146 @@ func Test_toZipkinTags(t *testing.T) { } } +func TestRemoteEndpointTransformation(t *testing.T) { + tests := []struct { + name string + data *tracesdk.SpanSnapshot + want *zkmodel.Endpoint + }{ + { + name: "nil-not-applicable", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindClient, + Attributes: []attribute.KeyValue{}, + }, + want: nil, + }, + { + name: "nil-not-found", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindConsumer, + Attributes: []attribute.KeyValue{ + attribute.String("attr", "test"), + }, + }, + want: nil, + }, + { + name: "peer-service-rank", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + semconv.PeerServiceKey.String("peer-service-test"), + semconv.NetPeerNameKey.String("peer-name-test"), + semconv.HTTPHostKey.String("http-host-test"), + }, + }, + want: &zkmodel.Endpoint{ + ServiceName: "peer-service-test", + }, + }, + { + name: "http-host-rank", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + semconv.HTTPHostKey.String("http-host-test"), + semconv.DBNameKey.String("db-name-test"), + }, + }, + want: &zkmodel.Endpoint{ + ServiceName: "http-host-test", + }, + }, + { + name: "db-name-rank", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + attribute.String("foo", "bar"), + semconv.DBNameKey.String("db-name-test"), + }, + }, + want: &zkmodel.Endpoint{ + ServiceName: "db-name-test", + }, + }, + { + name: "peer-hostname-rank", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + keyPeerHostname.String("peer-hostname-test"), + keyPeerAddress.String("peer-address-test"), + semconv.HTTPHostKey.String("http-host-test"), + semconv.DBNameKey.String("http-host-test"), + }, + }, + want: &zkmodel.Endpoint{ + ServiceName: "peer-hostname-test", + }, + }, + { + name: "peer-address-rank", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + keyPeerAddress.String("peer-address-test"), + semconv.HTTPHostKey.String("http-host-test"), + semconv.DBNameKey.String("http-host-test"), + }, + }, + want: &zkmodel.Endpoint{ + ServiceName: "peer-address-test", + }, + }, + { + name: "net-peer-invalid-ip", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + semconv.NetPeerIPKey.String("INVALID"), + }, + }, + want: nil, + }, + { + name: "net-peer-ipv6-no-port", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + semconv.NetPeerIPKey.String("0:0:1:5ee:bad:c0de:0:0"), + }, + }, + want: &zkmodel.Endpoint{ + IPv6: net.ParseIP("0:0:1:5ee:bad:c0de:0:0"), + }, + }, + { + name: "net-peer-ipv4-port", + data: &tracesdk.SpanSnapshot{ + SpanKind: trace.SpanKindProducer, + Attributes: []attribute.KeyValue{ + semconv.NetPeerIPKey.String("1.2.3.4"), + semconv.NetPeerPortKey.Int(9876), + }, + }, + want: &zkmodel.Endpoint{ + IPv4: net.ParseIP("1.2.3.4"), + Port: 9876, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := toZipkinRemoteEndpoint(tt.data) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("Diff%v", diff) + } + }) + } +} + func TestServiceName(t *testing.T) { attrs := []attribute.KeyValue{} assert.Empty(t, getServiceName(attrs)) diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index 404d495bebd..95b0b3d625b 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -296,8 +296,8 @@ func TestExportSpans(t *testing.T) { RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ - "otel.status_code": "Error", - "otel.status_description": "404, file not found", + "otel.status_code": "Error", + "error": "404, file not found", }, }, // model of child @@ -324,8 +324,8 @@ func TestExportSpans(t *testing.T) { RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ - "otel.status_code": "Error", - "otel.status_description": "403, forbidden", + "otel.status_code": "Error", + "error": "403, forbidden", }, }, }