Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

PHP 64 bit signed int causing issues with Trace ID #7

Closed
lookaflyingdonkey opened this issue Aug 20, 2018 · 6 comments
Closed

PHP 64 bit signed int causing issues with Trace ID #7

lookaflyingdonkey opened this issue Aug 20, 2018 · 6 comments

Comments

@lookaflyingdonkey
Copy link

The SpanConverter class uses hexdec to convert the traceId (both high and low is using 128bit ids) to an integer for the Thrift package, the issue is that PHP doesn't support unsigned ints, so any ID above 0x7FFFFFFFFFFFFFFF will be output as a float and will lose precision.

When converted back it will have changed the spanId

$original = 'ba3035a23b64558d';

echo "Original: {$original}\n";

$a = hexdec($original);

echo "As Int: {$a}\n";

$b = dechex($a);

echo "Back to Hex: {$b}\n";

will output

Original: ba3035a23b64558d
As Dec: 1.3416282260834E+19
Back to Hex: ba3035a23b645800

I have had to fall back to the zipkin exporter, even though I am pushing to Jaeger, as it uses strings for the ID and doesn't rely on the hex conversion

@chingor13
Copy link
Member

This would need to be fixed upstream in the Thrift PHP package as it expects integer values to TProtocol#writeI64($int).

@lookaflyingdonkey
Copy link
Author

Agreed, but how is anyone using the jaeger exporter at all (or Thrift PHP for that matter)? Are people just finding a way to limit Int64 generation in PHP to below the cutoff?

@chingor13
Copy link
Member

The core opencensus-php library only generates trace and span ids with 32 bits of randomness. For a standalone PHP application, it should be good enough. If it's in a SOA where the trace id is propagated and the trace id generated is 64-bit, then this will not work unless we can fix Thrift upstream (or switch Thrift implementations if there is one).

@lookaflyingdonkey
Copy link
Author

So from my example above the 64 bit int should be 13416282260834112909, which PHP cannot handle.

We can use BC Math to get the true value as a string.

function bchexdec($hex) {
$dec = 0;
$len = strlen($hex);
for ($i = 1; $i <= $len; $i++) {
$dec = bcadd($dec, bcmul(strval(hexdec($hex[$i - 1])), bcpow('16', strval($len - $i))));
}
return $dec;
}

That would return the correct value for my example id.

But it doesn't look like the Thrift library can support a string representing an I64.

I guess I could modify my upstream id generators to only use 32 bit ids, but as many are starting to use 128 bit IDs, would be good to get a fix.

@dragoscirjan
Copy link
Contributor

@chingor13 I think this should be fixed by my PR

@chingor13
Copy link
Member

Fixed in #8

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants