Skip to content

Commit

Permalink
[Infra] Limit the number of metrics accepted by Snapshot API (#188181)
Browse files Browse the repository at this point in the history
part of [3628](elastic/observability-dev#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

<img width="1713" alt="image"
src="https://github.com/user-attachments/assets/c784b08b-e118-4491-b53d-46bfde898216">

### 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 f2d1a8b)

# 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
  • Loading branch information
crespocarlos committed Jul 12, 2024
1 parent 4d78f78 commit 8caa263
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 17 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/infra/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export const CustomMetricForm = withTheme(
options={fieldOptions}
onChange={handleFieldChange}
isClearable={false}
data-test-subj="infraCustomMetricFieldSelect"
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -134,10 +135,13 @@ export const WaffleMetricControls = ({
return null;
}

const canAdd = options.length + customMetrics.length < SNAPSHOT_API_MAX_METRICS;

const button = (
<DropdownButton
onClick={handleToggle}
label={i18n.translate('xpack.infra.waffle.metriclabel', { defaultMessage: 'Metric' })}
data-test-subj="infraInventoryMetricDropdown"
>
{currentLabel}
</DropdownButton>
Expand Down Expand Up @@ -194,6 +198,7 @@ export const WaffleMetricControls = ({
mode={mode}
onSave={handleSaveEdit}
customMetrics={customMetrics}
disableAdd={!canAdd}
/>
</EuiPopover>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,11 @@ export const MetricsContextMenu = ({
},
];

return <EuiContextMenu initialPanelId={0} panels={panels} />;
return (
<EuiContextMenu
initialPanelId={0}
panels={panels}
data-test-subj="infraInventoryMetricsContextMenu"
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
}
Expand Down Expand Up @@ -91,20 +93,36 @@ export const ModeSwitcher = withTheme(
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
onClick={onAdd}
size="s"
flush="right"
aria-label={i18n.translate(
'xpack.infra.waffle.customMetrics.modeSwitcher.addMetricAriaLabel',
{ defaultMessage: 'Add custom metric' }
)}
<EuiToolTip
content={
disableAdd
? i18n.translate(
'xpack.infra.waffle.customMetrics.modeSwitcher.addDisabledTooltip',
{
defaultMessage: 'Maximum number of {maxMetrics} metrics reached.',
values: { maxMetrics: SNAPSHOT_API_MAX_METRICS },
}
)
: undefined
}
>
<FormattedMessage
id="xpack.infra.waffle.customMetrics.modeSwitcher.addMetric"
defaultMessage="Add metric"
/>
</EuiButtonEmpty>
<EuiButtonEmpty
data-test-subj="infraModeSwitcherAddMetricButton"
onClick={onAdd}
disabled={disableAdd}
size="s"
flush="right"
aria-label={i18n.translate(
'xpack.infra.waffle.customMetrics.modeSwitcher.addMetricAriaLabel',
{ defaultMessage: 'Add custom metric' }
)}
>
<FormattedMessage
id="xpack.infra.waffle.customMetrics.modeSwitcher.addMetric"
defaultMessage="Add metric"
/>
</EuiButtonEmpty>
</EuiToolTip>
</EuiFlexItem>
</>
)}
Expand Down
18 changes: 17 additions & 1 deletion x-pack/plugins/infra/server/routes/snapshot/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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 });
}
}
Expand Down
21 changes: 21 additions & 0 deletions x-pack/test/api_integration/apis/metrics_ui/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
});
});
}
33 changes: 33 additions & 0 deletions x-pack/test/functional/apps/infra/home_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
22 changes: 22 additions & 0 deletions x-pack/test/functional/page_objects/infra_home_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
},
};
}

0 comments on commit 8caa263

Please sign in to comment.