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.

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

[translator/jaeger] Convert ValueTypeBytes attributes in Jaeger translator #14574

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

mcardy
Copy link
Contributor

@mcardy mcardy commented Sep 28, 2022

Description: Adds support for ValueTypeBytes to Jaeger's trace to jaegerproto translator.
See open-telemetry/opentelemetry-collector#6153

Link to tracking Issue:

Testing: Added unit test case for the new type

Documentation: None, minor change

@mcardy mcardy requested review from a team, dashpole and jpkrohling September 28, 2022 17:24
@mcardy mcardy changed the title Convert ValueTypeBytes attributes in Jaeger translator [exporter/jaegerexporter] Convert ValueTypeBytes attributes in Jaeger translator Sep 28, 2022
@mcardy mcardy changed the title [exporter/jaegerexporter] Convert ValueTypeBytes attributes in Jaeger translator [translator/jaeger] Convert ValueTypeBytes attributes in Jaeger translator Sep 28, 2022
@jpkrohling jpkrohling requested a review from frzifus September 29, 2022 14:07
@jpkrohling
Copy link
Member

jpkrohling commented Sep 29, 2022

@frzifus, would you like to take a look?

Copy link
Member

@frzifus frzifus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :)

@frzifus
Copy link
Member

frzifus commented Sep 30, 2022

Seems there was 1 ci step failing: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/3145579848/jobs/5113189896#step:14:122

Is it just flaky?

@bogdandrutu bogdandrutu merged commit 7e904d8 into open-telemetry:main Sep 30, 2022
@mcardy mcardy deleted the jaeger_bytearray branch September 30, 2022 16:01
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants