[7.0] [APM] Fix missing ML data, and NaN issue (#34333)

x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts

@@ -4,35 +4,35 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { setupRequest } from './setup_request';
+import { Legacy } from 'kibana';
+import { isApmIndex, setupRequest } from './setup_request';
 
-describe('setupRequest', () => {
-  let callWithRequestSpy: jest.Mock;
-  let mockReq: any;
-
-  beforeEach(() => {
-    callWithRequestSpy = jest.fn();
-    mockReq = {
-      params: {},
-      query: {},
-      server: {
-        config: () => 'myConfig',
-        plugins: {
-          elasticsearch: {
-            getCluster: () => ({ callWithRequest: callWithRequestSpy })
-          }
+function getMockRequest() {
+  const callWithRequestSpy = jest.fn();
+  const mockRequest = ({
+    params: {},
+    query: {},
+    server: {
+      config: () => ({ get: () => 'apm-*' }),
+      plugins: {
+        elasticsearch: {
+          getCluster: () => ({ callWithRequest: callWithRequestSpy })
         }
-      },
-      getUiSettingsService: jest.fn(() => ({
-        get: jest.fn(() => Promise.resolve(false))
-      }))
-    };
-  });
+      }
+    },
+    getUiSettingsService: () => ({ get: async () => false })
+  } as any) as Legacy.Request;
+
+  return { callWithRequestSpy, mockRequest };
+}
 
-  it('should call callWithRequest with correct args', async () => {
-    const setup = setupRequest(mockReq);
-    await setup.client('myType', { body: { foo: 'bar' } });
-    expect(callWithRequestSpy).toHaveBeenCalledWith(mockReq, 'myType', {
+describe('setupRequest', () => {
+  it('should call callWithRequest with default args', async () => {
+    const { mockRequest, callWithRequestSpy } = getMockRequest();
+    const setup = setupRequest(mockRequest);
+    await setup.client('myType', { index: 'apm-*', body: { foo: 'bar' } });
+    expect(callWithRequestSpy).toHaveBeenCalledWith(mockRequest, 'myType', {
+      index: 'apm-*',
       body: {
         foo: 'bar',
         query: {
@@ -46,45 +46,125 @@ describe('setupRequest', () => {
     });
   });
 
-  it('should set ignore_throttled to false if includeFrozen is true', async () => {
-    // mock includeFrozen to return true
-    mockReq.getUiSettingsService.mockImplementation(() => ({
-      get: jest.fn(() => Promise.resolve(true))
-    }));
-    const setup = setupRequest(mockReq);
-    await setup.client('myType', {});
-    const params = callWithRequestSpy.mock.calls[0][2];
-    expect(params.ignore_throttled).toBe(false);
+  describe('observer.version_major filter', () => {
+    describe('if index is apm-*', () => {
+      it('should add `observer.version_major` filter if none exists', async () => {
+        const { mockRequest, callWithRequestSpy } = getMockRequest();
+        const setup = setupRequest(mockRequest);
+        await setup.client('myType', { index: 'apm-*' });
+        const params = callWithRequestSpy.mock.calls[0][2];
+        expect(params.body).toEqual({
+          query: {
+            bool: {
+              filter: [{ range: { 'observer.version_major': { gte: 7 } } }]
+            }
+          }
+        });
+      });
+
+      it('should merge `observer.version_major` filter with existing boolean filters', async () => {
+        const { mockRequest, callWithRequestSpy } = getMockRequest();
+        const setup = setupRequest(mockRequest);
+        await setup.client('myType', {
+          index: 'apm-*',
+          body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
+        });
+        const params = callWithRequestSpy.mock.calls[0][2];
+        expect(params.body).toEqual({
+          query: {
+            bool: {
+              filter: [
+                { term: 'someTerm' },
+                { range: { 'observer.version_major': { gte: 7 } } }
+              ]
+            }
+          }
+        });
+      });
+    });
+
+    it('if index is not an APM index, it should not add `observer.version_major` filter', async () => {
+      const { mockRequest, callWithRequestSpy } = getMockRequest();
+      const setup = setupRequest(mockRequest);
+      await setup.client('myType', {
+        index: '.ml-*',
+        body: {
+          query: { bool: { filter: [{ term: 'someTerm' }] } }
+        }
+      });
+      const params = callWithRequestSpy.mock.calls[0][2];
+      expect(params.body).toEqual({
+        query: {
+          bool: {
+            filter: [{ term: 'someTerm' }]
+          }
+        }
+      });
+    });
   });
 
-  it('should set filter if none exists', async () => {
-    const setup = setupRequest(mockReq);
-    await setup.client('myType', {});
-    const params = callWithRequestSpy.mock.calls[0][2];
-    expect(params.body).toEqual({
-      query: {
-        bool: { filter: [{ range: { 'observer.version_major': { gte: 7 } } }] }
-      }
+  describe('ignore_throttled', () => {
+    it('should set `ignore_throttled=true` if `includeFrozen=false`', async () => {
+      const { mockRequest, callWithRequestSpy } = getMockRequest();
+
+      // mock includeFrozen to return false
+      mockRequest.getUiSettingsService = () => ({ get: async () => false });
+      const setup = setupRequest(mockRequest);
+      await setup.client('myType', {});
+      const params = callWithRequestSpy.mock.calls[0][2];
+      expect(params.ignore_throttled).toBe(true);
+    });
+
+    it('should set `ignore_throttled=false` if `includeFrozen=true`', async () => {
+      const { mockRequest, callWithRequestSpy } = getMockRequest();
+
+      // mock includeFrozen to return true
+      mockRequest.getUiSettingsService = () => ({ get: async () => true });
+      const setup = setupRequest(mockRequest);
+      await setup.client('myType', {});
+      const params = callWithRequestSpy.mock.calls[0][2];
+      expect(params.ignore_throttled).toBe(false);
     });
   });
 
-  it('should merge filters if one exists', async () => {
-    const setup = setupRequest(mockReq);
-    await setup.client('myType', {
-      body: {
-        query: { bool: { filter: [{ term: 'someTerm' }] } }
-      }
+  describe('isApmIndex', () => {
+    const apmIndices = [
+      'apm-*-metric-*',
+      'apm-*-onboarding-*',
+      'apm-*-span-*',
+      'apm-*-transaction-*',
+      'apm-*-error-*'
+    ];
+    describe('when indexParam is a string', () => {
+      it('should return true if it matches any of the items in apmIndices', () => {
+        const indexParam = 'apm-*-transaction-*';
+        expect(isApmIndex(apmIndices, indexParam)).toBe(true);
+      });
+
+      it('should return false if it does not match any of the items in `apmIndices`', () => {
+        const indexParam = '.ml-anomalies-*';
+        expect(isApmIndex(apmIndices, indexParam)).toBe(false);
+      });
     });
-    const params = callWithRequestSpy.mock.calls[0][2];
-    expect(params.body).toEqual({
-      query: {
-        bool: {
-          filter: [
-            { term: 'someTerm' },
-            { range: { 'observer.version_major': { gte: 7 } } }
-          ]
-        }
-      }
+
+    describe('when indexParam is an array', () => {
+      it('should return true if all values in `indexParam` matches values in `apmIndices`', () => {
+        const indexParam = ['apm-*-transaction-*', 'apm-*-span-*'];
+        expect(isApmIndex(apmIndices, indexParam)).toBe(true);
+      });
+
+      it("should return false if some of the values don't match with `apmIndices`", () => {
+        const indexParam = ['apm-*-transaction-*', '.ml-anomalies-*'];
+        expect(isApmIndex(apmIndices, indexParam)).toBe(false);
+      });
+    });
+
+    describe('when indexParam is neither a string or an array', () => {
+      it('should return false', () => {
+        [true, false, undefined].forEach(indexParam => {
+          expect(isApmIndex(apmIndices, indexParam)).toBe(false);
+        });
+      });
     });
   });
 });

x-pack/plugins/apm/server/lib/helpers/setup_request.ts

@@ -11,7 +11,7 @@ import {
   SearchParams
 } from 'elasticsearch';
 import { Legacy } from 'kibana';
-import { merge } from 'lodash';
+import { cloneDeep, has, isString, set } from 'lodash';
 import moment from 'moment';
 import { OBSERVER_VERSION_MAJOR } from 'x-pack/plugins/apm/common/elasticsearch_fieldnames';
 
@@ -39,11 +39,43 @@ interface APMRequestQuery {
   esFilterQuery: string;
 }
 
-function addFilterForLegacyData(params: SearchParams) {
-  // ensure a filter exists
-  const nextParams = merge({}, params, {
-    body: { query: { bool: { filter: [] } } }
-  });
+function getApmIndices(config: Legacy.KibanaConfig) {
+  return [
+    config.get<string>('apm_oss.errorIndices'),
+    config.get<string>('apm_oss.metricsIndices'),
+    config.get<string>('apm_oss.onboardingIndices'),
+    config.get<string>('apm_oss.sourcemapIndices'),
+    config.get<string>('apm_oss.spanIndices'),
+    config.get<string>('apm_oss.transactionIndices')
+  ];
+}
+
+export function isApmIndex(
+  apmIndices: string[],
+  indexParam: SearchParams['index']
+) {
+  if (isString(indexParam)) {
+    return apmIndices.includes(indexParam);
+  } else if (Array.isArray(indexParam)) {
+    // return false if at least one of the indices is not an APM index
+    return indexParam.every(index => apmIndices.includes(index));
+  }
+  return false;
+}
+
+function addFilterForLegacyData(
+  apmIndices: string[],
+  params: SearchParams
+): SearchParams {
+  // search across all data (including 6.x data)
+  if (!isApmIndex(apmIndices, params.index)) {
+    return params;
+  }
+
+  const nextParams = cloneDeep(params);
+  if (!has(nextParams, 'body.query.bool.filter')) {
+    set(nextParams, 'body.query.bool.filter', []);
+  }
 
   // add to filter
   nextParams.body.query.bool.filter.push({
@@ -57,12 +89,14 @@ export function setupRequest(req: Legacy.Request): Setup {
   const query = (req.query as unknown) as APMRequestQuery;
   const cluster = req.server.plugins.elasticsearch.getCluster('data');
   const uiSettings = req.getUiSettingsService();
+  const config = req.server.config();
+  const apmIndices = getApmIndices(config);
 
   const client: ESClient = async (type, params) => {
     const includeFrozen = await uiSettings.get('search:includeFrozen');
 
     const nextParams = {
-      ...addFilterForLegacyData(params), // filter out pre-7.0 data
+      ...addFilterForLegacyData(apmIndices, params), // filter out pre-7.0 data
       ignore_throttled: !includeFrozen, // whether to query frozen indices or not
       rest_total_hits_as_int: true // ensure that ES returns accurate hits.total with pre-6.6 format
     };
@@ -87,6 +121,6 @@ export function setupRequest(req: Legacy.Request): Setup {
     end: moment.utc(query.end).valueOf(),
     esFilterQuery: decodeEsQuery(query.esFilterQuery),
     client,
-    config: req.server.config()
+    config
   };
 }