Graph: Fixed histogram bucket calculations to avoid missing buckets (#27883)

* Graph: Fixed histogram bucket calculations to avoid missing buckets

* Removed testdata

* Updated tests

public/app/plugins/panel/graph/histogram.ts

@@ -1,5 +1,6 @@
 import _ from 'lodash';
 import TimeSeries from 'app/core/time_series2';
+import { histogram } from 'd3';
 
 /**
  * Convert series into array of series values.
@@ -30,33 +31,16 @@ export function getSeriesValues(dataList: TimeSeries[]): number[] {
  * @param bucketSize
  */
 export function convertValuesToHistogram(values: number[], bucketSize: number, min: number, max: number): any[] {
-  const histogram: any = {};
-
   const minBound = getBucketBound(min, bucketSize);
   const maxBound = getBucketBound(max, bucketSize);
-  let bound = minBound;
-  let n = 0;
-  while (bound <= maxBound) {
-    histogram[bound] = 0;
-    bound = minBound + bucketSize * n;
-    n++;
-  }
 
-  for (let i = 0; i < values.length; i++) {
-    // filter out values outside the min and max boundaries
-    if (values[i] < min || values[i] > max) {
-      continue;
-    }
-    const bound = getBucketBound(values[i], bucketSize);
-    histogram[bound] = histogram[bound] + 1;
-  }
+  const histGenerator = histogram()
+    .domain([minBound, maxBound])
+    .thresholds(Math.round(max - min) / bucketSize);
 
-  const histogamSeries = _.map(histogram, (count, bound) => {
-    return [Number(bound), count];
+  return histGenerator(values).map(bin => {
+    return [bin.x0, bin.length];
   });
-
-  // Sort by Y axis values
-  return _.sortBy(histogamSeries, point => point[0]);
 }
 
 /**

public/app/plugins/panel/graph/specs/graph.test.ts

@@ -636,7 +636,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -672,7 +672,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -708,7 +708,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(250);
     });
   });
 
@@ -744,7 +744,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(250);
     });
   });
 
@@ -854,7 +854,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(250);
     });
   });
 
@@ -891,7 +891,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(250);
     });
   });
 
@@ -966,7 +966,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1003,7 +1003,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1040,7 +1040,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1077,7 +1077,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1114,7 +1114,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1150,7 +1150,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1186,7 +1186,7 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(300);
+      ).toBe(280);
     });
   });
 
@@ -1216,13 +1216,13 @@ describe('grafanaGraph', () => {
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(100);
+      ).toBe(90);
       expect(
         Math.max.apply(
           Math,
           nonZero.map((t: number[]) => t[0])
         )
-      ).toBe(100);
+      ).toBe(90);
     });
   });
 
@@ -1255,7 +1255,7 @@ describe('grafanaGraph', () => {
     beforeEach(() => {
       setupCtx(() => {
         ctrl.panel.xaxis.mode = 'histogram';
-        ctrl.panel.xaxis.max = 301;
+        ctrl.panel.xaxis.max = 400;
         ctrl.panel.stack = false;
         ctrl.hiddenSeries = {};
         ctx.data[0] = new TimeSeries({

public/app/plugins/panel/graph/specs/histogram.test.ts

@@ -6,7 +6,7 @@ describe('Graph Histogam Converter', () => {
     let bucketSize = 10;
 
     beforeEach(() => {
-      values = [1, 2, 10, 11, 17, 20, 29];
+      values = [29, 1, 2, 10, 11, 17, 20];
     });
 
     it('Should convert to series-like array', () => {
@@ -17,7 +17,7 @@ describe('Graph Histogam Converter', () => {
         [20, 2],
       ];
 
-      const histogram = convertValuesToHistogram(values, bucketSize, 1, 29);
+      const histogram = convertValuesToHistogram(values, bucketSize, 1, 30);
       expect(histogram).toMatchObject(expected);
     });
 
@@ -32,7 +32,16 @@ describe('Graph Histogam Converter', () => {
         [25, 1],
       ];
 
-      const histogram = convertValuesToHistogram(values, bucketSize, 1, 29);
+      const histogram = convertValuesToHistogram(values, bucketSize, 1, 30);
+      expect(histogram).toMatchObject(expected);
+    });
+  });
+
+  describe('Buckets to have correct decimals', () => {
+    it('Should convert to series-like array', () => {
+      const expected = [[1.7000000000000002, 1]];
+
+      const histogram = convertValuesToHistogram([1.715000033378601], 0.05, 1.7, 1.8);
       expect(histogram).toMatchObject(expected);
     });
   });