Revert "Merge branch '214627-fix-incorrect-csv-export' into 'master'"

This reverts merge request !36556

app/assets/javascripts/helpers/monitor_helper.js

@@ -49,7 +49,7 @@ const multiMetricLabel = metricAttributes => {
  * @param {Object} metricAttributes - Default metric attribute values (e.g. method, instance)
  * @returns {String} The formatted query label
  */
-export const getSeriesLabel = (queryLabel, metricAttributes) => {
+const getSeriesLabel = (queryLabel, metricAttributes) => {
   return (
     singleAttributeLabel(queryLabel, metricAttributes) ||
     templatedLabel(queryLabel, metricAttributes) ||
@@ -63,6 +63,7 @@ export const getSeriesLabel = (queryLabel, metricAttributes) => {
  * @param {Object} defaultConfig - Default chart config values (e.g. lineStyle, name)
  * @returns {Array} The formatted values
  */
+// eslint-disable-next-line import/prefer-default-export
 export const makeDataSeries = (queryResults, defaultConfig) =>
   queryResults.map(result => {
     return {

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

@@ -30,7 +30,6 @@ import MonitorStackedColumnChart from './charts/stacked_column.vue';
 import TrackEventDirective from '~/vue_shared/directives/track_event';
 import AlertWidget from './alert_widget.vue';
 import { timeRangeToUrl, downloadCSVOptions, generateLinkToChartOptions } from '../utils';
-import { graphDataToCsv } from '../csv_export';
 
 const events = {
   timeRangeZoom: 'timerangezoom',
@@ -149,10 +148,13 @@ export default {
       return null;
     },
     csvText() {
-      if (this.graphData) {
-        return graphDataToCsv(this.graphData);
-      }
-      return null;
+      const chartData = this.graphData?.metrics[0].result[0].values || [];
+      const yLabel = this.graphData.y_label;
+      const header = `timestamp,${yLabel}\r\n`; // eslint-disable-line @gitlab/require-i18n-strings
+      return chartData.reduce((csv, data) => {
+        const row = data.join(',');
+        return `${csv}${row}\r\n`;
+      }, header);
     },
     downloadCsv() {
       const data = new Blob([this.csvText], { type: 'text/plain' });

app/assets/javascripts/monitoring/csv_export.js

-import { getSeriesLabel } from '~/helpers/monitor_helper';
-
-/**
- * Returns a label for a header of the csv.
- *
- * Includes double quotes ("") in case the header includes commas or other separator.
- *
- * @param {String} axisLabel
- * @param {String} metricLabel
- * @param {Object} metricAttributes
- */
-const csvHeader = (axisLabel, metricLabel, metricAttributes = {}) =>
-  `${axisLabel} > ${getSeriesLabel(metricLabel, metricAttributes)}`;
-
-/**
- * Returns an array with the header labels given a list of metrics
- *
- * ```
- * metrics = [
- *   {
- *      label: "..." // user-defined label
- *      result: [
- *        {
- *           metric: { ... } // metricAttributes
- *        },
- *        ...
- *      ]
- *   },
- *   ...
- * ]
- * ```
- *
- * When metrics have a `label` or `metricAttributes`, they are
- * used to generate the column name.
- *
- * @param {String} axisLabel - Main label
- * @param {Array} metrics - Metrics with results
- */
-const csvMetricHeaders = (axisLabel, metrics) =>
-  metrics.flatMap(({ label, result }) =>
-    // The `metric` in a `result` is a map of `metricAttributes`
-    // contains key-values to identify the series, rename it
-    // here for clarity.
-    result.map(({ metric: metricAttributes }) => {
-      return csvHeader(axisLabel, label, metricAttributes);
-    }),
-  );
-
-/**
- * Returns a (flat) array with all the values arrays in each
- * metric and series
- *
- * ```
- * metrics = [
- *   {
- *      result: [
- *        {
- *           values: [ ... ] // `values`
- *        },
- *        ...
- *      ]
- *   },
- *   ...
- * ]
- * ```
- *
- * @param {Array} metrics - Metrics with results
- */
-const csvMetricValues = metrics =>
-  metrics.flatMap(({ result }) => result.map(res => res.values || []));
-
-/**
- * Returns headers and rows for csv, sorted by their timestamp.
- *
- * {
- *   headers: ["timestamp", "<col_1_name>", "col_2_name"],
- *   rows: [
- *     [ <timestamp>, <col_1_value>, <col_2_value> ],
- *     [ <timestamp>, <col_1_value>, <col_2_value> ]
- *     ...
- *   ]
- * }
- *
- * @param {Array} metricHeaders
- * @param {Array} metricValues
- */
-const csvData = (metricHeaders, metricValues) => {
-  const rowsByTimestamp = {};
-
-  metricValues.forEach((values, colIndex) => {
-    values.forEach(([timestamp, value]) => {
-      if (!rowsByTimestamp[timestamp]) {
-        rowsByTimestamp[timestamp] = [];
-      }
-      // `value` should be in the right column
-      rowsByTimestamp[timestamp][colIndex] = value;
-    });
-  });
-
-  const rows = Object.keys(rowsByTimestamp)
-    .sort()
-    .map(timestamp => {
-      // force each row to have the same number of entries
-      rowsByTimestamp[timestamp].length = metricHeaders.length;
-      // add timestamp as the first entry
-      return [timestamp, ...rowsByTimestamp[timestamp]];
-    });
-
-  // Escape double quotes and enclose headers:
-  // "If double-quotes are used to enclose fields, then a double-quote
-  // appearing inside a field must be escaped by preceding it with
-  // another double quote."
-  // https://tools.ietf.org/html/rfc4180#page-2
-  const headers = metricHeaders.map(header => `"${header.replace(/"/g, '""')}"`);
-
-  return {
-    headers: ['timestamp', ...headers],
-    rows,
-  };
-};
-
-/**
- * Returns dashboard panel's data in a string in CSV format
- *
- * @param {Object} graphData - Panel contents
- * @returns {String}
- */
-// eslint-disable-next-line import/prefer-default-export
-export const graphDataToCsv = graphData => {
-  const delimiter = ',';
-  const br = '\r\n';
-  const { metrics = [], y_label: axisLabel } = graphData;
-
-  const metricsWithResults = metrics.filter(metric => metric.result);
-  const metricHeaders = csvMetricHeaders(axisLabel, metricsWithResults);
-  const metricValues = csvMetricValues(metricsWithResults);
-  const { headers, rows } = csvData(metricHeaders, metricValues);
-
-  if (rows.length === 0) {
-    return '';
-  }
-
-  const headerLine = headers.join(delimiter) + br;
-  const lines = rows.map(row => row.join(delimiter));
-
-  return headerLine + lines.join(br) + br;
-};

changelogs/unreleased/214627-fix-incorrect-csv-export.yml

----
-title: Fix CSV downloads for multiple series in the same chart
-merge_request: 36556
-author:
-type: fixed

spec/frontend/helpers/monitor_helper_spec.js

-import { getSeriesLabel, makeDataSeries } from '~/helpers/monitor_helper';
+import * as monitorHelper from '~/helpers/monitor_helper';
 
 describe('monitor helper', () => {
   const defaultConfig = { default: true, name: 'default name' };
   const name = 'data name';
   const series = [[1, 1], [2, 2], [3, 3]];
-
-  describe('getSeriesLabel', () => {
-    const metricAttributes = { __name__: 'up', app: 'prometheus' };
-
-    it('gets a single attribute label', () => {
-      expect(getSeriesLabel('app', metricAttributes)).toBe('app: prometheus');
-    });
-
-    it('gets a templated label', () => {
-      expect(getSeriesLabel('{{__name__}}', metricAttributes)).toBe('up');
-      expect(getSeriesLabel('{{app}}', metricAttributes)).toBe('prometheus');
-      expect(getSeriesLabel('{{missing}}', metricAttributes)).toBe('{{missing}}');
-    });
-
-    it('gets a multiple label', () => {
-      expect(getSeriesLabel(null, metricAttributes)).toBe('__name__: up, app: prometheus');
-      expect(getSeriesLabel('', metricAttributes)).toBe('__name__: up, app: prometheus');
-    });
-
-    it('gets a simple label', () => {
-      expect(getSeriesLabel('A label', {})).toBe('A label');
-    });
-  });
+  const data = ({ metric = { default_name: name }, values = series } = {}) => [{ metric, values }];
 
   describe('makeDataSeries', () => {
-    const data = ({ metric = { default_name: name }, values = series } = {}) => [
-      { metric, values },
-    ];
-
     const expectedDataSeries = [
       {
         ...defaultConfig,
@@ -41,17 +15,19 @@ describe('monitor helper', () => {
     ];
 
     it('converts query results to data series', () => {
-      expect(makeDataSeries(data({ metric: {} }), defaultConfig)).toEqual(expectedDataSeries);
+      expect(monitorHelper.makeDataSeries(data({ metric: {} }), defaultConfig)).toEqual(
+        expectedDataSeries,
+      );
     });
 
     it('returns an empty array if no query results exist', () => {
-      expect(makeDataSeries([], defaultConfig)).toEqual([]);
+      expect(monitorHelper.makeDataSeries([], defaultConfig)).toEqual([]);
     });
 
     it('handles multi-series query results', () => {
       const expectedData = { ...expectedDataSeries[0], name: 'default name: data name' };
 
-      expect(makeDataSeries([...data(), ...data()], defaultConfig)).toEqual([
+      expect(monitorHelper.makeDataSeries([...data(), ...data()], defaultConfig)).toEqual([
         expectedData,
         expectedData,
       ]);
@@ -63,7 +39,10 @@ describe('monitor helper', () => {
         name: '{{cmd}}',
       };
 
-      const [result] = makeDataSeries([{ metric: { cmd: 'brpop' }, values: series }], config);
+      const [result] = monitorHelper.makeDataSeries(
+        [{ metric: { cmd: 'brpop' }, values: series }],
+        config,
+      );
 
       expect(result.name).toEqual('brpop');
     });
@@ -74,7 +53,7 @@ describe('monitor helper', () => {
         name: '',
       };
 
-      const [result] = makeDataSeries(
+      const [result] = monitorHelper.makeDataSeries(
         [
           {
             metric: {
@@ -100,7 +79,7 @@ describe('monitor helper', () => {
         name: 'backend: {{ backend }}',
       };
 
-      const [result] = makeDataSeries(
+      const [result] = monitorHelper.makeDataSeries(
         [{ metric: { backend: 'HA Server' }, values: series }],
         config,
       );
@@ -111,7 +90,10 @@ describe('monitor helper', () => {
     it('supports repeated template variables', () => {
       const config = { ...defaultConfig, name: '{{cmd}}, {{cmd}}' };
 
-      const [result] = makeDataSeries([{ metric: { cmd: 'brpop' }, values: series }], config);
+      const [result] = monitorHelper.makeDataSeries(
+        [{ metric: { cmd: 'brpop' }, values: series }],
+        config,
+      );
 
       expect(result.name).toEqual('brpop, brpop');
     });
@@ -119,7 +101,7 @@ describe('monitor helper', () => {
     it('supports hyphenated template variables', () => {
       const config = { ...defaultConfig, name: 'expired - {{ test-attribute }}' };
 
-      const [result] = makeDataSeries(
+      const [result] = monitorHelper.makeDataSeries(
         [{ metric: { 'test-attribute': 'test-attribute-value' }, values: series }],
         config,
       );
@@ -133,7 +115,7 @@ describe('monitor helper', () => {
         name: '{{job}}: {{cmd}}',
       };
 
-      const [result] = makeDataSeries(
+      const [result] = monitorHelper.makeDataSeries(
         [{ metric: { cmd: 'brpop', job: 'redis' }, values: series }],
         config,
       );
@@ -147,7 +129,7 @@ describe('monitor helper', () => {
         name: '{{cmd}}',
       };
 
-      const [firstSeries, secondSeries] = makeDataSeries(
+      const [firstSeries, secondSeries] = monitorHelper.makeDataSeries(
         [
           { metric: { cmd: 'brpop' }, values: series },
           { metric: { cmd: 'zrangebyscore' }, values: series },

spec/frontend/monitoring/components/dashboard_panel_spec.js

@@ -443,7 +443,7 @@ describe('Dashboard Panel', () => {
 
     describe('csvText', () => {
       it('converts metrics data from json to csv', () => {
-        const header = `timestamp,"${graphData.y_label} > ${graphData.metrics[0].label}"`;
+        const header = `timestamp,${graphData.y_label}`;
         const data = graphData.metrics[0].result[0].values;
         const firstRow = `${data[0][0]},${data[0][1]}`;
         const secondRow = `${data[1][0]},${data[1][1]}`;

spec/frontend/monitoring/csv_export_spec.js

-import { timeSeriesGraphData } from './graph_data';
-import { graphDataToCsv } from '~/monitoring/csv_export';
-
-describe('monitoring export_csv', () => {
-  describe('graphDataToCsv', () => {
-    const expectCsvToMatchLines = (csv, lines) => expect(`${lines.join('\r\n')}\r\n`).toEqual(csv);
-
-    it('should return a csv with 0 metrics', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 0 });
-
-      expect(graphDataToCsv(data)).toEqual('');
-    });
-
-    it('should return a csv with 1 metric with no data', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 1 });
-
-      // When state is NO_DATA, result is null
-      data.metrics[0].result = null;
-
-      expect(graphDataToCsv(data)).toEqual('');
-    });
-
-    it('should return a csv with multiple metrics and one with no data', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 2 });
-
-      // When state is NO_DATA, result is null
-      data.metrics[0].result = null;
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > Metric 2"`,
-        '2015-07-01T20:10:51.781Z,1',
-        '2015-07-01T20:11:06.781Z,2',
-        '2015-07-01T20:11:21.781Z,3',
-      ]);
-    });
-
-    it('should return a csv when not all metrics have the same timestamps', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 3 });
-
-      // Add an "odd" timestamp that is not in the dataset
-      Object.assign(data.metrics[2].result[0], {
-        value: ['2016-01-01T00:00:00.000Z', 9],
-        values: [['2016-01-01T00:00:00.000Z', 9]],
-      });
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > Metric 1","Y Axis > Metric 2","Y Axis > Metric 3"`,
-        '2015-07-01T20:10:51.781Z,1,1,',
-        '2015-07-01T20:11:06.781Z,2,2,',
-        '2015-07-01T20:11:21.781Z,3,3,',
-        '2016-01-01T00:00:00.000Z,,,9',
-      ]);
-    });
-
-    it('should return a csv with 1 metric', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 1 });
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > Metric 1"`,
-        '2015-07-01T20:10:51.781Z,1',
-        '2015-07-01T20:11:06.781Z,2',
-        '2015-07-01T20:11:21.781Z,3',
-      ]);
-    });
-
-    it('should escape double quotes in metric labels with two double quotes ("")', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 1 });
-
-      data.metrics[0].label = 'My "quoted" metric';
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > My ""quoted"" metric"`,
-        '2015-07-01T20:10:51.781Z,1',
-        '2015-07-01T20:11:06.781Z,2',
-        '2015-07-01T20:11:21.781Z,3',
-      ]);
-    });
-
-    it('should return a csv with multiple metrics', () => {
-      const data = timeSeriesGraphData({}, { metricCount: 3 });
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > Metric 1","Y Axis > Metric 2","Y Axis > Metric 3"`,
-        '2015-07-01T20:10:51.781Z,1,1,1',
-        '2015-07-01T20:11:06.781Z,2,2,2',
-        '2015-07-01T20:11:21.781Z,3,3,3',
-      ]);
-    });
-
-    it('should return a csv with 1 metric and multiple series with labels', () => {
-      const data = timeSeriesGraphData({}, { isMultiSeries: true });
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > Metric 1","Y Axis > Metric 1"`,
-        '2015-07-01T20:10:51.781Z,1,4',
-        '2015-07-01T20:11:06.781Z,2,5',
-        '2015-07-01T20:11:21.781Z,3,6',
-      ]);
-    });
-
-    it('should return a csv with 1 metric and multiple series', () => {
-      const data = timeSeriesGraphData({}, { isMultiSeries: true, withLabels: false });
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > __name__: up, job: prometheus, instance: localhost:9090","Y Axis > __name__: up, job: node, instance: localhost:9091"`,
-        '2015-07-01T20:10:51.781Z,1,4',
-        '2015-07-01T20:11:06.781Z,2,5',
-        '2015-07-01T20:11:21.781Z,3,6',
-      ]);
-    });
-
-    it('should return a csv with multiple metrics and multiple series', () => {
-      const data = timeSeriesGraphData(
-        {},
-        { metricCount: 3, isMultiSeries: true, withLabels: false },
-      );
-
-      expectCsvToMatchLines(graphDataToCsv(data), [
-        `timestamp,"Y Axis > __name__: up, job: prometheus, instance: localhost:9090","Y Axis > __name__: up, job: node, instance: localhost:9091","Y Axis > __name__: up, job: prometheus, instance: localhost:9090","Y Axis > __name__: up, job: node, instance: localhost:9091","Y Axis > __name__: up, job: prometheus, instance: localhost:9090","Y Axis > __name__: up, job: node, instance: localhost:9091"`,
-        '2015-07-01T20:10:51.781Z,1,4,1,4,1,4',
-        '2015-07-01T20:11:06.781Z,2,5,2,5,2,5',
-        '2015-07-01T20:11:21.781Z,3,6,3,6,3,6',
-      ]);
-    });
-  });
-});

spec/frontend/monitoring/graph_data.js

@@ -83,7 +83,7 @@ const matrixMultiResult = ({ values1 = ['1', '2', '3'], values2 = ['4', '5', '6'
  * @param {Object} dataOptions.isMultiSeries
  */
 export const timeSeriesGraphData = (panelOptions = {}, dataOptions = {}) => {
-  const { metricCount = 1, isMultiSeries = false, withLabels = true } = dataOptions;
+  const { metricCount = 1, isMultiSeries = false } = dataOptions;
 
   return mapPanelToViewModel({
     title: 'Time Series Panel',
@@ -91,7 +91,7 @@ export const timeSeriesGraphData = (panelOptions = {}, dataOptions = {}) => {
     x_label: 'X Axis',
     y_label: 'Y Axis',
     metrics: Array.from(Array(metricCount), (_, i) => ({
-      label: withLabels ? `Metric ${i + 1}` : undefined,
+      label: `Metric ${i + 1}`,
       state: metricStates.OK,
       result: isMultiSeries ? matrixMultiResult() : matrixSingleResult(),
     })),