From b2bc497b9ea535e0b1fa9540daf7ae6e7be9ab1e Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 23 Apr 2024 05:24:12 -0400 Subject: [PATCH] [pkg/translator/jaeger] Preserve binary attribute values (#32208) **Description:** translate binary attribute values to/from Jaeger as is, without encoding them as base64 strings **Link to tracking Issue:** Resolves #32204 **Testing:** unit tests in the package **Documentation:** none --------- Signed-off-by: Yuri Shkuro --- .../translator_jaeger_keep_bin_attrs.yaml | 27 +++++++++++++++++++ .../jaeger/jaegerproto_to_traces.go | 3 +-- .../jaeger/jaegerproto_to_traces_test.go | 2 +- .../jaeger/jaegerthrift_to_traces.go | 3 +-- .../jaeger/jaegerthrift_to_traces_test.go | 2 +- .../jaeger/traces_to_jaegerproto.go | 8 ++---- .../jaeger/traces_to_jaegerproto_test.go | 6 ++--- 7 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 .chloggen/translator_jaeger_keep_bin_attrs.yaml diff --git a/.chloggen/translator_jaeger_keep_bin_attrs.yaml b/.chloggen/translator_jaeger_keep_bin_attrs.yaml new file mode 100644 index 000000000000..f459acd6c645 --- /dev/null +++ b/.chloggen/translator_jaeger_keep_bin_attrs.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/translator/jaeger + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: translate binary attribute values to/from Jaeger as is, without encoding them as base64 strings + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [32204] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/pkg/translator/jaeger/jaegerproto_to_traces.go b/pkg/translator/jaeger/jaegerproto_to_traces.go index 1ac848cfaf77..099e13e5ec89 100644 --- a/pkg/translator/jaeger/jaegerproto_to_traces.go +++ b/pkg/translator/jaeger/jaegerproto_to_traces.go @@ -4,7 +4,6 @@ package jaeger // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" import ( - "encoding/base64" "encoding/binary" "fmt" "hash/fnv" @@ -240,7 +239,7 @@ func jTagsToInternalAttributes(tags []model.KeyValue, dest pcommon.Map) { case model.ValueType_FLOAT64: dest.PutDouble(tag.Key, tag.GetVFloat64()) case model.ValueType_BINARY: - dest.PutStr(tag.Key, base64.StdEncoding.EncodeToString(tag.GetVBinary())) + dest.PutEmptyBytes(tag.Key).FromRaw(tag.GetVBinary()) default: dest.PutStr(tag.Key, fmt.Sprintf("", tag.GetVType())) } diff --git a/pkg/translator/jaeger/jaegerproto_to_traces_test.go b/pkg/translator/jaeger/jaegerproto_to_traces_test.go index 9270e903fe7b..e196efd909ad 100644 --- a/pkg/translator/jaeger/jaegerproto_to_traces_test.go +++ b/pkg/translator/jaeger/jaegerproto_to_traces_test.go @@ -168,7 +168,7 @@ func TestJTagsToInternalAttributes(t *testing.T) { expected.PutInt("int-val", 123) expected.PutStr("string-val", "abc") expected.PutDouble("double-val", 1.23) - expected.PutStr("binary-val", "AAAAAABkfZg=") + expected.PutEmptyBytes("binary-val").FromRaw([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0x7D, 0x98}) got := pcommon.NewMap() jTagsToInternalAttributes(tags, got) diff --git a/pkg/translator/jaeger/jaegerthrift_to_traces.go b/pkg/translator/jaeger/jaegerthrift_to_traces.go index 4504d1cc364f..131b1e49b54a 100644 --- a/pkg/translator/jaeger/jaegerthrift_to_traces.go +++ b/pkg/translator/jaeger/jaegerthrift_to_traces.go @@ -4,7 +4,6 @@ package jaeger // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" import ( - "encoding/base64" "fmt" "reflect" @@ -150,7 +149,7 @@ func jThriftTagsToInternalAttributes(tags []*jaeger.Tag, dest pcommon.Map) { case jaeger.TagType_DOUBLE: dest.PutDouble(tag.Key, tag.GetVDouble()) case jaeger.TagType_BINARY: - dest.PutStr(tag.Key, base64.StdEncoding.EncodeToString(tag.GetVBinary())) + dest.PutEmptyBytes(tag.Key).FromRaw(tag.GetVBinary()) default: dest.PutStr(tag.Key, fmt.Sprintf("", tag.GetVType())) } diff --git a/pkg/translator/jaeger/jaegerthrift_to_traces_test.go b/pkg/translator/jaeger/jaegerthrift_to_traces_test.go index f9c69c5208f7..e3f2bb1cdf78 100644 --- a/pkg/translator/jaeger/jaegerthrift_to_traces_test.go +++ b/pkg/translator/jaeger/jaegerthrift_to_traces_test.go @@ -56,7 +56,7 @@ func TestJThriftTagsToInternalAttributes(t *testing.T) { expected.PutInt("int-val", 123) expected.PutStr("string-val", "abc") expected.PutDouble("double-val", 1.23) - expected.PutStr("binary-val", "AAAAAABkfZg=") + expected.PutEmptyBytes("binary-val").FromRaw([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0x7D, 0x98}) got := pcommon.NewMap() jThriftTagsToInternalAttributes(tags, got) diff --git a/pkg/translator/jaeger/traces_to_jaegerproto.go b/pkg/translator/jaeger/traces_to_jaegerproto.go index a0a24920e0ff..ecbd780f02bf 100644 --- a/pkg/translator/jaeger/traces_to_jaegerproto.go +++ b/pkg/translator/jaeger/traces_to_jaegerproto.go @@ -4,8 +4,6 @@ package jaeger // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" import ( - "encoding/base64" - "github.com/jaegertracing/jaeger/model" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" @@ -125,8 +123,6 @@ func attributeToJaegerProtoTag(key string, attr pcommon.Value) model.KeyValue { tag := model.KeyValue{Key: key} switch attr.Type() { case pcommon.ValueTypeStr: - // Jaeger-to-Internal maps binary tags to string attributes and encodes them as - // base64 strings. Blindingly attempting to decode base64 seems too much. tag.VType = model.ValueType_STRING tag.VStr = attr.Str() case pcommon.ValueTypeInt: @@ -139,8 +135,8 @@ func attributeToJaegerProtoTag(key string, attr pcommon.Value) model.KeyValue { tag.VType = model.ValueType_FLOAT64 tag.VFloat64 = attr.Double() case pcommon.ValueTypeBytes: - tag.VType = model.ValueType_STRING - tag.VStr = base64.StdEncoding.EncodeToString(attr.Bytes().AsRaw()) + tag.VType = model.ValueType_BINARY + tag.VBinary = attr.Bytes().AsRaw() case pcommon.ValueTypeMap, pcommon.ValueTypeSlice: tag.VType = model.ValueType_STRING tag.VStr = attr.AsString() diff --git a/pkg/translator/jaeger/traces_to_jaegerproto_test.go b/pkg/translator/jaeger/traces_to_jaegerproto_test.go index 744b9be19495..20d8ee1e3d00 100644 --- a/pkg/translator/jaeger/traces_to_jaegerproto_test.go +++ b/pkg/translator/jaeger/traces_to_jaegerproto_test.go @@ -195,9 +195,9 @@ func TestAttributesToJaegerProtoTags(t *testing.T) { VFloat64: 1.23, }, { - Key: "bytes-val", - VType: model.ValueType_STRING, - VStr: "AQIDBA==", // base64 encoding of the byte array [1,2,3,4] + Key: "bytes-val", + VType: model.ValueType_BINARY, + VBinary: []byte{1, 2, 3, 4}, }, { Key: conventions.AttributeServiceName,