Skip to content

Commit

Permalink
[7.17] [Infra] Limit the number of metrics accepted by Snapshot API (#…
Browse files Browse the repository at this point in the history
…188181) (#188242)

# Backport

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

<!--- 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-12T13:53:53Z","message":"[Infra]
Limit the number of metrics accepted by Snapshot API (#188181)\n\npart
of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more metrics and will see\r\nthe \"Add metric\" button
disabled with a tooltip\r\n\r\n<img width=\"1713\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/c784b08b-e118-4491-b53d-46bfde898216\">\r\n\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\r\n- Try to add more than 20
metrics in the Metrics
dropdown.","sha":"f2d1a8b6d24486cedb0dad97e71cd660845f353c","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":188181,"url":"https://github.com/elastic/kibana/pull/188181","mergeCommit":{"message":"[Infra]
Limit the number of metrics accepted by Snapshot API (#188181)\n\npart
of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more metrics and will see\r\nthe \"Add metric\" button
disabled with a tooltip\r\n\r\n<img width=\"1713\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/c784b08b-e118-4491-b53d-46bfde898216\">\r\n\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\r\n- Try to add more than 20
metrics in the Metrics
dropdown.","sha":"f2d1a8b6d24486cedb0dad97e71cd660845f353c"}},"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/188181","number":188181,"mergeCommit":{"message":"[Infra]
Limit the number of metrics accepted by Snapshot API (#188181)\n\npart
of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n-
private\r\n\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no
longer add more metrics and will see\r\nthe \"Add metric\" button
disabled with a tooltip\r\n\r\n<img width=\"1713\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/c784b08b-e118-4491-b53d-46bfde898216\">\r\n\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\r\n- Try to add more than 20
metrics in the Metrics
dropdown.","sha":"f2d1a8b6d24486cedb0dad97e71cd660845f353c"}},{"url":"https://github.com/elastic/kibana/pull/188231","number":188231,"branch":"8.15","state":"OPEN"}]}]
BACKPORT-->
  • Loading branch information
crespocarlos authored Jul 15, 2024
1 parent 4d78f78 commit 8197581
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 36 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
49 changes: 31 additions & 18 deletions 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,28 +32,34 @@ export const initSnapshotRoute = (libs: InfraBackendLibs) => {
},
},
async (requestContext, request, response) => {
const snapshotRequest = pipe(
SnapshotRequestRT.decode(request.body),
fold(throwErrors(Boom.badRequest), identity)
);
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
);
const compositeSize = libs.configuration.inventory.compositeSize;
const logQueryFields = await libs
.getLogQueryFields(
snapshotRequest.sourceId,
const source = await libs.sources.getSourceConfiguration(
requestContext.core.savedObjects.client,
requestContext.core.elasticsearch.client.asCurrentUser
)
.catch(() => undefined);
snapshotRequest.sourceId
);
const compositeSize = libs.configuration.inventory.compositeSize;
const logQueryFields = await libs
.getLogQueryFields(
snapshotRequest.sourceId,
requestContext.core.savedObjects.client,
requestContext.core.elasticsearch.client.asCurrentUser
)
.catch(() => undefined);

UsageCollector.countNode(snapshotRequest.nodeType);
const client = createSearchClient(requestContext, framework);
UsageCollector.countNode(snapshotRequest.nodeType);
const client = createSearchClient(requestContext, framework);

try {
const snapshotResponse = await getNodes(
client,
snapshotRequest,
Expand All @@ -64,6 +71,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
26 changes: 24 additions & 2 deletions x-pack/test/api_integration/apis/metrics_ui/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ export default function ({ getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const supertest = getService('supertest');
const fetchSnapshot = async (
body: SnapshotRequest
body: SnapshotRequest,
expectStatusCode = 200
): Promise<SnapshotNodeResponse | undefined> => {
const response = await supertest
.post('/api/metrics/snapshot')
.set('kbn-xsrf', 'xxx')
.send(body)
.expect(200);
.expect(expectStatusCode);
return response.body;
};

Expand Down Expand Up @@ -410,5 +411,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
);
});
});
});
}
34 changes: 34 additions & 0 deletions x-pack/test/functional/apps/infra/home_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';
import { DATES } from './constants';

Expand Down Expand Up @@ -55,6 +56,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 8197581

Please sign in to comment.