Skip to content

Commit

Permalink
Translate OC resource labels to Jaeger process tags (#325)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
owais authored and tigrannajaryan committed Sep 12, 2019
1 parent df62b61 commit 8fa6afe
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 4 deletions.
1 change: 1 addition & 0 deletions translator/trace/jaeger/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
opencensusLanguage = "opencensus.language"
opencensusExporterVersion = "opencensus.exporterversion"
opencensusCoreLibVersion = "opencensus.corelibversion"
opencensusResourceType = "opencensus.resourcetype"
)

var (
Expand Down
23 changes: 21 additions & 2 deletions translator/trace/jaeger/protospan_to_jaegerproto.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -41,15 +42,15 @@ func OCProtoToJaegerProto(td consumerdata.TraceData) (*jaeger.Batch, error) {
}

jb := &jaeger.Batch{
Process: ocNodeToJaegerProcessProto(td.Node),
Process: ocNodeAndResourceToJaegerProcessProto(td.Node, td.Resource),
Spans: jSpans,
}

return jb, nil
}

// 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
}
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions translator/trace/jaeger/protospan_to_jaegerproto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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},
Expand Down
23 changes: 21 additions & 2 deletions translator/trace/jaeger/protospan_to_jaegerthrift.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions translator/trace/jaeger/protospan_to_jaegerthrift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
},
Expand Down

0 comments on commit 8fa6afe

Please sign in to comment.