Overflow bug in ProbabilisticSampler.

[rnburn]

The ProbabilisticSampler uses the following logic to determine a sampling bound:

    explicit ProbabilisticSampler(double samplingRate)
        : _samplingRate(std::max(0.0, std::min(samplingRate, 1.0)))
        , _samplingBoundary(
              static_cast<uint64_t>(kMaxRandomNumber * _samplingRate))
        , _tags({ { kSamplerTypeTagKey, kSamplerTypeProbabilistic },
                  { kSamplerParamTagKey, _samplingRate } })
    {
    }

kMaxRandomNumber * _samplingRate gets converted to a double which rounds to a higher value than kMaxRandomNumber so that when it's casted back to a uint64_t it overflows to zero and nothing is sampled.

This short program will demonstrate the problem

#include <limits>
#include <cstddef>
#include <iostream>

int main() {
  constexpr auto max = std::numeric_limits<uint64_t>::max();
  std::cout << max << "\n";
  auto t = max*1.0;
  std::cout << t << "\n";
  auto x = static_cast<uint64_t>(t);
  std::cout << x << "\n";
  return 0;
}

It outputs

18446744073709551615
1.84467e+19
0

[yurishkuro]

Thanks for reporting. cc @isaachier

[isaachier]

Interesting. Will submit fix. Thanks!

[isaachier]

@rnburn let me know what you think of @black-adder's logic here: jaegertracing/jaeger-client-go#229.

[rnburn]

Not sure why go only uses 63-bit trace ids, but #13 gives you a way to fix the bug while still keeping the same type of sampling logic.

[yurishkuro]

Not sure why go only uses 63-bit trace ids

because that's what Go stdlib provided in the past, Int63 . I can see that Uint64 was added later, not sure from which version.