[Metrics UI] Fix p95/p99 charts and alerting error (#65579)

* [Metrics UI] Fix p95/p99 charts and alerting error

- Fixes #65561

* Fixing open in visualize for percentiles

* Adding test for P95; refactoring to use first consitently
This commit is contained in:
Chris Cowan 2020-05-08 10:55:43 -07:00 committed by GitHub
parent d3b155f843
commit 1c6e6cb7b7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 144 additions and 18 deletions

View file

@ -118,10 +118,7 @@ export const ExpressionChart: React.FC<Props> = ({
const series = {
...firstSeries,
rows: firstSeries.rows.map(row => {
const newRow: MetricsExplorerRow = {
timestamp: row.timestamp,
metric_0: row.metric_0 || null,
};
const newRow: MetricsExplorerRow = { ...row };
thresholds.forEach((thresholdValue, index) => {
newRow[`metric_threshold_${index}`] = thresholdValue;
});

View file

@ -234,4 +234,20 @@ export const aggregationType: { [key: string]: any } = {
value: AGGREGATION_TYPES.SUM,
validNormalizedTypes: ['number'],
},
p95: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.p95', {
defaultMessage: '95th Percentile',
}),
fieldRequired: false,
value: AGGREGATION_TYPES.P95,
validNormalizedTypes: ['number'],
},
p99: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.p99', {
defaultMessage: '99th Percentile',
}),
fieldRequired: false,
value: AGGREGATION_TYPES.P99,
validNormalizedTypes: ['number'],
},
};

View file

@ -29,6 +29,8 @@ export enum AGGREGATION_TYPES {
MAX = 'max',
RATE = 'rate',
CARDINALITY = 'cardinality',
P95 = 'p95',
P99 = 'p99',
}
export interface MetricThresholdAlertParams {

View file

@ -46,6 +46,24 @@ export const metricsExplorerMetricToTSVBMetric = (metric: MetricsExplorerOptions
field: derivativeId,
},
];
} else if (metric.aggregation === 'p95' || metric.aggregation === 'p99') {
const percentileValue = metric.aggregation === 'p95' ? '95' : '99';
return [
{
id: uuid.v1(),
type: 'percentile',
field: metric.field,
percentiles: [
{
id: uuid.v1(),
value: percentileValue,
mode: 'line',
percentile: '',
shade: 0.2,
},
],
},
];
} else {
return [
{

View file

@ -20,6 +20,7 @@ import {
MetricsExplorerOptionsMetric,
MetricsExplorerChartType,
} from '../hooks/use_metrics_explorer_options';
import { getMetricId } from './helpers/get_metric_id';
type NumberOrString = string | number;
@ -45,10 +46,12 @@ export const MetricsExplorerAreaChart = ({ metric, id, series, type, stack, opac
colorTransformer(MetricsExplorerColor.color0);
const yAccessors = Array.isArray(id)
? id.map(i => `metric_${i}`).slice(id.length - 1, id.length)
: [`metric_${id}`];
? id.map(i => getMetricId(metric, i)).slice(id.length - 1, id.length)
: [getMetricId(metric, id)];
const y0Accessors =
Array.isArray(id) && id.length > 1 ? id.map(i => `metric_${i}`).slice(0, 1) : undefined;
Array.isArray(id) && id.length > 1
? id.map(i => getMetricId(metric, i)).slice(0, 1)
: undefined;
const chartId = `series-${series.id}-${yAccessors.join('-')}`;
const seriesAreaStyle: RecursivePartial<AreaSeriesStyle> = {
@ -85,8 +88,10 @@ export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) =>
(metric.color && colorTransformer(metric.color)) ||
colorTransformer(MetricsExplorerColor.color0);
const yAccessor = `metric_${id}`;
const chartId = `series-${series.id}-${yAccessor}`;
const yAccessors = Array.isArray(id)
? id.map(i => getMetricId(metric, i)).slice(id.length - 1, id.length)
: [getMetricId(metric, id)];
const chartId = `series-${series.id}-${yAccessors.join('-')}`;
const seriesBarStyle: RecursivePartial<BarSeriesStyle> = {
rectBorder: {
@ -100,13 +105,13 @@ export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) =>
};
return (
<BarSeries
id={yAccessor}
id={chartId}
key={chartId}
name={createMetricLabel(metric)}
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
xAccessor="timestamp"
yAccessors={[yAccessor]}
yAccessors={yAccessors}
data={series.rows}
stackAccessors={stack ? ['timestamp'] : void 0}
barSeriesStyle={seriesBarStyle}

View file

@ -0,0 +1,22 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { Aggregators } from './types';
export const createPercentileAggregation = (
type: Aggregators.P95 | Aggregators.P99,
field: string
) => {
const value = type === Aggregators.P95 ? 95 : 99;
return {
aggregatedValue: {
percentiles: {
field,
percents: [value],
keyed: false,
},
},
};
};

View file

@ -233,6 +233,58 @@ describe('The metric threshold alert type', () => {
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
});
describe('querying with the p99 aggregator', () => {
const instanceID = 'test-*';
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
services,
params: {
criteria: [
{
...baseCriterion,
comparator,
threshold,
aggType: 'p99',
metric: 'test.metric.2',
},
],
},
});
test('alerts based on the p99 values', async () => {
await execute(Comparator.GT, [1]);
expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
expect(getState(instanceID).alertState).toBe(AlertStates.ALERT);
await execute(Comparator.LT, [1]);
expect(mostRecentAction(instanceID)).toBe(undefined);
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
});
describe('querying with the p95 aggregator', () => {
const instanceID = 'test-*';
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
services,
params: {
criteria: [
{
...baseCriterion,
comparator,
threshold,
aggType: 'p95',
metric: 'test.metric.1',
},
],
},
});
test('alerts based on the p95 values', async () => {
await execute(Comparator.GT, [0.25]);
expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
expect(getState(instanceID).alertState).toBe(AlertStates.ALERT);
await execute(Comparator.LT, [0.95]);
expect(mostRecentAction(instanceID)).toBe(undefined);
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
});
describe("querying a metric that hasn't reported data", () => {
const instanceID = 'test-*';
const execute = (alertOnNoData: boolean) =>

View file

@ -3,7 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { mapValues } from 'lodash';
import { mapValues, first } from 'lodash';
import { i18n } from '@kbn/i18n';
import { InfraDatabaseSearchResponse } from '../../adapters/framework/adapter_types';
import { createAfterKeyHandler } from '../../../utils/create_afterkey_handler';
@ -21,12 +21,16 @@ import { AlertServices, AlertExecutorOptions } from '../../../../../alerting/ser
import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds';
import { getDateHistogramOffset } from '../../snapshot/query_helpers';
import { InfraBackendLibs } from '../../infra_types';
import { createPercentileAggregation } from './create_percentile_aggregation';
const TOTAL_BUCKETS = 5;
interface Aggregation {
aggregatedIntervals: {
buckets: Array<{ aggregatedValue: { value: number }; doc_count: number }>;
buckets: Array<{
aggregatedValue: { value: number; values?: Array<{ key: number; value: number }> };
doc_count: number;
}>;
};
}
@ -47,6 +51,12 @@ const getCurrentValueFromAggregations = (
if (aggType === Aggregators.COUNT) {
return mostRecentBucket.doc_count;
}
if (aggType === Aggregators.P95 || aggType === Aggregators.P99) {
const values = mostRecentBucket.aggregatedValue?.values || [];
const firstValue = first(values);
if (!firstValue) return null;
return firstValue.value;
}
const { value } = mostRecentBucket.aggregatedValue;
return value;
} catch (e) {
@ -86,6 +96,8 @@ export const getElasticsearchMetricQuery = (
? {}
: aggType === Aggregators.RATE
? networkTraffic('aggregatedValue', metric)
: aggType === Aggregators.P95 || aggType === Aggregators.P99
? createPercentileAggregation(aggType, metric)
: {
aggregatedValue: {
[aggType]: {
@ -275,7 +287,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs, alertId: s
);
// Because each alert result has the same group definitions, just grap the groups from the first one.
const groups = Object.keys(alertResults[0]);
const groups = Object.keys(first(alertResults));
for (const group of groups) {
const alertInstance = services.alertInstanceFactory(`${alertId}-${group}`);

View file

@ -7,22 +7,22 @@
const bucketsA = [
{
doc_count: 2,
aggregatedValue: { value: 0.5 },
aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] },
},
{
doc_count: 3,
aggregatedValue: { value: 1.0 },
aggregatedValue: { value: 1.0, values: [{ key: 95.0, value: 1.0 }] },
},
];
const bucketsB = [
{
doc_count: 4,
aggregatedValue: { value: 2.5 },
aggregatedValue: { value: 2.5, values: [{ key: 99.0, value: 2.5 }] },
},
{
doc_count: 5,
aggregatedValue: { value: 3.5 },
aggregatedValue: { value: 3.5, values: [{ key: 99.0, value: 3.5 }] },
},
];

View file

@ -23,6 +23,8 @@ export enum Aggregators {
MAX = 'max',
RATE = 'rate',
CARDINALITY = 'cardinality',
P95 = 'p95',
P99 = 'p99',
}
export enum AlertStates {