Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Jaeger translator performs unnecessary conversion of binary attributes to strings #32204

Closed
yurishkuro opened this issue Apr 5, 2024 · 1 comment · Fixed by #32208
Closed

Jaeger translator performs unnecessary conversion of binary attributes to strings #32204

yurishkuro opened this issue Apr 5, 2024 · 1 comment · Fixed by #32208
Labels
bug Something isn't working pkg/translator/jaeger

Comments

@yurishkuro
Copy link
Member

Component(s)

pkg/translator/jaeger

What happened?

Description

When implementing e2e integration tests for jaeger-v2 we encountered a conversion issue jaegertracing/jaeger#5322 (comment)

Expected Result

binary attributes in OTEL should be represented as binary tags in Jaeger

Actual Result

transformers in both directions encode binary as base64. Behavior introduced in #14574

opentelemetry-collector-contrib/pkg/translator/jaeger/traces_to_jaegerproto.go

Lines 141 to 143 in 5f1488f

case pcommon.ValueTypeBytes:
tag.VType = model.ValueType_STRING
tag.VStr = base64.StdEncoding.EncodeToString(attr.Bytes().AsRaw())

opentelemetry-collector-contrib/pkg/translator/jaeger/jaegerproto_to_traces.go

Lines 242 to 243 in 5f1488f

case model.ValueType_BINARY:
dest.PutStr(tag.Key, base64.StdEncoding.EncodeToString(tag.GetVBinary()))

opentelemetry-collector-contrib/pkg/translator/jaeger/jaegerthrift_to_traces.go

Lines 152 to 153 in 5f1488f

case jaeger.TagType_BINARY:
dest.PutStr(tag.Key, base64.StdEncoding.EncodeToString(tag.GetVBinary()))

Collector version

0.97

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response
@yurishkuro yurishkuro added bug Something isn't working needs triage New item requiring triage labels Apr 5, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 5, 2024

Pinging code owners:

  • pkg/translator/jaeger: @open-telemetry/collector-approvers @frzifus

See Adding Labels via Comments if you do not have permissions to add labels yourself.

This was referenced Apr 5, 2024
Merged
Merged
@Frapschen Frapschen removed the needs triage New item requiring triage label Apr 7, 2024
@github-actions github-actions bot mentioned this issue Apr 9, 2024
Open
@mx-psi mx-psi closed this as completed in b2bc497 Apr 23, 2024
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@yurishkuro @rimitchell
[pkg/translator/jaeger] Preserve binary attribute values (open-teleme…
11f04d1 
…try#32208)

**Description:** translate binary attribute values to/from Jaeger as is,
without encoding them as base64 strings

**Link to tracking Issue:**  Resolves open-telemetry#32204

**Testing:** unit tests in the package

**Documentation:** none

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
jlg-io pushed a commit to jlg-io/opentelemetry-collector-contrib that referenced this issue May 14, 2024
@yurishkuro @jlg-io
[pkg/translator/jaeger] Preserve binary attribute values (open-teleme…
44fdfbc 
…try#32208)

**Description:** translate binary attribute values to/from Jaeger as is,
without encoding them as base64 strings

**Link to tracking Issue:**  Resolves open-telemetry#32204

**Testing:** unit tests in the package

**Documentation:** none

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working pkg/translator/jaeger
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[pkg/translator/jaeger] Preserve binary attribute values yurishkuro/opentelemetry-collector-contrib
2 participants
@yurishkuro @Frapschen