[Custom threshold] Reintroduce no data setting in the custom threshold rule (#188300)

maryam-saeidi · web-flow · commit 4552c76d38d7 · 2024-07-18T12:06:41.000+02:00
Fixes #188229, related to #183921 Documentation request: elastic/observability-docs#4068 ## Summary **Note**: I've added an item to deprecate/remove one of the no-data settings in v9. Fixes not showing no data setting and set the related settings to false by default. Based on @maciejforcone's input, we can combine these 2 settings for simplicity, as one of them works at a time. I also changed the tooltip according to which setting is relevant: (we use one action group for both of them in connectors) |No data (without group)|Missing group (with group)| |---|---| |![image](https://github.com/user-attachments/assets/ecf45dd2-d2a7-46ce-abd0-e2a07426f28e)|![image](https://github.com/user-attachments/assets/8dedd0fe-bb4b-4e51-808f-f65f54ee73fd)| Here is how the setting is applied in API: https://github.com/user-attachments/assets/52c52724-6011-4f6d-8464-023cd9a9ea10
diff --git a/x-pack/plugins/observability_solution/observability/public/components/custom_threshold/custom_threshold_rule_expression.test.tsx b/x-pack/plugins/observability_solution/observability/public/components/custom_threshold/custom_threshold_rule_expression.test.tsx
@@ -42,7 +42,7 @@ describe('Expression', () => {
 
   async function setup(
     currentOptions?: CustomThresholdPrefillOptions,
-    customRuleParams?: Record<string, unknown>
+    customRuleParams?: Partial<RuleTypeParams & AlertParams>
   ) {
     const ruleParams: RuleTypeParams & AlertParams = {
       criteria: [],
@@ -164,7 +164,8 @@ describe('Expression', () => {
   it('should prefill the rule using the context metadata', async () => {
     const index = 'changedMockedIndex';
     const currentOptions: CustomThresholdPrefillOptions = {
-      alertOnGroupDisappear: false,
+      alertOnGroupDisappear: true,
+      alertOnNoData: true,
       groupBy: ['host.hostname'],
       searchConfiguration: {
         index,
@@ -191,7 +192,8 @@ describe('Expression', () => {
 
     const { ruleParams } = await setup(currentOptions, { searchConfiguration: undefined });
 
-    expect(ruleParams.alertOnGroupDisappear).toEqual(false);
+    expect(ruleParams.alertOnGroupDisappear).toEqual(true);
+    expect(ruleParams.alertOnNoData).toEqual(true);
     expect(ruleParams.groupBy).toEqual(['host.hostname']);
     expect((ruleParams.searchConfiguration.query as Query).query).toBe('foo');
     expect(ruleParams.searchConfiguration.index).toBe(index);
@@ -211,6 +213,68 @@ describe('Expression', () => {
     ]);
   });
 
+  it('should only set alertOnGroupDisappear to true if there is a group by field', async () => {
+    const customRuleParams: Partial<RuleTypeParams & AlertParams> = {
+      groupBy: ['host.hostname'],
+    };
+
+    const { ruleParams, wrapper } = await setup({}, customRuleParams);
+
+    act(() => {
+      wrapper
+        .find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
+        .at(1)
+        .prop('onChange')?.({
+        target: { checked: true },
+      } as React.ChangeEvent<HTMLInputElement>);
+    });
+
+    expect(ruleParams.alertOnGroupDisappear).toEqual(true);
+    expect(ruleParams.alertOnNoData).toEqual(false);
+
+    // Uncheck
+    act(() => {
+      wrapper
+        .find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
+        .at(1)
+        .prop('onChange')?.({
+        target: { checked: false },
+      } as React.ChangeEvent<HTMLInputElement>);
+    });
+
+    expect(ruleParams.alertOnGroupDisappear).toEqual(false);
+    expect(ruleParams.alertOnNoData).toEqual(false);
+  });
+
+  it('should only set alertOnNoData to true if there is no group by', async () => {
+    const { ruleParams, wrapper } = await setup();
+
+    act(() => {
+      wrapper
+        .find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
+        .at(1)
+        .prop('onChange')?.({
+        target: { checked: true },
+      } as React.ChangeEvent<HTMLInputElement>);
+    });
+
+    expect(ruleParams.alertOnGroupDisappear).toEqual(false);
+    expect(ruleParams.alertOnNoData).toEqual(true);
+
+    // Uncheck
+    act(() => {
+      wrapper
+        .find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
+        .at(1)
+        .prop('onChange')?.({
+        target: { checked: false },
+      } as React.ChangeEvent<HTMLInputElement>);
+    });
+
+    expect(ruleParams.alertOnGroupDisappear).toEqual(false);
+    expect(ruleParams.alertOnNoData).toEqual(false);
+  });
+
   it('should show an error message when searchSource throws an error', async () => {
     const errorMessage = 'Error in searchSource create';
     const kibanaMock = kibanaStartMock.startContract();
diff --git a/x-pack/plugins/observability_solution/observability/public/components/custom_threshold/custom_threshold_rule_expression.tsx b/x-pack/plugins/observability_solution/observability/public/components/custom_threshold/custom_threshold_rule_expression.tsx
@@ -75,13 +75,22 @@ export default function Expressions(props: Props) {
     },
   } = useKibana().services;
 
+  const hasGroupBy = useMemo<boolean>(
+    () => !!ruleParams.groupBy && ruleParams.groupBy.length > 0,
+    [ruleParams.groupBy]
+  );
+
   const [timeSize, setTimeSize] = useState<number | undefined>(1);
   const [timeUnit, setTimeUnit] = useState<TimeUnitChar | undefined>('m');
   const [dataView, setDataView] = useState<DataView>();
   const [dataViewTimeFieldError, setDataViewTimeFieldError] = useState<string>();
   const [searchSource, setSearchSource] = useState<ISearchSource>();
   const [paramsError, setParamsError] = useState<Error>();
   const [paramsWarning, setParamsWarning] = useState<string>();
+  const [isNoDataChecked, setIsNoDataChecked] = useState<boolean>(
+    (hasGroupBy && !!ruleParams.alertOnGroupDisappear) ||
+      (!hasGroupBy && !!ruleParams.alertOnNoData)
+  );
   const derivedIndexPattern = useMemo<DataViewBase>(
     () => ({
       fields: dataView?.fields || [],
@@ -177,11 +186,15 @@ export default function Expressions(props: Props) {
     }
 
     if (typeof ruleParams.alertOnNoData === 'undefined') {
-      setRuleParams('alertOnNoData', true);
+      preFillAlertOnNoData();
     }
     if (typeof ruleParams.alertOnGroupDisappear === 'undefined') {
       preFillAlertOnGroupDisappear();
     }
+    setIsNoDataChecked(
+      (hasGroupBy && !!ruleParams.alertOnGroupDisappear) ||
+        (!hasGroupBy && !!ruleParams.alertOnNoData)
+    );
   }, [metadata]); // eslint-disable-line react-hooks/exhaustive-deps
 
   const onSelectDataView = useCallback(
@@ -250,9 +263,12 @@ export default function Expressions(props: Props) {
 
   const onGroupByChange = useCallback(
     (group: string | null | string[]) => {
+      const hasGroup = !!group && group.length > 0;
       setRuleParams('groupBy', group && group.length ? group : '');
+      setRuleParams('alertOnGroupDisappear', hasGroup && isNoDataChecked);
+      setRuleParams('alertOnNoData', !hasGroup && isNoDataChecked);
     },
-    [setRuleParams]
+    [setRuleParams, isNoDataChecked]
   );
 
   const emptyError = useMemo(() => {
@@ -314,20 +330,24 @@ export default function Expressions(props: Props) {
     }
   }, [metadata, setRuleParams]);
 
+  const preFillAlertOnNoData = useCallback(() => {
+    const md = metadata;
+    if (md && typeof md.currentOptions?.alertOnNoData !== 'undefined') {
+      setRuleParams('alertOnNoData', md.currentOptions.alertOnNoData);
+    } else {
+      setRuleParams('alertOnNoData', false);
+    }
+  }, [metadata, setRuleParams]);
+
   const preFillAlertOnGroupDisappear = useCallback(() => {
     const md = metadata;
     if (md && typeof md.currentOptions?.alertOnGroupDisappear !== 'undefined') {
       setRuleParams('alertOnGroupDisappear', md.currentOptions.alertOnGroupDisappear);
     } else {
-      setRuleParams('alertOnGroupDisappear', true);
+      setRuleParams('alertOnGroupDisappear', false);
     }
   }, [metadata, setRuleParams]);
 
-  const hasGroupBy = useMemo(
-    () => ruleParams.groupBy && ruleParams.groupBy.length > 0,
-    [ruleParams.groupBy]
-  );
-
   if (paramsError) {
     return (
       <>
@@ -540,30 +560,55 @@ export default function Expressions(props: Props) {
       <EuiSpacer size="s" />
       <EuiCheckbox
         id="metrics-alert-group-disappear-toggle"
+        data-test-subj="thresholdRuleAlertOnNoDataCheckbox"
         label={
           <>
             {i18n.translate(
               'xpack.observability.customThreshold.rule.alertFlyout.alertOnGroupDisappear',
               {
-                defaultMessage: 'Alert me if a group stops reporting data',
+                defaultMessage: "Alert me if there's no data",
               }
             )}{' '}
             <EuiIconTip
               type="questionInCircle"
               color="subdued"
-              content={i18n.translate(
-                'xpack.observability.customThreshold.rule.alertFlyout.groupDisappearHelpText',
-                {
-                  defaultMessage:
-                    'Enable this to trigger the action if a previously detected group begins to report no results. This is not recommended for dynamically scaling infrastructures that may rapidly start and stop nodes automatically.',
-                }
-              )}
+              content={
+                hasGroupBy
+                  ? i18n.translate(
+                      'xpack.observability.customThreshold.rule.alertFlyout.groupDisappearHelpText',
+                      {
+                        defaultMessage:
+                          'Enable this to trigger a no data alert if a previously detected group begins to report no results. This is not recommended for dynamically scaling infrastructures that may rapidly start and stop nodes automatically.',
+                      }
+                    )
+                  : i18n.translate(
+                      'xpack.observability.customThreshold.rule.alertFlyout.noDataHelpText',
+                      {
+                        defaultMessage:
+                          'Enable this to trigger a no data alert if the condition(s) do not report any data over the expected time period, or if the alert fails to query Elasticsearch',
+                      }
+                    )
+              }
             />
           </>
         }
-        disabled={!hasGroupBy}
-        checked={Boolean(hasGroupBy && ruleParams.alertOnGroupDisappear)}
-        onChange={(e) => setRuleParams('alertOnGroupDisappear', e.target.checked)}
+        checked={isNoDataChecked}
+        onChange={(e) => {
+          const checked = e.target.checked;
+          setIsNoDataChecked(checked);
+          if (!checked) {
+            setRuleParams('alertOnGroupDisappear', false);
+            setRuleParams('alertOnNoData', false);
+          } else {
+            if (hasGroupBy) {
+              setRuleParams('alertOnGroupDisappear', true);
+              setRuleParams('alertOnNoData', false);
+            } else {
+              setRuleParams('alertOnGroupDisappear', false);
+              setRuleParams('alertOnNoData', true);
+            }
+          }
+        }}
       />
       <EuiSpacer size="m" />
     </>
