[Uptime] Fix race on overview page query (elastic#67843) (elastic#68702)

Fixes elastic#67842 by requerying
during the refine phase to see if a newer matching doc has come in.

x-pack/plugins/uptime/server/lib/requests/search/__tests__/refine_potential_matches.test.ts

@@ -0,0 +1,112 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { fullyMatchingIds } from '../refine_potential_matches';
+import { MonitorLocCheckGroup } from '..';
+
+const mockQueryResult = (opts: { latestSummary: any; latestMatching: any }) => {
+  return {
+    aggregations: {
+      monitor: {
+        buckets: [
+          {
+            key: 'my-monitor',
+            location: {
+              buckets: [
+                {
+                  key: 'my-location',
+                  summaries: {
+                    latest: {
+                      hits: {
+                        hits: [
+                          {
+                            _source: opts.latestSummary,
+                          },
+                        ],
+                      },
+                    },
+                  },
+                  latest_matching: {
+                    top: {
+                      hits: {
+                        hits: [
+                          {
+                            _source: opts.latestMatching,
+                          },
+                        ],
+                      },
+                    },
+                  },
+                },
+              ],
+            },
+          },
+        ],
+      },
+    },
+  };
+};
+
+describe('fully matching IDs', () => {
+  it('should exclude items whose latest result does not match', () => {
+    const queryRes = mockQueryResult({
+      latestSummary: {
+        '@timestamp': '2020-06-04T12:39:54.698-0500',
+        monitor: {
+          check_group: 'latest-summary-check-group',
+        },
+        summary: {
+          up: 1,
+          down: 0,
+        },
+      },
+      latestMatching: {
+        '@timestamp': '2019-06-04T12:39:54.698-0500',
+        summary: {
+          up: 1,
+          down: 0,
+        },
+      },
+    });
+    const res = fullyMatchingIds(queryRes, undefined);
+    const expected = new Map<string, MonitorLocCheckGroup[]>();
+    expect(res).toEqual(expected);
+  });
+
+  it('should include items whose latest result does match', () => {
+    const queryRes = mockQueryResult({
+      latestSummary: {
+        '@timestamp': '2020-06-04T12:39:54.698-0500',
+        monitor: {
+          check_group: 'latest-summary-check-group',
+        },
+        summary: {
+          up: 1,
+          down: 0,
+        },
+      },
+      latestMatching: {
+        '@timestamp': '2020-06-04T12:39:54.698-0500',
+        summary: {
+          up: 1,
+          down: 0,
+        },
+      },
+    });
+    const res = fullyMatchingIds(queryRes, undefined);
+    const expected = new Map<string, MonitorLocCheckGroup[]>();
+    expected.set('my-monitor', [
+      {
+        checkGroup: 'latest-summary-check-group',
+        location: 'my-location',
+        monitorId: 'my-monitor',
+        status: 'up',
+        summaryTimestamp: new Date('2020-06-04T12:39:54.698-0500'),
+      },
+    ]);
+    expect(res).toEqual(expected);
+  });
+});

x-pack/plugins/uptime/server/lib/requests/search/fetch_chunk.ts

@@ -26,12 +26,12 @@ export const fetchChunk: ChunkFetcher = async (
   searchAfter: any,
   size: number
 ): Promise<ChunkResult> => {
-  const { monitorIds, checkGroups, searchAfter: foundSearchAfter } = await findPotentialMatches(
+  const { monitorIds, searchAfter: foundSearchAfter } = await findPotentialMatches(
     queryContext,
     searchAfter,
     size
   );
-  const matching = await refinePotentialMatches(queryContext, monitorIds, checkGroups);
+  const matching = await refinePotentialMatches(queryContext, monitorIds);
 
   return {
     monitorGroups: matching,

x-pack/plugins/uptime/server/lib/requests/search/find_potential_matches.ts

@@ -8,12 +8,8 @@ import { get, set } from 'lodash';
 import { CursorDirection } from '../../../../common/runtime_types';
 import { QueryContext } from './query_context';
 
-// This is the first phase of the query. In it, we find the most recent check groups that matched the given query.
-// Note that these check groups may not be the most recent groups for the matching monitor ID! We'll filter those
 /**
- * This is the first phase of the query. In it, we find the most recent check groups that matched the given query.
- * Note that these check groups may not be the most recent groups for the matching monitor ID. They'll be filtered
- * out in the next phase.
+ * This is the first phase of the query. In it, we find all monitor IDs that have ever matched the given filters.
  * @param queryContext the data and resources needed to perform the query
  * @param searchAfter indicates where Elasticsearch should continue querying on subsequent requests, if at all
  * @param size the minimum size of the matches to chunk
@@ -24,29 +20,14 @@ export const findPotentialMatches = async (
   size: number
 ) => {
   const queryResult = await query(queryContext, searchAfter, size);
-  const checkGroups = new Set<string>();
   const monitorIds: string[] = [];
   get<any>(queryResult, 'aggregations.monitors.buckets', []).forEach((b: any) => {
     const monitorId = b.key.monitor_id;
     monitorIds.push(monitorId);
-
-    // Doc count can be zero if status filter optimization does not match
-    if (b.doc_count > 0) {
-      // Here we grab the most recent 2 check groups per location and add them to the list.
-      // Why 2? Because the most recent one may be a partial result from mode: all, and hence not match a summary doc.
-      b.locations.buckets.forEach((lb: any) => {
-        lb.ips.buckets.forEach((ib: any) => {
-          ib.top.hits.hits.forEach((h: any) => {
-            checkGroups.add(h._source.monitor.check_group);
-          });
-        });
-      });
-    }
   });
 
   return {
     monitorIds,
-    checkGroups,
     searchAfter: queryResult.aggregations?.monitors?.after_key,
   };
 };
@@ -89,29 +70,6 @@ const queryBody = async (queryContext: QueryContext, searchAfter: any, size: num
             },
           ],
         },
-        aggs: {
-          // Here we grab the most recent 2 check groups per location.
-          // Why 2? Because the most recent one may not be for a summary, it may be incomplete.
-          locations: {
-            terms: { field: 'observer.geo.name', missing: '__missing__' },
-            aggs: {
-              ips: {
-                terms: { field: 'monitor.ip', missing: '0.0.0.0' },
-                aggs: {
-                  top: {
-                    top_hits: {
-                      sort: [{ '@timestamp': 'desc' }],
-                      _source: {
-                        includes: ['monitor.check_group', '@timestamp'],
-                      },
-                      size: 2,
-                    },
-                  },
-                },
-              },
-            },
-          },
-        },
       },
     },
   };

x-pack/plugins/uptime/server/lib/requests/search/refine_potential_matches.ts

@@ -18,18 +18,14 @@ import { MonitorGroups, MonitorLocCheckGroup } from './fetch_page';
 // check groups for their associated monitor IDs. If not, it discards the result.
 export const refinePotentialMatches = async (
   queryContext: QueryContext,
-  potentialMatchMonitorIDs: string[],
-  potentialMatchCheckGroups: Set<string>
+  potentialMatchMonitorIDs: string[]
 ): Promise<MonitorGroups[]> => {
   if (potentialMatchMonitorIDs.length === 0) {
     return [];
   }
 
-  const recentGroupsMatchingStatus = await fullyMatchingIds(
-    queryContext,
-    potentialMatchMonitorIDs,
-    potentialMatchCheckGroups
-  );
+  const queryResult = await query(queryContext, potentialMatchMonitorIDs);
+  const recentGroupsMatchingStatus = await fullyMatchingIds(queryResult, queryContext.statusFilter);
 
   // Return the monitor groups filtering out potential matches that weren't current
   const matches: MonitorGroups[] = potentialMatchMonitorIDs
@@ -49,27 +45,35 @@ export const refinePotentialMatches = async (
   return matches;
 };
 
-const fullyMatchingIds = async (
-  queryContext: QueryContext,
-  potentialMatchMonitorIDs: string[],
-  potentialMatchCheckGroups: Set<string>
-) => {
-  const mostRecentQueryResult = await mostRecentCheckGroups(queryContext, potentialMatchMonitorIDs);
-
+export const fullyMatchingIds = (queryResult: any, statusFilter?: string) => {
   const matching = new Map<string, MonitorLocCheckGroup[]>();
-  MonitorLoop: for (const monBucket of mostRecentQueryResult.aggregations.monitor.buckets) {
+  MonitorLoop: for (const monBucket of queryResult.aggregations.monitor.buckets) {
     const monitorId: string = monBucket.key;
     const groups: MonitorLocCheckGroup[] = [];
 
+    // Did at least one location match?
+    let matched = false;
     for (const locBucket of monBucket.location.buckets) {
       const location = locBucket.key;
-      const topSource = locBucket.top.hits.hits[0]._source;
-      const checkGroup = topSource.monitor.check_group;
-      const status = topSource.summary.down > 0 ? 'down' : 'up';
+      const latestSource = locBucket.summaries.latest.hits.hits[0]._source;
+      const latestStillMatchingSource = locBucket.latest_matching.top.hits.hits[0]?._source;
+      // If the most recent document still matches the most recent document matching the current filters
+      // we can include this in the result
+      //
+      // We just check if the timestamp is greater. Note this may match an incomplete check group
+      // that has not yet sent a summary doc
+      if (
+        latestStillMatchingSource &&
+        latestStillMatchingSource['@timestamp'] >= latestSource['@timestamp']
+      ) {
+        matched = true;
+      }
+      const checkGroup = latestSource.monitor.check_group;
+      const status = latestSource.summary.down > 0 ? 'down' : 'up';
 
       // This monitor doesn't match, so just skip ahead and don't add it to the output
       // Only skip in case of up statusFilter, for a monitor to be up, all checks should be up
-      if (queryContext?.statusFilter === 'up' && queryContext.statusFilter !== status) {
+      if (statusFilter === 'up' && statusFilter !== status) {
         continue MonitorLoop;
       }
 
@@ -78,20 +82,20 @@ const fullyMatchingIds = async (
         location,
         checkGroup,
         status,
-        summaryTimestamp: topSource['@timestamp'],
+        summaryTimestamp: new Date(latestSource['@timestamp']),
       });
     }
 
-    // We only truly match the monitor if one of the most recent check groups was found in the potential matches phase
-    if (groups.some((g) => potentialMatchCheckGroups.has(g.checkGroup))) {
+    // If one location matched, include data from all locations in the result set
+    if (matched) {
       matching.set(monitorId, groups);
     }
   }
 
   return matching;
 };
 
-export const mostRecentCheckGroups = async (
+export const query = async (
   queryContext: QueryContext,
   potentialMatchMonitorIDs: string[]
 ): Promise<any> => {
@@ -104,8 +108,6 @@ export const mostRecentCheckGroups = async (
           filter: [
             await queryContext.dateRangeFilter(),
             { terms: { 'monitor.id': potentialMatchMonitorIDs } },
-            // only match summary docs because we only want the latest *complete* check group.
-            { exists: { field: 'summary' } },
           ],
         },
       },
@@ -116,13 +118,39 @@ export const mostRecentCheckGroups = async (
             location: {
               terms: { field: 'observer.geo.name', missing: 'N/A', size: 100 },
               aggs: {
-                top: {
-                  top_hits: {
-                    sort: [{ '@timestamp': 'desc' }],
-                    _source: {
-                      includes: ['monitor.check_group', '@timestamp', 'summary.up', 'summary.down'],
+                summaries: {
+                  // only match summary docs because we only want the latest *complete* check group.
+                  filter: { exists: { field: 'summary' } },
+                  aggs: {
+                    latest: {
+                      top_hits: {
+                        sort: [{ '@timestamp': 'desc' }],
+                        _source: {
+                          includes: [
+                            'monitor.check_group',
+                            '@timestamp',
+                            'summary.up',
+                            'summary.down',
+                          ],
+                        },
+                        size: 1,
+                      },
+                    },
+                  },
+                },
+                // We want to find the latest check group, even if it's not part of a summary
+                latest_matching: {
+                  filter: queryContext.filterClause || { match_all: {} },
+                  aggs: {
+                    top: {
+                      top_hits: {
+                        sort: [{ '@timestamp': 'desc' }],
+                        _source: {
+                          includes: ['monitor.check_group', '@timestamp'],
+                        },
+                        size: 1,
+                      },
                     },
-                    size: 1,
                   },
                 },
               },