[ML] Fix errors from annotations searches when event mapping is incor…

…rect (#116101) * [ML] Fix errors from annotations searches when event mapping is incorrect * [ML] Delete tests on annotation errors due to incorrect mappings * [ML] Jest test fix and remove unused servuce method * [ML] type fix * [ML] Edits following review
elastic · Oct 26, 2021 · 9c92ac8 · 9c92ac8
1 parent f5463ce
commit 9c92ac8
Show file tree

Hide file tree

Showing 13 changed files with 37 additions and 274 deletions.
diff --git a/x-pack/plugins/ml/common/types/annotations.ts b/x-pack/plugins/ml/common/types/annotations.ts
@@ -118,33 +118,14 @@ export function isAnnotations(arg: any): arg is Annotations {
   return arg.every((d: Annotation) => isAnnotation(d));
 }
 
-export interface FieldToBucket {
-  field: string;
-  missing?: string | number;
-}
-
-export interface FieldToBucketResult {
-  key: string;
-  doc_count: number;
-}
-
-export interface TermAggregationResult {
-  doc_count_error_upper_bound: number;
-  sum_other_doc_count: number;
-  buckets: FieldToBucketResult[];
-}
-
-export type EsAggregationResult = Record<string, TermAggregationResult>;
-
 export interface GetAnnotationsResponse {
-  aggregations?: EsAggregationResult;
+  totalCount: number;
   annotations: Record<string, Annotations>;
   error?: string;
   success: boolean;
 }
 
 export interface AnnotationsTable {
   annotationsData: Annotations;
-  aggregations: EsAggregationResult;
   error?: string;
 }
diff --git a/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js b/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js
@@ -81,7 +81,6 @@ class AnnotationsTableUI extends Component {
     super(props);
     this.state = {
       annotations: [],
-      aggregations: null,
       isLoading: false,
       queryText: `event:(${ANNOTATION_EVENT_USER} or ${ANNOTATION_EVENT_DELAYED_DATA})`,
       searchError: undefined,
@@ -115,18 +114,11 @@ class AnnotationsTableUI extends Component {
           earliestMs: null,
           latestMs: null,
           maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
-          fields: [
-            {
-              field: 'event',
-              missing: ANNOTATION_EVENT_USER,
-            },
-          ],
         })
         .toPromise()
         .then((resp) => {
           this.setState((prevState, props) => ({
             annotations: resp.annotations[props.jobs[0].job_id] || [],
-            aggregations: resp.aggregations,
             errorMessage: undefined,
             isLoading: false,
             jobId: props.jobs[0].job_id,
@@ -570,41 +562,35 @@ class AnnotationsTableUI extends Component {
         onMouseLeave: () => this.onMouseLeaveRow(),
       };
     };
-    let filterOptions = [];
-    const aggregations = this.props.aggregations ?? this.state.aggregations;
-    if (aggregations) {
-      const buckets = aggregations.event.buckets;
-      let foundUser = false;
-      let foundDelayedData = false;
-
-      buckets.forEach((bucket) => {
-        if (bucket.key === ANNOTATION_EVENT_USER) {
-          foundUser = true;
-        }
-        if (bucket.key === ANNOTATION_EVENT_DELAYED_DATA) {
-          foundDelayedData = true;
-        }
-      });
-      const adjustedBuckets = [];
-      if (!foundUser) {
-        adjustedBuckets.push({ key: ANNOTATION_EVENT_USER, doc_count: 0 });
-      }
-      if (!foundDelayedData) {
-        adjustedBuckets.push({ key: ANNOTATION_EVENT_DELAYED_DATA, doc_count: 0 });
+
+    // Build the options to show in the Event type filter.
+    // Do not try and run a search using a terms agg on the event field
+    // because in 7.9 this field was incorrectly mapped as a text rather than keyword.
+
+    // Always display options for user and delayed data types.
+    const countsByEvent = {
+      [ANNOTATION_EVENT_USER]: 0,
+      [ANNOTATION_EVENT_DELAYED_DATA]: 0,
+    };
+    annotations.forEach((annotation) => {
+      // Default to user type for annotations created in early releases which didn't have an event field
+      const event = annotation.event ?? ANNOTATION_EVENT_USER;
+      if (countsByEvent[event] === undefined) {
+        countsByEvent[event] = 0;
       }
+      countsByEvent[event]++;
+    });
 
-      filterOptions = [...adjustedBuckets, ...buckets];
-    }
     const filters = [
       {
         type: 'field_value_selection',
         field: 'event',
         name: 'Event',
         multiSelect: 'or',
-        options: filterOptions.map((field) => ({
-          value: field.key,
-          name: field.key,
-          view: `${field.key} (${field.doc_count})`,
+        options: Object.entries(countsByEvent).map(([key, docCount]) => ({
+          value: key,
+          name: key,
+          view: `${key} (${docCount})`,
         })),
         'data-test-subj': 'mlAnnotationTableEventFilter',
       },

diff --git a/x-pack/plugins/ml/public/application/explorer/explorer.js b/x-pack/plugins/ml/public/application/explorer/explorer.js
@@ -255,13 +255,9 @@ export class ExplorerUI extends React.Component {
       tableData,
       swimLaneSeverity,
     } = this.props.explorerState;
-    const { annotationsData, aggregations, error: annotationsError } = annotations;
+    const { annotationsData, totalCount: allAnnotationsCnt, error: annotationsError } = annotations;
 
     const annotationsCnt = Array.isArray(annotationsData) ? annotationsData.length : 0;
-    const allAnnotationsCnt = Array.isArray(aggregations?.event?.buckets)
-      ? aggregations.event.buckets.reduce((acc, v) => acc + v.doc_count, 0)
-      : annotationsCnt;
-
     const badge =
       allAnnotationsCnt > annotationsCnt ? (
         <EuiBadge color={'hollow'}>
@@ -449,7 +445,6 @@ export class ExplorerUI extends React.Component {
                       <AnnotationsTable
                         jobIds={selectedJobIds}
                         annotations={annotationsData}
-                        aggregations={aggregations}
                         drillDown={true}
                         numberBadge={false}
                       />

diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_utils.js b/x-pack/plugins/ml/public/application/explorer/explorer_utils.js
@@ -35,7 +35,6 @@ import {
   SWIMLANE_TYPE,
   VIEW_BY_JOB_LABEL,
 } from './explorer_constants';
-import { ANNOTATION_EVENT_USER } from '../../../common/constants/annotations';
 
 // create new job objects based on standard job config objects
 // new job objects just contain job id, bucket span in seconds and a selected flag.
@@ -437,10 +436,7 @@ export function loadOverallAnnotations(selectedJobs, interval, bounds) {
 }
 
 export function loadAnnotationsTableData(selectedCells, selectedJobs, interval, bounds) {
-  const jobIds =
-    selectedCells !== undefined && selectedCells.viewByFieldName === VIEW_BY_JOB_LABEL
-      ? selectedCells.lanes
-      : selectedJobs.map((d) => d.id);
+  const jobIds = getSelectionJobIds(selectedCells, selectedJobs);
   const timeRange = getSelectionTimeRange(selectedCells, interval, bounds);
 
   return new Promise((resolve) => {
@@ -450,20 +446,14 @@ export function loadAnnotationsTableData(selectedCells, selectedJobs, interval,
         earliestMs: timeRange.earliestMs,
         latestMs: timeRange.latestMs,
         maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
-        fields: [
-          {
-            field: 'event',
-            missing: ANNOTATION_EVENT_USER,
-          },
-        ],
       })
       .toPromise()
       .then((resp) => {
         if (resp.error !== undefined || resp.annotations === undefined) {
           const errorMessage = extractErrorMessage(resp.error);
           return resolve({
             annotationsData: [],
-            aggregations: {},
+            totalCount: 0,
             error: errorMessage !== '' ? errorMessage : undefined,
           });
         }
@@ -485,14 +475,14 @@ export function loadAnnotationsTableData(selectedCells, selectedJobs, interval,
               d.key = (i + 1).toString();
               return d;
             }),
-          aggregations: resp.aggregations,
+          totalCount: resp.totalCount,
         });
       })
       .catch((resp) => {
         const errorMessage = extractErrorMessage(resp);
         return resolve({
           annotationsData: [],
-          aggregations: {},
+          totalCount: 0,
           error: errorMessage !== '' ? errorMessage : undefined,
         });
       });

diff --git a/x-pack/plugins/ml/public/application/explorer/reducers/explorer_reducer/state.ts b/x-pack/plugins/ml/public/application/explorer/reducers/explorer_reducer/state.ts
@@ -71,12 +71,10 @@ export function getExplorerDefaultState(): ExplorerState {
     overallAnnotations: {
       error: undefined,
       annotationsData: [],
-      aggregations: {},
     },
     annotations: {
       error: undefined,
       annotationsData: [],
-      aggregations: {},
     },
     anomalyChartsDataLoading: true,
     chartsData: getDefaultChartsData(),

diff --git a/x-pack/plugins/ml/public/application/services/ml_api_service/annotations.ts b/x-pack/plugins/ml/public/application/services/ml_api_service/annotations.ts
@@ -5,11 +5,7 @@
  * 2.0.
  */
 
-import {
-  Annotation,
-  FieldToBucket,
-  GetAnnotationsResponse,
-} from '../../../../common/types/annotations';
+import { Annotation, GetAnnotationsResponse } from '../../../../common/types/annotations';
 import { http, http$ } from '../http_service';
 import { basePath } from './index';
 
@@ -19,7 +15,6 @@ export const annotations = {
     earliestMs: number;
     latestMs: number;
     maxAnnotations: number;
-    fields?: FieldToBucket[];
     detectorIndex?: number;
     entities?: any[];
   }) {
@@ -36,7 +31,6 @@ export const annotations = {
     earliestMs: number | null;
     latestMs: number | null;
     maxAnnotations: number;
-    fields?: FieldToBucket[];
     detectorIndex?: number;
     entities?: any[];
   }) {

diff --git a/...lication/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx b/...lication/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx
@@ -15,7 +15,6 @@ import { extractErrorMessage } from '../../../../../common/util/errors';
 import { Annotation } from '../../../../../common/types/annotations';
 import { useMlKibana, useNotifications } from '../../../contexts/kibana';
 import { getBoundsRoundedToInterval } from '../../../util/time_buckets';
-import { ANNOTATION_EVENT_USER } from '../../../../../common/constants/annotations';
 import { getControlsForDetector } from '../../get_controls_for_detector';
 import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context';
 
@@ -88,12 +87,6 @@ export const TimeSeriesChartWithTooltips: FC<TimeSeriesChartWithTooltipsProps> =
           earliestMs: searchBounds.min.valueOf(),
           latestMs: searchBounds.max.valueOf(),
           maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
-          fields: [
-            {
-              field: 'event',
-              missing: ANNOTATION_EVENT_USER,
-            },
-          ],
           detectorIndex,
           entities: nonBlankEntities,
         });

diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js
@@ -104,7 +104,6 @@ function getTimeseriesexplorerDefaultState() {
     entitiesLoading: false,
     entityValues: {},
     focusAnnotationData: [],
-    focusAggregations: {},
     focusAggregationInterval: {},
     focusChartData: undefined,
     focusForecastData: undefined,
@@ -935,7 +934,6 @@ export class TimeSeriesExplorer extends React.Component {
       focusAggregationInterval,
       focusAnnotationError,
       focusAnnotationData,
-      focusAggregations,
       focusChartData,
       focusForecastData,
       fullRefresh,
@@ -1257,7 +1255,6 @@ export class TimeSeriesExplorer extends React.Component {
                       detectors={detectors}
                       jobIds={[this.props.selectedJobId]}
                       annotations={focusAnnotationData}
-                      aggregations={focusAggregations}
                       isSingleMetricViewerLinkVisible={false}
                       isNumberBadgeVisible={true}
                     />

diff --git a/...ugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/get_focus_data.ts b/...ugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/get_focus_data.ts
@@ -26,7 +26,6 @@ import {
 import { mlForecastService } from '../../services/forecast_service';
 import { mlFunctionToESAggregation } from '../../../../common/util/job_utils';
 import { GetAnnotationsResponse } from '../../../../common/types/annotations';
-import { ANNOTATION_EVENT_USER } from '../../../../common/constants/annotations';
 import { aggregationTypeTransform } from '../../../../common/util/anomaly_utils';
 
 export interface Interval {
@@ -42,7 +41,6 @@ export interface FocusData {
   focusAnnotationError?: string;
   focusAnnotationData?: any[];
   focusForecastData?: any;
-  focusAggregations?: any;
 }
 
 export function getFocusData(
@@ -98,20 +96,14 @@ export function getFocusData(
         earliestMs: searchBounds.min.valueOf(),
         latestMs: searchBounds.max.valueOf(),
         maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
-        fields: [
-          {
-            field: 'event',
-            missing: ANNOTATION_EVENT_USER,
-          },
-        ],
         detectorIndex,
         entities: nonBlankEntities,
       })
       .pipe(
         catchError((resp) =>
           of({
             annotations: {},
-            aggregations: {},
+            totalCount: 0,
             error: extractErrorMessage(resp),
             success: false,
           } as GetAnnotationsResponse)
@@ -168,7 +160,6 @@ export function getFocusData(
         if (annotations.error !== undefined) {
           refreshFocusData.focusAnnotationError = annotations.error;
           refreshFocusData.focusAnnotationData = [];
-          refreshFocusData.focusAggregations = {};
         } else {
           refreshFocusData.focusAnnotationData = (annotations.annotations[selectedJob.job_id] ?? [])
             .sort((a, b) => {
@@ -178,8 +169,6 @@ export function getFocusData(
               d.key = (i + 1).toString();
               return d;
             });
-
-          refreshFocusData.focusAggregations = annotations.aggregations;
         }
       }
 

diff --git a/x-pack/plugins/ml/server/models/annotation_service/__mocks__/get_annotations_request.json b/x-pack/plugins/ml/server/models/annotation_service/__mocks__/get_annotations_request.json
@@ -1,6 +1,7 @@
 {
   "index": ".ml-annotations-read",
   "size": 500,
+  "track_total_hits": true,
   "body": {
     "query": {
       "bool": {