From 3a48b7a84e587f0d5b9bffef0abecbce9c62e2e6 Mon Sep 17 00:00:00 2001
From: Walter Rafelsberger <walter.rafelsberger@elastic.co>
Date: Tue, 20 Dec 2022 16:31:12 +0100
Subject: [PATCH] [ML] Explain Log Rate Spikes: Fix mini histograms for groups
 with multiples values per field. (#147597)

For groups that have multiple values for the same field, the group
histogram query wasn't able to fetch data because it filters with a bool
`must` and individual `term` aggregations. This PR fixes it by using a
`terms` aggregation if there are multiple values for a field so just
these get treated as `OR`.
---
 .../server/routes/explain_log_rate_spikes.ts  |  8 +--
 .../routes/queries/get_group_filter.test.ts   | 54 +++++++++++++++++++
 .../server/routes/queries/get_group_filter.ts | 33 ++++++++++++
 3 files changed, 89 insertions(+), 6 deletions(-)
 create mode 100644 x-pack/plugins/aiops/server/routes/queries/get_group_filter.test.ts
 create mode 100644 x-pack/plugins/aiops/server/routes/queries/get_group_filter.ts

diff --git a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts
index 81477e64125e8b..03021415410ede 100644
--- a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts
+++ b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts
@@ -58,6 +58,7 @@ import {
   getSimpleHierarchicalTreeLeaves,
   markDuplicates,
 } from './queries/get_simple_hierarchical_tree';
+import { getGroupFilter } from './queries/get_group_filter';
 
 // 10s ping frequency to keep the stream alive.
 const PING_FREQUENCY = 10000;
@@ -639,12 +640,7 @@ export const defineExplainLogRateSpikesRoute = (
                   }
 
                   if (overallTimeSeries !== undefined) {
-                    const histogramQuery = getHistogramQuery(
-                      request.body,
-                      cpg.group.map((d) => ({
-                        term: { [d.fieldName]: d.fieldValue },
-                      }))
-                    );
+                    const histogramQuery = getHistogramQuery(request.body, getGroupFilter(cpg));
 
                     let cpgTimeSeries: NumericChartData;
                     try {
diff --git a/x-pack/plugins/aiops/server/routes/queries/get_group_filter.test.ts b/x-pack/plugins/aiops/server/routes/queries/get_group_filter.test.ts
new file mode 100644
index 00000000000000..432450ede9b455
--- /dev/null
+++ b/x-pack/plugins/aiops/server/routes/queries/get_group_filter.test.ts
@@ -0,0 +1,54 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { getGroupFilter } from './get_group_filter';
+
+const changePointGroups = [
+  {
+    id: '2038579476',
+    group: [
+      { fieldName: 'response_code', fieldValue: '500', duplicate: false },
+      { fieldName: 'url', fieldValue: 'home.php', duplicate: false },
+      { fieldName: 'url', fieldValue: 'login.php', duplicate: false },
+    ],
+    docCount: 792,
+    pValue: 0.010770456205312423,
+  },
+  {
+    id: '817080373',
+    group: [{ fieldName: 'user', fieldValue: 'Peter', duplicate: false }],
+    docCount: 1981,
+    pValue: 2.7454255728359757e-21,
+  },
+];
+
+describe('getGroupFilter', () => {
+  it('gets a query filter for the change points of a group with multiple values per field', () => {
+    expect(getGroupFilter(changePointGroups[0])).toStrictEqual([
+      {
+        term: {
+          response_code: '500',
+        },
+      },
+      {
+        terms: {
+          url: ['home.php', 'login.php'],
+        },
+      },
+    ]);
+  });
+
+  it('gets a query filter for the change points of a group with just a single field/value', () => {
+    expect(getGroupFilter(changePointGroups[1])).toStrictEqual([
+      {
+        term: {
+          user: 'Peter',
+        },
+      },
+    ]);
+  });
+});
diff --git a/x-pack/plugins/aiops/server/routes/queries/get_group_filter.ts b/x-pack/plugins/aiops/server/routes/queries/get_group_filter.ts
new file mode 100644
index 00000000000000..fdd97eb1010721
--- /dev/null
+++ b/x-pack/plugins/aiops/server/routes/queries/get_group_filter.ts
@@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
+
+import type { ChangePointGroup } from '@kbn/ml-agg-utils';
+
+// Transforms a list of change point items from a group in a query filter.
+// Uses a `term` filter for single field value combinations.
+// For fields with multiple values it creates a single `terms` filter that includes
+// all values. This avoids queries not returning any results otherwise because
+// separate `term` filter for multiple values for the same field would rule each other out.
+export function getGroupFilter(
+  changePointGroup: ChangePointGroup
+): estypes.QueryDslQueryContainer[] {
+  return Object.entries(
+    changePointGroup.group.reduce<Record<string, Array<string | number>>>((p, c) => {
+      if (p[c.fieldName]) {
+        p[c.fieldName].push(c.fieldValue);
+      } else {
+        p[c.fieldName] = [c.fieldValue];
+      }
+      return p;
+    }, {})
+  ).reduce<estypes.QueryDslQueryContainer[]>((p, [key, values]) => {
+    p.push(values.length > 1 ? { terms: { [key]: values } } : { term: { [key]: values[0] } });
+    return p;
+  }, []);
+}