Skip to content

Commit

Permalink
[7.17] [Infra] Limit the number of metrics accepted by Metrics Explor…
Browse files Browse the repository at this point in the history
…er API (elastic#188112) (elastic#188447)

# Backport

This will backport the following commits from `main` to `7.17`:
- [[Infra] Limit the number of metrics accepted by Metrics Explorer API
(elastic#188112)](elastic#188112)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Carlos
Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-16T13:44:05Z","message":"[Infra]
Limit the number of metrics accepted by Metrics Explorer API
(elastic#188112)\n\npart of
[3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img
width=\"1725\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n###
Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer`
route in infra. I\r\nremoved it. It should've been removed when the
`metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants`
field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to
test\r\n\r\n- Start a local Kibana instance pointing to an oblt
cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to
select more than 20 fields in the metrics
field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:all-open","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0"],"number":188112,"url":"https://github.com/elastic/kibana/pull/188112","mergeCommit":{"message":"[Infra]
Limit the number of metrics accepted by Metrics Explorer API
(elastic#188112)\n\npart of
[3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img
width=\"1725\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n###
Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer`
route in infra. I\r\nremoved it. It should've been removed when the
`metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants`
field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to
test\r\n\r\n- Start a local Kibana instance pointing to an oblt
cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to
select more than 20 fields in the metrics
field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188112","number":188112,"mergeCommit":{"message":"[Infra]
Limit the number of metrics accepted by Metrics Explorer API
(elastic#188112)\n\npart of
[3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img
width=\"1725\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n###
Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer`
route in infra. I\r\nremoved it. It should've been removed when the
`metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants`
field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to
test\r\n\r\n- Start a local Kibana instance pointing to an oblt
cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to
select more than 20 fields in the metrics
field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},{"url":"https://github.com/elastic/kibana/pull/188442","number":188442,"branch":"8.15","state":"OPEN"}]}]
BACKPORT-->

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
3 people authored and adelisle committed Aug 5, 2024
1 parent 314d9e6 commit 1348b58
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 46 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/infra/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ export const LOGS_FEATURE_ID = 'logs';
export type InfraFeatureId = typeof METRICS_FEATURE_ID | typeof LOGS_FEATURE_ID;

export const SNAPSHOT_API_MAX_METRICS = 20;
export const METRICS_EXPLORER_API_MAX_METRICS = 20;
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@
* 2.0.
*/

import { EuiComboBox } from '@elastic/eui';
import {
EuiComboBox,
EuiFlexGroup,
EuiFlexItem,
EuiText,
EuiIcon,
EuiComboBoxOptionOption,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import React, { useCallback, useState } from 'react';
import React, { useCallback, useState, useMemo } from 'react';
import { IFieldType } from 'src/plugins/data/public';
import { METRICS_EXPLORER_API_MAX_METRICS } from '../../../../../common/constants';
import { colorTransformer, Color } from '../../../../../common/color_palette';
import { MetricsExplorerMetric } from '../../../../../common/http_api/metrics_explorer';
import { MetricsExplorerOptions } from '../hooks/use_metrics_explorer_options';
Expand All @@ -26,10 +34,21 @@ interface SelectedOption {
label: string;
}

const placeholderText = i18n.translate('xpack.infra.metricsExplorer.metricComboBoxPlaceholder', {
defaultMessage: 'choose a metric to plot',
});

const comboValidationText = i18n.translate('xpack.infra.metricsExplorer.maxItemsSelected', {
defaultMessage: 'Maximum number of {maxMetrics} metrics reached.',
values: { maxMetrics: METRICS_EXPLORER_API_MAX_METRICS },
});

export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus = false }: Props) => {
const colors = Object.keys(Color) as Array<keyof typeof Color>;
const [shouldFocus, setShouldFocus] = useState(autoFocus);

const maxMetricsReached = options.metrics.length >= METRICS_EXPLORER_API_MAX_METRICS;

// the EuiCombobox forwards the ref to an input element
const autoFocusInputElement = useCallback(
(inputElement: HTMLInputElement | null) => {
Expand All @@ -54,7 +73,17 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus =
[onChange, options.aggregation, colors]
);

const comboOptions = fields.map((field) => ({ label: field.name, value: field.name }));
const comboOptions = useMemo(
(): EuiComboBoxOptionOption[] =>
maxMetricsReached
? [{ label: comboValidationText, disabled: true }]
: fields.map((field) => ({
label: field.name,
value: field.name,
})),
[maxMetricsReached, fields]
);

const selectedOptions = options.metrics
.filter((m) => m.aggregation !== 'count')
.map((metric) => ({
Expand All @@ -63,9 +92,37 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus =
color: colorTransformer(metric.color || Color.color0),
}));

const placeholderText = i18n.translate('xpack.infra.metricsExplorer.metricComboBoxPlaceholder', {
defaultMessage: 'choose a metric to plot',
});
const handleOnKeyDown = (ev: React.KeyboardEvent<HTMLDivElement>) => {
if (maxMetricsReached) {
ev.preventDefault();
}

return ev;
};

const renderFields = useCallback((option: EuiComboBoxOptionOption) => {
const { label, disabled } = option;

if (disabled) {
return (
<EuiFlexGroup
direction="column"
justifyContent="center"
alignItems="center"
data-test-subj="infraMetricsExplorerMaxMetricsReached"
>
<EuiFlexItem>
<EuiFlexGroup gutterSize="xs" justifyContent="center" alignItems="center">
<EuiIcon type="iInCircle" size="s" />
<EuiText size="xs">{label}</EuiText>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
);
}

return label;
}, []);

return (
<EuiComboBox
Expand All @@ -77,8 +134,10 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus =
options={comboOptions}
selectedOptions={selectedOptions}
onChange={handleChange}
isClearable={true}
onKeyDown={handleOnKeyDown}
isClearable
inputRef={autoFocusInputElement}
renderOption={renderFields}
/>
);
};
91 changes: 54 additions & 37 deletions x-pack/plugins/infra/server/routes/metrics_explorer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ import { findIntervalForMetrics } from './lib/find_interval_for_metrics';
import { query } from '../../lib/metrics';
import { queryTotalGroupings } from './lib/query_total_groupings';
import { transformSeries } from './lib/transform_series';
import { METRICS_EXPLORER_API_MAX_METRICS } from '../../../common/constants';

const escapeHatch = schema.object({}, { unknowns: 'allow' });

export const initMetricExplorerRoute = (libs: InfraBackendLibs) => {
const { framework } = libs;
const { framework, handleEsError } = libs;
framework.registerRoute(
{
method: 'post',
Expand All @@ -37,49 +38,65 @@ export const initMetricExplorerRoute = (libs: InfraBackendLibs) => {
},
},
async (requestContext, request, response) => {
const options = pipe(
metricsExplorerRequestBodyRT.decode(request.body),
fold(throwErrors(Boom.badRequest), identity)
);
try {
const options = pipe(
metricsExplorerRequestBodyRT.decode(request.body),
fold(throwErrors(Boom.badRequest), identity)
);

const client = createSearchClient(requestContext, framework);
const interval = await findIntervalForMetrics(client, options);
if (options.metrics.length > METRICS_EXPLORER_API_MAX_METRICS) {
throw Boom.badRequest(
`'metrics' size is greater than maximum of ${METRICS_EXPLORER_API_MAX_METRICS} allowed.`
);
}

const optionsWithInterval = options.forceInterval
? options
: {
...options,
timerange: {
...options.timerange,
interval: interval ? `>=${interval}s` : options.timerange.interval,
},
};
const client = createSearchClient(requestContext, framework);
const interval = await findIntervalForMetrics(client, options);

const metricsApiOptions = convertRequestToMetricsAPIOptions(optionsWithInterval);
const metricsApiResponse = await query(client, metricsApiOptions);
const totalGroupings = await queryTotalGroupings(client, metricsApiOptions);
const hasGroupBy =
Array.isArray(metricsApiOptions.groupBy) && metricsApiOptions.groupBy.length > 0;
const optionsWithInterval = options.forceInterval
? options
: {
...options,
timerange: {
...options.timerange,
interval: interval ? `>=${interval}s` : options.timerange.interval,
},
};

const pageInfo: MetricsExplorerPageInfo = {
total: totalGroupings,
afterKey: null,
};
const metricsApiOptions = convertRequestToMetricsAPIOptions(optionsWithInterval);
const metricsApiResponse = await query(client, metricsApiOptions);
const totalGroupings = await queryTotalGroupings(client, metricsApiOptions);
const hasGroupBy =
Array.isArray(metricsApiOptions.groupBy) && metricsApiOptions.groupBy.length > 0;

if (metricsApiResponse.info.afterKey) {
pageInfo.afterKey = metricsApiResponse.info.afterKey;
}
const pageInfo: MetricsExplorerPageInfo = {
total: totalGroupings,
afterKey: null,
};

if (metricsApiResponse.info.afterKey) {
pageInfo.afterKey = metricsApiResponse.info.afterKey;
}

// If we have a groupBy but there are ZERO groupings returned then we need to
// return an empty array. Otherwise we transform the series to match the current schema.
const series =
hasGroupBy && totalGroupings === 0
? []
: metricsApiResponse.series.map(transformSeries(hasGroupBy));
// If we have a groupBy but there are ZERO groupings returned then we need to
// return an empty array. Otherwise we transform the series to match the current schema.
const series =
hasGroupBy && totalGroupings === 0
? []
: metricsApiResponse.series.map(transformSeries(hasGroupBy));

return response.ok({
body: metricsExplorerResponseRT.encode({ series, pageInfo }),
});
return response.ok({
body: metricsExplorerResponseRT.encode({ series, pageInfo }),
});
} catch (err) {
if (Boom.isBoom(err)) {
return response.customError({
statusCode: err.output.statusCode,
body: { message: err.output.payload.message },
});
}
return handleEsError({ error: err, response });
}
}
);
};
5 changes: 3 additions & 2 deletions x-pack/test/api_integration/apis/metrics_ui/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ export default function ({ getService }: FtrProviderContext) {
after(() => esArchiver.unload('x-pack/test/functional/es_archives/infra/7.0.0/hosts'));

const fetchNodeDetails = async (
body: NodeDetailsRequest
body: NodeDetailsRequest,
expectedStatusCode = 200
): Promise<NodeDetailsMetricDataResponse | undefined> => {
const response = await supertest
.post('/api/metrics/node_details')
.set('kbn-xsrf', 'xxx')
.send(body)
.expect(200);
.expect(expectedStatusCode);
return response.body;
};

Expand Down
25 changes: 25 additions & 0 deletions x-pack/test/api_integration/apis/metrics_ui/metrics_explorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,31 @@ export default function ({ getService }: FtrProviderContext) {
total: 4,
});
});

it('should return 400 when requesting more than 20 metrics', async () => {
const postBody = {
timerange: {
field: '@timestamp',
to: max,
from: min,
interval: '>=1m',
},
indexPattern: 'metricbeat-*',
groupBy: ['host.name', 'system.network.name'],
limit: 3,
afterKey: null,
metrics: Array(21).fill({
aggregation: 'rate',
field: 'system.network.out.bytes',
}),
};

await supertest
.post('/api/infra/metrics_explorer')
.set('kbn-xsrf', 'xxx')
.send(postBody)
.expect(400);
});
});

describe('without data', () => {
Expand Down
33 changes: 33 additions & 0 deletions x-pack/test/functional/apps/infra/metrics_explorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,39 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
const chartType = await pageObjects.infraMetricsExplorer.getChartType(charts[0]);
expect(chartType).to.equal('bar chart');
});

it('should not allow adding more than 20 metrics', async () => {
await pageObjects.infraMetricsExplorer.clearMetrics();

const fields = [
'kubernetes.pod.cpu.usage.limit.pct',
'kubernetes.pod.memory.usage.bytes',
'kubernetes.pod.memory.usage.limit.pct',
'system.core.user.pct',
'system.core.nice.pct',
'system.core.idle.pct',
'system.core.iowait.pct',
'system.core.irq.pct',
'system.core.softirq.pct',
'system.core.steal.pct',
'system.cpu.nice.pct',
'system.cpu.idle.pct',
'system.cpu.iowait.pct',
'system.cpu.irq.pct',
'system.cpu.softirq.pct',
'system.cpu.steal.pct',
'system.cpu.user.norm.pct',
'kubernetes.pod.memory.usage.node.pct',
'kubernetes.pod.cpu.usage.node.pct',
'docker.cpu.total.pct',
];

for (const field of fields) {
await pageObjects.infraMetricsExplorer.addMetric(field);
}

await pageObjects.infraMetricsExplorer.ensureMaxMetricsLimiteReachedIsVisible();
});
});

// FLAKY: https://github.com/elastic/kibana/issues/111076
Expand Down
15 changes: 15 additions & 0 deletions x-pack/test/functional/page_objects/infra_metrics_explorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export function InfraMetricsExplorerProvider({ getService }: FtrProviderContext)
const comboBox = getService('comboBox');

return {
async clearMetrics() {
await comboBox.clear('metricsExplorer-metrics');
},

async getMetrics() {
const subject = await testSubjects.find('metricsExplorer-metrics');
return await subject.findAllByCssSelector('span.euiBadge');
Expand Down Expand Up @@ -56,5 +60,16 @@ export function InfraMetricsExplorerProvider({ getService }: FtrProviderContext)
const radioInput = await chartRadio.findByCssSelector(`label[for="${type}"]`);
return await radioInput.click();
},

async ensureMetricsExplorerFeedbackLinkIsVisible() {
await testSubjects.missingOrFail('loadingMessage', { timeout: 20000 });
await testSubjects.existOrFail('infraMetricsExplorerFeedbackLink');
},

async ensureMaxMetricsLimiteReachedIsVisible() {
const subject = await testSubjects.find('metricsExplorer-metrics');
await subject.click();
await testSubjects.existOrFail('infraMetricsExplorerMaxMetricsReached');
},
};
}

0 comments on commit 1348b58

Please sign in to comment.