From 3184942aebf4b86c0c215b9dbaed2d36ed5a7df0 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Tue, 10 Oct 2017 15:25:57 +0300 Subject: [PATCH] Fix NaN handling (#9469) * graph: fix NaN formatting, #9012 * singlestat: prevent null value coloring, #9012, #8404 * timeseries: add tests for #9012 and move test file to TS --- public/app/core/specs/time_series_specs.ts | 299 ++++++++++++++++++ public/app/core/time_series2.ts | 5 +- public/app/plugins/panel/singlestat/module.ts | 5 +- public/test/core/time_series_specs.js | 278 ---------------- 4 files changed, 307 insertions(+), 280 deletions(-) create mode 100644 public/app/core/specs/time_series_specs.ts delete mode 100644 public/test/core/time_series_specs.js diff --git a/public/app/core/specs/time_series_specs.ts b/public/app/core/specs/time_series_specs.ts new file mode 100644 index 00000000000..ddf08a9f704 --- /dev/null +++ b/public/app/core/specs/time_series_specs.ts @@ -0,0 +1,299 @@ +import {describe, beforeEach, it, expect} from 'test/lib/common'; +import TimeSeries from 'app/core/time_series2'; + +describe("TimeSeries", function() { + var points, series; + var yAxisFormats = ['short', 'ms']; + var testData; + + beforeEach(function() { + testData = { + alias: 'test', + datapoints: [ + [1,2],[null,3],[10,4],[8,5] + ] + }; + }); + + describe('when getting flot pairs', function() { + it('with connected style, should ignore nulls', function() { + series = new TimeSeries(testData); + points = series.getFlotPairs('connected', yAxisFormats); + expect(points.length).to.be(3); + }); + + it('with null as zero style, should replace nulls with zero', function() { + series = new TimeSeries(testData); + points = series.getFlotPairs('null as zero', yAxisFormats); + expect(points.length).to.be(4); + expect(points[1][1]).to.be(0); + }); + + it('if last is null current should pick next to last', function() { + series = new TimeSeries({ + datapoints: [[10,1], [null, 2]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.current).to.be(10); + }); + + it('max value should work for negative values', function() { + series = new TimeSeries({ + datapoints: [[-10,1], [-4, 2]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.max).to.be(-4); + }); + + it('average value should ignore nulls', function() { + series = new TimeSeries(testData); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.avg).to.be(6.333333333333333); + }); + + it('the delta value should account for nulls', function() { + series = new TimeSeries({ + datapoints: [[1,2],[3,3],[null,4],[10,5],[15,6]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.delta).to.be(14); + }); + + it('the delta value should account for nulls on first', function() { + series = new TimeSeries({ + datapoints: [[null,2],[1,3],[10,4],[15,5]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.delta).to.be(14); + }); + + it('the delta value should account for nulls on last', function() { + series = new TimeSeries({ + datapoints: [[1,2],[5,3],[10,4],[null,5]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.delta).to.be(9); + }); + + it('the delta value should account for resets', function() { + series = new TimeSeries({ + datapoints: [[1,2],[5,3],[10,4],[0,5],[10,6]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.delta).to.be(19); + }); + + it('the delta value should account for resets on last', function() { + series = new TimeSeries({ + datapoints: [[1,2],[2,3],[10,4],[8,5]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.delta).to.be(17); + }); + + it('the range value should be max - min', function() { + series = new TimeSeries(testData); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.range).to.be(9); + }); + + it('first value should ingone nulls', function() { + series = new TimeSeries(testData); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.first).to.be(1); + series = new TimeSeries({ + datapoints: [[null,2],[1,3],[10,4],[8,5]] + }); + series.getFlotPairs('null', yAxisFormats); + expect(series.stats.first).to.be(1); + }); + + it('with null as zero style, average value should treat nulls as 0', function() { + series = new TimeSeries(testData); + series.getFlotPairs('null as zero', yAxisFormats); + expect(series.stats.avg).to.be(4.75); + }); + + it('average value should be null if all values is null', function() { + series = new TimeSeries({ + datapoints: [[null,2],[null,3],[null,4],[null,5]] + }); + series.getFlotPairs('null'); + expect(series.stats.avg).to.be(null); + }); + }); + + describe('When checking if ms resolution is needed', function() { + describe('msResolution with second resolution timestamps', function() { + beforeEach(function() { + series = new TimeSeries({datapoints: [[45, 1234567890], [60, 1234567899]]}); + }); + + it('should set hasMsResolution to false', function() { + expect(series.hasMsResolution).to.be(false); + }); + }); + + describe('msResolution with millisecond resolution timestamps', function() { + beforeEach(function() { + series = new TimeSeries({datapoints: [[55, 1236547890001], [90, 1234456709000]]}); + }); + + it('should show millisecond resolution tooltip', function() { + expect(series.hasMsResolution).to.be(true); + }); + }); + + describe('msResolution with millisecond resolution timestamps but with trailing zeroes', function() { + beforeEach(function() { + series = new TimeSeries({datapoints: [[45, 1234567890000], [60, 1234567899000]]}); + }); + + it('should not show millisecond resolution tooltip', function() { + expect(series.hasMsResolution).to.be(false); + }); + }); + }); + + describe('can detect if series contains ms precision', function() { + var fakedata; + + beforeEach(function() { + fakedata = testData; + }); + + it('missing datapoint with ms precision', function() { + fakedata.datapoints[0] = [1337, 1234567890000]; + series = new TimeSeries(fakedata); + expect(series.isMsResolutionNeeded()).to.be(false); + }); + + it('contains datapoint with ms precision', function() { + fakedata.datapoints[0] = [1337, 1236547890001]; + series = new TimeSeries(fakedata); + expect(series.isMsResolutionNeeded()).to.be(true); + }); + }); + + describe('series overrides', function() { + var series; + beforeEach(function() { + series = new TimeSeries(testData); + }); + + describe('fill & points', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', fill: 0, points: true }]); + }); + + it('should set fill zero, and enable points', function() { + expect(series.lines.fill).to.be(0.001); + expect(series.points.show).to.be(true); + }); + }); + + describe('series option overrides, bars, true & lines false', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', bars: true, lines: false }]); + }); + + it('should disable lines, and enable bars', function() { + expect(series.lines.show).to.be(false); + expect(series.bars.show).to.be(true); + }); + }); + + describe('series option overrides, linewidth, stack', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', linewidth: 5, stack: false }]); + }); + + it('should disable stack, and set lineWidth', function() { + expect(series.stack).to.be(false); + expect(series.lines.lineWidth).to.be(5); + }); + }); + + describe('series option overrides, dashes and lineWidth', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', linewidth: 5, dashes: true }]); + }); + + it('should enable dashes, set dashes lineWidth to 5 and lines lineWidth to 0', function() { + expect(series.dashes.show).to.be(true); + expect(series.dashes.lineWidth).to.be(5); + expect(series.lines.lineWidth).to.be(0); + }); + }); + + describe('series option overrides, fill below to', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', fillBelowTo: 'min' }]); + }); + + it('should disable line fill and add fillBelowTo', function() { + expect(series.fillBelowTo).to.be('min'); + }); + }); + + describe('series option overrides, pointradius, steppedLine', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', pointradius: 5, steppedLine: true }]); + }); + + it('should set pointradius, and set steppedLine', function() { + expect(series.points.radius).to.be(5); + expect(series.lines.steps).to.be(true); + }); + }); + + describe('override match on regex', function() { + beforeEach(function() { + series.alias = 'test_01'; + series.applySeriesOverrides([{ alias: '/.*01/', lines: false }]); + }); + + it('should match second series', function() { + expect(series.lines.show).to.be(false); + }); + }); + + describe('override series y-axis, and z-index', function() { + beforeEach(function() { + series.alias = 'test'; + series.applySeriesOverrides([{ alias: 'test', yaxis: 2, zindex: 2 }]); + }); + + it('should set yaxis', function() { + expect(series.yaxis).to.be(2); + }); + + it('should set zindex', function() { + expect(series.zindex).to.be(2); + }); + }); + + }); + + describe('value formatter', function() { + var series; + beforeEach(function() { + series = new TimeSeries(testData); + }); + + it('should format non-numeric values as empty string', function() { + expect(series.formatValue(null)).to.be(""); + expect(series.formatValue(undefined)).to.be(""); + expect(series.formatValue(NaN)).to.be(""); + expect(series.formatValue(Infinity)).to.be(""); + expect(series.formatValue(-Infinity)).to.be(""); + }); + }); + +}); diff --git a/public/app/core/time_series2.ts b/public/app/core/time_series2.ts index c8ab8d9efcf..1c4c94da6fb 100644 --- a/public/app/core/time_series2.ts +++ b/public/app/core/time_series2.ts @@ -203,7 +203,7 @@ export default class TimeSeries { if (this.stats.max === -Number.MAX_VALUE) { this.stats.max = null; } if (this.stats.min === Number.MAX_VALUE) { this.stats.min = null; } - if (result.length) { + if (result.length && !this.allIsNull) { this.stats.avg = (this.stats.total / nonNulls); this.stats.current = result[result.length-1][1]; if (this.stats.current === null && result.length > 1) { @@ -228,6 +228,9 @@ export default class TimeSeries { } formatValue(value) { + if (!_.isFinite(value)) { + value = null; // Prevent NaN formatting + } return this.valueFormater(value, this.decimals, this.scaledDecimals); } diff --git a/public/app/plugins/panel/singlestat/module.ts b/public/app/plugins/panel/singlestat/module.ts index 92d03562557..744659e5134 100644 --- a/public/app/plugins/panel/singlestat/module.ts +++ b/public/app/plugins/panel/singlestat/module.ts @@ -606,7 +606,7 @@ class SingleStatCtrl extends MetricsPanelCtrl { var body = panel.gauge.show ? '' : getBigValueHtml(); - if (panel.colorBackground && !isNaN(data.value)) { + if (panel.colorBackground) { var color = getColorForValue(data, data.value); if (color) { $panelContainer.css('background-color', color); @@ -690,6 +690,9 @@ class SingleStatCtrl extends MetricsPanelCtrl { } function getColorForValue(data, value) { + if (!_.isFinite(value)) { + return null; + } for (var i = data.thresholds.length; i > 0; i--) { if (value >= data.thresholds[i-1]) { return data.colorMap[i]; diff --git a/public/test/core/time_series_specs.js b/public/test/core/time_series_specs.js deleted file mode 100644 index 08434e58b81..00000000000 --- a/public/test/core/time_series_specs.js +++ /dev/null @@ -1,278 +0,0 @@ -define([ - 'app/core/time_series' -], function(TimeSeries) { - 'use strict'; - - describe("TimeSeries", function() { - var points, series; - var yAxisFormats = ['short', 'ms']; - var testData; - - beforeEach(function() { - testData = { - alias: 'test', - datapoints: [ - [1,2],[null,3],[10,4],[8,5] - ] - }; - }); - - describe('when getting flot pairs', function() { - it('with connected style, should ignore nulls', function() { - series = new TimeSeries(testData); - points = series.getFlotPairs('connected', yAxisFormats); - expect(points.length).to.be(3); - }); - - it('with null as zero style, should replace nulls with zero', function() { - series = new TimeSeries(testData); - points = series.getFlotPairs('null as zero', yAxisFormats); - expect(points.length).to.be(4); - expect(points[1][1]).to.be(0); - }); - - it('if last is null current should pick next to last', function() { - series = new TimeSeries({ - datapoints: [[10,1], [null, 2]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.current).to.be(10); - }); - - it('max value should work for negative values', function() { - series = new TimeSeries({ - datapoints: [[-10,1], [-4, 2]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.max).to.be(-4); - }); - - it('average value should ignore nulls', function() { - series = new TimeSeries(testData); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.avg).to.be(6.333333333333333); - }); - - it('the delta value should account for nulls', function() { - series = new TimeSeries({ - datapoints: [[1,2],[3,3],[null,4],[10,5],[15,6]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.delta).to.be(14); - }); - - it('the delta value should account for nulls on first', function() { - series = new TimeSeries({ - datapoints: [[null,2],[1,3],[10,4],[15,5]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.delta).to.be(14); - }); - - it('the delta value should account for nulls on last', function() { - series = new TimeSeries({ - datapoints: [[1,2],[5,3],[10,4],[null,5]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.delta).to.be(9); - }); - - it('the delta value should account for resets', function() { - series = new TimeSeries({ - datapoints: [[1,2],[5,3],[10,4],[0,5],[10,6]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.delta).to.be(19); - }); - - it('the delta value should account for resets on last', function() { - series = new TimeSeries({ - datapoints: [[1,2],[2,3],[10,4],[8,5]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.delta).to.be(17); - }); - - it('the range value should be max - min', function() { - series = new TimeSeries(testData); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.range).to.be(9); - }); - - it('first value should ingone nulls', function() { - series = new TimeSeries(testData); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.first).to.be(1); - series = new TimeSeries({ - datapoints: [[null,2],[1,3],[10,4],[8,5]] - }); - series.getFlotPairs('null', yAxisFormats); - expect(series.stats.first).to.be(1); - }); - - it('with null as zero style, average value should treat nulls as 0', function() { - series = new TimeSeries(testData); - series.getFlotPairs('null as zero', yAxisFormats); - expect(series.stats.avg).to.be(4.75); - }); - }); - - describe('When checking if ms resolution is needed', function() { - describe('msResolution with second resolution timestamps', function() { - beforeEach(function() { - series = new TimeSeries({datapoints: [[45, 1234567890], [60, 1234567899]]}); - }); - - it('should set hasMsResolution to false', function() { - expect(series.hasMsResolution).to.be(false); - }); - }); - - describe('msResolution with millisecond resolution timestamps', function() { - beforeEach(function() { - series = new TimeSeries({datapoints: [[55, 1236547890001], [90, 1234456709000]]}); - }); - - it('should show millisecond resolution tooltip', function() { - expect(series.hasMsResolution).to.be(true); - }); - }); - - describe('msResolution with millisecond resolution timestamps but with trailing zeroes', function() { - beforeEach(function() { - series = new TimeSeries({datapoints: [[45, 1234567890000], [60, 1234567899000]]}); - }); - - it('should not show millisecond resolution tooltip', function() { - expect(series.hasMsResolution).to.be(false); - }); - }); - }); - - describe('can detect if series contains ms precision', function() { - var fakedata; - - beforeEach(function() { - fakedata = testData; - }); - - it('missing datapoint with ms precision', function() { - fakedata.datapoints[0] = [1337, 1234567890000]; - series = new TimeSeries(fakedata); - expect(series.isMsResolutionNeeded()).to.be(false); - }); - - it('contains datapoint with ms precision', function() { - fakedata.datapoints[0] = [1337, 1236547890001]; - series = new TimeSeries(fakedata); - expect(series.isMsResolutionNeeded()).to.be(true); - }); - }); - - describe('series overrides', function() { - var series; - beforeEach(function() { - series = new TimeSeries(testData); - }); - - describe('fill & points', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', fill: 0, points: true }]); - }); - - it('should set fill zero, and enable points', function() { - expect(series.lines.fill).to.be(0.001); - expect(series.points.show).to.be(true); - }); - }); - - describe('series option overrides, bars, true & lines false', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', bars: true, lines: false }]); - }); - - it('should disable lines, and enable bars', function() { - expect(series.lines.show).to.be(false); - expect(series.bars.show).to.be(true); - }); - }); - - describe('series option overrides, linewidth, stack', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', linewidth: 5, stack: false }]); - }); - - it('should disable stack, and set lineWidth', function() { - expect(series.stack).to.be(false); - expect(series.lines.lineWidth).to.be(5); - }); - }); - - describe('series option overrides, dashes and lineWidth', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', linewidth: 5, dashes: true }]); - }); - - it('should enable dashes, set dashes lineWidth to 5 and lines lineWidth to 0', function() { - expect(series.dashes.show).to.be(true); - expect(series.dashes.lineWidth).to.be(5); - expect(series.lines.lineWidth).to.be(0); - }); - }); - - describe('series option overrides, fill below to', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', fillBelowTo: 'min' }]); - }); - - it('should disable line fill and add fillBelowTo', function() { - expect(series.fillBelowTo).to.be('min'); - }); - }); - - describe('series option overrides, pointradius, steppedLine', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', pointradius: 5, steppedLine: true }]); - }); - - it('should set pointradius, and set steppedLine', function() { - expect(series.points.radius).to.be(5); - expect(series.lines.steps).to.be(true); - }); - }); - - describe('override match on regex', function() { - beforeEach(function() { - series.alias = 'test_01'; - series.applySeriesOverrides([{ alias: '/.*01/', lines: false }]); - }); - - it('should match second series', function() { - expect(series.lines.show).to.be(false); - }); - }); - - describe('override series y-axis, and z-index', function() { - beforeEach(function() { - series.alias = 'test'; - series.applySeriesOverrides([{ alias: 'test', yaxis: 2, zindex: 2 }]); - }); - - it('should set yaxis', function() { - expect(series.yaxis).to.be(2); - }); - - it('should set zindex', function() { - expect(series.zindex).to.be(2); - }); - }); - - }); - }); -});