Merge branch 'support-file-names-for-metrics-dashboards' into 'master'

Support loading metrics dashboard with file name

See merge request gitlab-org/gitlab!34115

app/assets/javascripts/monitoring/monitoring_app.js

@@ -22,6 +22,7 @@ export default (props = {}) => {
       currentEnvironmentName,
       dashboardTimezone,
       metricsDashboardBasePath,
+      customDashboardBasePath,
       ...dataProps
     } = el.dataset;
 
@@ -34,6 +35,7 @@ export default (props = {}) => {
       projectPath,
       logsPath,
       currentEnvironmentName,
+      customDashboardBasePath,
     });
 
     // HTML attributes are always strings, parse other types.

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

@@ -117,12 +117,12 @@ export const fetchData = ({ dispatch }) => {
 
 // Metrics dashboard
 
-export const fetchDashboard = ({ state, commit, dispatch }) => {
+export const fetchDashboard = ({ state, commit, dispatch, getters }) => {
   dispatch('requestMetricsDashboard');
 
   const params = {};
-  if (state.currentDashboard) {
-    params.dashboard = state.currentDashboard;
+  if (getters.fullDashboardPath) {
+    params.dashboard = getters.fullDashboardPath;
   }
 
   return backOffRequest(() => axios.get(state.dashboardEndpoint, { params }))
@@ -204,7 +204,7 @@ export const fetchDashboardData = ({ state, dispatch, getters }) => {
 
   return Promise.all(promises)
     .then(() => {
-      const dashboardType = state.currentDashboard === '' ? 'default' : 'custom';
+      const dashboardType = getters.fullDashboardPath === '' ? 'default' : 'custom';
       trackDashboardLoad({
         label: `${dashboardType}_metrics_dashboard`,
         value: getters.metricsWithData().length,
@@ -322,9 +322,9 @@ export const receiveEnvironmentsDataFailure = ({ commit }) => {
   commit(types.RECEIVE_ENVIRONMENTS_DATA_FAILURE);
 };
 
-export const fetchAnnotations = ({ state, dispatch }) => {
+export const fetchAnnotations = ({ state, dispatch, getters }) => {
   const { start } = convertToFixedRange(state.timeRange);
-  const dashboardPath = state.currentDashboard || DEFAULT_DASHBOARD_PATH;
+  const dashboardPath = getters.fullDashboardPath || DEFAULT_DASHBOARD_PATH;
   return gqClient
     .mutate({
       mutation: getAnnotations,

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

 import { NOT_IN_DB_PREFIX } from '../constants';
-import { addPrefixToCustomVariableParams, addDashboardMetaDataToLink } from './utils';
+import {
+  addPrefixToCustomVariableParams,
+  addDashboardMetaDataToLink,
+  normalizeCustomDashboardPath,
+} from './utils';
 
 const metricsIdsInPanel = panel =>
   panel.metrics.filter(metric => metric.metricId && metric.result).map(metric => metric.metricId);
@@ -10,10 +14,10 @@ const metricsIdsInPanel = panel =>
  *
  * @param {Object} state
  */
-export const selectedDashboard = state => {
+export const selectedDashboard = (state, getters) => {
   const { allDashboards } = state;
   return (
-    allDashboards.find(d => d.path === state.currentDashboard) ||
+    allDashboards.find(d => d.path === getters.fullDashboardPath) ||
     allDashboards.find(d => d.default) ||
     null
   );
@@ -154,5 +158,15 @@ export const getCustomVariablesParams = state =>
     return acc;
   }, {});
 
+/**
+ * For a given custom dashboard file name, this method
+ * returns the full file path.
+ *
+ * @param {Object} state
+ * @returns {String} full dashboard path
+ */
+export const fullDashboardPath = state =>
+  normalizeCustomDashboardPath(state.currentDashboard, state.customDashboardBasePath);
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

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

@@ -9,6 +9,13 @@ export default () => ({
 
   // Dashboard request parameters
   timeRange: null,
+  /**
+   * Currently selected dashboard. For custom dashboards,
+   * this could be the filename or the file path.
+   *
+   * If this is the filename and full path is required,
+   * getters.fullDashboardPath should be used.
+   */
   currentDashboard: null,
 
   // Dashboard data
@@ -58,4 +65,7 @@ export default () => ({
   // GitLab paths to other pages
   projectPath: null,
   logsPath: invalidUrl,
+
+  // static paths
+  customDashboardBasePath: '',
 });

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

@@ -3,10 +3,10 @@ import createGqClient, { fetchPolicies } from '~/lib/graphql';
 import { SUPPORTED_FORMATS } from '~/lib/utils/unit_format';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { parseTemplatingVariables } from './variable_mapping';
-import { NOT_IN_DB_PREFIX, linkTypes } from '../constants';
 import { DATETIME_RANGE_TYPES } from '~/lib/utils/constants';
 import { timeRangeToParams, getRangeType } from '~/lib/utils/datetime_range';
 import { isSafeURL, mergeUrlParams } from '~/lib/utils/url_utility';
+import { NOT_IN_DB_PREFIX, linkTypes, DEFAULT_DASHBOARD_PATH } from '../constants';
 
 export const gqClient = createGqClient(
   {},
@@ -440,3 +440,31 @@ export const normalizeQueryResponseData = data => {
  * @returns {String}
  */
 export const addPrefixToCustomVariableParams = key => `variables[${key}]`;
+
+/**
+ * Normalize custom dashboard paths. This method helps support
+ * metrics dashboard to work with custom dashboard file names instead
+ * of the entire path.
+ *
+ * If dashboard is empty, it is the default dashboard.
+ * If dashboard is set, it usually is a custom dashboard unless
+ * explicitly it is set to default dashboard path.
+ *
+ * @param {String} dashboard dashboard path
+ * @param {String} dashboardPrefix custom dashboard directory prefix
+ * @returns {String} normalized dashboard path
+ */
+export const normalizeCustomDashboardPath = (dashboard, dashboardPrefix = '') => {
+  const currDashboard = dashboard || '';
+  let dashboardPath = `${dashboardPrefix}/${currDashboard}`;
+
+  if (!currDashboard) {
+    dashboardPath = '';
+  } else if (
+    currDashboard.startsWith(dashboardPrefix) ||
+    currDashboard.startsWith(DEFAULT_DASHBOARD_PATH)
+  ) {
+    dashboardPath = currDashboard;
+  }
+  return dashboardPath;
+};

changelogs/unreleased/support-file-names-for-metrics-dashboards.yml

+---
+title: Support metrics dashboard with file name
+merge_request: 34115
+author:
+type: added

spec/frontend/monitoring/mock_data.js

@@ -22,6 +22,8 @@ export const propsData = {
   validateQueryPath: '',
 };
 
+export const customDashboardBasePath = '.gitlab/dashboards';
+
 const customDashboardsData = new Array(30).fill(null).map((_, idx) => ({
   default: false,
   display_name: `Custom Dashboard ${idx}`,

spec/frontend/monitoring/store/actions_spec.js

@@ -6,6 +6,7 @@ import statusCodes from '~/lib/utils/http_status';
 import * as commonUtils from '~/lib/utils/common_utils';
 import createFlash from '~/flash';
 import { defaultTimeRange } from '~/vue_shared/constants';
+import * as getters from '~/monitoring/stores/getters';
 import { ENVIRONMENT_AVAILABLE_STATE } from '~/monitoring/constants';
 
 import { createStore } from '~/monitoring/stores';
@@ -62,7 +63,7 @@ describe('Monitoring store actions', () => {
   let state;
 
   beforeEach(() => {
-    store = createStore();
+    store = createStore({ getters });
     state = store.state.monitoringDashboard;
     mock = new MockAdapter(axios);
 
@@ -265,6 +266,11 @@ describe('Monitoring store actions', () => {
       state.projectPath = 'gitlab-org/gitlab-test';
       state.currentEnvironmentName = 'production';
       state.currentDashboard = '.gitlab/dashboards/custom_dashboard.yml';
+      // testAction doesn't have access to getters. The state is passed in as getters
+      // instead of the actual getters inside the testAction method implementation.
+      // All methods downstream that needs access to getters will throw and error.
+      // For that reason, the result of the getter is set as a state variable.
+      state.fullDashboardPath = store.getters['monitoringDashboard/fullDashboardPath'];
     });
 
     it('fetches annotations data and dispatches receiveAnnotationsSuccess', () => {
@@ -581,9 +587,12 @@ describe('Monitoring store actions', () => {
       let result;
       beforeEach(() => {
         const params = {};
+        const localGetters = {
+          fullDashboardPath: store.getters['monitoringDashboard/fullDashboardPath'],
+        };
         result = () => {
           mock.onGet(state.dashboardEndpoint).replyOnce(500, mockDashboardsErrorResponse);
-          return fetchDashboard({ state, commit, dispatch }, params);
+          return fetchDashboard({ state, commit, dispatch, getters: localGetters }, params);
         };
       });
 
@@ -712,10 +721,10 @@ describe('Monitoring store actions', () => {
     });
 
     it('commits empty state when state.groups is empty', done => {
-      const getters = {
+      const localGetters = {
         metricsWithData: () => [],
       };
-      fetchDashboardData({ state, commit, dispatch, getters })
+      fetchDashboardData({ state, commit, dispatch, getters: localGetters })
         .then(() => {
           expect(Tracking.event).toHaveBeenCalledWith(
             document.body.dataset.page,
@@ -740,11 +749,11 @@ describe('Monitoring store actions', () => {
       );
 
       const [metric] = state.dashboard.panelGroups[0].panels[0].metrics;
-      const getters = {
+      const localGetters = {
         metricsWithData: () => [metric.id],
       };
 
-      fetchDashboardData({ state, commit, dispatch, getters })
+      fetchDashboardData({ state, commit, dispatch, getters: localGetters })
         .then(() => {
           expect(dispatch).toHaveBeenCalledWith('fetchPrometheusMetric', {
             metric,

spec/frontend/monitoring/store/getters_spec.js

@@ -4,6 +4,7 @@ import mutations from '~/monitoring/stores/mutations';
 import * as types from '~/monitoring/stores/mutation_types';
 import { metricStates } from '~/monitoring/constants';
 import {
+  customDashboardBasePath,
   environmentData,
   metricsResult,
   dashboardGitResponse,
@@ -364,45 +365,53 @@ describe('Monitoring store Getters', () => {
 
   describe('selectedDashboard', () => {
     const { selectedDashboard } = getters;
+    const localGetters = state => ({
+      fullDashboardPath: getters.fullDashboardPath(state),
+    });
 
     it('returns a dashboard', () => {
       const state = {
         allDashboards: dashboardGitResponse,
         currentDashboard: dashboardGitResponse[0].path,
+        customDashboardBasePath,
       };
-      expect(selectedDashboard(state)).toEqual(dashboardGitResponse[0]);
+      expect(selectedDashboard(state, localGetters(state))).toEqual(dashboardGitResponse[0]);
     });
 
     it('returns a non-default dashboard', () => {
       const state = {
         allDashboards: dashboardGitResponse,
         currentDashboard: dashboardGitResponse[1].path,
+        customDashboardBasePath,
       };
-      expect(selectedDashboard(state)).toEqual(dashboardGitResponse[1]);
+      expect(selectedDashboard(state, localGetters(state))).toEqual(dashboardGitResponse[1]);
     });
 
     it('returns a default dashboard when no dashboard is selected', () => {
       const state = {
         allDashboards: dashboardGitResponse,
         currentDashboard: null,
+        customDashboardBasePath,
       };
-      expect(selectedDashboard(state)).toEqual(dashboardGitResponse[0]);
+      expect(selectedDashboard(state, localGetters(state))).toEqual(dashboardGitResponse[0]);
     });
 
     it('returns a default dashboard when dashboard cannot be found', () => {
       const state = {
         allDashboards: dashboardGitResponse,
         currentDashboard: 'wrong_path',
+        customDashboardBasePath,
       };
-      expect(selectedDashboard(state)).toEqual(dashboardGitResponse[0]);
+      expect(selectedDashboard(state, localGetters(state))).toEqual(dashboardGitResponse[0]);
     });
 
     it('returns null when no dashboards are present', () => {
       const state = {
         allDashboards: [],
         currentDashboard: dashboardGitResponse[0].path,
+        customDashboardBasePath,
       };
-      expect(selectedDashboard(state)).toEqual(null);
+      expect(selectedDashboard(state, localGetters(state))).toEqual(null);
     });
   });
 

spec/frontend/monitoring/store/utils_spec.js

@@ -8,6 +8,7 @@ import {
   normalizeQueryResponseData,
   convertToGrafanaTimeRange,
   addDashboardMetaDataToLink,
+  normalizeCustomDashboardPath,
 } from '~/monitoring/stores/utils';
 import { annotationsData } from '../mock_data';
 import { NOT_IN_DB_PREFIX } from '~/monitoring/constants';
@@ -700,3 +701,24 @@ describe('normalizeQueryResponseData', () => {
     ]);
   });
 });
+
+describe('normalizeCustomDashboardPath', () => {
+  it.each`
+    input                                                               | expected
+    ${[undefined]}                                                      | ${''}
+    ${[null]}                                                           | ${''}
+    ${[]}                                                               | ${''}
+    ${['links.yml']}                                                    | ${'links.yml'}
+    ${['links.yml', '.gitlab/dashboards']}                              | ${'.gitlab/dashboards/links.yml'}
+    ${['config/prometheus/common_metrics.yml']}                         | ${'config/prometheus/common_metrics.yml'}
+    ${['config/prometheus/common_metrics.yml', '.gitlab/dashboards']}   | ${'config/prometheus/common_metrics.yml'}
+    ${['dir1/links.yml', '.gitlab/dashboards']}                         | ${'.gitlab/dashboards/dir1/links.yml'}
+    ${['dir1/dir2/links.yml', '.gitlab/dashboards']}                    | ${'.gitlab/dashboards/dir1/dir2/links.yml'}
+    ${['.gitlab/dashboards/links.yml']}                                 | ${'.gitlab/dashboards/links.yml'}
+    ${['.gitlab/dashboards/links.yml', '.gitlab/dashboards']}           | ${'.gitlab/dashboards/links.yml'}
+    ${['.gitlab/dashboards/dir1/links.yml', '.gitlab/dashboards']}      | ${'.gitlab/dashboards/dir1/links.yml'}
+    ${['.gitlab/dashboards/dir1/dir2/links.yml', '.gitlab/dashboards']} | ${'.gitlab/dashboards/dir1/dir2/links.yml'}
+  `(`normalizeCustomDashboardPath returns $expected for $input`, ({ input, expected }) => {
+    expect(normalizeCustomDashboardPath(...input)).toEqual(expected);
+  });
+});