Fix overflow bug. (#13)

jaegertracing · Nov 22, 2017 · a1bea40 · a1bea40
1 parent 64f66e3
commit a1bea40
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 2 deletions.
diff --git a/src/jaegertracing/samplers/ProbabilisticSampler.h b/src/jaegertracing/samplers/ProbabilisticSampler.h
@@ -27,8 +27,7 @@ class ProbabilisticSampler : public Sampler {
   public:
     explicit ProbabilisticSampler(double samplingRate)
         : _samplingRate(std::max(0.0, std::min(samplingRate, 1.0)))
-        , _samplingBoundary(
-              static_cast<uint64_t>(kMaxRandomNumber * _samplingRate))
+        , _samplingBoundary(computeSamplingBoundary(_samplingRate))
         , _tags({ { kSamplerTypeTagKey, kSamplerTypeProbabilistic },
                   { kSamplerParamTagKey, _samplingRate } })
     {
@@ -53,6 +52,20 @@ class ProbabilisticSampler : public Sampler {
     double _samplingRate;
     uint64_t _samplingBoundary;
     std::vector<Tag> _tags;
+
+    static uint64_t computeSamplingBoundary(long double samplingRate)
+    {
+        const long double maxRandNumber = kMaxRandomNumber;
+        const auto samplingBoundary = samplingRate * maxRandNumber;
+
+        // Protect against overflow in case samplingBoundary rounds
+        // higher than kMaxRandNumber.
+        if (samplingBoundary == maxRandNumber) {
+            return kMaxRandomNumber;
+        }
+
+        return static_cast<uint64_t>(samplingBoundary);
+    }
 };
 
 }  // namespace samplers

diff --git a/src/jaegertracing/samplers/SamplerTest.cpp b/src/jaegertracing/samplers/SamplerTest.cpp
@@ -105,6 +105,7 @@ TEST(Sampler, testProbabilisticSamplerErrors)
 
 TEST(Sampler, testProbabilisticSampler)
 {
+  {
     ProbabilisticSampler sampler(0.5);
     auto result =
         sampler.isSampled(TraceID(0, kTestMaxID + 10), kTestOperationName);
@@ -114,6 +115,16 @@ TEST(Sampler, testProbabilisticSampler)
     result = sampler.isSampled(TraceID(0, kTestMaxID - 20), kTestOperationName);
     ASSERT_TRUE(result.isSampled());
     CMP_TAGS(testProbablisticExpectedTags, result.tags());
+  }
+  {
+    ProbabilisticSampler sampler(1.0);
+    auto result =
+        sampler.isSampled(TraceID(0, kTestMaxID), kTestOperationName);
+    ASSERT_TRUE(result.isSampled());
+
+    result = sampler.isSampled(TraceID(0, kTestMaxID - 20), kTestOperationName);
+    ASSERT_TRUE(result.isSampled());
+  }
 }
 
 TEST(Sampler, testProbabilisticSamplerPerformance)