diff options
author | Miguel Rincon <mrincon@gitlab.com> | 2019-09-10 10:39:29 +0200 |
---|---|---|
committer | Miguel Rincon <mrincon@gitlab.com> | 2019-09-12 16:26:47 +0200 |
commit | bb53d262cfb8713543cb6709f43474db2d36f588 (patch) | |
tree | 29ebdd8a2008a3ae27874cf67db6b68823e09cba | |
parent | 660ddf5791b8a3c3fe62d4373503bf1c706d36ff (diff) | |
download | gitlab-ce-5366-display-anomaly-deviation-boundaries-on-dashboard-ce.tar.gz |
Fix code based on reviews:5366-display-anomaly-deviation-boundaries-on-dashboard-ce
- Add extra checks for null in getEarliestDatapoint
- Simplify validator setup for graphData prop
- Remove showBorder
- Add specs for new utils
- Add documentation
-rw-r--r-- | app/assets/javascripts/monitoring/components/charts/anomaly.vue | 100 | ||||
-rw-r--r-- | app/assets/javascripts/monitoring/components/panel_type.vue | 47 | ||||
-rw-r--r-- | app/assets/javascripts/monitoring/utils.js | 43 | ||||
-rw-r--r-- | spec/frontend/monitoring/mock_data.js | 4 | ||||
-rw-r--r-- | spec/javascripts/monitoring/mock_data.js | 4 | ||||
-rw-r--r-- | spec/javascripts/monitoring/utils_spec.js | 26 |
6 files changed, 101 insertions, 123 deletions
diff --git a/app/assets/javascripts/monitoring/components/charts/anomaly.vue b/app/assets/javascripts/monitoring/components/charts/anomaly.vue index 72f5412d456..20c8450f423 100644 --- a/app/assets/javascripts/monitoring/components/charts/anomaly.vue +++ b/app/assets/javascripts/monitoring/components/charts/anomaly.vue @@ -35,7 +35,7 @@ export default { graphData: { type: Object, required: true, - validator: graphDataValidatorForAnomalyValues.bind(null, false), + validator: graphDataValidatorForAnomalyValues, }, containerWidth: { type: Number, @@ -51,11 +51,6 @@ export default { required: false, default: '', }, - showBorder: { - type: Boolean, - required: false, - default: false, - }, singleEmbed: { type: Boolean, required: false, @@ -213,12 +208,10 @@ export default { const handleIcon = this.svgs['scroll-handle']; return handleIcon ? { handleIcon } : {}; }, - isMultiSeries() { - return this.tooltip.content.length > 1; - }, recentDeployments() { return this.deploymentData.reduce((acc, deployment) => { - if (deployment.created_at >= getEarliestDatapoint(this.chartData)) { + const earliestDatapoint = getEarliestDatapoint(this.chartData); + if (earliestDatapoint && deployment.created_at >= getEarliestDatapoint(this.chartData)) { const { id, created_at, sha, ref, tag } = deployment; acc.push({ id, @@ -333,54 +326,49 @@ export default { </script> <template> - <div - class="prometheus-graph col-12" - :class="[showBorder ? 'p-2' : 'p-0', { 'col-lg-6': !singleEmbed }]" - > - <div :class="{ 'prometheus-graph-embed w-100 p-3': showBorder }"> - <div class="prometheus-graph-header"> - <h5 class="prometheus-graph-title js-graph-title">{{ graphData.title }}</h5> - <div class="prometheus-graph-widgets js-graph-widgets"> - <slot></slot> - </div> + <div class="prometheus-graph col-12" :class="{ 'col-lg-6': !singleEmbed }"> + <div class="prometheus-graph-header"> + <h5 class="prometheus-graph-title js-graph-title">{{ graphData.title }}</h5> + <div class="prometheus-graph-widgets js-graph-widgets"> + <slot></slot> </div> - <gl-line-chart - ref="chart" - v-bind="$attrs" - :data="chartData" - :option="chartOptions" - :include-legend-avg-max="false" - :format-tooltip-text="formatTooltipText" - :thresholds="thresholds" - :width="width" - :height="height" - @updated="onChartUpdated" - > - <template v-if="tooltip.isDeployment"> - <template slot="tooltipTitle">{{ __('Deployed') }}</template> - <div slot="tooltipContent" class="d-flex align-items-center"> - <icon name="commit" class="mr-2" /> - <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link> - </div> + </div> + <gl-line-chart + ref="chart" + v-bind="$attrs" + :data="chartData" + :option="chartOptions" + :include-legend-avg-max="false" + :format-tooltip-text="formatTooltipText" + :thresholds="thresholds" + :width="width" + :height="height" + @updated="onChartUpdated" + > + <template v-if="tooltip.isDeployment"> + <template slot="tooltipTitle">{{ __('Deployed') }}</template> + <div slot="tooltipContent" class="d-flex align-items-center"> + <icon name="commit" class="mr-2" /> + <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link> + </div> + </template> + <template v-else> + <template slot="tooltipTitle"> + <div class="text-nowrap">{{ tooltip.title }}</div> </template> - <template v-else> - <template slot="tooltipTitle"> - <div class="text-nowrap">{{ tooltip.title }}</div> - </template> - <template slot="tooltipContent"> - <div - v-for="(content, key) in tooltip.content" - :key="key" - class="d-flex justify-content-between" - > - <gl-chart-series-label :color="isMultiSeries ? content.color : ''">{{ - content.name - }}</gl-chart-series-label> - <div class="prepend-left-32">{{ content.value }}</div> - </div> - </template> + <template slot="tooltipContent"> + <div + v-for="(seriesLabel, key) in tooltip.content" + :key="key" + class="d-flex justify-content-between" + > + <gl-chart-series-label :color="tooltip.content.length > 1 ? seriesLabel.color : ''">{{ + seriesLabel.name + }}</gl-chart-series-label> + <div class="prepend-left-32">{{ seriesLabel.value }}</div> + </div> </template> - </gl-line-chart> - </div> + </template> + </gl-line-chart> </div> </template> diff --git a/app/assets/javascripts/monitoring/components/panel_type.vue b/app/assets/javascripts/monitoring/components/panel_type.vue index c56bcfb8db2..73ff651d510 100644 --- a/app/assets/javascripts/monitoring/components/panel_type.vue +++ b/app/assets/javascripts/monitoring/components/panel_type.vue @@ -11,7 +11,6 @@ import { } from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; import MonitorTimeSeriesChart from './charts/time_series.vue'; -import MonitorAnomalyChart from './charts/anomaly.vue'; import MonitorSingleStatChart from './charts/single_stat.vue'; import MonitorEmptyChart from './charts/empty_chart.vue'; @@ -19,7 +18,6 @@ export default { components: { MonitorSingleStatChart, MonitorTimeSeriesChart, - MonitorAnomalyChart, MonitorEmptyChart, Icon, GlDropdown, @@ -94,51 +92,6 @@ export default { v-if="isPanelType('single-stat') && graphDataHasMetrics" :graph-data="graphData" /> - <monitor-anomaly-chart - v-else-if="isPanelType('anomaly-chart') && graphDataHasMetrics" - :graph-data="graphData" - :deployment-data="deploymentData" - :project-path="projectPath" - :thresholds="getGraphAlertValues(graphData.queries)" - :container-width="dashboardWidth" - group-id="monitor-anomaly-chart" - > - <div class="d-flex align-items-center"> - <alert-widget - v-if="alertWidgetAvailable && graphData" - :modal-id="`alert-modal-${index}`" - :alerts-endpoint="alertsEndpoint" - :relevant-queries="graphData.queries" - :alerts-to-manage="getGraphAlerts(graphData.queries)" - @setAlerts="setAlerts" - /> - <gl-dropdown - v-gl-tooltip - class="mx-2" - toggle-class="btn btn-transparent border-0" - :right="true" - :no-caret="true" - :title="__('More actions')" - > - <template slot="button-content"> - <icon name="ellipsis_v" class="text-secondary" /> - </template> - <gl-dropdown-item :href="downloadCsv" download="chart_metrics.csv"> - {{ __('Download CSV') }} - </gl-dropdown-item> - <gl-dropdown-item - class="js-chart-link" - :data-clipboard-text="clipboardText" - @click="showToast" - > - {{ __('Generate link to chart') }} - </gl-dropdown-item> - <gl-dropdown-item v-if="alertWidgetAvailable" v-gl-modal="`alert-modal-${index}`"> - {{ __('Alerts') }} - </gl-dropdown-item> - </gl-dropdown> - </div> - </monitor-anomaly-chart> <monitor-time-series-chart v-else-if="graphDataHasMetrics" :graph-data="graphData" diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js index 3df9387b4f2..0f6f4ad1fd6 100644 --- a/app/assets/javascripts/monitoring/utils.js +++ b/app/assets/javascripts/monitoring/utils.js @@ -45,15 +45,30 @@ export const graphDataValidatorForValues = (isValues, graphData) => { ); }; -export const graphDataValidatorForAnomalyValues = (isValues, graphData) => { +/** + * This function validates the graph data contains exactly 3 queries plus + * value validations from graphDataValidatorForValues. + * @param {Object} isValues + * @param {Object} graphData the graph data response from a prometheus request + * @returns {boolean} true if the data is valid + */ +export const graphDataValidatorForAnomalyValues = graphData => { const anomalySeriesCount = 3; // metric, upper, lower return ( graphData.queries && graphData.queries.length === anomalySeriesCount && - graphDataValidatorForValues(isValues, graphData) + graphDataValidatorForValues(false, graphData) ); }; +/** + * This function returns the earliest time value in all series of a chart. + * @param {Object} chartData chart data with data to populate a timeseries. + * data should be an array of data points [t, y] where t is a ISO formatted date, + * and is sorted by t (time). + * @returns {(String|null)} earliest x value from all series, or null when the + * chart series data is empty. + */ export const getEarliestDatapoint = chartData => chartData.reduce((acc, series) => { const { data } = series; @@ -69,18 +84,16 @@ export const getEarliestDatapoint = chartData => return seriesEarliest < acc || acc === null ? seriesEarliest : acc; }, null); -export const makeTimeAxis = config => { - const defaults = { - name: __('Time'), - type: 'time', - axisLabel: { - formatter: date => dateFormat(date, dateFormats.timeOfDay), - }, - axisPointer: { - snap: true, - }, - }; - return { ...defaults, ...config }; -}; +export const makeTimeAxis = config => ({ + name: __('Time'), + type: 'time', + axisLabel: { + formatter: date => dateFormat(date, dateFormats.timeOfDay), + }, + axisPointer: { + snap: true, + }, + ...config, +}); export default {}; diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js index 38afe2b9e3b..88d56b3e382 100644 --- a/spec/frontend/monitoring/mock_data.js +++ b/spec/frontend/monitoring/mock_data.js @@ -1,6 +1,4 @@ -import * as mockData from '../../javascripts/monitoring/mock_data'; - -export const { mockProjectPath } = mockData; +export const mockProjectPath = '/frontend-fixtures/environments-project'; export const anomalyDeploymentData = [ { diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 17e7314e214..e10108b6026 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -1,3 +1,7 @@ +import { anomalyMockGraphData as importedAnomalyMockGraphData } from '../../frontend/monitoring/mock_data'; + +export const anomalyMockGraphData = importedAnomalyMockGraphData; + export const mockApiEndpoint = `${gl.TEST_HOST}/monitoring/mock`; export const mockProjectPath = '/frontend-fixtures/environments-project'; diff --git a/spec/javascripts/monitoring/utils_spec.js b/spec/javascripts/monitoring/utils_spec.js index e22e8cdc03d..1cd3cc27e88 100644 --- a/spec/javascripts/monitoring/utils_spec.js +++ b/spec/javascripts/monitoring/utils_spec.js @@ -1,6 +1,14 @@ -import { getTimeDiff, graphDataValidatorForValues } from '~/monitoring/utils'; +import { + getTimeDiff, + graphDataValidatorForValues, + graphDataValidatorForAnomalyValues, +} from '~/monitoring/utils'; import { timeWindows } from '~/monitoring/constants'; -import { graphDataPrometheusQuery, graphDataPrometheusQueryRange } from './mock_data'; +import { + graphDataPrometheusQuery, + graphDataPrometheusQueryRange, + anomalyMockGraphData, +} from './mock_data'; describe('getTimeDiff', () => { function secondsBetween({ start, end }) { @@ -62,3 +70,17 @@ describe('graphDataValidatorForValues', () => { expect(validGraphData).toBe(true); }); }); + +describe('graphDataValidatorForAnomalyValues', () => { + /* + * Anomaly charts can accept results for exactly 3 queries, + * Anomaly validation also require graphDataValidatorForValues to have `values` + */ + it('validates data with the query format', () => { + const validAnomalyData = graphDataValidatorForAnomalyValues(anomalyMockGraphData); + const validGraphData = graphDataValidatorForValues(false, anomalyMockGraphData); + + expect(validAnomalyData).toBe(true); + expect(validGraphData).toBe(true); + }); +}); |