Skip to content

G
gitlab-ce
Commit 2aafa3bb authored by Andrew Fontaine's avatar Andrew Fontaine
Browse files
Options

Merge branch 'revert-e58a2eea' into 'master' 

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

See merge request gitlab-org/gitlab!37362
parents 373aea95 da0413d9
Hide whitespace changes
Inline Side-by-side
Showing with 32 additions and 325 deletions
app/assets/javascripts/helpers/monitor_helper.js
View file @ 2aafa3bb
......@@ -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
View file @ 2aafa3bb
......@@ -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 deleted 100644 → 0
View file @ 373aea95
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 deleted 100644 → 0
View file @ 373aea95
---
title: Fix CSV downloads for multiple series in the same chart
merge_request: 36556
author:
type: fixed
spec/frontend/helpers/monitor_helper_spec.js
View file @ 2aafa3bb
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
View file @ 2aafa3bb
......@@ -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 deleted 100644 → 0
View file @ 373aea95
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
View file @ 2aafa3bb
......@@ -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(),
})),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7