[Monitoring] Out of the box alert tweaks (elastic#71942)

* Tweaks to thresholds and throttle periods * Fixes * Type fix, and more defensive against no alerts * Remove unnecessary restrictions
chrisronline · Jul 16, 2020 · 44f80e6 · 44f80e6
1 parent eec3ae8
commit 44f80e6
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 16 deletions.
diff --git a/x-pack/plugins/monitoring/public/alerts/callout.tsx b/x-pack/plugins/monitoring/public/alerts/callout.tsx
@@ -24,7 +24,7 @@ const TYPES = [
     severity: AlertSeverity.Danger,
     color: 'danger',
     label: i18n.translate('xpack.monitoring.alerts.callout.dangerLabel', {
-      defaultMessage: 'DAnger alert(s)',
+      defaultMessage: 'Danger alert(s)',
     }),
   },
 ];

diff --git a/x-pack/plugins/monitoring/public/alerts/flyout_expressions/alert_param_duration.tsx b/x-pack/plugins/monitoring/public/alerts/flyout_expressions/alert_param_duration.tsx
@@ -49,7 +49,7 @@ interface Props {
   setAlertParams: (property: string, value: any) => void;
 }
 
-const parseRegex = /(\d+)(\smhd)/;
+const parseRegex = /(\d+)([smhd]{1})/;
 export const AlertParamDuration: React.FC<Props> = (props: Props) => {
   const { name, label, setAlertParams, errors } = props;
   const parsed = parseRegex.exec(props.duration);

diff --git a/x-pack/plugins/monitoring/public/alerts/lib/should_show_alert_badge.ts b/x-pack/plugins/monitoring/public/alerts/lib/should_show_alert_badge.ts
@@ -10,6 +10,9 @@ export function shouldShowAlertBadge(
   alerts: { [alertTypeId: string]: CommonAlertStatus },
   alertTypeIds: string[]
 ) {
+  if (!alerts) {
+    return false;
+  }
   const inSetupMode = isInSetupMode();
   return inSetupMode || alertTypeIds.find((name) => alerts[name] && alerts[name].states.length);
 }
diff --git a/x-pack/plugins/monitoring/server/alerts/base_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/base_alert.test.ts
@@ -63,7 +63,7 @@ describe('BaseAlert', () => {
             interval: '1m',
           },
           tags: [],
-          throttle: '1m',
+          throttle: '1d',
         },
       });
     });

diff --git a/x-pack/plugins/monitoring/server/alerts/base_alert.ts b/x-pack/plugins/monitoring/server/alerts/base_alert.ts
@@ -40,7 +40,7 @@ import { MonitoringLicenseService } from '../types';
 export class BaseAlert {
   public type!: string;
   public label!: string;
-  public defaultThrottle: string = '1m';
+  public defaultThrottle: string = '1d';
   public defaultInterval: string = '1m';
   public rawAlert: Alert | undefined;
   public isLegacy: boolean = false;

diff --git a/x-pack/plugins/monitoring/server/alerts/cluster_health_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/cluster_health_alert.test.ts
@@ -22,7 +22,7 @@ describe('ClusterHealthAlert', () => {
     const alert = new ClusterHealthAlert();
     expect(alert.type).toBe(ALERT_CLUSTER_HEALTH);
     expect(alert.label).toBe('Cluster health');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/cpu_usage_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/cpu_usage_alert.test.ts
@@ -22,9 +22,9 @@ describe('CpuUsageAlert', () => {
     const alert = new CpuUsageAlert();
     expect(alert.type).toBe(ALERT_CPU_USAGE);
     expect(alert.label).toBe('CPU Usage');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
-    expect(alert.defaultParams).toStrictEqual({ threshold: 90, duration: '5m' });
+    expect(alert.defaultParams).toStrictEqual({ threshold: 85, duration: '5m' });
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/cpu_usage_alert.ts b/x-pack/plugins/monitoring/server/alerts/cpu_usage_alert.ts
@@ -39,7 +39,7 @@ const FIRING = i18n.translate('xpack.monitoring.alerts.cpuUsage.firing', {
   defaultMessage: 'firing',
 });
 
-const DEFAULT_THRESHOLD = 90;
+const DEFAULT_THRESHOLD = 85;
 const DEFAULT_DURATION = '5m';
 
 interface CpuUsageParams {
@@ -393,7 +393,16 @@ export class CpuUsageAlert extends BaseAlert {
         continue;
       }
 
-      const instance = services.alertInstanceFactory(`${this.type}:${cluster.clusterUuid}`);
+      const firingNodeUuids = nodes.reduce((list: string[], node) => {
+        const stat = node.meta as AlertCpuUsageNodeStats;
+        if (node.shouldFire) {
+          list.push(stat.nodeId);
+        }
+        return list;
+      }, [] as string[]);
+      firingNodeUuids.sort(); // It doesn't matter how we sort, but keep the order consistent
+      const instanceId = `${this.type}:${cluster.clusterUuid}:${firingNodeUuids.join(',')}`;
+      const instance = services.alertInstanceFactory(instanceId);
       const state = (instance.getState() as unknown) as AlertInstanceState;
       const alertInstanceState: AlertInstanceState = { alertStates: state?.alertStates || [] };
       let shouldExecuteActions = false;

diff --git a/x-pack/plugins/monitoring/server/alerts/elasticsearch_version_mismatch_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/elasticsearch_version_mismatch_alert.test.ts
@@ -22,7 +22,7 @@ describe('ElasticsearchVersionMismatchAlert', () => {
     const alert = new ElasticsearchVersionMismatchAlert();
     expect(alert.type).toBe(ALERT_ELASTICSEARCH_VERSION_MISMATCH);
     expect(alert.label).toBe('Elasticsearch version mismatch');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/kibana_version_mismatch_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/kibana_version_mismatch_alert.test.ts
@@ -22,7 +22,7 @@ describe('KibanaVersionMismatchAlert', () => {
     const alert = new KibanaVersionMismatchAlert();
     expect(alert.type).toBe(ALERT_KIBANA_VERSION_MISMATCH);
     expect(alert.label).toBe('Kibana version mismatch');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/license_expiration_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/license_expiration_alert.test.ts
@@ -29,7 +29,7 @@ describe('LicenseExpirationAlert', () => {
     const alert = new LicenseExpirationAlert();
     expect(alert.type).toBe(ALERT_LICENSE_EXPIRATION);
     expect(alert.label).toBe('License expiration');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/logstash_version_mismatch_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/logstash_version_mismatch_alert.test.ts
@@ -22,7 +22,7 @@ describe('LogstashVersionMismatchAlert', () => {
     const alert = new LogstashVersionMismatchAlert();
     expect(alert.type).toBe(ALERT_LOGSTASH_VERSION_MISMATCH);
     expect(alert.label).toBe('Logstash version mismatch');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/nodes_changed_alert.test.ts b/x-pack/plugins/monitoring/server/alerts/nodes_changed_alert.test.ts
@@ -29,7 +29,7 @@ describe('NodesChangedAlert', () => {
     const alert = new NodesChangedAlert();
     expect(alert.type).toBe(ALERT_NODES_CHANGED);
     expect(alert.label).toBe('Nodes changed');
-    expect(alert.defaultThrottle).toBe('1m');
+    expect(alert.defaultThrottle).toBe('1d');
     // @ts-ignore
     expect(alert.actionVariables).toStrictEqual([
       {

diff --git a/x-pack/plugins/monitoring/server/alerts/nodes_changed_alert.ts b/x-pack/plugins/monitoring/server/alerts/nodes_changed_alert.ts
@@ -155,6 +155,21 @@ export class NodesChangedAlert extends BaseAlert {
       };
     }
 
+    if (
+      Object.values(states.added).length === 0 &&
+      Object.values(states.removed).length === 0 &&
+      Object.values(states.restarted).length === 0
+    ) {
+      return {
+        text: i18n.translate(
+          'xpack.monitoring.alerts.nodesChanged.ui.nothingDetectedFiringMessage',
+          {
+            defaultMessage: `Elasticsearch nodes have changed`,
+          }
+        ),
+      };
+    }
+
     const addedText =
       Object.values(states.added).length > 0
         ? i18n.translate('xpack.monitoring.alerts.nodesChanged.ui.addedFiringMessage', {

diff --git a/x-pack/plugins/monitoring/server/routes/api/v1/alerts/enable.ts b/x-pack/plugins/monitoring/server/routes/api/v1/alerts/enable.ts
@@ -18,7 +18,6 @@ export function enableAlertsRoute(server: any, npRoute: RouteDependencies) {
   npRoute.router.post(
     {
       path: '/api/monitoring/v1/alerts/enable',
-      options: { tags: ['access:monitoring'] },
       validate: false,
     },
     async (context, request, response) => {

diff --git a/x-pack/plugins/monitoring/server/routes/api/v1/alerts/status.ts b/x-pack/plugins/monitoring/server/routes/api/v1/alerts/status.ts
@@ -15,7 +15,6 @@ export function alertStatusRoute(server: any, npRoute: RouteDependencies) {
   npRoute.router.post(
     {
       path: '/api/monitoring/v1/alert/{clusterUuid}/status',
-      options: { tags: ['access:monitoring'] },
       validate: {
         params: schema.object({
           clusterUuid: schema.string(),