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

Fix rust WorkUnit span_id bug #8129

Merged
merged 1 commit into from Aug 1, 2019

Conversation

@cattibrie
Copy link
Contributor

commented Jul 30, 2019

Problem

span_id for Zipkin trace should be a 64-bit size and it is represented as a 16-digit hex string.
In rust part of the code WorkUnits sometimes had the wrong format (instead of leading 0 it had leading spaces which is not a hex string char).

@stuhood
Copy link
Member

left a comment

Thanks!

@stuhood stuhood added this to the 1.19.x milestone Jul 30, 2019

let hex_string = hex_16_digit_string(number);
assert_eq!(16, hex_string.len());
for ch in hex_string.chars() {
assert!(ch.is_ascii_hexdigit())

This comment has been minimized.

Copy link
@pierrechevalier83

pierrechevalier83 Jul 31, 2019

Contributor

This commit is looking good to me.
Maybe the test could be a tad more stringent, for instance by verifying that the hex encoding turned out OK.
You could to this by taking a somewhat random u64, but writing it down in hex with the 0x prefix, so it's easy to visually check the correctness.
So something like this:

assert_eq("0123456789abcdef", hex_16_digit_string(0x0123456789abcdef));

This comment has been minimized.

Copy link
@stuhood

stuhood Aug 1, 2019

Member

Since things were green, I went ahead and merged. Can discuss expanding this in a followup. Thanks!

@stuhood stuhood merged commit d80d23c into pantsbuild:master Aug 1, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

blorente added a commit that referenced this pull request Aug 1, 2019

Fix span id bug (#8129)
### Problem

span_id for Zipkin trace should be a 64-bit size and it is represented as a 16-digit hex string.
In rust part of the code WorkUnits sometimes had the wrong format (instead of leading 0 it had leading spaces which is not a hex string char).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.