From 06210a4a22fcc84b09be2f71d8fd27ce41285c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 12 Aug 2016 10:32:46 +0200 Subject: [PATCH] feat(thresholds): refactoring and adding many unit tests --- CHANGELOG.md | 2 +- .../app/features/alerting/alert_tab_ctrl.ts | 7 +- public/app/plugins/panel/graph/graph.js | 91 +------------- .../plugins/panel/graph/specs/graph_specs.ts | 31 ----- .../graph/specs/threshold_manager_specs.ts | 115 ++++++++++++++++++ .../{thresholds.ts => threshold_manager.ts} | 81 +++++++++++- 6 files changed, 205 insertions(+), 122 deletions(-) create mode 100644 public/app/plugins/panel/graph/specs/threshold_manager_specs.ts rename public/app/plugins/panel/graph/{thresholds.ts => threshold_manager.ts} (63%) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2461e1efd2..d6d3653bcaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ * **Templating**: Update panel repeats for variables that change on time refresh, closes [#5021](https://github.com/grafana/grafana/issues/5021) * **Elasticsearch**: Support to set Precision Threshold for Unique Count metric, closes [#4689](https://github.com/grafana/grafana/issues/4689) -#b 3.1.1 (unreleased / v3.1.x branch) +# 3.1.1 (unreleased / v3.1.x branch) * **IFrame embedding**: Fixed issue of using full iframe height, fixes [#5605](https://github.com/grafana/grafana/issues/5606) * **Panel PNG rendering**: Fixed issue detecting render completion, fixes [#5605](https://github.com/grafana/grafana/issues/5606) * **Elasticsearch**: Fixed issue with templating query and json parse error, fixes [#5615](https://github.com/grafana/grafana/issues/5615) diff --git a/public/app/features/alerting/alert_tab_ctrl.ts b/public/app/features/alerting/alert_tab_ctrl.ts index 25ea009f80f..c02b4597ae6 100644 --- a/public/app/features/alerting/alert_tab_ctrl.ts +++ b/public/app/features/alerting/alert_tab_ctrl.ts @@ -129,7 +129,8 @@ export class AlertTabCtrl { graphThresholdChanged(evt) { for (var condition of this.alert.conditions) { if (condition.type === 'query') { - condition.evaluator.params[0] = evt.threshold.value; + condition.evaluator.params[evt.handleIndex] = evt.threshold.value; + this.evaluatorParamsChanged(); break; } } @@ -140,7 +141,7 @@ export class AlertTabCtrl { type: 'query', query: {params: ['A', '5m', 'now']}, reducer: {type: 'avg', params: []}, - evaluator: {type: '>', params: [null]}, + evaluator: {type: 'gt', params: [null]}, }; } @@ -171,7 +172,7 @@ export class AlertTabCtrl { } delete() { - this.alert.enabled = false; + this.panel.alert = {enabled: false}; this.initModel(); } diff --git a/public/app/plugins/panel/graph/graph.js b/public/app/plugins/panel/graph/graph.js index bce8058ba4f..c71321f4478 100755 --- a/public/app/plugins/panel/graph/graph.js +++ b/public/app/plugins/panel/graph/graph.js @@ -5,7 +5,7 @@ define([ 'lodash', 'app/core/utils/kbn', './graph_tooltip', - './thresholds', + './threshold_manager', 'jquery.flot', 'jquery.flot.selection', 'jquery.flot.time', @@ -15,15 +15,12 @@ define([ 'jquery.flot.crosshair', './jquery.flot.events', ], -function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { +function (angular, $, moment, _, kbn, GraphTooltip, thresholdManExports) { 'use strict'; var module = angular.module('grafana.directives'); var labelWidthCache = {}; - // systemjs export - var ThresholdControls = thresholds.ThresholdControls; - module.directive('grafanaGraph', function($rootScope, timeSrv) { return { restrict: 'A', @@ -37,7 +34,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { var legendSideLastValue = null; var rootScope = scope.$root; var panelWidth = 0; - var thresholdControls = new ThresholdControls(ctrl); + var thresholdManager = new thresholdManExports.ThresholdManager(ctrl); rootScope.onAppEvent('setCrosshair', function(event, info) { // do not need to to this if event is from this panel @@ -161,7 +158,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { rightLabel[0].style.marginTop = (getLabelWidth(panel.yaxes[1].label, rightLabel) / 2) + 'px'; } - thresholdControls.draw(plot); + thresholdManager.draw(plot); } function processOffsetHook(plot, gridMargin) { @@ -180,7 +177,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { } // give space to alert editing - thresholdControls.prepare(elem); + thresholdManager.prepare(elem); var stack = panel.stack ? true : null; @@ -252,7 +249,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { } addTimeAxis(options); - addGridThresholds(options, panel); + thresholdManager.addPlotOptions(options, panel); addAnnotations(options); configureAxisOptions(data, options); @@ -315,82 +312,6 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) { }; } - function addGridThresholds(options, panel) { - if (!panel.thresholds || panel.thresholds.length === 0) { - return; - } - - var gtLimit = Infinity; - var ltLimit = -Infinity; - var i, threshold, other; - - for (i = 0; i < panel.thresholds.length; i++) { - threshold = panel.thresholds[i]; - if (!_.isNumber(threshold.value)) { - continue; - } - - var limit; - switch(threshold.op) { - case 'gt': { - limit = gtLimit; - // if next threshold is less then op and greater value, then use that as limit - if (panel.thresholds.length > i+1) { - other = panel.thresholds[i+1]; - if (other.value > threshold.value) { - limit = other.value; - ltLimit = limit; - } - } - break; - } - case 'lt': { - limit = ltLimit; - // if next threshold is less then op and greater value, then use that as limit - if (panel.thresholds.length > i+1) { - other = panel.thresholds[i+1]; - if (other.value < threshold.value) { - limit = other.value; - gtLimit = limit; - } - } - break; - } - } - - switch(threshold.colorMode) { - case 'critical': { - threshold.fillColor = 'rgba(234, 112, 112, 0.12)'; - threshold.lineColor = 'rgba(237, 46, 24, 0.60)'; - break; - } - case 'warning': { - threshold.fillColor = 'rgba(235, 138, 14, 0.12)'; - threshold.lineColor = 'rgba(247, 149, 32, 0.60)'; - break; - } - case 'ok': { - threshold.fillColor = 'rgba(11, 237, 50, 0.090)'; - threshold.lineColor = 'rgba(6,163,69, 0.60)'; - break; - } - case 'custom': { - threshold.fillColor = threshold.fillColor; - threshold.lineColor = threshold.lineColor; - break; - } - } - - // fill - if (threshold.fill) { - options.grid.markings.push({yaxis: {from: threshold.value, to: limit}, color: threshold.fillColor}); - } - if (threshold.line) { - options.grid.markings.push({yaxis: {from: threshold.value, to: threshold.value}, color: threshold.lineColor}); - } - } - } - function addAnnotations(options) { if(!annotations || annotations.length === 0) { return; diff --git a/public/app/plugins/panel/graph/specs/graph_specs.ts b/public/app/plugins/panel/graph/specs/graph_specs.ts index 7a0fcd6305c..633832d4866 100644 --- a/public/app/plugins/panel/graph/specs/graph_specs.ts +++ b/public/app/plugins/panel/graph/specs/graph_specs.ts @@ -112,37 +112,6 @@ describe('grafanaGraph', function() { }); }); - graphScenario('grid thresholds 100, 200', function(ctx) { - ctx.setup(function(ctrl) { - ctrl.panel.thresholds = [ - {op: "gt", value: 300, fillColor: 'red', lineColor: 'blue', fill: true, line: true}, - {op: "gt", value: 200, fillColor: '#ed2e18', fill: true} - ]; - }); - - it('should add fill for threshold with fill: true', function() { - var markings = ctx.plotOptions.grid.markings; - - expect(markings[0].yaxis.from).to.be(300); - expect(markings[0].yaxis.to).to.be(Infinity); - expect(markings[0].color).to.be('red'); - }); - - it('should add line', function() { - var markings = ctx.plotOptions.grid.markings; - - expect(markings[1].yaxis.from).to.be(300); - expect(markings[1].yaxis.to).to.be(300); - expect(markings[1].color).to.be('blue'); - }); - - it('should add fill for second thresholds to previous threshold', function() { - var markings = ctx.plotOptions.grid.markings; - expect(markings[2].yaxis.from).to.be(200); - expect(markings[2].yaxis.to).to.be(300); - }); - }); - graphScenario('when logBase is log 10', function(ctx) { ctx.setup(function(ctrl) { ctrl.panel.yaxes[0].logBase = 10; diff --git a/public/app/plugins/panel/graph/specs/threshold_manager_specs.ts b/public/app/plugins/panel/graph/specs/threshold_manager_specs.ts new file mode 100644 index 00000000000..de598ea73ed --- /dev/null +++ b/public/app/plugins/panel/graph/specs/threshold_manager_specs.ts @@ -0,0 +1,115 @@ +/// + +import {describe, beforeEach, it, sinon, expect, angularMocks} from '../../../../../test/lib/common'; + +import {ThresholdManager} from '../threshold_manager'; + +describe('ThresholdManager', function() { + + function plotOptionsScenario(desc, func) { + describe(desc, function() { + var ctx: any = { + panel: { + thresholds: [], + }, + options: { + grid: {markings: []}, + }, + panelCtrl: {}, + }; + + ctx.setup = function(thresholds) { + ctx.panel.thresholds = thresholds; + var manager = new ThresholdManager(ctx.panelCtrl); + manager.addPlotOptions(ctx.options, ctx.panel); + }; + + func(ctx); + }); + } + + describe("When creating plot markings", () => { + + plotOptionsScenario("for simple gt threshold", ctx => { + ctx.setup([ + {op: 'gt', value: 300, fill: true, line: true, colorMode: 'critical'}, + ]); + + it('should add fill for threshold with fill: true', function() { + var markings = ctx.options.grid.markings; + + expect(markings[0].yaxis.from).to.be(300); + expect(markings[0].yaxis.to).to.be(Infinity); + expect(markings[0].color).to.be('rgba(234, 112, 112, 0.12)'); + }); + + it('should add line', function() { + var markings = ctx.options.grid.markings; + expect(markings[1].yaxis.from).to.be(300); + expect(markings[1].yaxis.to).to.be(300); + expect(markings[1].color).to.be('rgba(237, 46, 24, 0.60)'); + }); + }); + + plotOptionsScenario("for two gt thresholds", ctx => { + ctx.setup([ + {op: 'gt', value: 200, fill: true, colorMode: 'warning'}, + {op: 'gt', value: 300, fill: true, colorMode: 'critical'}, + ]); + + it('should add fill for first thresholds to next threshold', function() { + var markings = ctx.options.grid.markings; + expect(markings[0].yaxis.from).to.be(200); + expect(markings[0].yaxis.to).to.be(300); + }); + + it('should add fill for last thresholds to infinity', function() { + var markings = ctx.options.grid.markings; + expect(markings[1].yaxis.from).to.be(300); + expect(markings[1].yaxis.to).to.be(Infinity); + }); + + }); + + plotOptionsScenario("for lt then gt threshold (inside)", ctx => { + ctx.setup([ + {op: 'lt', value: 300, fill: true, colorMode: 'critical'}, + {op: 'gt', value: 200, fill: true, colorMode: 'critical'}, + ]); + + it('should add fill for first thresholds to next threshold', function() { + var markings = ctx.options.grid.markings; + expect(markings[0].yaxis.from).to.be(300); + expect(markings[0].yaxis.to).to.be(200); + }); + + it('should add fill for last thresholds to itself', function() { + var markings = ctx.options.grid.markings; + expect(markings[1].yaxis.from).to.be(200); + expect(markings[1].yaxis.to).to.be(200); + }); + + }); + + plotOptionsScenario("for gt then lt threshold (outside)", ctx => { + ctx.setup([ + {op: 'gt', value: 300, fill: true, colorMode: 'critical'}, + {op: 'lt', value: 200, fill: true, colorMode: 'critical'}, + ]); + + it('should add fill for first thresholds to next threshold', function() { + var markings = ctx.options.grid.markings; + expect(markings[0].yaxis.from).to.be(300); + expect(markings[0].yaxis.to).to.be(Infinity); + }); + + it('should add fill for last thresholds to itself', function() { + var markings = ctx.options.grid.markings; + expect(markings[1].yaxis.from).to.be(200); + expect(markings[1].yaxis.to).to.be(-Infinity); + }); + + }); + + }); +}); diff --git a/public/app/plugins/panel/graph/thresholds.ts b/public/app/plugins/panel/graph/threshold_manager.ts similarity index 63% rename from public/app/plugins/panel/graph/thresholds.ts rename to public/app/plugins/panel/graph/threshold_manager.ts index 267a6754acb..1fbdfa99cc8 100644 --- a/public/app/plugins/panel/graph/thresholds.ts +++ b/public/app/plugins/panel/graph/threshold_manager.ts @@ -4,7 +4,7 @@ import 'jquery.flot'; import $ from 'jquery'; import _ from 'lodash'; -export class ThresholdControls { +export class ThresholdManager { plot: any; placeholder: any; height: any; @@ -69,7 +69,7 @@ export class ThresholdControls { // trigger digest and render panelCtrl.$scope.$apply(function() { panelCtrl.render(); - panelCtrl.events.emit('threshold-changed', {threshold: model, index: handleIndex}); + panelCtrl.events.emit('threshold-changed', {threshold: model, handleIndex: handleIndex}); }); } @@ -145,5 +145,82 @@ export class ThresholdControls { this.placeholder.on('mousedown', '.alert-handle', this.initDragging.bind(this)); this.needsCleanup = true; } + + addPlotOptions(options, panel) { + if (!panel.thresholds || panel.thresholds.length === 0) { + return; + } + + var gtLimit = Infinity; + var ltLimit = -Infinity; + var i, threshold, other; + + for (i = 0; i < panel.thresholds.length; i++) { + threshold = panel.thresholds[i]; + if (!_.isNumber(threshold.value)) { + continue; + } + + var limit; + switch (threshold.op) { + case 'gt': { + limit = gtLimit; + // if next threshold is less then op and greater value, then use that as limit + if (panel.thresholds.length > i+1) { + other = panel.thresholds[i+1]; + if (other.value > threshold.value) { + limit = other.value; + ltLimit = limit; + } + } + break; + } + case 'lt': { + limit = ltLimit; + // if next threshold is less then op and greater value, then use that as limit + if (panel.thresholds.length > i+1) { + other = panel.thresholds[i+1]; + if (other.value < threshold.value) { + limit = other.value; + gtLimit = limit; + } + } + break; + } + } + + switch (threshold.colorMode) { + case 'critical': { + threshold.fillColor = 'rgba(234, 112, 112, 0.12)'; + threshold.lineColor = 'rgba(237, 46, 24, 0.60)'; + break; + } + case 'warning': { + threshold.fillColor = 'rgba(235, 138, 14, 0.12)'; + threshold.lineColor = 'rgba(247, 149, 32, 0.60)'; + break; + } + case 'ok': { + threshold.fillColor = 'rgba(11, 237, 50, 0.090)'; + threshold.lineColor = 'rgba(6,163,69, 0.60)'; + break; + } + case 'custom': { + threshold.fillColor = threshold.fillColor; + threshold.lineColor = threshold.lineColor; + break; + } + } + + // fill + if (threshold.fill) { + options.grid.markings.push({yaxis: {from: threshold.value, to: limit}, color: threshold.fillColor}); + } + if (threshold.line) { + options.grid.markings.push({yaxis: {from: threshold.value, to: threshold.value}, color: threshold.lineColor}); + } + } + } + }