search

tryfer / node-tryfer

A Node Zipkin Tracer Library.
Apache License 2.0
44 stars 27 forks source link

lower limit for the upper bound #33

Closed fr0stbyte closed 9 years ago

dustyburwell commented 9 years ago

This seems reasonable given Zipkin is jvm (scala specifically). Actually, it sounds like this would be a major issue resulting in failures for standard installs of Zipkin.

That said the upper bounds are slightly off here. The max long value in Scala is 9223372036854775807L (dec) or 0x7FFF FFFF FFFF FFFF (hex). So we shouldn't limit the lower word in this way, a value of 0x100000000 is fine. For the upper word, the max should be set at 0x80000000 since the Math.random() range is [0, 1) (1 exclusive, i.e. the maximal value would be 0x7FFFFFFF).

Line 29 should therefore read:

var random = new Int64(Math.random() * 0x80000000, Math.random() * 0x100000000);
cyli commented 9 years ago

Apologies, when I moved the repo I forgot I needed to reauthorize travis for this organization, which is why this PR did not build on travis. I'm going to try to trigger a build by closing and reopening this PR.

dustyburwell commented 9 years ago

Looks like the conversation on #34 approves this. So I'll go ahead and merge.

dustyburwell commented 9 years ago

@cyli would you mind bumping the version and deploying a patch release?

cyli commented 9 years ago

@dustyburwell Done