Skip to content

Convert unsigned IDs to signed IDs for Thrift - #3

Open
KronicDeth wants to merge 1 commit into
opencensus-beam:masterfrom
KronicDeth:signed-thrift-ids
Open

Convert unsigned IDs to signed IDs for Thrift#3
KronicDeth wants to merge 1 commit into
opencensus-beam:masterfrom
KronicDeth:signed-thrift-ids

Conversation

@KronicDeth

Copy link
Copy Markdown

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

pdobacz commented Jan 14, 2021

Copy link
Copy Markdown

@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