Skip to content

Commit

Permalink
Temporary fix for blank monitor name on Monitors page issue. (#593) (#…
Browse files Browse the repository at this point in the history
…604)

* Refactored the cluster metrics monitor configuration component to not automatically execute the monitor dryrun when there are no triggers defined.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>

* Temporary fix for bug described in issue 569.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>

* Fixed display issue.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>

* Fixed unit test. Updated snapshots.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>

---------

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
(cherry picked from commit c152885)

Co-authored-by: AWSHurneyt <hurneyt@amazon.com>
  • Loading branch information
opensearch-trigger-bot[bot] and AWSHurneyt committed Sep 12, 2023
1 parent 96494b5 commit 964d4de
Show file tree
Hide file tree
Showing 14 changed files with 255 additions and 42 deletions.
73 changes: 73 additions & 0 deletions cypress/fixtures/sample_cluster_metrics_stats_monitor.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{
"name": "sample_cluster_metrics_stats_monitor",
"type": "monitor",
"monitor_type": "cluster_metrics_monitor",
"enabled": true,
"schedule": {
"period": {
"unit": "MINUTES",
"interval": 1
}
},
"inputs": [
{
"uri": {
"api_type": "CLUSTER_STATS",
"path": "_cluster/stats/",
"path_params": "",
"url": "http://localhost:9200/_cluster/stats/"
}
}
],
"triggers": [
{
"query_level_trigger": {
"id": "Y5mmA4kBIezNcMbMJnEy",
"name": "sample_cluster_metrics_stats_monitor-trigger1",
"severity": "1",
"condition": {
"script": {
"source": "ctx.results[0].indices.count >= 0",
"lang": "painless"
}
},
"actions": []
}
}
],
"ui_metadata": {
"schedule": {
"timezone": null,
"frequency": "interval",
"period": {
"unit": "MINUTES",
"interval": 1
},
"daily": 0,
"weekly": {
"tue": false,
"wed": false,
"thur": false,
"sat": false,
"fri": false,
"mon": false,
"sun": false
},
"monthly": {
"type": "day",
"day": 1
},
"cronExpression": "0 */1 * * *"
},
"search": {
"searchType": "clusterMetrics",
"timeField": "",
"aggregations": [],
"groupBy": [],
"bucketValue": 1,
"bucketUnitOfTime": "h",
"filters": []
},
"monitor_type": "cluster_metrics_monitor"
}
}
146 changes: 146 additions & 0 deletions cypress/integration/monitors_dashboard_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { INDEX, PLUGIN_NAME } from '../support/constants';
import sampleAlertsFlyoutBucketMonitor from '../fixtures/sample_alerts_flyout_bucket_level_monitor.json';
import sampleAlertsFlyoutQueryMonitor from '../fixtures/sample_alerts_flyout_query_level_monitor.json';
import sampleClusterMetricsHealthMonitor from '../fixtures/sample_cluster_metrics_health_monitor.json';
import sampleClusterMetricsStatsMonitor from '../fixtures/sample_cluster_metrics_stats_monitor.json';

const queryMonitor = {
...sampleAlertsFlyoutQueryMonitor,
id: 'monitors_dashboard_cypress_query_level',
name: 'monitors_dashboard_cypress_query_level',
enabled: false,
};
const bucketMonitor = {
...sampleAlertsFlyoutBucketMonitor,
id: 'monitors_dashboard_cypress_bucket_level',
name: 'monitors_dashboard_cypress_bucket_level',
enabled: false,
};
const clusterHealthMonitor = {
...sampleClusterMetricsHealthMonitor,
id: 'monitors_dashboard_cypress_cluster_health',
name: 'monitors_dashboard_cypress_cluster_health',
enabled: false,
triggers: [
{
query_level_trigger: {
id: 'WJmlA4kBIezNcMbMwnFg',
name: 'sample_cluster_metrics_health_monitor-trigger1',
severity: '1',
condition: {
script: {
source: 'ctx.results[0].status != "green"',
lang: 'painless',
},
},
actions: [],
},
},
],
};
const clusterStatsMonitor = {
...sampleClusterMetricsStatsMonitor,
enabled: false,
id: 'monitors_dashboard_cypress_cluster_stats',
name: 'monitors_dashboard_cypress_cluster_stats',
};
const testMonitors = [
{
monitor: queryMonitor,
expectedAlertsCount: 1,
triggerName: queryMonitor.triggers[0].query_level_trigger.name,
},
{
monitor: bucketMonitor,
expectedAlertsCount: 46,
triggerName: bucketMonitor.triggers[0].bucket_level_trigger.name,
},
{
monitor: clusterHealthMonitor,
expectedAlertsCount: 1,
triggerName: clusterHealthMonitor.triggers[0].query_level_trigger.name,
},
{
monitor: clusterStatsMonitor,
expectedAlertsCount: 1,
triggerName: clusterStatsMonitor.triggers[0].query_level_trigger.name,
},
];

describe('Monitors dashboard page', () => {
before(() => {
// Delete any existing monitors
cy.deleteAllMonitors()
.then(() => {
// Load sample data
cy.loadSampleEcommerceData();
})
.then(() => {
// Short wait to reduce flakiness while ecommerce data is loaded
cy.wait(5000);

// Create the test monitors
testMonitors.forEach((entry) => cy.createAndExecuteMonitor(entry.monitor));
});

// Visit Alerting OpenSearch Dashboards
cy.visit(`${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/monitors`);
});

beforeEach(() => {
// Refresh Alerting OpenSearch Dashboards
cy.visit(`${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/monitors`);

// Common text to wait for to confirm page loaded, give up to 20 seconds for initial load
cy.contains('Create monitor', { timeout: 20000 });
});

it('Displays expected number of alerts', () => {
// Ensure the 'Monitor name' column is sorted in ascending order by sorting another column first
cy.contains('Last updated by').click({ force: true });
cy.contains('Monitor name').click({ force: true });

testMonitors.forEach((entry) => {
cy.get('tbody > tr')
.filter(`:contains(${entry.monitor.name})`, { timeout: 20000 })
.within(() => {
cy.get('[class="euiTableRowCell"]')
.filter(':contains(Latest alert)', { timeout: 20000 })
.should('contain', entry.triggerName);

cy.get('[class="euiTableRowCell"]')
.filter(':contains(State)', { timeout: 20000 })
.should('contain', 'Disabled');

cy.get('[class="euiTableRowCell"]')
.filter(':contains(Active)', { timeout: 20000 })
.should('contain', entry.expectedAlertsCount);

cy.get('[class="euiTableRowCell"]')
.filter(':contains(Acknowledged)', { timeout: 20000 })
.should('contain', 0);

cy.get('[class="euiTableRowCell"]')
.filter(':contains(Errors)', { timeout: 20000 })
.should('contain', 0);

cy.get('[class="euiTableRowCell"]')
.filter(':contains(Ignored)', { timeout: 20000 })
.should('contain', 0);
});
});
});

after(() => {
// Delete all monitors
cy.deleteAllMonitors();

// Delete sample data
cy.deleteIndexByName(INDEX.SAMPLE_DATA_ECOMMERCE);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ const ClusterMetricsMonitor = ({
label: (
<div>
<EuiText size={'xs'}>
<strong>Query parameters</strong>
<strong>Path parameters</strong>
{!requirePathParams && <i> - optional </i>}
</EuiText>
<EuiText color={'subdued'} size={'xs'}>
Expand Down Expand Up @@ -243,7 +243,7 @@ const ClusterMetricsMonitor = ({
size={'s'}
style={{ backgroundColor: 'transparent', paddingRight: !hidePathParams && '0px' }}
>
GET {_.get(API_TYPES, `${apiType}.prependText`)}
GET {'/' + _.get(API_TYPES, `${apiType}.prependText`) + '/'}
</EuiText>
),
append: !_.isEmpty(_.get(API_TYPES, `${apiType}.appendText`)) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

export const URL_DEFAULT_PREFIX = 'http://localhost:9200';
export const API_PATH_REQUIRED_PLACEHOLDER_TEXT = 'Select an API.';
export const EMPTY_PATH_PARAMS_TEXT = 'Enter remaining path components and path parameters';
export const GET_API_TYPE_DEBUG_TEXT =
Expand Down Expand Up @@ -37,7 +38,7 @@ export const API_TYPES = {
exampleText: 'indexAlias1,indexAlias2...',
label: 'Cluster health',
paths: {
withPathParams: '_cluster/health/',
withPathParams: '_cluster/health',
withoutPathParams: '_cluster/health',
},
get prependText() {
Expand All @@ -55,7 +56,7 @@ export const API_TYPES = {
exampleText: 'nodeFilter1,nodeFilter2...',
label: 'Cluster stats',
paths: {
withPathParams: '_cluster/stats/nodes/',
withPathParams: '_cluster/stats/nodes',
withoutPathParams: '_cluster/stats',
},
get prependText() {
Expand Down Expand Up @@ -127,7 +128,7 @@ export const API_TYPES = {
exampleText: 'index1,index2...',
label: 'Recovery',
paths: {
withPathParams: '_cat/recovery/',
withPathParams: '_cat/recovery',
withoutPathParams: '_cat/recovery',
},
get prependText() {
Expand All @@ -145,7 +146,7 @@ export const API_TYPES = {
exampleText: 'repositoryName',
label: 'List snapshots',
paths: {
withPathParams: '_cat/snapshots/',
withPathParams: '_cat/snapshots',
withoutPathParams: undefined,
},
get prependText() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export const getExamplePathParams = (apiType) => {

export const getApiType = (uri) => {
let apiType = '';
const path = _.get(uri, 'path');
// Trim '/' characters from the beginning and end of the path
const path = _.get(uri, 'path')?.replace(/^\/+|\/+$/g, '');
if (_.isEmpty(path)) return apiType;
_.keys(API_TYPES).forEach((apiTypeKey) => {
const withPathParams = _.get(API_TYPES, `${apiTypeKey}.paths.withPathParams`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('clusterMetricsMonitorHelpers', () => {
api_type: API_TYPES.CLUSTER_HEALTH.type,
path: path,
path_params: pathParams,
url: `http://localhost:9200/${path}${pathParams}`,
url: `http://localhost:9200/${path}/${pathParams}`,
},
};
expect(buildClusterMetricsRequest(values)).toEqual(expectedResult.uri);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ exports[`formikToClusterMetricsUri can build a ClusterMetricsMonitor request wit
Object {
"uri": Object {
"api_type": "",
"path": "params",
"path": "",
"path_params": "",
"url": "",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import moment from 'moment-timezone';
import { BUCKET_COUNT, DEFAULT_COMPOSITE_AGG_SIZE, FORMIK_INITIAL_VALUES } from './constants';
import { MONITOR_TYPE, SEARCH_TYPE } from '../../../../../utils/constants';
import { OPERATORS_QUERY_MAP } from './whereFilters';
import { API_TYPES } from '../../../components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants';
import {
API_TYPES,
URL_DEFAULT_PREFIX,
} from '../../../components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants';
import {
getApiPath,
getApiType,
Expand Down Expand Up @@ -118,13 +121,13 @@ export function formikToClusterMetricsInput(values) {
let pathParams = _.get(values, 'uri.path_params', FORMIK_INITIAL_VALUES.uri.path_params);
pathParams = _.trim(pathParams);
const hasPathParams = !_.isEmpty(pathParams);
if (hasPathParams) _.concat(pathParams, _.get(API_TYPES, `${apiType}.appendText`, ''));
let path = _.get(values, 'uri.path', FORMIK_INITIAL_VALUES.uri.path);
if (_.isEmpty(path)) path = getApiPath(hasPathParams, apiType);
const canConstructUrl = !_.isEmpty(apiType);
const url = canConstructUrl
? `http://localhost:9200/${path}${pathParams}`
: FORMIK_INITIAL_VALUES.uri.url;
const path = getApiPath(hasPathParams, apiType);
let url = FORMIK_INITIAL_VALUES.uri.url;
if (!_.isEmpty(apiType)) {
url = URL_DEFAULT_PREFIX;
if (!_.isEmpty(path)) url = url + '/' + path;
if (hasPathParams) url = url + '/' + pathParams + _.get(API_TYPES, `${apiType}.appendText`, '');
}
return {
uri: {
api_type: apiType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class ConfigureTriggers extends React.Component {
this.onQueryMappings();
break;
case MONITOR_TYPE.CLUSTER_METRICS:
if (canExecuteClusterMetricsMonitor(uri)) this.onRunExecute();
const numOfTriggers = _.get(this.props.triggerValues, 'triggerDefinitions', []).length;
if (numOfTriggers > 0 && canExecuteClusterMetricsMonitor(uri)) this.onRunExecute();
break;
}
};
Expand Down
25 changes: 2 additions & 23 deletions public/pages/Monitors/containers/Monitors/Monitors.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import MonitorEmptyPrompt from '../../components/MonitorEmptyPrompt';
import { DEFAULT_PAGE_SIZE_OPTIONS, DEFAULT_QUERY_PARAMS } from './utils/constants';
import { getURLQueryParams } from './utils/helpers';
import { columns as staticColumns } from './utils/tableUtils';
import { MONITOR_ACTIONS, MONITOR_TYPE } from '../../../../utils/constants';
import { MONITOR_ACTIONS } from '../../../../utils/constants';
import { backendErrorNotification } from '../../../../utils/helpers';
import { displayAcknowledgedAlertsToast } from '../../../Dashboard/utils/helpers';

Expand Down Expand Up @@ -129,27 +129,6 @@ export default class Monitors extends Component {
};
}

// TODO: The getMonitors API is wrapping the 'monitor' field for ClusterMetrics monitors in an additional 'monitor' object.
// This formatGetMonitorsResponse method is a temporary means of resolving that issue until it can be debugged on the backend.
formatGetMonitorsResponse = (monitors) => {
const unwrappedMonitors = [];
monitors.forEach((monitor) => {
const monitorType = _.get(monitor, 'monitor.monitor.monitor_type', 'monitor.monitor_type');
switch (monitorType) {
case MONITOR_TYPE.CLUSTER_METRICS:
let unwrappedMonitor = monitor.monitor;
_.set(monitor, 'monitor', unwrappedMonitor.monitor);
_.set(monitor, 'name', monitor.monitor.name);
_.set(monitor, 'enabled', monitor.monitor.enabled);
unwrappedMonitors.push(monitor);
break;
default:
unwrappedMonitors.push(monitor);
}
});
return unwrappedMonitors;
};

async getMonitors(from, size, search, sortField, sortDirection, state) {
this.setState({ loadingMonitors: true });
try {
Expand All @@ -160,7 +139,7 @@ export default class Monitors extends Component {
const response = await httpClient.get('../api/alerting/monitors', { query: params });
if (response.ok) {
const { monitors, totalMonitors } = response;
this.setState({ monitors: this.formatGetMonitorsResponse(monitors), totalMonitors });
this.setState({ monitors, totalMonitors });
} else {
console.log('error getting monitors:', response);
// TODO: 'response.ok' is 'false' when there is no alerting config index in the cluster, and notification should not be shown to new Alerting users
Expand Down

0 comments on commit 964d4de

Please sign in to comment.