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

Convert unsigned IDs to signed IDs for Thrift #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KronicDeth
Copy link

Fixes jaegertracing/jaeger#1951

Changelog

Bug Fixes

  • Convert unsigned IDs to signed IDs for Thrift
    Thrift IDs are i64, which is a signed 64-bit integer, but the Jaeger protocol says IDs are unsigned, so in our records we store them unsigned.  When sending the IDs through Thrift, we need to convert the bits directly from unsigned to signed.  This matches the behavior in the Python client as pointed out in the bug report triage, but we get to use type specifiers to make it clearer (:tada: Erlang!) instead of bit shifts and masking that Python needs to do.

    Screen Shot 2020-10-20 at 9 47 58 PM
    Screen Shot 2020-10-20 at 9 48 37 PM

Fixes jaegertracing/jaeger#1951

Thrift IDs are i64, which is a _signed_ 64-bit integer, but the Jaeger protocol says IDs are unsigned, so in our records we store them unsigned.  When sending the IDs through Thrift, we need to convert the bits directly from unsigned to signed.  This matches the behavior in the Python client (https://github.com/jaegertracing/jaeger-client-python/blob/51d9027c3e0b87f6721dd14e7bc2b3303ce682c2/jaeger_client/thrift.py#L32-L47) as pointed out here (jaegertracing/jaeger#1951 (comment)), but we get to use type specifiers to make it clearer (:tada: Erlang!) instead of bit shifts and masking that Python needs to do.
@pdobacz
Copy link

pdobacz commented Jan 14, 2021

@KronicDeth Hello, and thank you for sharing this!

I have been experiencing same issue as descrbed in jaegertracing/jaeger#1951.

I tried applying your fix my mix.depsing from your branch, but the issue still seems to have persisted for me. I'm assuming you were able to resolve using this on your end?

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.

Jaeger mangles (?) Trace IDs sent with Thrift
2 participants