[SLO] Add support for document count to custom metric indicator (elas…

…tic#170913) ## 🍒 Summary This PR fixes elastic#170905 by adding the aggregation menu to the Custom Metric indicator to allow the user to pick either `doc_count` or `sum` for the aggregation. <img width="1152" alt="image" src="https://github.com/elastic/kibana/assets/41702/35aea8bd-d21c-4780-bad6-1efe5fc8902b">
e40pud · Nov 9, 2023 · b9c08ba · b9c08ba
1 parent e0da2ae
commit b9c08ba
Show file tree

Hide file tree

Showing 12 changed files with 274 additions and 73 deletions.
diff --git a/x-pack/packages/kbn-slo-schema/src/schema/indicators.ts b/x-pack/packages/kbn-slo-schema/src/schema/indicators.ts
@@ -136,22 +136,29 @@ const timesliceMetricIndicatorSchema = t.type({
   ]),
 });
 
-const metricCustomValidAggregations = t.keyof({
-  sum: true,
-});
+const metricCustomDocCountMetric = t.intersection([
+  t.type({
+    name: t.string,
+    aggregation: t.literal('doc_count'),
+  }),
+  t.partial({
+    filter: t.string,
+  }),
+]);
+
+const metricCustomBasicMetric = t.intersection([
+  t.type({
+    name: t.string,
+    aggregation: t.literal('sum'),
+    field: t.string,
+  }),
+  t.partial({
+    filter: t.string,
+  }),
+]);
+
 const metricCustomMetricDef = t.type({
-  metrics: t.array(
-    t.intersection([
-      t.type({
-        name: t.string,
-        aggregation: metricCustomValidAggregations,
-        field: t.string,
-      }),
-      t.partial({
-        filter: t.string,
-      }),
-    ])
-  ),
+  metrics: t.array(t.union([metricCustomBasicMetric, metricCustomDocCountMetric])),
   equation: t.string,
 });
 const metricCustomIndicatorTypeSchema = t.literal('sli.metric.custom');
@@ -267,6 +274,8 @@ export {
   kqlCustomIndicatorTypeSchema,
   metricCustomIndicatorSchema,
   metricCustomIndicatorTypeSchema,
+  metricCustomDocCountMetric,
+  metricCustomBasicMetric,
   timesliceMetricComparatorMapping,
   timesliceMetricIndicatorSchema,
   timesliceMetricIndicatorTypeSchema,

diff --git a/...plugins/observability/public/pages/slo_edit/components/custom_metric/metric_indicator.tsx b/...plugins/observability/public/pages/slo_edit/components/custom_metric/metric_indicator.tsx
@@ -22,6 +22,10 @@ import { first, range, xor } from 'lodash';
 import React, { useEffect, useState } from 'react';
 import { Controller, useFieldArray, useFormContext } from 'react-hook-form';
 import { Field } from '../../../../hooks/slo/use_fetch_index_pattern_fields';
+import {
+  aggValueToLabel,
+  CUSTOM_METRIC_AGGREGATION_OPTIONS,
+} from '../../helpers/aggregation_options';
 import { createOptionsFromFields, Option } from '../../helpers/create_options';
 import { CreateSLOForm } from '../../types';
 import { QueryBuilder } from '../common/query_builder';
@@ -62,7 +66,8 @@ const metricTooltip = (
     content={i18n.translate(
       'xpack.observability.slo.sloEdit.sliType.customMetric.totalMetric.tooltip',
       {
-        defaultMessage: 'This data from this field will be aggregated with the "sum" aggregation.',
+        defaultMessage:
+          'This data from this field will be aggregated with the "sum" aggregation or document count.',
       }
     )}
     position="top"
@@ -89,6 +94,7 @@ const equationTooltip = (
 export function MetricIndicator({ type, metricFields, isLoadingIndex }: MetricIndicatorProps) {
   const { control, watch, setValue, register, getFieldState } = useFormContext<CreateSLOForm>();
   const [options, setOptions] = useState<Option[]>(createOptionsFromFields(metricFields));
+  const [aggregationOptions, setAggregationOptions] = useState(CUSTOM_METRIC_AGGREGATION_OPTIONS);
 
   useEffect(() => {
     setOptions(createOptionsFromFields(metricFields));
@@ -131,20 +137,25 @@ export function MetricIndicator({ type, metricFields, isLoadingIndex }: MetricIn
         {fields?.map((metric, index) => (
           <EuiFlexGroup alignItems="center" gutterSize="xs" key={metric.id}>
             <input hidden {...register(`indicator.params.${type}.metrics.${index}.name`)} />
-            <input hidden {...register(`indicator.params.${type}.metrics.${index}.aggregation`)} />
             <EuiFlexItem>
               <EuiFormRow
                 fullWidth
-                isInvalid={getFieldState(`indicator.params.${type}.metrics.${index}.field`).invalid}
+                isInvalid={
+                  getFieldState(`indicator.params.${type}.metrics.${index}.aggregation`).invalid
+                }
                 label={
                   <span>
-                    {metricLabel} {metric.name} {metricTooltip}
+                    {i18n.translate(
+                      'xpack.observability.slo.sloEdit.customMetric.aggregationLabel',
+                      { defaultMessage: 'Aggregation' }
+                    )}{' '}
+                    {metric.name}
                   </span>
                 }
               >
                 <Controller
-                  name={`indicator.params.${type}.metrics.${index}.field`}
-                  defaultValue=""
+                  name={`indicator.params.${type}.metrics.${index}.aggregation`}
+                  defaultValue="sum"
                   rules={{ required: true }}
                   control={control}
                   render={({ field: { ref, ...field }, fieldState }) => (
@@ -153,17 +164,13 @@ export function MetricIndicator({ type, metricFields, isLoadingIndex }: MetricIn
                       async
                       fullWidth
                       singleSelection={{ asPlainText: true }}
-                      prepend={i18n.translate(
-                        'xpack.observability.slo.sloEdit.sliType.customMetric.sumLabel',
-                        { defaultMessage: 'Sum of' }
-                      )}
                       placeholder={i18n.translate(
-                        'xpack.observability.slo.sloEdit.sliType.customMetric.metricField.placeholder',
-                        { defaultMessage: 'Select a metric field' }
+                        'xpack.observability.slo.sloEdit.sliType.customMetric.aggregation.placeholder',
+                        { defaultMessage: 'Select an aggregation' }
                       )}
                       aria-label={i18n.translate(
-                        'xpack.observability.slo.sloEdit.sliType.customMetric.metricField.placeholder',
-                        { defaultMessage: 'Select a metric field' }
+                        'xpack.observability.slo.sloEdit.sliType.customMetric.aggregation.placeholder',
+                        { defaultMessage: 'Select an aggregation' }
                       )}
                       isClearable
                       isInvalid={fieldState.invalid}
@@ -178,40 +185,112 @@ export function MetricIndicator({ type, metricFields, isLoadingIndex }: MetricIn
                       selectedOptions={
                         !!indexPattern &&
                         !!field.value &&
-                        metricFields.some((metricField) => metricField.name === field.value)
+                        CUSTOM_METRIC_AGGREGATION_OPTIONS.some((agg) => agg.value === field.value)
                           ? [
                               {
                                 value: field.value,
-                                label: field.value,
+                                label: aggValueToLabel(field.value),
                               },
                             ]
                           : []
                       }
                       onSearchChange={(searchValue: string) => {
-                        setOptions(
-                          createOptionsFromFields(metricFields, ({ value }) =>
+                        setAggregationOptions(
+                          CUSTOM_METRIC_AGGREGATION_OPTIONS.filter(({ value }) =>
                             value.includes(searchValue)
                           )
                         );
                       }}
-                      options={options}
+                      options={aggregationOptions}
                     />
                   )}
                 />
               </EuiFormRow>
             </EuiFlexItem>
+            {watch(`indicator.params.${type}.metrics.${index}.aggregation`) !== 'doc_count' && (
+              <EuiFlexItem>
+                <EuiFormRow
+                  fullWidth
+                  isInvalid={
+                    getFieldState(`indicator.params.${type}.metrics.${index}.field`).invalid
+                  }
+                  label={
+                    <span>
+                      {metricLabel} {metric.name} {metricTooltip}
+                    </span>
+                  }
+                >
+                  <Controller
+                    name={`indicator.params.${type}.metrics.${index}.field`}
+                    defaultValue=""
+                    rules={{ required: true }}
+                    shouldUnregister
+                    control={control}
+                    render={({ field: { ref, ...field }, fieldState }) => (
+                      <EuiComboBox
+                        {...field}
+                        async
+                        fullWidth
+                        singleSelection={{ asPlainText: true }}
+                        placeholder={i18n.translate(
+                          'xpack.observability.slo.sloEdit.sliType.customMetric.metricField.placeholder',
+                          { defaultMessage: 'Select a metric field' }
+                        )}
+                        aria-label={i18n.translate(
+                          'xpack.observability.slo.sloEdit.sliType.customMetric.metricField.placeholder',
+                          { defaultMessage: 'Select a metric field' }
+                        )}
+                        isClearable
+                        isInvalid={fieldState.invalid}
+                        isDisabled={isLoadingIndex || !indexPattern}
+                        isLoading={!!indexPattern && isLoadingIndex}
+                        onChange={(selected: EuiComboBoxOptionOption[]) => {
+                          if (selected.length) {
+                            return field.onChange(selected[0].value);
+                          }
+                          field.onChange('');
+                        }}
+                        selectedOptions={
+                          !!indexPattern &&
+                          !!field.value &&
+                          metricFields.some((metricField) => metricField.name === field.value)
+                            ? [
+                                {
+                                  value: field.value,
+                                  label: field.value,
+                                },
+                              ]
+                            : []
+                        }
+                        onSearchChange={(searchValue: string) => {
+                          setOptions(
+                            createOptionsFromFields(metricFields, ({ value }) =>
+                              value.includes(searchValue)
+                            )
+                          );
+                        }}
+                        options={options}
+                      />
+                    )}
+                  />
+                </EuiFormRow>
+              </EuiFlexItem>
+            )}
             <EuiFlexItem>
               <QueryBuilder
                 dataTestSubj="customKqlIndicatorFormGoodQueryInput"
                 indexPatternString={watch('indicator.params.index')}
                 label={`${filterLabel} ${metric.name}`}
                 name={`indicator.params.${type}.metrics.${index}.filter`}
-                placeholder=""
+                placeholder={i18n.translate(
+                  'xpack.observability.slo.sloEdit.sliType.customMetric.placeholder',
+                  { defaultMessage: 'KQL filter' }
+                )}
                 required={false}
                 tooltip={
                   <EuiIconTip
                     content={i18n.translate(
-                      'xpack.observability.slo.sloEdit.sliType.customMetric.goodQuery.tooltip',
+                      'xpack.observability.slo.sloEdit.sliType.customMetric.tooltip',
                       {
                         defaultMessage: 'This KQL query should return a subset of events.',
                       }

diff --git a/.../plugins/observability/public/pages/slo_edit/components/timeslice_metric/metric_input.tsx b/.../plugins/observability/public/pages/slo_edit/components/timeslice_metric/metric_input.tsx
@@ -132,7 +132,7 @@ export function MetricInput({
                 selectedOptions={
                   !!indexPattern &&
                   !!field.value &&
-                  AGGREGATION_OPTIONS.some((agg) => agg.value === agg.value)
+                  AGGREGATION_OPTIONS.some((agg) => agg.value === field.value)
                     ? [
                         {
                           value: field.value,

diff --git a/x-pack/plugins/observability/public/pages/slo_edit/helpers/aggregation_options.ts b/x-pack/plugins/observability/public/pages/slo_edit/helpers/aggregation_options.ts
@@ -75,6 +75,10 @@ export const AGGREGATION_OPTIONS = [
   },
 ];
 
+export const CUSTOM_METRIC_AGGREGATION_OPTIONS = AGGREGATION_OPTIONS.filter((option) =>
+  ['doc_count', 'sum'].includes(option.value)
+);
+
 export function aggValueToLabel(value: string) {
   const aggregation = AGGREGATION_OPTIONS.find((agg) => agg.value === value);
   if (aggregation) {

diff --git a/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_section_form_validation.ts b/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_section_form_validation.ts
@@ -6,6 +6,8 @@
  */
 
 import {
+  metricCustomBasicMetric,
+  metricCustomDocCountMetric,
   MetricCustomIndicator,
   timesliceMetricBasicMetricWithField,
   TimesliceMetricIndicator,
@@ -31,7 +33,16 @@ export function useSectionFormValidation({ getFieldState, getValues, formState,
         const data = getValues('indicator.params.good') as MetricCustomIndicator['params']['good'];
         const isEquationValid = !getFieldState('indicator.params.good.equation').invalid;
         const areMetricsValid =
-          isObject(data) && (data.metrics ?? []).every((metric) => Boolean(metric.field));
+          isObject(data) &&
+          (data.metrics ?? []).every((metric) => {
+            if (metricCustomDocCountMetric.is(metric)) {
+              return true;
+            }
+            if (metricCustomBasicMetric.is(metric) && metric.field != null) {
+              return true;
+            }
+            return false;
+          });
         return isEquationValid && areMetricsValid;
       };
 
@@ -41,7 +52,16 @@ export function useSectionFormValidation({ getFieldState, getValues, formState,
         ) as MetricCustomIndicator['params']['total'];
         const isEquationValid = !getFieldState('indicator.params.total.equation').invalid;
         const areMetricsValid =
-          isObject(data) && (data.metrics ?? []).every((metric) => Boolean(metric.field));
+          isObject(data) &&
+          (data.metrics ?? []).every((metric) => {
+            if (metricCustomDocCountMetric.is(metric)) {
+              return true;
+            }
+            if (metricCustomBasicMetric.is(metric) && metric.field != null) {
+              return true;
+            }
+            return false;
+          });
         return isEquationValid && areMetricsValid;
       };
 

diff --git a/...vices/slo/aggregations/__snapshots__/get_custom_metric_indicator_aggregation.test.ts.snap b/...vices/slo/aggregations/__snapshots__/get_custom_metric_indicator_aggregation.test.ts.snap