Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposed common EuiExpressions to separate components be able to reuse for building new for Alert Types #56466

Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8c7ed23
Exposed common Expression to separate components be able to reuse
YulNaumenko Jan 31, 2020
e090eca
Merge remote-tracking branch 'upstream/master' into alerts-create-com…
YulNaumenko Feb 1, 2020
b54cdd6
Expressions with unit tests
YulNaumenko Feb 3, 2020
7c5031c
Fixed type check
YulNaumenko Feb 3, 2020
221194b
Merge remote-tracking branch 'upstream/master' into alerts-create-com…
YulNaumenko Feb 3, 2020
a110f9e
Merge remote-tracking branch 'upstream/master' into alerts-create-com…
YulNaumenko Feb 4, 2020
db7678b
Fixed merge issues
YulNaumenko Feb 4, 2020
293d985
Fixed due to review
YulNaumenko Feb 5, 2020
e4e9f4b
Cleaned up some not used params and added position popover definition
YulNaumenko Feb 5, 2020
f310132
Merge remote-tracking branch 'upstream/master' into alerts-create-com…
YulNaumenko Feb 5, 2020
73bc017
fixed type check
YulNaumenko Feb 6, 2020
1ad46d5
Unbinded alerting reusable components from application context
YulNaumenko Feb 6, 2020
2bd5bef
Added consumer and alertTypeId with enable change alert type button p…
YulNaumenko Feb 6, 2020
b8a18f4
Fixed case for default alert type id was set
YulNaumenko Feb 6, 2020
b9a1a56
Merge remote-tracking branch 'upstream/master' into alerts-create-com…
YulNaumenko Feb 6, 2020
2fcd805
Fixed chart visualization issues
YulNaumenko Feb 6, 2020
e414ef7
Exposed registry in triggers and actions ui
YulNaumenko Feb 7, 2020
f8438c9
Fixed alert_list to enable charts
YulNaumenko Feb 7, 2020
c7485d5
Fixed due to comments and simplified props
YulNaumenko Feb 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { i18n } from '@kbn/i18n';
import { Alert, AlertTypeModel, ValidationResult } from '../../../../types';
import { IndexThresholdAlertTypeExpression, aggregationTypes, groupByTypes } from './expression';
import { AlertTypeModel, ValidationResult } from '../../../../types';
import { IndexThresholdAlertTypeExpression } from './expression';
import { IndexThresholdAlertParams } from '../types';
import { builtInGroupByTypes, builtInAggregationTypes } from '../../../../common/constants';

export function getAlertType(): AlertTypeModel {
return {
id: 'threshold',
name: 'Index Threshold',
iconClass: 'alert',
alertParamsExpression: IndexThresholdAlertTypeExpression,
validate: (alert: Alert): ValidationResult => {
validate: (alertParams: IndexThresholdAlertParams): ValidationResult => {
const {
index,
timeField,
Expand All @@ -24,7 +26,7 @@ export function getAlertType(): AlertTypeModel {
termField,
threshold,
timeWindowSize,
} = alert.params;
} = alertParams;
const validationResult = { errors: {} };
const errors = {
aggField: new Array<string>(),
Expand All @@ -51,7 +53,7 @@ export function getAlertType(): AlertTypeModel {
})
);
}
if (aggType && aggregationTypes[aggType].fieldRequired && !aggField) {
if (aggType && builtInAggregationTypes[aggType].fieldRequired && !aggField) {
errors.aggField.push(
i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredAggFieldText', {
defaultMessage: 'Aggregation field is required.',
Expand All @@ -65,7 +67,7 @@ export function getAlertType(): AlertTypeModel {
})
);
}
if (groupBy && groupByTypes[groupBy].sizeRequired && !termField) {
if (!termField && groupBy && builtInGroupByTypes[groupBy].sizeRequired) {
errors.termField.push(
i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredtTermFieldText', {
defaultMessage: 'Term field is required.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import { EuiCallOut, EuiLoadingChart, EuiSpacer, EuiEmptyPrompt, EuiText } from
import { FormattedMessage } from '@kbn/i18n/react';
import { npStart } from 'ui/new_platform';
import { getThresholdAlertVisualizationData } from './lib/api';
import { comparators, aggregationTypes } from './expression';
import { useAppDependencies } from '../../../app_context';
import { Alert } from '../../../../types';
import { AggregationType, Comparator } from '../../../../common/types';
import { IndexThresholdAlertParams } from '../types';

const customTheme = () => {
return {
Expand Down Expand Up @@ -76,13 +76,6 @@ const getDomain = (alertParams: any) => {
};
};

const getThreshold = (alertParams: any) => {
return alertParams.threshold.slice(
0,
comparators[alertParams.thresholdComparator].requiredValues
);
};

const getTimeBuckets = (alertParams: any) => {
const domain = getDomain(alertParams);
const timeBuckets = new TimeBuckets();
Expand All @@ -91,10 +84,18 @@ const getTimeBuckets = (alertParams: any) => {
};

interface Props {
alert: Alert;
alertParams: IndexThresholdAlertParams;
aggregationTypes: { [key: string]: AggregationType };
comparators: {
[key: string]: Comparator;
};
}

export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }) => {
export const ThresholdVisualization: React.FunctionComponent<Props> = ({
alertParams,
aggregationTypes,
comparators,
}) => {
const { http, uiSettings, toastNotifications } = useAppDependencies();
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<undefined | any>(undefined);
Expand All @@ -104,8 +105,6 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
const {
index,
timeField,
triggerIntervalSize,
triggerIntervalUnit,
aggType,
aggField,
termSize,
Expand All @@ -115,9 +114,9 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
timeWindowUnit,
groupBy,
threshold,
} = alert.params;
} = alertParams;

const domain = getDomain(alert.params);
const domain = getDomain(alertParams);
const timeBuckets = new TimeBuckets();
timeBuckets.setBounds(domain);
const interval = timeBuckets.getInterval().expression;
Expand All @@ -129,7 +128,7 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
};

// Fetching visualization data is independent of alert actions
const alertWithoutActions = { ...alert.params, actions: [], type: 'threshold' };
const alertWithoutActions = { ...alertParams, actions: [], type: 'threshold' };

useEffect(() => {
(async () => {
Expand Down Expand Up @@ -158,8 +157,6 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
}, [
index,
timeField,
triggerIntervalSize,
triggerIntervalUnit,
aggType,
aggField,
termSize,
Expand Down Expand Up @@ -210,11 +207,17 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
);
}

const getThreshold = () => {
return thresholdComparator
? threshold.slice(0, comparators[thresholdComparator].requiredValues)
: [];
};

if (visualizationData) {
const alertVisualizationDataKeys = Object.keys(visualizationData);
const timezone = getTimezone(uiSettings);
const actualThreshold = getThreshold(alert.params);
let maxY = actualThreshold[actualThreshold.length - 1];
const actualThreshold = getThreshold();
let maxY = actualThreshold[actualThreshold.length - 1] as any;

(Object.values(visualizationData) as number[][][]).forEach(data => {
data.forEach(([, y]) => {
Expand All @@ -226,7 +229,7 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({ alert }
const dateFormatter = (d: number) => {
return moment(d)
.tz(timezone)
.format(getTimeBuckets(alert.params).getScaledDateFormat());
.format(getTimeBuckets(alertParams).getScaledDateFormat());
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to avoid any and the as number[][][] here?

For the any: Looking at the underlying type, it looks like it uses number explicitly, so we shouldn't need to cast this here. 🤔

For the number[][][]: Looks the core issue is the any in getThresholdAlertVisualizationData, so can we perhaps validate the result of the API call in getThresholdAlertVisualizationData and avoid the any there too?

const aggLabel = aggregationTypes[aggType].text;
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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.
*/

export interface IndexThresholdAlertParams {
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
index: string[];
timeField?: string;
aggType: string;
aggField?: string;
groupBy?: string;
termSize?: number;
termField?: string;
thresholdComparator?: string;
threshold: number[];
timeWindowSize: number;
timeWindowUnit: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const AlertAdd = () => {

const alertType = alertTypeRegistry.get(alert.alertTypeId);
const errors = {
...(alertType ? alertType.validate(alert).errors : []),
...(alertType ? alertType.validate(alert.params).errors : []),
...validateBaseProperties(alert).errors,
} as IErrorObject;
const hasErrors = !!Object.keys(errors).find(errorKey => errors[errorKey].length >= 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ import {
ActionConnector,
AlertTypeIndex,
} from '../../../types';
import { getTimeOptions } from '../../lib/get_time_options';
import { SectionLoading } from '../../components/section_loading';
import { ConnectorAddModal } from '../action_connector_form/connector_add_modal';
import { getTimeOptions } from '../../../common/lib/get_time_options';

export function validateBaseProperties(alertObject: Alert) {
const validationResult = { errors: {} };
Expand Down Expand Up @@ -609,7 +609,7 @@ export const AlertForm = ({
</EuiFlexGroup>
{AlertParamsExpressionComponent ? (
<AlertParamsExpressionComponent
alert={alert}
alertParams={alert.params}
errors={errors}
setAlertParams={setAlertParams}
setAlertProperty={setAlertProperty}
Expand Down Expand Up @@ -774,7 +774,7 @@ export const AlertForm = ({
fullWidth
compressed
value={alertIntervalUnit}
options={getTimeOptions((alertInterval ? alertInterval : 1).toString())}
options={getTimeOptions(alertInterval ?? 1)}
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
onChange={e => {
setAlertIntervalUnit(e.target.value);
setScheduleProperty('interval', `${alertInterval}${e.target.value}`);
Expand Down Expand Up @@ -806,7 +806,7 @@ export const AlertForm = ({
<EuiSelect
compressed
value={alertThrottleUnit}
options={getTimeOptions((alertThrottle ? alertThrottle : 1).toString())}
options={getTimeOptions(alertThrottle ?? 1)}
onChange={e => {
setAlertThrottleUnit(e.target.value);
setAlertProperty('throttle', `${alertThrottle}${e.target.value}`);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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 { AggregationType } from '../types';

export enum AGGREGATION_TYPES {
COUNT = 'count',
AVERAGE = 'avg',
SUM = 'sum',
MIN = 'min',
MAX = 'max',
}

export const builtInAggregationTypes: { [key: string]: AggregationType } = {
count: {
text: 'count()',
fieldRequired: false,
value: AGGREGATION_TYPES.COUNT,
validNormalizedTypes: [],
},
avg: {
text: 'average()',
fieldRequired: true,
validNormalizedTypes: ['number'],
value: AGGREGATION_TYPES.AVERAGE,
},
sum: {
text: 'sum()',
fieldRequired: true,
validNormalizedTypes: ['number'],
value: AGGREGATION_TYPES.SUM,
},
min: {
text: 'min()',
fieldRequired: true,
validNormalizedTypes: ['number', 'date'],
value: AGGREGATION_TYPES.MIN,
},
max: {
text: 'max()',
fieldRequired: true,
validNormalizedTypes: ['number', 'date'],
value: AGGREGATION_TYPES.MAX,
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
* 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 { i18n } from '@kbn/i18n';
import { Comparator } from '../types';

export enum COMPARATORS {
GREATER_THAN = '>',
GREATER_THAN_OR_EQUALS = '>=',
BETWEEN = 'between',
LESS_THAN = '<',
LESS_THAN_OR_EQUALS = '<=',
}

export const builtInComparators: { [key: string]: Comparator } = {
[COMPARATORS.GREATER_THAN]: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isAboveLabel',
{
defaultMessage: 'Is above',
}
),
value: COMPARATORS.GREATER_THAN,
requiredValues: 1,
},
[COMPARATORS.GREATER_THAN_OR_EQUALS]: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isAboveOrEqualsLabel',
{
defaultMessage: 'Is above or equals',
}
),
value: COMPARATORS.GREATER_THAN_OR_EQUALS,
requiredValues: 1,
},
[COMPARATORS.LESS_THAN]: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isBelowLabel',
{
defaultMessage: 'Is below',
}
),
value: COMPARATORS.LESS_THAN,
requiredValues: 1,
},
[COMPARATORS.LESS_THAN_OR_EQUALS]: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isBelowOrEqualsLabel',
{
defaultMessage: 'Is below or equals',
}
),
value: COMPARATORS.LESS_THAN_OR_EQUALS,
requiredValues: 1,
},
[COMPARATORS.BETWEEN]: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isBetweenLabel',
{
defaultMessage: 'Is between',
}
),
value: COMPARATORS.BETWEEN,
requiredValues: 2,
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
* 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 { i18n } from '@kbn/i18n';
import { GroupByType } from '../types';

export const builtInGroupByTypes: { [key: string]: GroupByType } = {
all: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.groupByLabel.allDocumentsLabel',
{
defaultMessage: 'all documents',
}
),
sizeRequired: false,
value: 'all',
validNormalizedTypes: [],
},
top: {
text: i18n.translate(
'xpack.triggersActionsUI.sections.alertAdd.threshold.groupByLabel.topLabel',
{
defaultMessage: 'top',
}
),
sizeRequired: true,
value: 'top',
validNormalizedTypes: ['number', 'date', 'keyword'],
},
};
Loading