From c152885b23ae2a883d8c726a60c7ad9154671174 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Mon, 10 Jul 2023 13:42:20 -0700 Subject: [PATCH] Temporary fix for blank monitor name on Monitors page issue. (#593) * Refactored the cluster metrics monitor configuration component to not automatically execute the monitor dryrun when there are no triggers defined. Signed-off-by: AWSHurneyt * Temporary fix for bug described in issue 569. Signed-off-by: AWSHurneyt * Fixed display issue. Signed-off-by: AWSHurneyt * Fixed unit test. Updated snapshots. Signed-off-by: AWSHurneyt --------- Signed-off-by: AWSHurneyt --- ...ample_cluster_metrics_health_monitor.json} | 0 .../sample_cluster_metrics_stats_monitor.json | 73 +++++++++ .../integration/monitors_dashboard_spec.js | 146 ++++++++++++++++++ .../ClusterMetricsMonitor.js | 4 +- .../utils/clusterMetricsMonitorConstants.js | 9 +- .../utils/clusterMetricsMonitorHelpers.js | 3 +- .../clusterMetricsMonitorHelpers.test.js | 2 +- .../formikToMonitor.test.js.snap | 2 +- .../CreateMonitor/utils/formikToMonitor.js | 19 ++- .../ConfigureTriggers/ConfigureTriggers.js | 3 +- .../Monitors/containers/Monitors/Monitors.js | 25 +-- .../__snapshots__/Monitors.test.js.snap | 4 + .../containers/Monitors/utils/tableUtils.js | 4 + server/services/MonitorService.js | 3 +- 14 files changed, 255 insertions(+), 42 deletions(-) rename cypress/fixtures/{sample_cluster_metrics_monitor.json => sample_cluster_metrics_health_monitor.json} (100%) create mode 100644 cypress/fixtures/sample_cluster_metrics_stats_monitor.json create mode 100644 cypress/integration/monitors_dashboard_spec.js diff --git a/cypress/fixtures/sample_cluster_metrics_monitor.json b/cypress/fixtures/sample_cluster_metrics_health_monitor.json similarity index 100% rename from cypress/fixtures/sample_cluster_metrics_monitor.json rename to cypress/fixtures/sample_cluster_metrics_health_monitor.json diff --git a/cypress/fixtures/sample_cluster_metrics_stats_monitor.json b/cypress/fixtures/sample_cluster_metrics_stats_monitor.json new file mode 100644 index 000000000..a258b0586 --- /dev/null +++ b/cypress/fixtures/sample_cluster_metrics_stats_monitor.json @@ -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" + } +} diff --git a/cypress/integration/monitors_dashboard_spec.js b/cypress/integration/monitors_dashboard_spec.js new file mode 100644 index 000000000..61facec7a --- /dev/null +++ b/cypress/integration/monitors_dashboard_spec.js @@ -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); + }); +}); diff --git a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/ClusterMetricsMonitor.js b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/ClusterMetricsMonitor.js index d4fc032f0..4389309f9 100644 --- a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/ClusterMetricsMonitor.js +++ b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/ClusterMetricsMonitor.js @@ -211,7 +211,7 @@ const ClusterMetricsMonitor = ({ label: (
- Query parameters + Path parameters {!requirePathParams && - optional } @@ -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`) + '/'} ), append: !_.isEmpty(_.get(API_TYPES, `${apiType}.appendText`)) && ( diff --git a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants.js b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants.js index 9245d1aee..20532552d 100644 --- a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants.js +++ b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants.js @@ -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 = @@ -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() { @@ -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() { @@ -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() { @@ -145,7 +146,7 @@ export const API_TYPES = { exampleText: 'repositoryName', label: 'List snapshots', paths: { - withPathParams: '_cat/snapshots/', + withPathParams: '_cat/snapshots', withoutPathParams: undefined, }, get prependText() { diff --git a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.js b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.js index 55ea7752d..ccccb04d8 100644 --- a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.js +++ b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.js @@ -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`); diff --git a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.test.js b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.test.js index ee48e8a1e..fee91cd81 100644 --- a/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.test.js +++ b/public/pages/CreateMonitor/components/ClusterMetricsMonitor/utils/clusterMetricsMonitorHelpers.test.js @@ -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); diff --git a/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap b/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap index 8421a65b5..530cf4593 100644 --- a/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap +++ b/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap @@ -49,7 +49,7 @@ exports[`formikToClusterMetricsUri can build a ClusterMetricsMonitor request wit Object { "uri": Object { "api_type": "", - "path": "params", + "path": "", "path_params": "", "url": "", }, diff --git a/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js b/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js index 18066b278..84981cb62 100644 --- a/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js +++ b/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js @@ -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, @@ -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, diff --git a/public/pages/CreateTrigger/containers/ConfigureTriggers/ConfigureTriggers.js b/public/pages/CreateTrigger/containers/ConfigureTriggers/ConfigureTriggers.js index 90d63ebfc..9f4e46d07 100644 --- a/public/pages/CreateTrigger/containers/ConfigureTriggers/ConfigureTriggers.js +++ b/public/pages/CreateTrigger/containers/ConfigureTriggers/ConfigureTriggers.js @@ -105,7 +105,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; } }; diff --git a/public/pages/Monitors/containers/Monitors/Monitors.js b/public/pages/Monitors/containers/Monitors/Monitors.js index 9367aafa0..e543471bb 100644 --- a/public/pages/Monitors/containers/Monitors/Monitors.js +++ b/public/pages/Monitors/containers/Monitors/Monitors.js @@ -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'; @@ -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 { @@ -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 diff --git a/public/pages/Monitors/containers/Monitors/__snapshots__/Monitors.test.js.snap b/public/pages/Monitors/containers/Monitors/__snapshots__/Monitors.test.js.snap index 5dcbfd468..929c0d132 100644 --- a/public/pages/Monitors/containers/Monitors/__snapshots__/Monitors.test.js.snap +++ b/public/pages/Monitors/containers/Monitors/__snapshots__/Monitors.test.js.snap @@ -74,24 +74,28 @@ exports[`Monitors renders 1`] = ` Object { "field": "active", "name": "Active", + "render": [Function], "sortable": true, "truncateText": false, }, Object { "field": "acknowledged", "name": "Acknowledged", + "render": [Function], "sortable": true, "truncateText": false, }, Object { "field": "errors", "name": "Errors", + "render": [Function], "sortable": true, "truncateText": false, }, Object { "field": "ignored", "name": "Ignored", + "render": [Function], "sortable": true, "truncateText": false, }, diff --git a/public/pages/Monitors/containers/Monitors/utils/tableUtils.js b/public/pages/Monitors/containers/Monitors/utils/tableUtils.js index 4aab897e9..c51204f1f 100644 --- a/public/pages/Monitors/containers/Monitors/utils/tableUtils.js +++ b/public/pages/Monitors/containers/Monitors/utils/tableUtils.js @@ -69,23 +69,27 @@ export const columns = [ name: 'Active', sortable: true, truncateText: false, + render: (count) => count || 0, }, { field: 'acknowledged', name: 'Acknowledged', sortable: true, truncateText: false, + render: (count) => count || 0, }, { field: 'errors', name: 'Errors', sortable: true, truncateText: false, + render: (count) => count || 0, }, { field: 'ignored', name: 'Ignored', sortable: true, truncateText: false, + render: (count) => count || 0, }, ]; diff --git a/server/services/MonitorService.js b/server/services/MonitorService.js index 7654faabf..475aea4b5 100644 --- a/server/services/MonitorService.js +++ b/server/services/MonitorService.js @@ -228,8 +228,9 @@ export default class MonitorService { _version: version, _seq_no: ifSeqNo, _primary_term: ifPrimaryTerm, - _source: monitor, + _source, } = result; + const monitor = _source.monitor ? _source.monitor : _source; const { name, enabled } = monitor; return [id, { id, version, ifSeqNo, ifPrimaryTerm, name, enabled, monitor }]; }, {});