From 7547192fa2665f98b34bf30d89fbf034c1267816 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 31 Aug 2017 15:41:30 -0700 Subject: [PATCH 1/2] fix inclusive-exclusiveness of histogram ToString --- monitoring/histogram.cc | 3 ++- monitoring/histogram_test.cc | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/monitoring/histogram.cc b/monitoring/histogram.cc index b3c01a78e08..d2b7ff8442c 100644 --- a/monitoring/histogram.cc +++ b/monitoring/histogram.cc @@ -209,7 +209,8 @@ std::string HistogramStat::ToString() const { if (bucket_value <= 0.0) continue; cumulative_sum += bucket_value; snprintf(buf, sizeof(buf), - "[ %7" PRIu64 ", %7" PRIu64 " ) %8" PRIu64 " %7.3f%% %7.3f%% ", + "%c %7" PRIu64 ", %7" PRIu64 " ] %8" PRIu64 " %7.3f%% %7.3f%% ", + (b == 0) ? '[' : '(', (b == 0) ? 0 : bucketMapper.BucketLimit(b-1), // left bucketMapper.BucketLimit(b), // right bucket_value, // count diff --git a/monitoring/histogram_test.cc b/monitoring/histogram_test.cc index b4e3c981c8e..950e6c80e76 100644 --- a/monitoring/histogram_test.cc +++ b/monitoring/histogram_test.cc @@ -85,6 +85,15 @@ TEST_F(HistogramTest, BasicOperation) { BasicOperation(histogramWindowing); } +TEST_F(HistogramTest, BoundaryValue) { + HistogramImpl histogram; + // both should be in [0, 1], never (1, 2]. + histogram.Add(0); + histogram.Add(1); + + ASSERT_LE(fabs(histogram.Percentile(50.0) - 0.5), kIota); +} + TEST_F(HistogramTest, MergeHistogram) { HistogramImpl histogram; HistogramImpl other; From a126ec7d703af82af40cf1acf6255d8a3fa698fe Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 31 Aug 2017 17:23:28 -0700 Subject: [PATCH 2/2] improve test case comment --- monitoring/histogram_test.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/monitoring/histogram_test.cc b/monitoring/histogram_test.cc index 950e6c80e76..df58822fc21 100644 --- a/monitoring/histogram_test.cc +++ b/monitoring/histogram_test.cc @@ -87,7 +87,11 @@ TEST_F(HistogramTest, BasicOperation) { TEST_F(HistogramTest, BoundaryValue) { HistogramImpl histogram; - // both should be in [0, 1], never (1, 2]. + // - both should be in [0, 1] bucket because we place values on bucket + // boundaries in the lower bucket. + // - all points are in [0, 1] bucket, so p50 will be 0.5 + // - the test cannot be written with a single point since histogram won't + // report percentiles lower than the min or greater than the max. histogram.Add(0); histogram.Add(1);