From 8caa26357b703ebfe2d587614c3907fd8b7fa152 Mon Sep 17 00:00:00 2001 From: Carlos Crespo Date: Fri, 12 Jul 2024 15:53:53 +0200 Subject: [PATCH] [Infra] Limit the number of metrics accepted by Snapshot API (#188181) part of [3628](https://github.com/elastic/observability-dev/issues/3628) - private ## Summary After adding 20 items, users can no longer add more metrics and will see the "Add metric" button disabled with a tooltip image ### How to test - Start a local Kibana instance pointing to an oblt cluster - Navigate to Infrastructure - Try to add more than 20 metrics in the Metrics dropdown. (cherry picked from commit f2d1a8b6d24486cedb0dad97e71cd660845f353c) # Conflicts: # x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/index.tsx # x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/mode_switcher.tsx # x-pack/plugins/infra/server/routes/snapshot/index.ts # x-pack/plugins/observability_solution/infra/common/constants.ts # x-pack/plugins/observability_solution/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx # x-pack/test/functional/page_objects/infra_home_page.ts --- x-pack/plugins/infra/common/constants.ts | 2 + .../metric_control/custom_metric_form.tsx | 1 + .../waffle/metric_control/index.tsx | 5 ++ .../metric_control/metrics_context_menu.tsx | 8 +++- .../waffle/metric_control/mode_switcher.tsx | 48 +++++++++++++------ .../infra/server/routes/snapshot/index.ts | 18 ++++++- .../apis/metrics_ui/snapshot.ts | 21 ++++++++ .../test/functional/apps/infra/home_page.ts | 33 +++++++++++++ .../page_objects/infra_home_page.ts | 22 +++++++++ 9 files changed, 141 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/infra/common/constants.ts b/x-pack/plugins/infra/common/constants.ts index 1c3aa550f2f62..48f728ef7f88b 100644 --- a/x-pack/plugins/infra/common/constants.ts +++ b/x-pack/plugins/infra/common/constants.ts @@ -16,3 +16,5 @@ export const METRICS_FEATURE_ID = 'infrastructure'; export const LOGS_FEATURE_ID = 'logs'; export type InfraFeatureId = typeof METRICS_FEATURE_ID | typeof LOGS_FEATURE_ID; + +export const SNAPSHOT_API_MAX_METRICS = 20; diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx index 38018bf233205..9750bea77fb5c 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx @@ -196,6 +196,7 @@ export const CustomMetricForm = withTheme( options={fieldOptions} onChange={handleFieldChange} isClearable={false} + data-test-subj="infraCustomMetricFieldSelect" /> diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/index.tsx b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/index.tsx index 16950cee86b6a..4397d6d242152 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/index.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/index.tsx @@ -9,6 +9,7 @@ import { EuiPopover } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React, { useState, useCallback } from 'react'; import { IFieldType } from 'src/plugins/data/public'; +import { SNAPSHOT_API_MAX_METRICS } from '../../../../../../../common/constants'; import { getCustomMetricLabel } from '../../../../../../../common/formatters/get_custom_metric_label'; import { SnapshotMetricInput, @@ -134,10 +135,13 @@ export const WaffleMetricControls = ({ return null; } + const canAdd = options.length + customMetrics.length < SNAPSHOT_API_MAX_METRICS; + const button = ( {currentLabel} @@ -194,6 +198,7 @@ export const WaffleMetricControls = ({ mode={mode} onSave={handleSaveEdit} customMetrics={customMetrics} + disableAdd={!canAdd} /> diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/metrics_context_menu.tsx b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/metrics_context_menu.tsx index fa3d35ef1f97c..0fc9881e0ee26 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/metrics_context_menu.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/metrics_context_menu.tsx @@ -73,5 +73,11 @@ export const MetricsContextMenu = ({ }, ]; - return ; + return ( + + ); }; diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/mode_switcher.tsx b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/mode_switcher.tsx index a0efed2b512d4..c75efb35a0755 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/mode_switcher.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/mode_switcher.tsx @@ -5,10 +5,11 @@ * 2.0. */ -import { EuiFlexGroup, EuiFlexItem, EuiButtonEmpty, EuiButton } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiButtonEmpty, EuiButton, EuiToolTip } from '@elastic/eui'; import React from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; +import { SNAPSHOT_API_MAX_METRICS } from '../../../../../../../common/constants'; import { CustomMetricMode } from './types'; import { SnapshotCustomMetricInput } from '../../../../../../../common/http_api/snapshot_api'; import { EuiTheme, withTheme } from '../../../../../../../../../../src/plugins/kibana_react/common'; @@ -21,10 +22,11 @@ interface Props { onEditCancel: () => void; mode: CustomMetricMode; customMetrics: SnapshotCustomMetricInput[]; + disableAdd?: boolean; } export const ModeSwitcher = withTheme( - ({ onSave, onEditCancel, onEdit, onAdd, mode, customMetrics, theme }: Props) => { + ({ onSave, onEditCancel, onEdit, onAdd, mode, customMetrics, disableAdd, theme }: Props) => { if (['editMetric', 'addMetric'].includes(mode)) { return null; } @@ -91,20 +93,36 @@ export const ModeSwitcher = withTheme( - - - + + + + )} diff --git a/x-pack/plugins/infra/server/routes/snapshot/index.ts b/x-pack/plugins/infra/server/routes/snapshot/index.ts index b86eb9f7d4c95..a8ebd2bb6f737 100644 --- a/x-pack/plugins/infra/server/routes/snapshot/index.ts +++ b/x-pack/plugins/infra/server/routes/snapshot/index.ts @@ -10,6 +10,7 @@ import { schema } from '@kbn/config-schema'; import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; +import { SNAPSHOT_API_MAX_METRICS } from '../../../common/constants'; import { InfraBackendLibs } from '../../lib/infra_types'; import { UsageCollector } from '../../usage/usage_collector'; import { SnapshotRequestRT, SnapshotNodeResponseRT } from '../../../common/http_api/snapshot_api'; @@ -31,11 +32,19 @@ export const initSnapshotRoute = (libs: InfraBackendLibs) => { }, }, async (requestContext, request, response) => { +try { + try { const snapshotRequest = pipe( SnapshotRequestRT.decode(request.body), fold(throwErrors(Boom.badRequest), identity) ); + if (snapshotRequest.metrics.length > SNAPSHOT_API_MAX_METRICS) { + throw Boom.badRequest( + `'metrics' size is greater than maximum of ${SNAPSHOT_API_MAX_METRICS} allowed.` + ); + } + const source = await libs.sources.getSourceConfiguration( requestContext.core.savedObjects.client, snapshotRequest.sourceId @@ -52,7 +61,8 @@ export const initSnapshotRoute = (libs: InfraBackendLibs) => { UsageCollector.countNode(snapshotRequest.nodeType); const client = createSearchClient(requestContext, framework); - try { + + const snapshotResponse = await getNodes( client, snapshotRequest, @@ -64,6 +74,12 @@ export const initSnapshotRoute = (libs: InfraBackendLibs) => { body: SnapshotNodeResponseRT.encode(snapshotResponse), }); } catch (err) { + if (Boom.isBoom(err)) { + return response.customError({ + statusCode: err.output.statusCode, + body: { message: err.output.payload.message }, + }); + } return handleEsError({ error: err, response }); } } diff --git a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts index 4181344f064b9..fef70ca04912b 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts @@ -410,5 +410,26 @@ export default function ({ getService }: FtrProviderContext) { }); }); }); + + describe('request validation', () => { + it('should return 400 when requesting more than 20 metrics', async () => { + const { min, max } = DATES['8.0.0'].logs_and_metrics; + await fetchSnapshot( + { + sourceId: 'default', + timerange: { + to: max, + from: min, + interval: '1m', + }, + metrics: Array(21).fill({ type: 'cpu' }), + nodeType: 'host', + groupBy: [{ field: 'service.type' }], + includeTimeseries: true, + }, + 400 + ); + }); + }); }); } diff --git a/x-pack/test/functional/apps/infra/home_page.ts b/x-pack/test/functional/apps/infra/home_page.ts index 84e070db13177..d44aa61a3df20 100644 --- a/x-pack/test/functional/apps/infra/home_page.ts +++ b/x-pack/test/functional/apps/infra/home_page.ts @@ -55,6 +55,39 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await pageObjects.infraHome.goToTime(DATE_WITHOUT_DATA); await pageObjects.infraHome.getNoMetricsDataPrompt(); }); + + it('should not allow adding more than 20 custom metrics', async () => { + // open + await pageObjects.infraHome.clickCustomMetricDropdown(); + + const fields = [ + 'process.cpu.pct', + 'process.memory.pct', + 'system.core.total.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', + ]; + + for (const field of fields) { + await pageObjects.infraHome.addCustomMetric(field); + } + const metricsCount = await pageObjects.infraHome.getMetricsContextMenuItemsCount(); + // there are 6 default metrics in the context menu for hosts + expect(metricsCount).to.eql(20); + + await pageObjects.infraHome.ensureCustomMetricAddButtonIsDisabled(); + // close + await pageObjects.infraHome.clickCustomMetricDropdown(); + }); }); describe('alerts flyouts', () => { diff --git a/x-pack/test/functional/page_objects/infra_home_page.ts b/x-pack/test/functional/page_objects/infra_home_page.ts index 788dcd18bef73..035bdccb0b779 100644 --- a/x-pack/test/functional/page_objects/infra_home_page.ts +++ b/x-pack/test/functional/page_objects/infra_home_page.ts @@ -15,6 +15,7 @@ export function InfraHomePageProvider({ getService, getPageObjects }: FtrProvide const retry = getService('retry'); const find = getService('find'); const pageObjects = getPageObjects(['common']); + const comboBox = getService('comboBox'); return { async goToTime(time: string) { @@ -255,5 +256,26 @@ export function InfraHomePageProvider({ getService, getPageObjects }: FtrProvide async closeAlertFlyout() { await testSubjects.click('euiFlyoutCloseButton'); }, + async clickCustomMetricDropdown() { + await testSubjects.click('infraInventoryMetricDropdown'); + }, + + async addCustomMetric(field: string) { + await testSubjects.click('infraModeSwitcherAddMetricButton'); + const groupByCustomField = await testSubjects.find('infraCustomMetricFieldSelect'); + await comboBox.setElement(groupByCustomField, field); + await testSubjects.click('infraCustomMetricFormSaveButton'); + }, + + async getMetricsContextMenuItemsCount() { + const contextMenu = await testSubjects.find('infraInventoryMetricsContextMenu'); + const menuItems = await contextMenu.findAllByCssSelector('button.euiContextMenuItem'); + return menuItems.length; + }, + + async ensureCustomMetricAddButtonIsDisabled() { + const button = await testSubjects.find('infraModeSwitcherAddMetricButton'); + expect(await button.getAttribute('disabled')).to.be('true'); + }, }; }