Add prefix to template vars in url

Currently, templating vars are parsed and appended
to the URL as is. However, this can cause collisions
with other URL params that are essential for other
features

app/assets/javascripts/monitoring/constants.js

@@ -216,3 +216,14 @@ export const VARIABLE_TYPES = {
   custom: 'custom',
   text: 'text',
 };
+
+/**
+ * The names of templating variables defined in the dashboard yml
+ * file are prefixed with a constant so that it doesn't collide with
+ * other URL params that the monitoring dashboard relies on for
+ * features like panel fullscreen etc.
+ *
+ * The prefix is added before it is appended to the URL and removed
+ * before passing the data to the backend.
+ */
+export const VARIABLE_PREFIX = 'var-';

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

+import { flatMap } from 'lodash';
+import { removePrefixFromLabels } from './utils';
 import { NOT_IN_DB_PREFIX } from '../constants';
 
 const metricsIdsInPanel = panel =>
@@ -110,16 +112,19 @@ export const filteredEnvironments = state =>
   );
 
 /**
- * Maps an variables object to an array
+ * Maps an variables object to an array along with stripping
+ * the variable prefix.
+ *
  * @param {Object} variables - Custom variables provided by the user
  * @returns {Array} The custom variables array to be send to the API
  * in the format of [variable1, variable1_value]
  */
 
 export const getCustomVariablesArray = state =>
-  Object.entries(state.promVariables)
-    .flat()
-    .map(encodeURIComponent);
+  flatMap(state.promVariables, (val, key) => [
+    encodeURIComponent(removePrefixFromLabels(key)),
+    encodeURIComponent(val),
+  ]);
 
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

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

@@ -2,7 +2,7 @@ import { slugify } from '~/lib/utils/text_utility';
 import createGqClient, { fetchPolicies } from '~/lib/graphql';
 import { SUPPORTED_FORMATS } from '~/lib/utils/unit_format';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
-import { NOT_IN_DB_PREFIX } from '../constants';
+import { NOT_IN_DB_PREFIX, VARIABLE_PREFIX } from '../constants';
 
 export const gqClient = createGqClient(
   {},
@@ -229,3 +229,22 @@ export const normalizeQueryResult = timeSeries => {
 
   return normalizedResult;
 };
+
+/**
+ * Variable labels are used as names for the dropdowns and also
+ * as URL params. Prefixing the name reduces the risk of
+ * collision with other URL params
+ *
+ * @param {String} label label for the template variable
+ * @returns {String}
+ */
+export const addPrefixToLabels = label => `${VARIABLE_PREFIX}${label}`;
+
+/**
+ * Before the templating variables are passed to the backend the
+ * prefix needs to be removed.
+ *
+ * @param {String} label label to remove prefix from
+ * @returns {String}
+ */
+export const removePrefixFromLabels = label => label.replace(VARIABLE_PREFIX, '');

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

 import { isString } from 'lodash';
+import { addPrefixToLabels } from './utils';
 import { VARIABLE_TYPES } from '../constants';
 
 /**
@@ -149,7 +150,7 @@ export const parseTemplatingVariables = ({ variables = {} } = {}) =>
     if (parsedVar) {
       acc[key] = {
         ...parsedVar,
-        label: parsedVar.label || key,
+        label: addPrefixToLabels(parsedVar.label || key),
       };
     }
     return acc;

app/assets/javascripts/monitoring/utils.js

-import { omit } from 'lodash';
+import { pickBy } from 'lodash';
 import { queryToObject, mergeUrlParams, removeParams } from '~/lib/utils/url_utility';
 import {
   timeRangeParamNames,
   timeRangeFromParams,
   timeRangeToParams,
 } from '~/lib/utils/datetime_range';
+import { VARIABLE_PREFIX } from './constants';
 
 /**
  * List of non time range url parameters
@@ -122,19 +123,16 @@ export const timeRangeFromUrl = (search = window.location.search) => {
 };
 
 /**
- * Returns an array with user defined variables from the URL
+ * User-defined variables from the URL are extracted. The variables
+ * begin with a constant prefix so that it doesn't collide with
+ * other URL params.
  *
- * @returns {Array} The custom variables defined by the
- * user in the URL
  * @param {String} New URL
+ * @returns {Object} The custom variables defined by the user in the URL
  */
 
-export const promCustomVariablesFromUrl = (search = window.location.search) => {
-  const params = queryToObject(search);
-  const paramsToRemove = timeRangeParamNames.concat(dashboardParams);
-
-  return omit(params, paramsToRemove);
-};
+export const promCustomVariablesFromUrl = (search = window.location.search) =>
+  pickBy(queryToObject(search), (val, key) => key.startsWith(VARIABLE_PREFIX));
 
 /**
  * Returns a URL with no time range based on the current URL.

changelogs/unreleased/dbodicherla-add-prefix-for-templating-variables.yml

+---
+title: Add prefix to template variables in URL in the monitoring dashboard
+merge_request: 31690
+author:
+type: changed

spec/frontend/monitoring/components/variables_section_spec.js

@@ -15,8 +15,8 @@ describe('Metrics dashboard/variables section component', () => {
   let store;
   let wrapper;
   const sampleVariables = {
-    label1: 'pod',
-    label2: 'main',
+    'var-label1': 'pod',
+    'var-label2': 'main',
   };
 
   const createShallowWrapper = () => {

spec/frontend/monitoring/mock_data.js

@@ -642,7 +642,7 @@ const generateMockTemplatingData = data => {
 
 const responseForSimpleTextVariable = {
   simpleText: {
-    label: 'simpleText',
+    label: 'var-simpleText',
     type: 'text',
     value: 'Simple text',
   },
@@ -650,7 +650,7 @@ const responseForSimpleTextVariable = {
 
 const responseForAdvTextVariable = {
   advText: {
-    label: 'Variable 4',
+    label: 'var-Variable 4',
     type: 'text',
     value: 'default',
   },
@@ -658,7 +658,7 @@ const responseForAdvTextVariable = {
 
 const responseForSimpleCustomVariable = {
   simpleCustom: {
-    label: 'simpleCustom',
+    label: 'var-simpleCustom',
     options: [
       {
         default: false,
@@ -682,7 +682,7 @@ const responseForSimpleCustomVariable = {
 
 const responseForAdvancedCustomVariableWithoutOptions = {
   advCustomWithoutOpts: {
-    label: 'advCustomWithoutOpts',
+    label: 'var-advCustomWithoutOpts',
     options: [],
     type: 'custom',
   },
@@ -690,7 +690,7 @@ const responseForAdvancedCustomVariableWithoutOptions = {
 
 const responseForAdvancedCustomVariableWithoutLabel = {
   advCustomWithoutLabel: {
-    label: 'advCustomWithoutLabel',
+    label: 'var-advCustomWithoutLabel',
     options: [
       {
         default: false,
@@ -710,7 +710,7 @@ const responseForAdvancedCustomVariableWithoutLabel = {
 const responseForAdvancedCustomVariable = {
   ...responseForSimpleCustomVariable,
   advCustomNormal: {
-    label: 'Advanced Var',
+    label: 'var-Advanced Var',
     options: [
       {
         default: false,

spec/frontend/monitoring/store/getters_spec.js

@@ -327,7 +327,8 @@ describe('Monitoring store Getters', () => {
   describe('getCustomVariablesArray', () => {
     let state;
     const sampleVariables = {
-      label1: 'pod',
+      'var-label1': 'pod',
+      'var-label2': 'env',
     };
 
     beforeEach(() => {
@@ -340,7 +341,7 @@ describe('Monitoring store Getters', () => {
       mutations[types.SET_PROM_QUERY_VARIABLES](state, sampleVariables);
       const variablesArray = getters.getCustomVariablesArray(state);
 
-      expect(variablesArray).toEqual(['label1', 'pod']);
+      expect(variablesArray).toEqual(['label1', 'pod', 'label2', 'env']);
     });
 
     it('transforms the promVariables object to an empty array when no keys are present', () => {

spec/frontend/monitoring/utils_spec.js

@@ -192,9 +192,10 @@ describe('monitoring/utils', () => {
         direction: 'left',
         anchor: 'top',
         pod: 'POD',
+        'var-pod': 'POD',
       });
 
-      expect(promCustomVariablesFromUrl()).toEqual(expect.objectContaining({ pod: 'POD' }));
+      expect(promCustomVariablesFromUrl()).toEqual(expect.objectContaining({ 'var-pod': 'POD' }));
     });
 
     it('returns an empty object when no custom variables are present', () => {