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

Fixed a bug that displayed all alerts for a monitor on individual triggers' flyouts. Fixed a bug that displayed incorrect source for the condition field on the alerts flyout. Fixed a bug that displayed incorrect severity on the alerts flyout. Fixed a bug that prevented selecting query-level monitor alerts 1 by 1. Fixed bug relating to validation of popovers when defining monitor queries. #123

Merged
merged 38 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
24e0bc7
Create opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md…
annie3431 Sep 8, 2021
1480661
Update version in package.json (#102)
annie3431 Sep 8, 2021
7e781aa
Text updates (#105)
annie3431 Sep 10, 2021
588114c
Added badges to the package README, and the Uploads coverage job to t…
AWSHurneyt Sep 10, 2021
ae78928
Update jest unit tests (#112)
annie3431 Sep 15, 2021
ed6fad5
Update jest unit tests (#112)
annie3431 Sep 15, 2021
0ba0ece
Fixed a few bugs
Sep 30, 2021
65eed16
Merge remote-tracking branch 'upstream/main' into bug-fix
Sep 30, 2021
3473d51
Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md
Sep 30, 2021
ebda05c
Fixed a bug that displayed all alerts for a monitor on individual tri…
AWSHurneyt Oct 1, 2021
77a8cd1
Updated release notes to reflect PR 122 bug fix.
AWSHurneyt Oct 1, 2021
6c00fb5
Fixing number of alerts displayed on Monitors tab.
AWSHurneyt Oct 1, 2021
4a2a6d8
Merge remote-tracking branch 'annie/bug-fix' into alertFlyoutBugFix
AWSHurneyt Oct 1, 2021
0add9a2
Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md
Sep 30, 2021
b790ddc
Merge branch 'bug-fix' of https://github.com/leeyun-amzn/alerting-das…
Oct 1, 2021
9ac8b2d
More bug fix
Oct 1, 2021
c865807
Skip test based on modification
Oct 1, 2021
1ae2215
Skip test based on modification
Oct 1, 2021
d2058b1
Update popover windows to remove item when filed is not defined
Oct 1, 2021
6eb7379
Merge branch 'bug-fix' of https://github.com/leeyun-amzn/alerting-das…
Oct 1, 2021
0e84b73
Merge branch 'bug-fix' of https://github.com/leeyun-amzn/alerting-das…
Oct 1, 2021
756793c
Update field validation
Oct 1, 2021
c69a674
Merge branch 'bug-fix' of https://github.com/leeyun-amzn/alerting-das…
Oct 1, 2021
017cf70
Merge branch 'bug-fix' of github.com:leeyun-amzn/alerting-dashboards-…
AWSHurneyt Oct 2, 2021
559fb68
Fixed a bug that prevented selecting query-level monitor alerts 1 by …
AWSHurneyt Oct 2, 2021
c0d3417
Merge remote-tracking branch 'thomas/alertFlyoutBugFix' into bug-fix
Oct 3, 2021
c4171e1
Update Dashboard.js
Oct 3, 2021
794be0b
Support data filter when using null operator
Oct 4, 2021
78916c2
Update WhereExpression.js
Oct 4, 2021
93cebca
Removing experimental dev code that was accidentally left in a previo…
AWSHurneyt Oct 4, 2021
63f20e1
Fixed a bug that was causing incorrect pagination display on alerts f…
AWSHurneyt Oct 4, 2021
de67be8
Removed redundant validation from filter values that was generating e…
AWSHurneyt Oct 4, 2021
c6fb682
Removed redundant validation from filter values that was generating e…
AWSHurneyt Oct 4, 2021
5919589
Merge remote-tracking branch 'origin/bugFixes' into bugFixes
AWSHurneyt Oct 4, 2021
49348e7
Update metric error for query monitors
Oct 4, 2021
ddfc601
Update MetricExpression.js
Oct 4, 2021
dc37a6f
Removed experimental dev code.
AWSHurneyt Oct 4, 2021
4fadf64
Updated release notes.
AWSHurneyt Oct 4, 2021
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
8 changes: 5 additions & 3 deletions public/components/Flyout/flyouts/alertsDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const getBucketLevelGraphFilter = (trigger) => {

const alertsDashboard = (payload) => {
const {
alerts,
history,
httpClient,
last_notification_time,
Expand All @@ -90,9 +91,8 @@ const alertsDashboard = (payload) => {
monitor_name,
notifications,
setFlyout,
severity,
start_time,
triggerId,
triggerID,
trigger_name,
} = payload;
const monitor = _.get(_.find(monitors, { _id: monitor_id }), '_source');
Expand All @@ -104,10 +104,11 @@ const alertsDashboard = (payload) => {
monitorType === MONITOR_TYPE.QUERY_LEVEL ? TRIGGER_TYPE.QUERY_LEVEL : TRIGGER_TYPE.BUCKET_LEVEL;

let trigger = _.get(monitor, 'triggers', []).find((trigger) => {
return trigger[triggerType].triggerId === triggerId;
return trigger[triggerType].id === triggerID;
});
trigger = _.get(trigger, triggerType);

const severity = _.get(trigger, 'severity');
const groupBy = _.get(monitor, MONITOR_GROUP_BY);

const condition =
Expand Down Expand Up @@ -260,6 +261,7 @@ const alertsDashboard = (payload) => {
monitorType={monitorType}
perAlertView={true}
groupBy={groupBy}
flyoutAlerts={alerts}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { EuiText, EuiButtonEmpty, EuiSpacer } from '@elastic/eui';
import { getIndexFields } from './utils/dataTypes';
import { getGroupByExpressionAllowedTypes } from './utils/helpers';
import GroupByItem from './GroupByItem';
import { GROUP_BY_ERROR } from './utils/constants';
import { GROUP_BY_ERROR, QUERY_TYPE_GROUP_BY_ERROR } from './utils/constants';
import { MONITOR_TYPE } from '../../../../../utils/constants';
import { inputLimitText } from '../../../../../utils/helpers';
import {
Expand Down Expand Up @@ -86,6 +86,8 @@ class GroupByExpression extends Component {
const isBucketLevelMonitor = values.monitor_type === MONITOR_TYPE.BUCKET_LEVEL;
if (!values.groupBy.length && isBucketLevelMonitor) {
errors.groupBy = GROUP_BY_ERROR;
} else if (!isBucketLevelMonitor && values.groupBy.length > MAX_NUM_QUERY_LEVEL_GROUP_BYS) {
errors.groupBy = QUERY_TYPE_GROUP_BY_ERROR;
} else {
delete errors.groupBy;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import React, { useState } from 'react';
import _ from 'lodash';
import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
import { GroupByPopover } from './index';

Expand All @@ -18,6 +19,7 @@ export default function GroupByItem(
) {
const [isPopoverOpen, setIsPopoverOpen] = useState(groupByItem === '');
const closePopover = () => {
if (_.isEmpty(groupByItem)) arrayHelpers.remove(index);
setIsPopoverOpen(false);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
import React, { Component } from 'react';
import { connect } from 'formik';
import { EuiText, EuiButtonEmpty, EuiSpacer, EuiBadge, EuiToolTip, EuiIcon } from '@elastic/eui';
import _ from 'lodash';
import { getIndexFields } from './utils/dataTypes';
import { getMetricExpressionAllowedTypes, validateAggregationsDuplicates } from './utils/helpers';
import _ from 'lodash';
import {
FORMIK_INITIAL_AGG_VALUES,
METRIC_TOOLTIP_TEXT,
Expand All @@ -38,6 +38,7 @@ import { MetricItem } from './index';
import { MONITOR_TYPE } from '../../../../../utils/constants';
import { inputLimitText } from '../../../../../utils/helpers';
import IconToolTip from '../../../../../components/IconToolTip';
import { QUERY_TYPE_METRIC_ERROR } from './utils/constants';

export const MAX_NUM_QUERY_LEVEL_METRICS = 1;
export const MAX_NUM_BUCKET_LEVEL_METRICS = 5;
Expand Down Expand Up @@ -107,6 +108,11 @@ class MetricExpression extends Component {

if (validateAggregationsDuplicates(aggregations)) {
errors.aggregations = `You have defined duplicated metrics.`;
} else if (
MONITOR_TYPE.QUERY_LEVEL === monitorType &&
aggregations.length > MAX_NUM_QUERY_LEVEL_METRICS
) {
errors.aggregations = QUERY_TYPE_METRIC_ERROR;
} else {
delete errors.aggregations;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import React, { useState } from 'react';
import _ from 'lodash';
import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
import MetricPopover from './MetricPopover';

Expand All @@ -18,6 +19,7 @@ export default function MetricItem(
) {
const [isPopoverOpen, setIsPopoverOpen] = useState(aggregation.fieldName === '');
const closePopover = () => {
if (_.isEmpty(aggregation.fieldName)) arrayHelpers.remove(index);
setIsPopoverOpen(false);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,7 @@ import {
isNullOperator,
isRangeOperator,
} from './utils/whereHelpers';
import {
hasError,
isInvalid,
required,
validateRequiredNumber,
} from '../../../../../utils/validate';
import { hasError, isInvalid } from '../../../../../utils/validate';
import {
FormikComboBox,
FormikSelect,
Expand All @@ -66,7 +61,6 @@ import { getFilteredIndexFields, getIndexFields } from './utils/dataTypes';
import {
FILTERS_TOOLTIP_TEXT,
FORMIK_INITIAL_VALUES,
TIME_RANGE_TOOLTIP_TEXT,
} from '../../../containers/CreateMonitor/utils/constants';
import { DATA_TYPES } from '../../../../../utils/constants';
import {
Expand Down Expand Up @@ -105,9 +99,10 @@ class WhereExpression extends Component {
}
};

handleOperatorChange = (e, field) => {
handleOperatorChange = (e, field, form) => {
this.props.onMadeChanges();
field.onChange(e);
form.setFieldError('where', undefined);
};

handleChangeWrapper = (e, field) => {
Expand All @@ -123,9 +118,16 @@ class WhereExpression extends Component {
} = this.props;
// Explicitly invoking validation, this component unmount after it closes.
const fieldName = _.get(values, `${fieldPath}where.fieldName`, '');
const fieldOperator = _.get(values, `${fieldPath}where.operator`, 'is');
const fieldValue = _.get(values, `${fieldPath}where.fieldValue`, '');
if (fieldName > 0) {
await this.props.formik.validateForm();
}
if (
_.isEmpty(fieldName) ||
(!isNullOperator(fieldOperator) && _.isEmpty(fieldValue.toString()))
)
this.resetValues();
closeExpression(Expressions.WHERE);
};

Expand Down Expand Up @@ -182,7 +184,6 @@ class WhereExpression extends Component {
) : (
<FormikFieldNumber
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: validateRequiredNumber }}
inputProps={{ onChange: this.handleChangeWrapper }}
formRow
rowProps={{ isInvalid, error: hasError }}
Expand All @@ -192,7 +193,6 @@ class WhereExpression extends Component {
return (
<FormikSelect
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: required }}
inputProps={{
onChange: this.handleChangeWrapper,
options: WHERE_BOOLEAN_FILTERS,
Expand All @@ -204,7 +204,6 @@ class WhereExpression extends Component {
return (
<FormikFieldText
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: required }}
inputProps={{ onChange: this.handleChangeWrapper, isInvalid }}
/>
);
Expand Down Expand Up @@ -267,7 +266,7 @@ class WhereExpression extends Component {
iconSide="right"
iconType="cross"
iconOnClick={() => this.resetValues()}
iconOnClickAriaLabel="Remove where filter"
iconOnClickAriaLabel="Remove filter"
onClick={() => {
openExpression(Expressions.WHERE);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,6 @@ export const AGGREGATION_TYPES = [
];

export const GROUP_BY_ERROR = 'Must specify at least 1 group by expression.';
export const QUERY_TYPE_GROUP_BY_ERROR = 'Can have a maximum of 1 group by selections.';

export const QUERY_TYPE_METRIC_ERROR = 'Can have a maximum of 1 metric selections.';
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default class VisualGraph extends Component {
renderAggregationXYPlot = (data, groupedData) => {
const { annotation, thresholdValue, values, fieldName, aggregationType } = this.props;
const { hint } = this.state;
const xDomain = getBufferedXDomain(data);
const xDomain = getBufferedXDomain(data, values);
const yDomain = getYDomain(data);
const annotations = getAnnotationData(xDomain, yDomain, thresholdValue);
const xTitle = values.timeField;
Expand Down
23 changes: 15 additions & 8 deletions public/pages/CreateMonitor/components/VisualGraph/utils/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*/

import _ from 'lodash';
import moment from 'moment';

import { selectOptionValueToText } from '../../MonitorExpressions/expressions/utils/helpers';
import {
Expand Down Expand Up @@ -61,10 +62,14 @@ export function getXDomain(data) {
return [minDate.x, maxDate.x];
}

export function getBufferedXDomain(data) {
export function getBufferedXDomain(data, values) {
const { bucketValue, bucketUnitOfTime } = values;
const minDate = data[0].x;
const maxDate = data[data.length - 1].x;
const timeRange = maxDate - minDate;
// If minDate equals to maxDate, then use bucketValue and bucketUnitOfTime as timeRange.
let timeRange = maxDate - minDate;
if (!timeRange) timeRange = moment.duration(bucketValue, bucketUnitOfTime);

const minDateBuffer = minDate - timeRange * X_DOMAIN_BUFFER;
const maxDateBuffer = maxDate.getTime() + timeRange * X_DOMAIN_BUFFER;
return [minDateBuffer, maxDateBuffer];
Expand Down Expand Up @@ -111,13 +116,13 @@ export function getDataFromResponse(response, fieldName, monitorType) {
}
}

// Function for aggregation type monitors to get Map of data.
// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points.
// The number n is based on the constant BAY_KEY_COUNT.
// Function for aggregation type monitors to get Map of data.
// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points.
// The number n is based on the constant BAY_KEY_COUNT.
export function getMapDataFromResponse(response, fieldName, groupByFields) {
if (!response) return [];
const buckets = _.get(response, 'aggregations.composite_agg.buckets', []);
let allData = new Map();
const allData = new Map();
lezzago marked this conversation as resolved.
Show resolved Hide resolved
buckets.map((bucket) => {
const dataPoint = getXYValuesByFieldName(bucket, fieldName);
// Key of object is the string concat by group by field values
Expand All @@ -126,7 +131,7 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {
? allData.set(key, [dataPoint, ...allData.get(key)])
: allData.set(key, [dataPoint]);
});
let entryLength = [];
const entryLength = [];
for (const [key, value] of allData.entries()) {
allData.set(key, _.filter(value, filterInvalidYValues));
entryLength.push({ key, length: value.length });
Expand All @@ -141,7 +146,9 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {

export function getXYValuesByFieldName(bucket, fieldName) {
const x = new Date(bucket.key.date);
const path = bucket[fieldName] ? `${fieldName}.value` : 'doc_count';
// Parse the fieldName containing "." to "_"
const parsedFieldName = fieldName.replace(/\./g, '_');
const path = bucket[parsedFieldName] ? `${parsedFieldName}.value` : 'doc_count';
const y = _.get(bucket, path, null);
return { x, y };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class DefineMonitor extends Component {
// Default `count of documents` graph when using Bucket-level monitor
let graphs = [
<Fragment key={`multi-visual-graph-0`}>
<VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />,
<VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />
</Fragment>,
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import _ from 'lodash';
import { FORMIK_INITIAL_TRIGGER_VALUES, TRIGGER_TYPE } from '../../CreateTrigger/utils/constants';

export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (value) => {
if (!value) return 'Required';
if (!value) return 'Required.';
const nameExists = triggers.filter((trigger) => {
const triggerId = _.get(
trigger,
Expand All @@ -44,7 +44,7 @@ export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (valu
return triggerToEditId !== triggerId && triggerName.toLowerCase() === value.toLowerCase();
});
if (nameExists.length > 0) {
return 'Trigger name already used';
return 'Trigger name already used.';
}
// TODO: character restrictions
// TODO: character limits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ describe('validateTriggerName', () => {
});

test('returns Required string if falsy value', () => {
expect(validateTriggerName([], {})()).toBe('Required');
expect(validateTriggerName([], {})('')).toBe('Required');
expect(validateTriggerName([], {})()).toBe('Required.');
expect(validateTriggerName([], {})('')).toBe('Required.');
});
test('returns false if name already exists in monitor while creates new trigger', () => {
const triggers = [{ [TRIGGER_TYPE.QUERY_LEVEL]: { id: '123', name: 'Test' } }];
expect(validateTriggerName(triggers, { [TRIGGER_TYPE.QUERY_LEVEL]: {} })('Test')).toBe(
'Trigger name already used'
'Trigger name already used.'
);
});

Expand Down
Loading