From 40bb1bb363830763f33474a9f576d5a43c69bb5c Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Thu, 29 Nov 2018 19:22:43 +0000 Subject: [PATCH] Add empty state for graphs with no values --- .../monitoring/components/graph.vue | 28 +++++++--- .../monitoring/stores/monitoring_store.js | 34 ++++++++++-- ...ivl-add-empty-state-graphs-null-values.yml | 5 ++ locale/gitlab.pot | 3 ++ spec/javascripts/monitoring/graph_spec.js | 20 +++++++ spec/javascripts/monitoring/mock_data.js | 53 ++++++++++++++++++- .../monitoring/monitoring_store_spec.js | 32 ++++++----- 7 files changed, 149 insertions(+), 26 deletions(-) create mode 100644 changelogs/unreleased/jivl-add-empty-state-graphs-null-values.yml diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 815063237fc..64a1df80a8e 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -105,6 +105,9 @@ export default { deploymentFlagData() { return this.reducedDeploymentData.find(deployment => deployment.showDeploymentFlag); }, + shouldRenderData() { + return this.graphData.queries.filter(s => s.result.length > 0).length > 0; + }, }, watch: { hoverData() { @@ -120,17 +123,17 @@ export default { }, draw() { const breakpointSize = bp.getBreakpointSize(); - const query = this.graphData.queries[0]; const svgWidth = this.$refs.baseSvg.getBoundingClientRect().width; + this.margin = measurements.large.margin; + if (this.smallGraph || breakpointSize === 'xs' || breakpointSize === 'sm') { this.graphHeight = 300; this.margin = measurements.small.margin; this.measurements = measurements.small; } - this.unitOfDisplay = query.unit || ''; + this.yAxisLabel = this.graphData.y_label || 'Values'; - this.legendTitle = query.label || 'Average'; this.graphWidth = svgWidth - this.margin.left - this.margin.right; this.graphHeight = this.graphHeight - this.margin.top - this.margin.bottom; this.baseGraphHeight = this.graphHeight - 50; @@ -139,8 +142,15 @@ export default { // pixel offsets inside the svg and outside are not 1:1 this.realPixelRatio = svgWidth / this.baseGraphWidth; - this.renderAxesPaths(); - this.formatDeployments(); + // set the legends on the axes + const [query] = this.graphData.queries; + this.legendTitle = query ? query.label : 'Average'; + this.unitOfDisplay = query ? query.unit : ''; + + if (this.shouldRenderData) { + this.renderAxesPaths(); + this.formatDeployments(); + } }, handleMouseOverGraph(e) { let point = this.$refs.graphData.createSVGPoint(); @@ -266,7 +276,7 @@ export default { :y-axis-label="yAxisLabel" :unit-of-display="unitOfDisplay" /> - + + + + {{ s__('Metrics|No data to display') }} + + { + const newTimeSeries = timeSeries; + const hasValue = series => + !Number.isNaN(series.value) && (series.value !== null || series.value !== undefined); + const hasNonNullValue = timeSeries.values.find(hasValue); + + newTimeSeries.values = hasNonNullValue ? newTimeSeries.values : []; + + return newTimeSeries.values.length > 0; + }), + }; +} + +function removeTimeSeriesNoData(queries) { + return queries.reduce((series, query) => series.concat(checkQueryEmptyData(query)), []); +} + function normalizeMetrics(metrics) { - return metrics.map(metric => ({ - ...metric, - queries: metric.queries.map(query => ({ + return metrics.map(metric => { + const queries = metric.queries.map(query => ({ ...query, result: query.result.map(result => ({ ...result, @@ -19,8 +38,13 @@ function normalizeMetrics(metrics) { value: Number(value), })), })), - })), - })); + })); + + return { + ...metric, + queries: removeTimeSeriesNoData(queries), + }; + }); } export default class MonitoringStore { diff --git a/changelogs/unreleased/jivl-add-empty-state-graphs-null-values.yml b/changelogs/unreleased/jivl-add-empty-state-graphs-null-values.yml new file mode 100644 index 00000000000..d21254b16d0 --- /dev/null +++ b/changelogs/unreleased/jivl-add-empty-state-graphs-null-values.yml @@ -0,0 +1,5 @@ +--- +title: Add empty state for graphs with no values +merge_request: 22630 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 076996e2231..dff5b098dd4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4067,6 +4067,9 @@ msgstr "" msgid "Metrics|Learn about environments" msgstr "" +msgid "Metrics|No data to display" +msgstr "" + msgid "Metrics|No deployed environments" msgstr "" diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index 4cc18afdf24..59d6d4f3a7f 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -5,6 +5,7 @@ import { deploymentData, convertDatesMultipleSeries, singleRowMetricsMultipleSeries, + queryWithoutData, } from './mock_data'; const tagsPath = 'http://test.host/frontend-fixtures/environments-project/tags'; @@ -104,4 +105,23 @@ describe('Graph', () => { expect(component.currentData).toBe(component.timeSeries[0].values[10]); }); + + describe('Without data to display', () => { + it('shows a "no data to display" empty state on a graph', done => { + const component = createComponent({ + graphData: queryWithoutData, + deploymentData, + tagsPath, + projectPath, + }); + + Vue.nextTick(() => { + expect( + component.$el.querySelector('.js-no-data-to-display text').textContent.trim(), + ).toEqual('No data to display'); + + done(); + }); + }); + }); }); diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 6c833b17f98..18ad9843d22 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -14,7 +14,7 @@ export const metricsGroupsAPIResponse = { queries: [ { query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20', - y_label: 'Memory', + label: 'Memory', unit: 'MiB', result: [ { @@ -324,12 +324,15 @@ export const metricsGroupsAPIResponse = { ], }, { + id: 6, title: 'CPU usage', weight: 1, queries: [ { query_range: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100', + label: 'Core Usage', + unit: 'Cores', result: [ { metric: {}, @@ -639,6 +642,39 @@ export const metricsGroupsAPIResponse = { }, ], }, + { + group: 'NGINX', + priority: 2, + metrics: [ + { + id: 100, + title: 'Http Error Rate', + weight: 100, + queries: [ + { + query_range: + 'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"nginx-test-8691397-production-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"nginx-test-8691397-production-.*"}[2m])) * 100', + label: '5xx errors', + unit: '%', + result: [ + { + metric: {}, + values: [ + [1495700554.925, NaN], + [1495700614.925, NaN], + [1495700674.925, NaN], + [1495700734.925, NaN], + [1495700794.925, NaN], + [1495700854.925, NaN], + [1495700914.925, NaN], + ], + }, + ], + }, + ], + }, + ], + }, ], last_update: '2017-05-25T13:18:34.949Z', }; @@ -6526,6 +6562,21 @@ export const singleRowMetricsMultipleSeries = [ }, ]; +export const queryWithoutData = { + title: 'HTTP Error rate', + weight: 10, + y_label: 'Http Error Rate', + queries: [ + { + query_range: + 'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"nginx-test-8691397-production-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"nginx-test-8691397-production-.*"}[2m])) * 100', + label: '5xx errors', + unit: '%', + result: [], + }, + ], +}; + export function convertDatesMultipleSeries(multipleSeries) { const convertedMultiple = multipleSeries; multipleSeries.forEach((column, index) => { diff --git a/spec/javascripts/monitoring/monitoring_store_spec.js b/spec/javascripts/monitoring/monitoring_store_spec.js index bf68c911549..d8a980c874d 100644 --- a/spec/javascripts/monitoring/monitoring_store_spec.js +++ b/spec/javascripts/monitoring/monitoring_store_spec.js @@ -1,31 +1,35 @@ import MonitoringStore from '~/monitoring/stores/monitoring_store'; import MonitoringMock, { deploymentData, environmentData } from './mock_data'; -describe('MonitoringStore', function() { - this.store = new MonitoringStore(); - this.store.storeMetrics(MonitoringMock.data); +describe('MonitoringStore', () => { + const store = new MonitoringStore(); + store.storeMetrics(MonitoringMock.data); - it('contains one group that contains two queries sorted by priority', () => { - expect(this.store.groups).toBeDefined(); - expect(this.store.groups.length).toEqual(1); - expect(this.store.groups[0].metrics.length).toEqual(2); + it('contains two groups that contains, one of which has two queries sorted by priority', () => { + expect(store.groups).toBeDefined(); + expect(store.groups.length).toEqual(2); + expect(store.groups[0].metrics.length).toEqual(2); }); it('gets the metrics count for every group', () => { - expect(this.store.getMetricsCount()).toEqual(2); + expect(store.getMetricsCount()).toEqual(3); }); it('contains deployment data', () => { - this.store.storeDeploymentData(deploymentData); + store.storeDeploymentData(deploymentData); - expect(this.store.deploymentData).toBeDefined(); - expect(this.store.deploymentData.length).toEqual(3); - expect(typeof this.store.deploymentData[0]).toEqual('object'); + expect(store.deploymentData).toBeDefined(); + expect(store.deploymentData.length).toEqual(3); + expect(typeof store.deploymentData[0]).toEqual('object'); }); it('only stores environment data that contains deployments', () => { - this.store.storeEnvironmentsData(environmentData); + store.storeEnvironmentsData(environmentData); - expect(this.store.environmentsData.length).toEqual(2); + expect(store.environmentsData.length).toEqual(2); + }); + + it('removes the data if all the values from a query are not defined', () => { + expect(store.groups[1].metrics[0].queries[0].result.length).toEqual(0); }); });