Merge branch 'minimized-multiple-queries' into 'master'

Support multiple queries on a single chart in Metrics

See merge request gitlab-org/gitlab-ee!9707

app/assets/javascripts/monitoring/components/charts/area.vue

@@ -42,10 +42,10 @@ export default {
       required: false,
       default: () => [],
     },
-    alertData: {
-      type: Object,
+    thresholds: {
+      type: Array,
       required: false,
-      default: () => ({}),
+      default: () => [],
     },
   },
   data() {
@@ -64,6 +64,9 @@ export default {
   },
   computed: {
     chartData() {
+      // Transforms & supplements query data to render appropriate labels & styles
+      // Input: [{ queryAttributes1 }, { queryAttributes2 }]
+      // Output: [{ seriesAttributes1 }, { seriesAttributes2 }]
       return this.graphData.queries.reduce((acc, query) => {
         const { appearance } = query;
         const lineType =
@@ -121,6 +124,9 @@ export default {
     },
     earliestDatapoint() {
       return this.chartData.reduce((acc, series) => {
+        if (!series.data.length) {
+          return acc;
+        }
         const [[timestamp]] = series.data.sort(([a], [b]) => {
           if (a < b) {
             return -1;
@@ -235,7 +241,7 @@ export default {
       :data="chartData"
       :option="chartOptions"
       :format-tooltip-text="formatTooltipText"
-      :thresholds="alertData"
+      :thresholds="thresholds"
       :width="width"
       :height="height"
       @updated="onChartUpdated"

app/assets/javascripts/monitoring/components/dashboard.vue

 <script>
 import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
+import _ from 'underscore';
 import { s__ } from '~/locale';
 import Icon from '~/vue_shared/components/icon.vue';
 import '~/vue_shared/mixins/is_ee';
@@ -142,8 +143,13 @@ export default {
     }
   },
   methods: {
-    getGraphAlerts(graphId) {
-      return this.alertData ? this.alertData[graphId] || {} : {};
+    getGraphAlerts(queries) {
+      if (!this.allAlerts) return {};
+      const metricIdsForChart = queries.map(q => q.metricId);
+      return _.pick(this.allAlerts, alert => metricIdsForChart.includes(alert.metricId));
+    },
+    getGraphAlertValues(queries) {
+      return Object.values(this.getGraphAlerts(queries));
     },
     getGraphsData() {
       this.state = 'loading';
@@ -199,17 +205,15 @@ export default {
         :key="graphIndex"
         :graph-data="graphData"
         :deployment-data="store.deploymentData"
-        :alert-data="getGraphAlerts(graphData.id)"
+        :thresholds="getGraphAlertValues(graphData.queries)"
         :container-width="elWidth"
         group-id="monitor-area-chart"
       >
         <alert-widget
-          v-if="isEE && prometheusAlertsAvailable && alertsEndpoint && graphData.id"
+          v-if="isEE && prometheusAlertsAvailable && alertsEndpoint && graphData"
           :alerts-endpoint="alertsEndpoint"
-          :label="getGraphLabel(graphData)"
-          :current-alerts="getQueryAlerts(graphData)"
-          :custom-metric-id="graphData.id"
-          :alert-data="alertData[graphData.id]"
+          :relevant-queries="graphData.queries"
+          :alerts-to-manage="getGraphAlerts(graphData.queries)"
           @setAlerts="setAlerts"
         />
       </monitor-area-chart>

app/assets/javascripts/monitoring/stores/monitoring_store.js

@@ -27,10 +27,47 @@ function removeTimeSeriesNoData(queries) {
   return queries.reduce((series, query) => series.concat(checkQueryEmptyData(query)), []);
 }
 
+// Metrics and queries are currently stored 1:1, so `queries` is an array of length one.
+// We want to group queries onto a single chart by title & y-axis label.
+// This function will no longer be required when metrics:queries are 1:many,
+// though there is no consequence if the function stays in use.
+// @param metrics [Array<Object>]
+//      Ex) [
+//            { id: 1, title: 'title', y_label: 'MB', queries: [{ ...query1Attrs }] },
+//            { id: 2, title: 'title', y_label: 'MB', queries: [{ ...query2Attrs }] },
+//            { id: 3, title: 'new title', y_label: 'MB', queries: [{ ...query3Attrs }] }
+//          ]
+// @return [Array<Object>]
+//      Ex) [
+//            { title: 'title', y_label: 'MB', queries: [{ metricId: 1, ...query1Attrs },
+//                                                       { metricId: 2, ...query2Attrs }] },
+//            { title: 'new title', y_label: 'MB', queries: [{ metricId: 3, ...query3Attrs }]}
+//          ]
+function groupQueriesByChartInfo(metrics) {
+  const metricsByChart = metrics.reduce((accumulator, metric) => {
+    const { id, queries, ...chart } = metric;
+
+    const chartKey = `${chart.title}|${chart.y_label}`;
+    accumulator[chartKey] = accumulator[chartKey] || { ...chart, queries: [] };
+
+    queries.forEach(queryAttrs =>
+      accumulator[chartKey].queries.push({ metricId: id.toString(), ...queryAttrs }),
+    );
+
+    return accumulator;
+  }, {});
+
+  return Object.values(metricsByChart);
+}
+
 function normalizeMetrics(metrics) {
-  return metrics.map(metric => {
+  const groupedMetrics = groupQueriesByChartInfo(metrics);
+
+  return groupedMetrics.map(metric => {
     const queries = metric.queries.map(query => ({
       ...query,
+      // custom metrics do not require a label, so we should ensure this attribute is defined
+      label: query.label || metric.y_label,
       result: query.result.map(result => ({
         ...result,
         values: result.values.map(([timestamp, value]) => [

changelogs/unreleased/minimized-multiple-queries-ce.yml

+---
+title: Support multiple queries per chart on metrics dash
+merge_request: 25758
+author:
+type: added

doc/user/project/integrations/prometheus.md

@@ -105,10 +105,12 @@ A few fields are required:
 
 - **Name**: Chart title
 - **Type**: Type of metric. Metrics of the same type will be shown together.
-- **Query**: Valid [PromQL query](https://prometheus.io/docs/prometheus/latest/querying/basics/). Note, no validation is performed at this time. If the query is not valid, the dashboard will display an error.
+- **Query**: Valid [PromQL query](https://prometheus.io/docs/prometheus/latest/querying/basics/).
 - **Y-axis label**: Y axis title to display on the dashboard.
 - **Unit label**: Query units, for example `req / sec`. Shown next to the value.
 
+Multiple metrics can be displayed on the same chart if the fields **Name**, **Type**, and **Y-axis label** match between metrics. For example, a metric with **Name** `Requests Rate`, **Type** `Business`, and **Y-axis label** `rec / sec` would display on the same chart as a second metric with the same values. A **Legend label** is suggested if this feature used.
+
 #### Query Variables
 
 GitLab supports a limited set of [CI variables](../../../ci/variables/README.html) in the Prometheus query. This is particularly useful for identifying a specific environment, for example with `CI_ENVIRONMENT_SLUG`. The supported variables are:

ee/app/assets/javascripts/monitoring/components/alert_widget.vue

@@ -3,6 +3,7 @@ import { s__ } from '~/locale';
 import Icon from '~/vue_shared/components/icon.vue';
 import AlertWidgetForm from './alert_widget_form.vue';
 import AlertsService from '../services/alerts_service';
+import { alertsValidator, queriesValidator } from '../validators';
 import { GlLoadingIcon } from '@gitlab/ui';
 
 export default {
@@ -16,24 +17,20 @@ export default {
       type: String,
       required: true,
     },
-    label: {
-      type: String,
-      required: true,
-    },
-    currentAlerts: {
-      type: Array,
-      require: false,
-      default: () => [],
-    },
-    customMetricId: {
-      type: Number,
-      require: false,
-      default: null,
-    },
-    alertData: {
+    // { [alertPath]: { alert_attributes } }. Populated from subsequent API calls.
+    // Includes only the metrics/alerts to be managed by this widget.
+    alertsToManage: {
       type: Object,
       required: false,
       default: () => ({}),
+      validator: alertsValidator,
+    },
+    // [{ metric+query_attributes }]. Represents queries (and alerts) we know about
+    // on intial fetch. Essentially used for reference.
+    relevantQueries: {
+      type: Array,
+      required: true,
+      validator: queriesValidator,
     },
   },
   data() {
@@ -42,14 +39,14 @@ export default {
       errorMessage: null,
       isLoading: false,
       isOpen: false,
-      alerts: this.currentAlerts,
+      apiAction: 'create',
     };
   },
   computed: {
     alertSummary() {
-      const data = this.firstAlertData;
-      if (!data) return null;
-      return `${this.label} ${data.operator} ${data.threshold}`;
+      return Object.keys(this.alertsToManage)
+        .map(this.formatAlertSummary)
+        .join(', ');
     },
     alertIcon() {
       return this.hasAlerts ? 'notifications' : 'notifications-off';
@@ -60,18 +57,12 @@ export default {
         : s__('PrometheusAlerts|No alert set');
     },
     dropdownTitle() {
-      return this.hasAlerts
-        ? s__('PrometheusAlerts|Edit alert')
-        : s__('PrometheusAlerts|Add alert');
+      return this.apiAction === 'create'
+        ? s__('PrometheusAlerts|Add alert')
+        : s__('PrometheusAlerts|Edit alert');
     },
     hasAlerts() {
-      return Object.keys(this.alertData).length > 0;
-    },
-    firstAlert() {
-      return this.hasAlerts ? this.alerts[0] : undefined;
-    },
-    firstAlertData() {
-      return this.hasAlerts ? this.alertData[this.alerts[0]] : undefined;
+      return !!Object.keys(this.alertsToManage).length;
     },
     formDisabled() {
       return !!(this.errorMessage || this.isLoading);
@@ -97,14 +88,14 @@ export default {
   methods: {
     fetchAlertData() {
       this.isLoading = true;
+
+      const queriesWithAlerts = this.relevantQueries.filter(query => query.alert_path);
+
       return Promise.all(
-        this.alerts.map(alertPath =>
-          this.service.readAlert(alertPath).then(alertData => {
-            this.$emit('setAlerts', this.customMetricId, {
-              ...this.alertData,
-              [alertPath]: alertData,
-            });
-          }),
+        queriesWithAlerts.map(query =>
+          this.service
+            .readAlert(query.alert_path)
+            .then(alertAttributes => this.setAlert(alertAttributes, query.metricId)),
         ),
       )
         .then(() => {
@@ -115,6 +106,18 @@ export default {
           this.isLoading = false;
         });
     },
+    setAlert(alertAttributes, metricId) {
+      this.$emit('setAlerts', alertAttributes.alert_path, { ...alertAttributes, metricId });
+    },
+    removeAlert(alertPath) {
+      this.$emit('setAlerts', alertPath, null);
+    },
+    formatAlertSummary(alertPath) {
+      const alert = this.alertsToManage[alertPath];
+      const alertQuery = this.relevantQueries.find(query => query.metricId === alert.metricId);
+
+      return `${alertQuery.label} ${alert.operator} ${alert.threshold}`;
+    },
     handleDropdownToggle() {
       this.isOpen = !this.isOpen;
     },
@@ -122,22 +125,23 @@ export default {
       this.isOpen = false;
     },
     handleOutsideClick(event) {
-      if (!this.$refs.dropdownMenu.contains(event.target)) {
+      if (
+        !this.$refs.dropdownMenu.contains(event.target) &&
+        !this.$refs.dropdownMenuToggle.contains(event.target)
+      ) {
         this.isOpen = false;
       }
     },
-    handleCreate({ operator, threshold }) {
-      const newAlert = { operator, threshold, prometheus_metric_id: this.customMetricId };
+    handleSetApiAction(apiAction) {
+      this.apiAction = apiAction;
+    },
+    handleCreate({ operator, threshold, prometheus_metric_id }) {
+      const newAlert = { operator, threshold, prometheus_metric_id };
       this.isLoading = true;
       this.service
         .createAlert(newAlert)
-        .then(response => {
-          const alertPath = response.alert_path;
-          this.alerts.unshift(alertPath);
-          this.$emit('setAlerts', this.customMetricId, {
-            ...this.alertData,
-            [alertPath]: newAlert,
-          });
+        .then(alertAttributes => {
+          this.setAlert(alertAttributes, prometheus_metric_id);
           this.isLoading = false;
           this.handleDropdownClose();
         })
@@ -151,11 +155,8 @@ export default {
       this.isLoading = true;
       this.service
         .updateAlert(alert, updatedAlert)
-        .then(() => {
-          this.$emit('setAlerts', this.customMetricId, {
-            ...this.alertData,
-            [alert]: updatedAlert,
-          });
+        .then(alertAttributes => {
+          this.setAlert(alertAttributes, this.alertsToManage[alert].metricId);
           this.isLoading = false;
           this.handleDropdownClose();
         })
@@ -169,9 +170,7 @@ export default {
       this.service
         .deleteAlert(alert)
         .then(() => {
-          const { [alert]: _, ...otherItems } = this.alertData;
-          this.$emit('setAlerts', this.customMetricId, otherItems);
-          this.alerts = this.alerts.filter(alertPath => alert !== alertPath);
+          this.removeAlert(alert);
           this.isLoading = false;
           this.handleDropdownClose();
         })
@@ -185,13 +184,14 @@ export default {
 </script>
 
 <template>
-  <div :class="{ show: isOpen }" class="prometheus-alert-widget dropdown">
+  <div class="prometheus-alert-widget dropdown d-flex align-items-center">
     <span v-if="errorMessage" class="alert-error-message"> {{ errorMessage }} </span>
     <span v-else class="alert-current-setting">
       <gl-loading-icon v-show="isLoading" :inline="true" />
       {{ alertSummary }}
     </span>
     <button
+      ref="dropdownMenuToggle"
       :aria-label="alertStatus"
       class="btn btn-sm alert-dropdown-button"
       type="button"
@@ -200,7 +200,7 @@ export default {
       <icon :name="alertIcon" :size="16" aria-hidden="true" />
       <icon :size="16" name="arrow-down" aria-hidden="true" class="chevron" />
     </button>
-    <div ref="dropdownMenu" class="dropdown-menu alert-dropdown-menu">
+    <div ref="dropdownMenu" :class="{ show: isOpen }" class="dropdown-menu alert-dropdown-menu">
       <div class="dropdown-title">
         <span>{{ dropdownTitle }}</span>
         <button
@@ -216,12 +216,13 @@ export default {
         <alert-widget-form
           ref="widgetForm"
           :disabled="formDisabled"
-          :alert="firstAlert"
-          :alert-data="firstAlertData"
+          :alerts-to-manage="alertsToManage"
+          :relevant-queries="relevantQueries"
           @create="handleCreate"
           @update="handleUpdate"
           @delete="handleDelete"
           @cancel="handleDropdownClose"
+          @setAction="handleSetApiAction"
         />
       </div>
     </div>

ee/app/assets/javascripts/monitoring/components/alert_widget_form.vue

 <script>
-import { __ } from '~/locale';
+import { __, s__ } from '~/locale';
+import _ from 'underscore';
 import Vue from 'vue';
 import Translate from '~/vue_shared/translate';
+import { alertsValidator, queriesValidator } from '../validators';
+import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 
 Vue.use(Translate);
 
@@ -24,39 +27,58 @@ const OPERATORS = {
 };
 
 export default {
+  components: {
+    GlDropdown,
+    GlDropdownItem,
+  },
   props: {
     disabled: {
       type: Boolean,
       required: true,
     },
-    alert: {
-      type: String,
-      required: false,
-      default: null,
-    },
-    alertData: {
+    alertsToManage: {
       type: Object,
       required: false,
       default: () => ({}),
+      validator: alertsValidator,
+    },
+    relevantQueries: {
+      type: Array,
+      required: true,
+      validator: queriesValidator,
     },
   },
   data() {
     return {
       operators: OPERATORS,
-      operator: this.alertData.operator,
-      threshold: this.alertData.threshold,
+      operator: null,
+      threshold: null,
+      prometheusMetricId: null,
+      selectedAlert: {},
     };
   },
   computed: {
+    currentQuery() {
+      return this.relevantQueries.find(query => query.metricId === this.prometheusMetricId) || {};
+    },
+    formDisabled() {
+      // We need a prometheusMetricId to determine whether we're
+      // creating/updating/deleting
+      return this.disabled || !this.prometheusMetricId;
+    },
+    queryDropdownLabel() {
+      return this.currentQuery.label || s__('PrometheusAlerts|Select query');
+    },
     haveValuesChanged() {
       return (
         this.operator &&
         this.threshold === Number(this.threshold) &&
-        (this.operator !== this.alertData.operator || this.threshold !== this.alertData.threshold)
+        (this.operator !== this.selectedAlert.operator ||
+          this.threshold !== this.selectedAlert.threshold)
       );
     },
     submitAction() {
-      if (!this.alert) return 'create';
+      if (_.isEmpty(this.selectedAlert)) return 'create';
       if (this.haveValuesChanged) return 'update';
       return 'delete';
     },
@@ -71,11 +93,30 @@ export default {
     },
   },
   watch: {
-    alertData() {
+    alertsToManage() {
       this.resetAlertData();
     },
+    submitAction() {
+      this.$emit('setAction', this.submitAction);
+    },
   },
   methods: {
+    selectQuery(queryId) {
+      const existingAlertPath = _.findKey(this.alertsToManage, alert => alert.metricId === queryId);
+      const existingAlert = this.alertsToManage[existingAlertPath];
+
+      if (existingAlert) {
+        this.selectedAlert = existingAlert;
+        this.operator = existingAlert.operator;
+        this.threshold = existingAlert.threshold;
+      } else {
+        this.selectedAlert = {};
+        this.operator = null;
+        this.threshold = null;
+      }
+
+      this.prometheusMetricId = queryId;
+    },
     handleCancel() {
       this.resetAlertData();
       this.$emit('cancel');
@@ -83,14 +124,17 @@ export default {
     handleSubmit() {
       this.$refs.submitButton.blur();
       this.$emit(this.submitAction, {
-        alert: this.alert,
+        alert: this.selectedAlert.alert_path,
         operator: this.operator,
         threshold: this.threshold,
+        prometheus_metric_id: this.prometheusMetricId,
       });
     },
     resetAlertData() {
-      this.operator = this.alertData.operator;
-      this.threshold = this.alertData.threshold;
+      this.operator = null;
+      this.threshold = null;
+      this.prometheusMetricId = null;
+      this.selectedAlert = {};
     },
   },
 };
@@ -98,10 +142,19 @@ export default {
 
 <template>
   <div class="alert-form">
+    <gl-dropdown :text="queryDropdownLabel" class="form-group" toggle-class="dropdown-menu-toggle">
+      <gl-dropdown-item
+        v-for="query in relevantQueries"
+        :key="query.metricId"
+        @click="selectQuery(query.metricId)"
+      >
+        {{ `${query.label} (${query.unit})` }}
+      </gl-dropdown-item>
+    </gl-dropdown>
     <div :aria-label="s__('PrometheusAlerts|Operator')" class="form-group btn-group" role="group">
       <button
         :class="{ active: operator === operators.greaterThan }"
-        :disabled="disabled"
+        :disabled="formDisabled"
         type="button"
         class="btn btn-default"
         @click="operator = operators.greaterThan"
@@ -110,7 +163,7 @@ export default {
       </button>
       <button
         :class="{ active: operator === operators.equalTo }"
-        :disabled="disabled"
+        :disabled="formDisabled"
         type="button"
         class="btn btn-default"
         @click="operator = operators.equalTo"
@@ -119,7 +172,7 @@ export default {
       </button>
       <button
         :class="{ active: operator === operators.lessThan }"
-        :disabled="disabled"
+        :disabled="formDisabled"
         type="button"
         class="btn btn-default"
         @click="operator = operators.lessThan"
@@ -129,12 +182,17 @@ export default {
     </div>
     <div class="form-group">
       <label>{{ s__('PrometheusAlerts|Threshold') }}</label>
-      <input v-model.number="threshold" :disabled="disabled" type="number" class="form-control" />
+      <input
+        v-model.number="threshold"
+        :disabled="formDisabled"
+        type="number"
+        class="form-control"
+      />
     </div>
     <div class="action-group">
       <button
         ref="cancelButton"
-        :disabled="disabled"
+        :disabled="formDisabled"
         type="button"
         class="btn btn-default"
         @click="handleCancel"

ee/app/assets/javascripts/monitoring/components/dashboard.vue

@@ -21,20 +21,21 @@ export default {
   },
   data() {
     return {
-      alertData: {},
+      allAlerts: {},
     };
   },
-  methods: {
-    getGraphLabel(graphData) {
-      if (!graphData.queries || !graphData.queries[0]) return undefined;
-      return graphData.queries[0].label || graphData.y_label || 'Average';
-    },
-    getQueryAlerts(graphData) {
-      if (!graphData.queries) return [];
-      return graphData.queries.map(query => query.alert_path).filter(Boolean);
+  computed: {
+    alertsAvailable() {
+      return this.prometheusAlertsAvailable && this.alertsEndpoint;
     },
-    setAlerts(metricId, alertData) {
-      this.$set(this.alertData, metricId, alertData);
+  },
+  methods: {
+    setAlerts(alertPath, alertAttributes) {
+      if (alertAttributes) {
+        this.$set(this.allAlerts, alertPath, alertAttributes);
+      } else {
+        this.$delete(this.allAlerts, alertPath);
+      }
     },
   },
 };

ee/app/assets/javascripts/monitoring/validators.js

+// Prop validator for alert information, expecting an object like the example below.
+//
+// {
+//   '/root/autodevops-deploy/prometheus/alerts/16.json?environment_id=37': {
+//     alert_path: "/root/autodevops-deploy/prometheus/alerts/16.json?environment_id=37",
+//     metricId: '1',
+//     operator: ">",
+//     query: "rate(http_requests_total[5m])[30m:1m]",
+//     threshold: 0.002,
+//     title: "Core Usage (Total)",
+//   }
+// }
+export function alertsValidator(value) {
+  return Object.keys(value).every(key => {
+    const alert = value[key];
+    return (
+      alert.alert_path &&
+      key === alert.alert_path &&
+      alert.metricId &&
+      typeof alert.metricId === 'string' &&
+      alert.operator &&
+      typeof alert.threshold === 'number'
+    );
+  });
+}
+
+// Prop validator for query information, expecting an array like the example below.
+//
+// [
+//   {
+//     metricId: '16',
+//     label: 'Total Cores'
+//   },
+//   {
+//     metricId: '17',
+//     label: 'Sub-total Cores'
+//   }
+// ]
+export function queriesValidator(value) {
+  return value.every(query => query.metricId && typeof query.metricId === 'string' && query.label);
+}

ee/spec/javascripts/monitoring/alert_widget_form_spec.js

@@ -5,8 +5,22 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper';
 describe('AlertWidgetForm', () => {
   let AlertWidgetFormComponent;
   let vm;
+
+  const metricId = '8';
+  const alertPath = 'alert';
+  const relevantQueries = [{ metricId, alert_path: alertPath, label: 'alert-label' }];
+
   const props = {
     disabled: false,
+    relevantQueries,
+  };
+
+  const propsWithAlertData = {
+    ...props,
+    relevantQueries,
+    alertsToManage: {
+      alert: { alert_path: alertPath, operator: '<', threshold: 5, metricId },
+    },
   };
 
   beforeAll(() => {
@@ -20,6 +34,15 @@ describe('AlertWidgetForm', () => {
   it('disables the input when disabled prop is set', () => {
     vm = mountComponent(AlertWidgetFormComponent, { ...props, disabled: true });
 
+    vm.prometheusMetricId = 6;
+
+    expect(vm.$refs.cancelButton).toBeDisabled();
+    expect(vm.$refs.submitButton).toBeDisabled();
+  });
+
+  it('disables the input if no query is selected', () => {
+    vm = mountComponent(AlertWidgetFormComponent, props);
+
     expect(vm.$refs.cancelButton).toBeDisabled();
     expect(vm.$refs.submitButton).toBeDisabled();
   });
@@ -30,15 +53,17 @@ describe('AlertWidgetForm', () => {
     expect(vm.$refs.submitButton.innerText).toContain('Add');
     vm.$once('create', alert => {
       expect(alert).toEqual({
-        alert: null,
+        alert: undefined,
         operator: '<',
         threshold: 5,
+        prometheus_metric_id: '8',
       });
       done();
     });
 
     // the button should be disabled until an operator and threshold are selected
     expect(vm.$refs.submitButton).toBeDisabled();
+    vm.selectQuery('8');
     vm.operator = '<';
     vm.threshold = 5;
     Vue.nextTick(() => {
@@ -47,47 +72,46 @@ describe('AlertWidgetForm', () => {
   });
 
   it('emits a "delete" event when form submitted with existing alert and no changes are made', done => {
-    vm = mountComponent(AlertWidgetFormComponent, {
-      ...props,
-      alert: 'alert',
-      alertData: { operator: '<', threshold: 5 },
-    });
+    vm = mountComponent(AlertWidgetFormComponent, propsWithAlertData);
+    vm.selectQuery('8');
 
     vm.$once('delete', alert => {
       expect(alert).toEqual({
         alert: 'alert',
         operator: '<',
         threshold: 5,
+        prometheus_metric_id: '8',
       });
       done();
     });
 
-    expect(vm.$refs.submitButton.innerText).toContain('Delete');
-    vm.$refs.submitButton.click();
+    Vue.nextTick(() => {
+      expect(vm.$refs.submitButton.innerText).toContain('Delete');
+      vm.$refs.submitButton.click();
+    });
   });
 
   it('emits a "update" event when form submitted with existing alert', done => {
-    vm = mountComponent(AlertWidgetFormComponent, {
-      ...props,
-      alert: 'alert',
-      alertData: { operator: '<', threshold: 5 },
-    });
-
-    expect(vm.$refs.submitButton.innerText).toContain('Delete');
+    vm = mountComponent(AlertWidgetFormComponent, propsWithAlertData);
+    vm.selectQuery('8');
     vm.$once('update', alert => {
       expect(alert).toEqual({
         alert: 'alert',
         operator: '=',
         threshold: 5,
+        prometheus_metric_id: '8',
       });
       done();
     });
-
-    // change operator to allow update
-    vm.operator = '=';
     Vue.nextTick(() => {
-      expect(vm.$refs.submitButton.innerText).toContain('Save');
-      vm.$refs.submitButton.click();
+      expect(vm.$refs.submitButton.innerText).toContain('Delete');
+
+      // change operator to allow update
+      vm.operator = '=';
+      Vue.nextTick(() => {
+        expect(vm.$refs.submitButton.innerText).toContain('Save');
+        vm.$refs.submitButton.click();
+      });
     });
   });
 });

ee/spec/javascripts/monitoring/alert_widget_spec.js

@@ -6,16 +6,33 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper';
 describe('AlertWidget', () => {
   let AlertWidgetComponent;
   let vm;
+
+  const metricId = '5';
+  const alertPath = 'my/alert.json';
+  const relevantQueries = [{ metricId, label: 'alert-label', alert_path: alertPath }];
+
   const props = {
     alertsEndpoint: '',
-    customMetricId: 5,
-    label: 'alert-label',
-    currentAlerts: ['my/alert.json'],
+    relevantQueries,
+    alertsToManage: {},
+  };
+
+  const propsWithAlert = {
+    ...props,
+    relevantQueries,
   };
 
-  const mockSetAlerts = (_, data) => {
-    /* eslint-disable-next-line no-underscore-dangle */
-    Vue.set(vm._props, 'alertData', data);
+  const propsWithAlertData = {
+    ...props,
+    relevantQueries,
+    alertsToManage: {
+      [alertPath]: { operator: '>', threshold: 42, alert_path: alertPath, metricId },
+    },
+  };
+
+  const mockSetAlerts = (path, data) => {
+    const alerts = data ? { [path]: data } : {};
+    Vue.set(vm, 'alertsToManage', alerts);
   };
 
   beforeAll(() => {
@@ -38,7 +55,7 @@ describe('AlertWidget', () => {
         resolveReadAlert = cb;
       }),
     );
-    vm = mountComponent(AlertWidgetComponent, props, '#alert-widget');
+    vm = mountComponent(AlertWidgetComponent, propsWithAlert, '#alert-widget');
 
     // expect loading spinner to exist during fetch
     expect(vm.isLoading).toBeTruthy();
@@ -58,7 +75,7 @@ describe('AlertWidget', () => {
 
   it('displays an error message when fetch fails', done => {
     spyOn(AlertsService.prototype, 'readAlert').and.returnValue(Promise.reject());
-    vm = mountComponent(AlertWidgetComponent, props, '#alert-widget');
+    vm = mountComponent(AlertWidgetComponent, propsWithAlert, '#alert-widget');
 
     setTimeout(() =>
       vm.$nextTick(() => {
@@ -70,28 +87,48 @@ describe('AlertWidget', () => {
     );
   });
 
-  it('displays an alert summary when fetch succeeds', done => {
+  it('displays an alert summary when there is a single alert', () => {
     spyOn(AlertsService.prototype, 'readAlert').and.returnValue(
       Promise.resolve({ operator: '>', threshold: 42 }),
     );
-    const propsWithAlertData = {
-      ...props,
-      alertData: { 'my/alert.json': { operator: '>', threshold: 42 } },
-    };
     vm = mountComponent(AlertWidgetComponent, propsWithAlertData, '#alert-widget');
 
-    setTimeout(() =>
-      vm.$nextTick(() => {
-        expect(vm.isLoading).toEqual(false);
-        expect(vm.alertSummary).toBe('alert-label > 42');
-        expect(vm.$el.querySelector('.alert-current-setting')).toBeVisible();
-        done();
-      }),
+    expect(vm.alertSummary).toBe('alert-label > 42');
+    expect(vm.$el.querySelector('.alert-current-setting')).toBeVisible();
+  });
+
+  it('displays a combined alert summary when there are multiple alerts', () => {
+    spyOn(AlertsService.prototype, 'readAlert').and.returnValue(
+      Promise.resolve({ operator: '>', threshold: 42 }),
     );
+    const propsWithManyAlerts = {
+      ...props,
+      relevantQueries: relevantQueries.concat([
+        { metricId: '6', alert_path: 'my/alert2.json', label: 'alert-label2' },
+      ]),
+      alertsToManage: {
+        'my/alert.json': {
+          operator: '>',
+          threshold: 42,
+          alert_path: alertPath,
+          metricId,
+        },
+        'my/alert2.json': {
+          operator: '=',
+          threshold: 900,
+          alert_path: 'my/alert2.json',
+          metricId: '6',
+        },
+      },
+    };
+    vm = mountComponent(AlertWidgetComponent, propsWithManyAlerts, '#alert-widget');
+
+    expect(vm.alertSummary).toBe('alert-label > 42, alert-label2 = 900');
+    expect(vm.$el.querySelector('.alert-current-setting')).toBeVisible();
   });
 
   it('opens and closes a dropdown menu by clicking close button', done => {
-    vm = mountComponent(AlertWidgetComponent, { ...props, currentAlerts: [] });
+    vm = mountComponent(AlertWidgetComponent, props);
 
     expect(vm.isOpen).toEqual(false);
     expect(vm.$el.querySelector('.alert-dropdown-menu')).toBeHidden();
@@ -100,20 +137,18 @@ describe('AlertWidget', () => {
 
     Vue.nextTick(() => {
       expect(vm.isOpen).toEqual(true);
-      expect(vm.$el).toHaveClass('show');
 
       vm.$el.querySelector('.dropdown-menu-close').click();
 
       Vue.nextTick(() => {
         expect(vm.isOpen).toEqual(false);
-        expect(vm.$el).not.toHaveClass('show');
         done();
       });
     });
   });
 
   it('opens and closes a dropdown menu by clicking outside the menu', done => {
-    vm = mountComponent(AlertWidgetComponent, { ...props, currentAlerts: [] });
+    vm = mountComponent(AlertWidgetComponent, props);
 
     expect(vm.isOpen).toEqual(false);
     expect(vm.$el.querySelector('.alert-dropdown-menu')).toBeHidden();
@@ -122,13 +157,11 @@ describe('AlertWidget', () => {
 
     Vue.nextTick(() => {
       expect(vm.isOpen).toEqual(true);
-      expect(vm.$el).toHaveClass('show');
 
       document.body.click();
 
       Vue.nextTick(() => {
         expect(vm.isOpen).toEqual(false);
-        expect(vm.$el).not.toHaveClass('show');
         done();
       });
     });
@@ -138,17 +171,14 @@ describe('AlertWidget', () => {
     const alertParams = {
       operator: '<',
       threshold: 4,
-      prometheus_metric_id: 5,
+      prometheus_metric_id: '5',
     };
 
     spyOn(AlertsService.prototype, 'createAlert').and.returnValue(
-      Promise.resolve({
-        alert_path: 'foo/bar',
-        ...alertParams,
-      }),
+      Promise.resolve({ alert_path: 'foo/bar', ...alertParams }),
     );
 
-    vm = mountComponent(AlertWidgetComponent, { ...props, currentAlerts: [] });
+    vm = mountComponent(AlertWidgetComponent, props);
     vm.$on('setAlerts', mockSetAlerts);
 
     vm.$refs.widgetForm.$emit('create', alertParams);
@@ -156,54 +186,40 @@ describe('AlertWidget', () => {
     expect(AlertsService.prototype.createAlert).toHaveBeenCalledWith(alertParams);
     Vue.nextTick(() => {
       expect(vm.isLoading).toEqual(false);
-      expect(vm.alertSummary).toBe('alert-label < 4');
       done();
     });
   });
 
   it('updates an alert with an appropriate handler', done => {
-    const alertPath = 'my/test/alert.json';
-    const alertParams = {
-      operator: '<',
-      threshold: 4,
-    };
+    const alertParams = { operator: '<', threshold: 4, alert_path: alertPath };
+    const newAlertParams = { operator: '=', threshold: 12 };
 
     spyOn(AlertsService.prototype, 'readAlert').and.returnValue(Promise.resolve(alertParams));
-    spyOn(AlertsService.prototype, 'updateAlert').and.returnValue(Promise.resolve());
+    spyOn(AlertsService.prototype, 'updateAlert').and.returnValue(Promise.resolve({}));
 
-    vm = mountComponent(AlertWidgetComponent, { ...props, currentAlerts: [alertPath] });
+    vm = mountComponent(AlertWidgetComponent, propsWithAlertData);
     vm.$on('setAlerts', mockSetAlerts);
 
     vm.$refs.widgetForm.$emit('update', {
-      ...alertParams,
       alert: alertPath,
-      operator: '=',
-      threshold: 12,
+      ...newAlertParams,
+      prometheus_metric_id: '5',
     });
 
-    expect(AlertsService.prototype.updateAlert).toHaveBeenCalledWith(alertPath, {
-      ...alertParams,
-      operator: '=',
-      threshold: 12,
-    });
+    expect(AlertsService.prototype.updateAlert).toHaveBeenCalledWith(alertPath, newAlertParams);
     Vue.nextTick(() => {
       expect(vm.isLoading).toEqual(false);
-      expect(vm.alertSummary).toBe('alert-label = 12');
       done();
     });
   });
 
   it('deletes an alert with an appropriate handler', done => {
-    const alertPath = 'my/test/alert.json';
-    const alertParams = {
-      operator: '<',
-      threshold: 4,
-    };
+    const alertParams = { alert_path: alertPath, operator: '>', threshold: 42 };
 
     spyOn(AlertsService.prototype, 'readAlert').and.returnValue(Promise.resolve(alertParams));
-    spyOn(AlertsService.prototype, 'deleteAlert').and.returnValue(Promise.resolve());
+    spyOn(AlertsService.prototype, 'deleteAlert').and.returnValue(Promise.resolve({}));
 
-    vm = mountComponent(AlertWidgetComponent, { ...props, currentAlerts: [alertPath] });
+    vm = mountComponent(AlertWidgetComponent, propsWithAlert);
     vm.$on('setAlerts', mockSetAlerts);
 
     vm.$refs.widgetForm.$emit('delete', { alert: alertPath });

ee/spec/javascripts/monitoring/validators_spec.js

+import { alertsValidator, queriesValidator } from 'ee/monitoring/validators';
+
+describe('alertsValidator', () => {
+  const validAlert = {
+    alert_path: 'my/alert.json',
+    operator: '<',
+    threshold: 5,
+    metricId: '8',
+  };
+
+  it('requires all alerts to have an alert path', () => {
+    const { operator, threshold, metricId } = validAlert;
+    const input = { [validAlert.alert_path]: { operator, threshold, metricId } };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('requires that the object key matches the alert path', () => {
+    const input = { undefined: validAlert };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('requires all alerts to have a metric id', () => {
+    const input = { [validAlert.alert_path]: { ...validAlert, metricId: undefined } };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('requires the metricId to be a string', () => {
+    const input = { [validAlert.alert_path]: { ...validAlert, metricId: 8 } };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('requires all alerts to have an operator', () => {
+    const input = { [validAlert.alert_path]: { ...validAlert, operator: '' } };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('requires all alerts to have an numeric threshold', () => {
+    const input = { [validAlert.alert_path]: { ...validAlert, threshold: '60' } };
+
+    expect(alertsValidator(input)).toEqual(false);
+  });
+
+  it('correctly identifies a valid alerts object', () => {
+    const input = { [validAlert.alert_path]: validAlert };
+
+    expect(alertsValidator(input)).toEqual(true);
+  });
+});
+
+describe('queriesValidator', () => {
+  const validQuery = { metricId: '8', alert_path: 'alert', label: 'alert-label' };
+
+  it('requires all alerts to have a metric id', () => {
+    const input = [{ ...validQuery, metricId: undefined }];
+
+    expect(queriesValidator(input)).toEqual(false);
+  });
+
+  it('requires the metricId to be a string', () => {
+    const input = [{ ...validQuery, metricId: 8 }];
+
+    expect(queriesValidator(input)).toEqual(false);
+  });
+
+  it('requires all queries to have a label', () => {
+    const input = [{ ...validQuery, label: undefined }];
+
+    expect(queriesValidator(input)).toEqual(false);
+  });
+
+  it('correctly identifies a valid queries array', () => {
+    const input = [validQuery];
+
+    expect(queriesValidator(input)).toEqual(true);
+  });
+});

locale/gitlab.pot

@@ -8460,6 +8460,9 @@ msgstr ""
 msgid "PrometheusAlerts|Operator"
 msgstr ""
 
+msgid "PrometheusAlerts|Select query"
+msgstr ""
+
 msgid "PrometheusAlerts|Threshold"
 msgstr ""
 

package.json

@@ -34,7 +34,7 @@
     "@babel/preset-env": "^7.3.1",
     "@gitlab/csslab": "^1.9.0",
     "@gitlab/svgs": "^1.58.0",
-    "@gitlab/ui": "^3.0.0",
+    "@gitlab/ui": "^3.0.1",
     "apollo-cache-inmemory": "^1.5.1",
     "apollo-client": "^2.5.1",
     "apollo-upload-client": "^10.0.0",

spec/javascripts/monitoring/charts/area_spec.js

@@ -65,7 +65,7 @@ describe('Area component', () => {
         expect(props.data).toBe(areaChart.vm.chartData);
         expect(props.option).toBe(areaChart.vm.chartOptions);
         expect(props.formatTooltipText).toBe(areaChart.vm.formatTooltipText);
-        expect(props.thresholds).toBe(areaChart.props('alertData'));
+        expect(props.thresholds).toBe(areaChart.vm.thresholds);
       });
 
       it('recieves a tooltip title', () => {
@@ -105,12 +105,13 @@ describe('Area component', () => {
             seriesName: areaChart.vm.chartData[0].name,
             componentSubType: type,
             value: [mockDate, 5.55555],
+            seriesIndex: 0,
           },
         ],
         value: mockDate,
       });
 
-      describe('series is of line type', () => {
+      describe('when series is of line type', () => {
         beforeEach(() => {
           areaChart.vm.formatTooltipText(generateSeriesData('line'));
         });
@@ -131,7 +132,7 @@ describe('Area component', () => {
         });
       });
 
-      describe('series is of scatter type', () => {
+      describe('when series is of scatter type', () => {
         beforeEach(() => {
           areaChart.vm.formatTooltipText(generateSeriesData('scatter'));
         });

yarn.lock

@@ -663,10 +663,10 @@
   resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.58.0.tgz#bb05263ff2eb7ca09a25cd14d0b1a932d2ea9c2f"
   integrity sha512-RlWSjjBT4lMIFuNC1ziCO1nws9zqZtxCjhrqK2DxDDTgp2W0At9M/BFkHp8RHyMCrO3g1fHTrLPUgzr5oR3Epg==
 
-"@gitlab/ui@^3.0.0":
-  version "3.0.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-3.0.0.tgz#33ca2808dbd4395e69a366a219d1edc1f3dbccd5"
-  integrity sha512-pDEa2k6ln5GE/N2z0V7dNEeFtSTW0p9ipO2/N9q6QMxO7fhhOhpMC0QVbdIljKTbglspDWI5v6BcqUjzYri5Pg==
+"@gitlab/ui@^3.0.1":
+  version "3.0.2"
+  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-3.0.2.tgz#29a17699751261657487b939c651c0f93264df2a"
+  integrity sha512-JZhcS5cDxtpxopTc55UWvUbZAwKvxygYHT9I01QmUtKgaKIJlnjBj8zkcg1xHazX7raSjjtjqfDEla39a+luuQ==
   dependencies:
     "@babel/standalone" "^7.0.0"
     bootstrap-vue "^2.0.0-rc.11"