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

Temporary fix for blank monitor name on Monitors page issue. #593

Merged
merged 5 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -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;
}
};
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