[Alerting]: get type-checking, tests, and ui working for index threshold (#59064)

This is a follow-on to #57030 ,
"[alerting] initial index threshold alertType and supporting APIs",
to get it working with the existing alerting UI.  The parameter shape
was different between the two, so the alertType was changed to fix
the existing UI shapes expected.

Author: pmuellr

x-pack/plugins/alerting_builtins/server/alert_types/index_threshold/README.md

@@ -58,43 +58,47 @@ Finally, create the alert:
 ```
 kbn-alert create .index-threshold 'es-hb-sim threshold' 1s \
   '{
-    index:       es-hb-sim
-    timeField:   @timestamp
-    aggType:     average
-    aggField:    summary.up
-    groupField:  monitor.name.keyword
-    window:      5s
-    comparator:  lessThan
-    threshold:   [ 0.6 ]
+    index:                es-hb-sim
+    timeField:            @timestamp
+    aggType:              avg
+    aggField:             summary.up
+    groupBy:              top
+    termSize:             100
+    termField:            monitor.name.keyword
+    timeWindowSize:       5
+    timeWindowUnit:       s
+    thresholdComparator:  <
+    threshold:            [ 0.6 ]
   }' \
   "[
      {
-       group:       threshold met
-       id:         '$ACTION_ID'
+       group:     threshold met
+       id:        '$ACTION_ID'
        params: {
-         level:    warn
-         message: '{{context.message}}'
+         level:   warn
+         message: '{{{context.message}}}'
        }
      }
    ]"
 ```
 
 This alert will run a query over the `es-hb-sim` index, using the `@timestamp`
-field as the date field, using an `average` aggregation over the `summary.up`
-field.  The results are then aggregated by `monitor.name.keyword`.  If we ran
+field as the date field, aggregating over groups of the field value
+`monitor.name.keyword` (the top 100 groups), then aggregating those values
+using an `average` aggregation over the `summary.up` field.  If we ran
 another instance of `es-hb-sim`, using `host-B` instead of `host-A`, then the
 alert will end up potentially scheduling actions for both, independently.
 Within the alerting plugin, this grouping is also referred to as "instanceIds"
 (`host-A` and `host-B` being distinct instanceIds, which can have actions
 scheduled against them independently).
 
-The `window` is set to `5s` which is 5 seconds.  That means, every time the
+The time window is set to 5 seconds.  That means, every time the
 alert runs it's queries (every second, in the example above), it will run it's
 ES query over the last 5 seconds.  Thus, the queries, over time, will overlap.
 Sometimes that's what you want.  Other times, maybe you just want to do 
 sampling, running an alert every hour, with a 5 minute window.  Up to the you!
 
-Using the `comparator` `lessThan` and `threshold` `[0.6]`, the alert will 
+Using the `thresholdComparator` `<` and `threshold` `[0.6]`, the alert will 
 calculate the average of all the `summary.up` fields for each unique
 `monitor.name.keyword`, and then if the value is less than 0.6, it will
 schedule the specified action (server log) to run.  The `message` param
@@ -110,11 +114,10 @@ working:
 
 ```
 server    log   [17:32:10.060] [warning][actions][actions][plugins] \
-   Server log: alert es-hb-sim threshold instance host-A value 0 \
-   exceeded threshold average(summary.up) lessThan 0.6 over 5s \
+   Server log: alert es-hb-sim threshold group host-A value 0 \
+   exceeded threshold avg(summary.up) < 0.6 over 5s \
    on 2020-02-20T22:32:07.000Z
 ```
-
 [kbn-action]: https://github.com/pmuellr/kbn-action
 [es-hb-sim]: https://github.com/pmuellr/es-hb-sim
 [now-iso]: https://github.com/pmuellr/now-iso
@@ -144,15 +147,18 @@ This example uses [now-iso][] to generate iso date strings.
 ```console
 curl -k  "https://elastic:changeme@localhost:5601/api/alerting_builtins/index_threshold/_time_series_query" \
     -H "kbn-xsrf: foo" -H "content-type: application/json"   -d "{
-    \"index\":         \"es-hb-sim\",
-    \"timeField\":     \"@timestamp\",
-    \"aggType\":       \"average\",
-    \"aggField\":      \"summary.up\",
-    \"groupField\":    \"monitor.name.keyword\",
-    \"interval\":      \"1s\",
-    \"dateStart\":     \"`now-iso -10s`\",
-    \"dateEnd\":       \"`now-iso`\",
-    \"window\":        \"5s\"
+    \"index\":           \"es-hb-sim\",
+    \"timeField\":       \"@timestamp\",
+    \"aggType\":         \"avg\",
+    \"aggField\":        \"summary.up\",
+    \"groupBy\":         \"top\",
+    \"termSize\":        100,
+    \"termField\":       \"monitor.name.keyword\",
+    \"interval\":        \"1s\",
+    \"dateStart\":       \"`now-iso -10s`\",
+    \"dateEnd\":         \"`now-iso`\",
+    \"timeWindowSize\":  5,
+    \"timeWindowUnit\":  \"s\"
 }"
 ```
 
@@ -184,13 +190,16 @@ To get the current value of the calculated metric, you can leave off the date:
 ```
 curl -k  "https://elastic:changeme@localhost:5601/api/alerting_builtins/index_threshold/_time_series_query" \
     -H "kbn-xsrf: foo" -H "content-type: application/json"   -d '{
-    "index":         "es-hb-sim",
-    "timeField":     "@timestamp",
-    "aggType":       "average",
-    "aggField":      "summary.up",
-    "groupField":    "monitor.name.keyword",
-    "interval":      "1s",
-    "window":        "5s"
+    "index":           "es-hb-sim",
+    "timeField":       "@timestamp",
+    "aggType":         "avg",
+    "aggField":        "summary.up",
+    "groupBy":         "top",
+    "termField":       "monitor.name.keyword",
+    "termSize":        100,
+    "interval":        "1s",
+    "timeWindowSize":  5,
+    "timeWindowUnit":  "s"
 }'
 ```
 
@@ -254,7 +263,7 @@ be ~24 time series points in the output.
 
 For preview purposes:
 
-- The `groupLimit` parameter should be used to help cut
+- The `termSize` parameter should be used to help cut
 down on the amount of work ES does, and keep the generated graphs a little
 simpler.  Probably something like `10`.
 
@@ -263,9 +272,9 @@ simpler.  Probably something like `10`.
 could result in a lot of time-series points being generated, which is both
 costly in ES, and may result in noisy graphs.
 
-- The `window` parameter should be the same as what the alert is using, 
+- The `timeWindow*` parameters should be the same as what the alert is using, 
 especially for the `count` and `sum` aggregation types.  Those aggregations
 don't scale the same way the others do, when the window changes.  Even for
 the other aggregations, changing the window could result in dramatically
-different values being generated - `averages` will be more "average-y", `min`
+different values being generated - `avg` will be more "average-y", `min`
 and `max` will be a little stickier.

x-pack/plugins/alerting_builtins/server/alert_types/index_threshold/action_context.test.ts

@@ -21,16 +21,20 @@ describe('ActionContext', () => {
       index: '[index]',
       timeField: '[timeField]',
       aggType: 'count',
-      window: '5m',
-      comparator: 'greaterThan',
+      groupBy: 'top',
+      termField: 'x',
+      termSize: 100,
+      timeWindowSize: 5,
+      timeWindowUnit: 'm',
+      thresholdComparator: '>',
       threshold: [4],
     });
     const context = addMessages(base, params);
     expect(context.subject).toMatchInlineSnapshot(
       `"alert [name] group [group] exceeded threshold"`
     );
     expect(context.message).toMatchInlineSnapshot(
-      `"alert [name] group [group] value 42 exceeded threshold count greaterThan 4 over 5m on 2020-01-01T00:00:00.000Z"`
+      `"alert [name] group [group] value 42 exceeded threshold count > 4 over 5m on 2020-01-01T00:00:00.000Z"`
     );
   });
 
@@ -46,18 +50,22 @@ describe('ActionContext', () => {
     const params = ParamsSchema.validate({
       index: '[index]',
       timeField: '[timeField]',
-      aggType: 'average',
+      aggType: 'avg',
+      groupBy: 'top',
+      termField: 'x',
+      termSize: 100,
       aggField: '[aggField]',
-      window: '5m',
-      comparator: 'greaterThan',
+      timeWindowSize: 5,
+      timeWindowUnit: 'm',
+      thresholdComparator: '>',
       threshold: [4.2],
     });
     const context = addMessages(base, params);
     expect(context.subject).toMatchInlineSnapshot(
       `"alert [name] group [group] exceeded threshold"`
     );
     expect(context.message).toMatchInlineSnapshot(
-      `"alert [name] group [group] value 42 exceeded threshold average([aggField]) greaterThan 4.2 over 5m on 2020-01-01T00:00:00.000Z"`
+      `"alert [name] group [group] value 42 exceeded threshold avg([aggField]) > 4.2 over 5m on 2020-01-01T00:00:00.000Z"`
     );
   });
 
@@ -74,8 +82,12 @@ describe('ActionContext', () => {
       index: '[index]',
       timeField: '[timeField]',
       aggType: 'count',
-      window: '5m',
-      comparator: 'between',
+      groupBy: 'top',
+      termField: 'x',
+      termSize: 100,
+      timeWindowSize: 5,
+      timeWindowUnit: 'm',
+      thresholdComparator: 'between',
       threshold: [4, 5],
     });
     const context = addMessages(base, params);

x-pack/plugins/alerting_builtins/server/alert_types/index_threshold/action_context.ts

@@ -47,8 +47,9 @@ export function addMessages(c: BaseActionContext, p: Params): ActionContext {
   );
 
   const agg = p.aggField ? `${p.aggType}(${p.aggField})` : `${p.aggType}`;
-  const humanFn = `${agg} ${p.comparator} ${p.threshold.join(',')}`;
+  const humanFn = `${agg} ${p.thresholdComparator} ${p.threshold.join(',')}`;
 
+  const window = `${p.timeWindowSize}${p.timeWindowUnit}`;
   const message = i18n.translate(
     'xpack.alertingBuiltins.indexThreshold.alertTypeContextMessageDescription',
     {
@@ -59,7 +60,7 @@ export function addMessages(c: BaseActionContext, p: Params): ActionContext {
         group: c.group,
         value: c.value,
         function: humanFn,
-        window: p.window,
+        window,
         date: c.date,
       },
     }

x-pack/plugins/alerting_builtins/server/alert_types/index_threshold/alert_type.test.ts

@@ -6,6 +6,7 @@
 
 import { loggingServiceMock } from '../../../../../../src/core/server/mocks';
 import { getAlertType } from './alert_type';
+import { Params } from './alert_type_params';
 
 describe('alertType', () => {
   const service = {
@@ -24,12 +25,14 @@ describe('alertType', () => {
   });
 
   it('validator succeeds with valid params', async () => {
-    const params = {
+    const params: Partial<Writable<Params>> = {
       index: 'index-name',
       timeField: 'time-field',
       aggType: 'count',
-      window: '5m',
-      comparator: 'greaterThan',
+      groupBy: 'all',
+      timeWindowSize: 5,
+      timeWindowUnit: 'm',
+      thresholdComparator: '<',
       threshold: [0],
     };
 
@@ -40,12 +43,14 @@ describe('alertType', () => {
     const paramsSchema = alertType.validate?.params;
     if (!paramsSchema) throw new Error('params validator not set');
 
-    const params = {
+    const params: Partial<Writable<Params>> = {
       index: 'index-name',
       timeField: 'time-field',
       aggType: 'foo',
-      window: '5m',
-      comparator: 'greaterThan',
+      groupBy: 'all',
+      timeWindowSize: 5,
+      timeWindowUnit: 'm',
+      thresholdComparator: '>',
       threshold: [0],
     };
 

x-pack/plugins/alerting_builtins/server/alert_types/index_threshold/alert_type.ts

@@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n';
 import { AlertType, AlertExecutorOptions } from '../../types';
 import { Params, ParamsSchema } from './alert_type_params';
 import { BaseActionContext, addMessages } from './action_context';
+import { TimeSeriesQuery } from './lib/time_series_query';
 
 export const ID = '.index-threshold';
 
@@ -46,24 +47,26 @@ export function getAlertType(service: Service): AlertType {
     const { alertId, name, services } = options;
     const params: Params = options.params as Params;
 
-    const compareFn = ComparatorFns.get(params.comparator);
+    const compareFn = ComparatorFns.get(params.thresholdComparator);
     if (compareFn == null) {
-      throw new Error(getInvalidComparatorMessage(params.comparator));
+      throw new Error(getInvalidComparatorMessage(params.thresholdComparator));
     }
 
     const callCluster = services.callCluster;
     const date = new Date().toISOString();
     // the undefined values below are for config-schema optional types
-    const queryParams = {
+    const queryParams: TimeSeriesQuery = {
       index: params.index,
       timeField: params.timeField,
       aggType: params.aggType,
       aggField: params.aggField,
-      groupField: params.groupField,
-      groupLimit: params.groupLimit,
+      groupBy: params.groupBy,
+      termField: params.termField,
+      termSize: params.termSize,
       dateStart: date,
       dateEnd: date,
-      window: params.window,
+      timeWindowSize: params.timeWindowSize,
+      timeWindowUnit: params.timeWindowUnit,
       interval: undefined,
     };
     const result = await service.indexThreshold.timeSeriesQuery({
@@ -100,7 +103,7 @@ export function getAlertType(service: Service): AlertType {
 
 export function getInvalidComparatorMessage(comparator: string) {
   return i18n.translate('xpack.alertingBuiltins.indexThreshold.invalidComparatorErrorMessage', {
-    defaultMessage: 'invalid comparator specified: {comparator}',
+    defaultMessage: 'invalid thresholdComparator specified: {comparator}',
     values: {
       comparator,
     },
@@ -111,10 +114,10 @@ type ComparatorFn = (value: number, threshold: number[]) => boolean;
 
 function getComparatorFns(): Map<string, ComparatorFn> {
   const fns: Record<string, ComparatorFn> = {
-    lessThan: (value: number, threshold: number[]) => value < threshold[0],
-    lessThanOrEqual: (value: number, threshold: number[]) => value <= threshold[0],
-    greaterThanOrEqual: (value: number, threshold: number[]) => value >= threshold[0],
-    greaterThan: (value: number, threshold: number[]) => value > threshold[0],
+    '<': (value: number, threshold: number[]) => value < threshold[0],
+    '<=': (value: number, threshold: number[]) => value <= threshold[0],
+    '>=': (value: number, threshold: number[]) => value >= threshold[0],
+    '>': (value: number, threshold: number[]) => value > threshold[0],
     between: (value: number, threshold: number[]) => value >= threshold[0] && value <= threshold[1],
     notBetween: (value: number, threshold: number[]) =>
       value < threshold[0] || value > threshold[1],