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

Composite monitors #611

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
28b8162
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 9, 2023
c068abd
Merge branch 'main' of https://github.com/opensearch-project/alerting…
jovancvetkovic3006 Jun 9, 2023
11b5b0d
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 15, 2023
a8aa47a
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 15, 2023
83d6eb6
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 16, 2023
567db1d
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 16, 2023
f6b39fe
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 18, 2023
2201c0e
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 19, 2023
654a5f1
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 19, 2023
39b4d0d
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 19, 2023
be4b4bf
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 19, 2023
1792d4c
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 19, 2023
e60c4ff
Merge branch 'main' of https://github.com/opensearch-project/alerting…
jovancvetkovic3006 Jun 19, 2023
8e5723b
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 20, 2023
12f0e6f
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 20, 2023
88d7ac9
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 20, 2023
11ebb8d
[FEATURE] Add composite monitor type #573
jovancvetkovic3006 Jun 21, 2023
59ec9d1
new columns with deletion modal for monitors list; added condition co…
amsiglan Jun 22, 2023
727a692
Code review #573
jovancvetkovic3006 Jun 22, 2023
bf802c1
Code review #573
jovancvetkovic3006 Jun 22, 2023
475807f
Code review #573
jovancvetkovic3006 Jun 22, 2023
c796666
Code review #573
jovancvetkovic3006 Jun 22, 2023
2ad090a
Code review #573
jovancvetkovic3006 Jun 22, 2023
bcd173b
Code review #573
jovancvetkovic3006 Jun 22, 2023
8a02403
Code review #573
jovancvetkovic3006 Jun 22, 2023
87829fd
Code review #573
jovancvetkovic3006 Jun 22, 2023
4e66dd9
Code review #573
jovancvetkovic3006 Jun 22, 2023
277506e
Code review #573
jovancvetkovic3006 Jun 22, 2023
64aca92
Code review #573
jovancvetkovic3006 Jun 22, 2023
f989735
Code review #573
jovancvetkovic3006 Jun 22, 2023
a80bb3d
Merge remote-tracking branch 'jovan/composite_monitors' into composit…
amsiglan Jun 22, 2023
ed738c8
Code review #573
jovancvetkovic3006 Jun 23, 2023
5a7fe5e
Code review #573
jovancvetkovic3006 Jun 23, 2023
b43dda7
Code review #573
jovancvetkovic3006 Jun 23, 2023
5b01da5
Code review #573
jovancvetkovic3006 Jun 23, 2023
c806609
Code review #573
jovancvetkovic3006 Jun 23, 2023
77c23d7
Code review #573
jovancvetkovic3006 Jun 23, 2023
fa03e0f
Code review #573
jovancvetkovic3006 Jun 23, 2023
5ec5aad
Code review #573
jovancvetkovic3006 Jun 23, 2023
a9cef2e
Code review #573
jovancvetkovic3006 Jun 23, 2023
c9df028
Cypress tests
jovancvetkovic3006 Jun 26, 2023
dcedac0
Cypress tests
jovancvetkovic3006 Jun 26, 2023
ed1bef7
Unit tests
jovancvetkovic3006 Jun 27, 2023
308b70c
Unit tests
jovancvetkovic3006 Jun 27, 2023
fa4f600
Add visual editor callout message for advanced conditions
jovancvetkovic3006 Jun 27, 2023
43c689f
Add visual editor callout message for advanced conditions
jovancvetkovic3006 Jun 27, 2023
314c47e
Add visual editor callout message for advanced conditions
jovancvetkovic3006 Jun 27, 2023
bd403f3
auto add cinditions when associated monitor is selected
jovancvetkovic3006 Jun 27, 2023
18f548e
getting associated monitors count
amsiglan Jun 27, 2023
37e2e8b
Merge remote-tracking branch 'jovan/composite_monitors' into composit…
amsiglan Jun 27, 2023
e0f9abb
Merge remote-tracking branch 'jovan/composite_monitors_follow_up' int…
amsiglan Jun 27, 2023
5f689d5
ux updates
amsiglan Jun 28, 2023
06584f6
Adds multiple triggers
jovancvetkovic3006 Jun 28, 2023
335b87c
Adds multiple triggers
jovancvetkovic3006 Jun 28, 2023
abaa7e5
Adds multiple triggers
jovancvetkovic3006 Jun 28, 2023
0f560a6
Adds multiple triggers
jovancvetkovic3006 Jun 28, 2023
cdf168f
Adds multiple triggers
jovancvetkovic3006 Jun 28, 2023
3a3578f
integrated all apis; monitor deletion added
amsiglan Jun 30, 2023
f5265fc
fixed delete modal
amsiglan Jun 30, 2023
3f29148
fixed ux issues from bug bash
amsiglan Jul 7, 2023
5d5e236
addressed UX feedback
amsiglan Jul 11, 2023
c918bb8
merged main
amsiglan Jul 11, 2023
b6d6593
fixed monitor service
amsiglan Jul 11, 2023
531f0c1
fixed build error; schedule
amsiglan Jul 11, 2023
9cde458
updated snapshots
amsiglan Jul 11, 2023
07e9116
addressed PR comments; changed description texts
amsiglan Jul 11, 2023
a492813
fixed cypress tests
amsiglan Jul 11, 2023
2c363ec
updated cypress tests
amsiglan Jul 11, 2023
e2204d5
removed commented code
amsiglan Jul 11, 2023
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
49 changes: 4 additions & 45 deletions cypress/integration/composite_level_monitor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('CompositeLevelMonitor', () => {

it('by visual editor', () => {
// Select visual editor for method of definition
// cy.intercept('GET', '/api/notifications/get_configs?*', channelResponse);
amsiglan marked this conversation as resolved.
Show resolved Hide resolved
cy.get('[data-test-subj="visualEditorRadioCard"]').click({ force: true });

// Wait for input to load and then type in the monitor name
Expand All @@ -68,30 +69,17 @@ describe('CompositeLevelMonitor', () => {
.type('monitorTwo', { delay: 50 })
.type('{enter}');

cy.get('button').contains('Add trigger').click({ force: true });

// Type trigger name
cy.get('[data-test-subj="composite-trigger-name"]')
.type('{selectall}')
.type('{backspace}')
.type('Composite trigger');

// Add associated monitors to condition
cy.get('[data-test-subj="condition-add-options-btn"]').click({ force: true });

cy.get('[data-test-subj="select-expression_0"]').click({ force: true });
cy.wait(1000);
cy.get('[data-test-subj="monitors-combobox-0"]')
.type('monitorOne', { delay: 50 })
.type('{enter}');

cy.get('[data-test-subj="select-expression_1"]').click({ force: true });
cy.wait(1000);
cy.get('[data-test-subj="monitors-combobox-1"]')
.type('monitorTwo', { delay: 50 })
.type('{enter}');

// TODO: Test with Notifications plugin
// Select notification channel
// cy.get('[title="Notification 1"]').type('Channel name');
// cy.get('[name="channel_name_0_0"]').find('input').type('Slack QA').type('{enter}');

cy.intercept('api/alerting/workflows').as('createMonitorRequest');
cy.intercept(`api/alerting/monitors?*`).as('getMonitorsRequest');
Expand Down Expand Up @@ -172,35 +160,6 @@ describe('CompositeLevelMonitor', () => {
}
});
});

it('by visual editor', () => {
// Verify edit page
cy.contains('Edit monitor', { timeout: 20000 });
cy.get('input[name="name"]').type('_edited');

cy.get('label').contains('Visual editor').click({ force: true });

cy.get('button').contains('Associate another monitor').click({ force: true });

cy.get('[data-test-subj="monitors_list_2"]')
.type('monitorThree', { delay: 50 })
.type('{enter}');

cy.get('[data-test-subj="condition-add-options-btn"]').click({ force: true });
cy.get('[data-test-subj="select-expression_2"]').click({ force: true });
cy.wait(1000);
cy.get('[data-test-subj="monitors-combobox-2"]')
.type('monitorThree', { delay: 50 })
.type('{enter}');

cy.intercept('api/alerting/workflows/*').as('updateMonitorRequest');
cy.get('button').contains('Update').click({ force: true });

// Wait for monitor to be created
cy.wait('@updateMonitorRequest').then(() => {
cy.get('.euiTitle--large').contains(`${SAMPLE_VISUAL_EDITOR_MONITOR}_edited`);
});
});
});

after(() => clearAll());
Expand Down
2 changes: 2 additions & 0 deletions cypress/integration/query_level_monitor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ describe('Query-Level Monitors', () => {

// Click the Delete button
cy.contains('Delete').click({ force: true });
cy.wait(1000);
cy.get('[data-test-subj="confirmModalConfirmButton"]').click({ force: true });

// Confirm we can see an empty monitor list
cy.contains('There are no existing monitors');
Expand Down
2 changes: 1 addition & 1 deletion public/components/DeleteModal/DeleteMonitorModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const DeleteMonitorModal = ({
warningHeading = `Unable to delete ${monitorNames[0]}`;
warningBody = (
<>
{`The monitor ${monitorNames[0]} is currently associated with composite monitors. Unlink from the composite monitors before deleting this monitor.`}
{`The monitor ${monitorNames[0]} is currently being used as a delegate monitor for composite monitors. Unlink from the following composite monitors before deleting this monitor:`}
{ associatedWorkflows ?
<ul>
{associatedWorkflows.map(({ id, name }) => <li><EuiLink target='_blank' href={`${PLUGIN_NAME}#/monitors/${id}?type=workflow`}>{name}</EuiLink></li>)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React, { Fragment, useState, useEffect } from 'react';
import { EuiSpacer, EuiText } from '@elastic/eui';
import { EuiLink, EuiSpacer, EuiText } from '@elastic/eui';
import MonitorsList from './components/MonitorsList';
import MonitorsEditor from './components/MonitorsEditor';
import { monitorTypesForComposition } from '../../../../utils/constants';
Expand Down Expand Up @@ -46,10 +46,19 @@ const AssociateMonitors = ({ isDarkMode, values, httpClient, errors }) => {
return (
<Fragment>
<EuiText size={'m'} style={{ paddingBottom: '0px', marginBottom: '0px' }}>
<h4>Associate monitors</h4>
<h4>Delegate monitors</h4>
</EuiText>
<EuiText color={'subdued'} size={'xs'}>
Associate two or more monitors to run as part of this workflow.
Delegate two or more monitors to run as part of this workflow. The monitor types per query,
per bucket, and per document are supported.{' '}
<EuiLink
href={
'https://opensearch.org/docs/latest/observing-your-data/alerting/monitors/#monitor-types'
}
target="_blank"
>
Learn more.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: Do we typically have a . in the Learn more link text? A quick search suggests we don't typically include it https://github.com/search?q=repo%3Aopensearch-project%2Falerting-dashboards-plugin+%22Learn+more%22&type=code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me fix this in a follow up PR

</EuiLink>
</EuiText>

<EuiSpacer size="m" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Array [
style="padding-bottom:0px;margin-bottom:0px"
>
<h4>
Associate monitors
Delegate monitors
</h4>
</div>,
<div
Expand All @@ -16,7 +16,23 @@ Array [
<div
class="euiTextColor euiTextColor--subdued"
>
Associate two or more monitors to run as part of this workflow.
Delegate two or more monitors to run as part of this workflow. The monitor types per query, per bucket, and per document are supported.
<a
class="euiLink euiLink--primary"
href="https://opensearch.org/docs/latest/observing-your-data/alerting/monitors/#monitor-types"
rel="noopener noreferrer"
target="_blank"
>
Learn more.
<div>
EuiIconMock
</div>
<span
class="euiScreenReaderOnly"
>
(opens in a new tab or window)
</span>
</a>
</div>
</div>,
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from '../../../../../components/FormControls';
import { DEFAULT_ASSOCIATED_MONITORS_VALUE } from '../../../containers/CreateMonitor/utils/constants';
import { getMonitors } from '../AssociateMonitors';
import { required } from '../../../../../utils/validate';

const MonitorsList = ({ values, httpClient }) => {
const formikFieldName = 'associatedMonitorsList';
Expand Down Expand Up @@ -172,15 +173,11 @@ const MonitorsList = ({ values, httpClient }) => {

const isValid = () => Object.keys(selection).length > 1;

const validate = () => {
if (!isValid()) return 'Required.';
};

return (
<FormikInputWrapper
name={formikFieldName}
fieldProps={{
validate: validate,
validate: required,
}}
render={({ form }) => (
<FormikFormRow
Expand All @@ -189,7 +186,7 @@ const MonitorsList = ({ values, httpClient }) => {
rowProps={{
label: 'Monitor',
isInvalid: () => form.touched[formikFieldName] && !isValid(),
error: () => validate(),
error: () => required(),
}}
>
<Fragment>
Expand Down Expand Up @@ -271,10 +268,10 @@ const MonitorsList = ({ values, httpClient }) => {
onClick={() => onAddMonitor()}
disabled={fields.length >= 10 || fields.length >= options.length}
>
Associate another monitor
Add another monitor
</EuiButton>
<EuiText color={'subdued'} size={'xs'}>
You can associate up to {10 - fields.length} more monitors.
You can add up to {10 - fields.length} more monitors.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-blocking nitpick: This text could potentially be You can add up to 1 more monitors. This helper function might be helpful here.

export const inputLimitText = (

</EuiText>
</Fragment>
</FormikFormRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ exports[`MonitorsList renders 1`] = `
<span
class="euiButton__text"
>
Associate another monitor
Add another monitor
</span>
</span>
</button>
Expand All @@ -176,7 +176,7 @@ exports[`MonitorsList renders 1`] = `
<div
class="euiTextColor euiTextColor--subdued"
>
You can associate up to 8 more monitors.
You can add up to 8 more monitors.
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,29 @@ const onChangeDefinition = (e, form) => {

const queryLevelDescription = (
<EuiText color={'subdued'} size={'xs'} style={{ paddingBottom: '10px', paddingTop: '0px' }}>
Per query monitors run a specified query and define triggers that check the results of that
query.
Per query monitors run a query and generate alerts based on trigger criteria that match query
results.
</EuiText>
);

const bucketLevelDescription = (
<EuiText color={'subdued'} size={'xs'} style={{ paddingBottom: '10px', paddingTop: '0px' }}>
Per bucket monitors allow you to group results into buckets and define triggers that check each
bucket.
Per bucket monitors run a query that evaluates trigger criteria based on aggregated values in
the dataset.
</EuiText>
);

const clusterMetricsDescription = (
<EuiText color={'subdued'} size={'xs'} style={{ paddingBottom: '10px', paddingTop: '0px' }}>
Per cluster metrics monitors allow you to alert based on responses to common REST APIs.
Per cluster metrics monitors run API requests to monitor the cluster’s health.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-blocking nitpick: Replacing health with state might better represent the functionality of these monitors, but I don't have strong opinions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The text came from content writers, so let's evaluate this as a fast follow

</EuiText>
);

const documentLevelDescription = // TODO DRAFT: confirm wording
(
<EuiText color={'subdued'} size={'xs'} style={{ paddingBottom: '10px', paddingTop: '0px' }}>
Per document monitors allow you to run queries on new documents as they're indexed.
Per document monitors run queries that return individual documents matching the trigger
conditions.
</EuiText>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ exports[`MonitorType renders 1`] = `
<div
class="euiTextColor euiTextColor--subdued"
>
Per query monitors run a specified query and define triggers that check the results of that query.
Per query monitors run a query and generate alerts based on trigger criteria that match query results.
</div>
</div>
</div>
Expand Down Expand Up @@ -129,7 +129,7 @@ exports[`MonitorType renders 1`] = `
<div
class="euiTextColor euiTextColor--subdued"
>
Per bucket monitors allow you to group results into buckets and define triggers that check each bucket.
Per bucket monitors run a query that evaluates trigger criteria based on aggregated values in the dataset.
</div>
</div>
</div>
Expand Down Expand Up @@ -191,7 +191,7 @@ exports[`MonitorType renders 1`] = `
<div
class="euiTextColor euiTextColor--subdued"
>
Per cluster metrics monitors allow you to alert based on responses to common REST APIs.
Per cluster metrics monitors run API requests to monitor the cluster’s health.
</div>
</div>
</div>
Expand Down Expand Up @@ -253,7 +253,7 @@ exports[`MonitorType renders 1`] = `
<div
class="euiTextColor euiTextColor--subdued"
>
Per document monitors allow you to run queries on new documents as they're indexed.
Per document monitors run queries that return individual documents matching the trigger conditions.
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ export const FORMIK_INITIAL_WHERE_EXPRESSION_VALUES = {
fieldRangeEnd: undefined,
};

/** Sample delegate
* {
order: 1,
monitor_id: '{{m1}}',
}
*/
export const DEFAULT_ASSOCIATED_MONITORS_VALUE = {
sequence: {
delegates: [
/*{
order: 1,
monitor_id: '{{m1}}',
}*/
],
delegates: [],
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { Fragment } from 'react';
import React from 'react';
import ContentPanel from '../../../../components/ContentPanel';
import Schedule from '../../components/Schedule';
import AssociateMonitors from '../../components/AssociateMonitors/AssociateMonitors';
import { EuiSpacer } from '@elastic/eui';
import { MONITOR_TYPE, SEARCH_TYPE } from '../../../../utils/constants';

const WorkflowDetails = ({ values, isDarkMode, httpClient, errors }) => {
const isAd = values.searchType === SEARCH_TYPE.AD;
const isComposite = values.monitor_type === MONITOR_TYPE.COMPOSITE_LEVEL;
return (
<ContentPanel
title="Workflow"
Expand All @@ -24,19 +21,15 @@ const WorkflowDetails = ({ values, isDarkMode, httpClient, errors }) => {
paddingTop: '20px',
}}
>
<Schedule isAd={isAd} />
<Schedule isAd={false} />

{isComposite && (
<Fragment>
<EuiSpacer size="xl" />
<AssociateMonitors
isDarkMode={isDarkMode}
values={values}
httpClient={httpClient}
errors={errors}
/>
</Fragment>
)}
<EuiSpacer size="xl" />
<AssociateMonitors
isDarkMode={isDarkMode}
values={values}
httpClient={httpClient}
errors={errors}
/>
</ContentPanel>
);
};
Expand Down
Loading