From 8fa6afefef141acdb0b98b39da0338f5a0602355 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Thu, 12 Sep 2019 21:03:30 +0800 Subject: [PATCH] Translate OC resource labels to Jaeger process tags (#325) Preserve OC resource labels during translation to Jaeger by adding them to the process tags Right now we translate OC node to Jaeger process while completely ignoring OC resource labels resulting in loss of this data. This PR patches the OC > Jaeger translators by making sure the resource labels are recorded as Jaegerprocess tags. While this preserves process labels and prevents loss of such data when translating to Jaeger, it's not a perfect solution because: 1. It will replace any tags from OC Node with the same names. Current behaviour is to give precedence to resource labels over node attributes. We can make it configurable or revisit the decision later. 2. Translation is not reversible Translating back from Jaeger to OC will still not re-create the OC process object. It'll instead translate all the Jaeger process tags to OC node attributes. We could add some semantic convention to denote resource labels differently or add an additional tag to specify the resource tag names but doing so would add additional complexity to backends processing traces in the Jaeger format. Also, in practice, it's going to be very rare for people to translate from OC to Jaeger and then back from Jaeger to OC in the same data pipeline. While this doesn't solve the cases perfectly, we think it's better than completely dropping the resource data. --- translator/trace/jaeger/constants.go | 1 + .../trace/jaeger/protospan_to_jaegerproto.go | 23 +++++++++++++++++-- .../jaeger/protospan_to_jaegerproto_test.go | 5 ++++ .../trace/jaeger/protospan_to_jaegerthrift.go | 23 +++++++++++++++++-- .../jaeger/protospan_to_jaegerthrift_test.go | 5 ++++ .../proto_batch_no_binary_tags_01.json | 8 +++++++ .../thrift_batch_no_binary_tags_01.json | 10 ++++++++ 7 files changed, 71 insertions(+), 4 deletions(-) diff --git a/translator/trace/jaeger/constants.go b/translator/trace/jaeger/constants.go index ff6eb8d64c3..769dcd790a0 100644 --- a/translator/trace/jaeger/constants.go +++ b/translator/trace/jaeger/constants.go @@ -31,6 +31,7 @@ const ( opencensusLanguage = "opencensus.language" opencensusExporterVersion = "opencensus.exporterversion" opencensusCoreLibVersion = "opencensus.corelibversion" + opencensusResourceType = "opencensus.resourcetype" ) var ( diff --git a/translator/trace/jaeger/protospan_to_jaegerproto.go b/translator/trace/jaeger/protospan_to_jaegerproto.go index 64dc6f7849f..5b8f930feff 100644 --- a/translator/trace/jaeger/protospan_to_jaegerproto.go +++ b/translator/trace/jaeger/protospan_to_jaegerproto.go @@ -19,6 +19,7 @@ import ( "time" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/timestamp" @@ -41,7 +42,7 @@ func OCProtoToJaegerProto(td consumerdata.TraceData) (*jaeger.Batch, error) { } jb := &jaeger.Batch{ - Process: ocNodeToJaegerProcessProto(td.Node), + Process: ocNodeAndResourceToJaegerProcessProto(td.Node, td.Resource), Spans: jSpans, } @@ -49,7 +50,7 @@ func OCProtoToJaegerProto(td consumerdata.TraceData) (*jaeger.Batch, error) { } // Replica of protospan_to_jaegerthrift.ocNodeToJaegerProcess -func ocNodeToJaegerProcessProto(node *commonpb.Node) *jaeger.Process { +func ocNodeAndResourceToJaegerProcessProto(node *commonpb.Node, resource *resourcepb.Resource) *jaeger.Process { if node == nil { return unknownProcessProto } @@ -134,6 +135,24 @@ func ocNodeToJaegerProcessProto(node *commonpb.Node) *jaeger.Process { serviceName = node.ServiceInfo.Name } + if resource != nil { + resourceType := resource.GetType() + if resourceType != "" { + jTags = append(jTags, jaeger.KeyValue{ + Key: opencensusResourceType, + VType: jaeger.ValueType_STRING, + VStr: resourceType, + }) + } + for k, v := range resource.GetLabels() { + jTags = append(jTags, jaeger.KeyValue{ + Key: k, + VType: jaeger.ValueType_STRING, + VStr: v, + }) + } + } + if serviceName == "" && len(jTags) == 0 { // No info to put in the process... return nil diff --git a/translator/trace/jaeger/protospan_to_jaegerproto_test.go b/translator/trace/jaeger/protospan_to_jaegerproto_test.go index 5482de3ed88..efdf1602853 100644 --- a/translator/trace/jaeger/protospan_to_jaegerproto_test.go +++ b/translator/trace/jaeger/protospan_to_jaegerproto_test.go @@ -21,6 +21,7 @@ import ( "testing" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" "github.com/google/go-cmp/cmp" @@ -351,6 +352,10 @@ var ocBatches = []consumerdata.TraceData{ "ip": "10.53.69.61", }, }, + Resource: &resourcepb.Resource{ + Type: "k8s.io/container", + Labels: map[string]string{"resource_key1": "resource_val1"}, + }, Spans: []*tracepb.Span{ { TraceId: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x52, 0x96, 0x9A, 0x89, 0x55, 0x57, 0x1A, 0x3F}, diff --git a/translator/trace/jaeger/protospan_to_jaegerthrift.go b/translator/trace/jaeger/protospan_to_jaegerthrift.go index da4a11fcbdf..4a888fd7e57 100644 --- a/translator/trace/jaeger/protospan_to_jaegerthrift.go +++ b/translator/trace/jaeger/protospan_to_jaegerthrift.go @@ -18,6 +18,7 @@ import ( "fmt" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/timestamp" @@ -39,14 +40,14 @@ func OCProtoToJaegerThrift(td consumerdata.TraceData) (*jaeger.Batch, error) { } jb := &jaeger.Batch{ - Process: ocNodeToJaegerProcess(td.Node), + Process: ocNodeAndResourceToJaegerProcess(td.Node, td.Resource), Spans: jSpans, } return jb, nil } -func ocNodeToJaegerProcess(node *commonpb.Node) *jaeger.Process { +func ocNodeAndResourceToJaegerProcess(node *commonpb.Node, resource *resourcepb.Resource) *jaeger.Process { if node == nil { // Jaeger requires a non-nil Process return unknownProcess @@ -132,6 +133,24 @@ func ocNodeToJaegerProcess(node *commonpb.Node) *jaeger.Process { serviceName = node.ServiceInfo.Name } + if resource != nil { + resourceType := resource.GetType() + if resourceType != "" { + jTags = append(jTags, &jaeger.Tag{ + Key: opencensusResourceType, + VType: jaeger.TagType_STRING, + VStr: &resourceType, + }) + } + for k, v := range resource.GetLabels() { + jTags = append(jTags, &jaeger.Tag{ + Key: k, + VType: jaeger.TagType_STRING, + VStr: &v, + }) + } + } + if serviceName == "" && len(jTags) == 0 { // No info to put in the process... return nil diff --git a/translator/trace/jaeger/protospan_to_jaegerthrift_test.go b/translator/trace/jaeger/protospan_to_jaegerthrift_test.go index 70e59520089..d38ef2ecdae 100644 --- a/translator/trace/jaeger/protospan_to_jaegerthrift_test.go +++ b/translator/trace/jaeger/protospan_to_jaegerthrift_test.go @@ -22,6 +22,7 @@ import ( "testing" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/golang/protobuf/ptypes/timestamp" "github.com/google/go-cmp/cmp" @@ -405,6 +406,10 @@ var tds = []consumerdata.TraceData{ "ip": "10.53.69.61", }, }, + Resource: &resourcepb.Resource{ + Type: "k8s.io/container", + Labels: map[string]string{"resource_key1": "resource_val1"}, + }, Spans: []*tracepb.Span{ { TraceId: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x52, 0x96, 0x9A, 0x89, 0x55, 0x57, 0x1A, 0x3F}, diff --git a/translator/trace/jaeger/testdata/proto_batch_no_binary_tags_01.json b/translator/trace/jaeger/testdata/proto_batch_no_binary_tags_01.json index 40eda44c594..572575d249c 100644 --- a/translator/trace/jaeger/testdata/proto_batch_no_binary_tags_01.json +++ b/translator/trace/jaeger/testdata/proto_batch_no_binary_tags_01.json @@ -38,6 +38,14 @@ { "key": "a.binary", "v_str": "AQIDBAMCAQ==" + }, + { + "key": "resource_key1", + "v_str": "resource_val1" + }, + { + "key": "opencensus.resourcetype", + "v_str": "k8s.io/container" } ] }, diff --git a/translator/trace/jaeger/testdata/thrift_batch_no_binary_tags_01.json b/translator/trace/jaeger/testdata/thrift_batch_no_binary_tags_01.json index 94cb5f4669f..181d66ff391 100644 --- a/translator/trace/jaeger/testdata/thrift_batch_no_binary_tags_01.json +++ b/translator/trace/jaeger/testdata/thrift_batch_no_binary_tags_01.json @@ -46,6 +46,16 @@ "key": "a.binary", "vType": "STRING", "vStr": "AQIDBAMCAQ==" + }, + { + "key": "resource_key1", + "vType": "STRING", + "vStr": "resource_val1" + }, + { + "key": "opencensus.resourcetype", + "vType": "STRING", + "vStr": "k8s.io/container" } ] },