[APM] Fix missing ML data, and NaN issue

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 };
+}
 
+describe('setupRequest', () => {
   it('should call callWithRequest with default args', async () => {
-    const setup = setupRequest(mockReq);
-    await setup.client('myType', { body: { foo: 'bar' } });
-    expect(callWithRequestSpy).toHaveBeenCalledWith(mockReq, 'myType', {
+    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: {
@@ -47,51 +47,82 @@ describe('setupRequest', () => {
   });
 
   describe('omitLegacyData', () => {
-    it('should add `observer.version_major` filter if `omitLegacyData=true` ', async () => {
-      const setup = setupRequest(mockReq);
-      await setup.client('myType', {
-        omitLegacyData: true,
-        body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
-      });
-      expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
-        query: {
-          bool: {
-            filter: [
-              { term: 'someTerm' },
-              { range: { 'observer.version_major': { gte: 7 } } }
-            ]
+    describe('if index is apm-*', () => {
+      it('should add `observer.version_major` filter if `omitLegacyData=true` ', async () => {
+        const { mockRequest, callWithRequestSpy } = getMockRequest();
+        const setup = setupRequest(mockRequest);
+        await setup.client('myType', {
+          index: 'apm-*',
+          omitLegacyData: true,
+          body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
+        });
+        expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
+          query: {
+            bool: {
+              filter: [
+                { term: 'someTerm' },
+                { range: { 'observer.version_major': { gte: 7 } } }
+              ]
+            }
           }
-        }
+        });
       });
-    });
 
-    it('should not add `observer.version_major` filter if `omitLegacyData=false` ', async () => {
-      const setup = setupRequest(mockReq);
-      await setup.client('myType', {
-        omitLegacyData: false,
-        body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
+      it('should not add `observer.version_major` filter if `omitLegacyData=false` ', async () => {
+        const { mockRequest, callWithRequestSpy } = getMockRequest();
+        const setup = setupRequest(mockRequest);
+        await setup.client('myType', {
+          index: 'apm-*',
+          omitLegacyData: false,
+          body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
+        });
+        expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
+          query: { bool: { filter: [{ term: 'someTerm' }] } }
+        });
       });
-      expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
-        query: { bool: { filter: [{ term: 'someTerm' }] } }
+
+      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 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 } } }]
+      it('should have `omitLegacyData=true` as default and merge 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('should have `omitLegacyData=true` as default and merge boolean filters', async () => {
-      const setup = setupRequest(mockReq);
+    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' }] } }
         }
@@ -100,24 +131,75 @@ describe('setupRequest', () => {
       expect(params.body).toEqual({
         query: {
           bool: {
-            filter: [
-              { term: 'someTerm' },
-              { range: { 'observer.version_major': { gte: 7 } } }
-            ]
+            filter: [{ term: 'someTerm' }]
           }
         }
       });
     });
   });
 
-  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('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);
+    });
+  });
+
+  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);
+      });
+    });
+
+    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 { cloneDeep, has, set } 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';
 
@@ -43,12 +43,36 @@ interface APMRequestQuery {
   esFilterQuery: string;
 }
 
-function addFilterForLegacyData({
-  omitLegacyData = true,
-  ...params
-}: APMSearchParams): SearchParams {
+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[],
+  { omitLegacyData = true, ...params }: APMSearchParams
+): SearchParams {
   // search across all data (including data)
-  if (!omitLegacyData) {
+  if (!omitLegacyData || !isApmIndex(apmIndices, params.index)) {
     return params;
   }
 
@@ -69,12 +93,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
     };
@@ -99,6 +125,6 @@ export function setupRequest(req: Legacy.Request): Setup {
     end: moment.utc(query.end).valueOf(),
     esFilterQuery: decodeEsQuery(query.esFilterQuery),
     client,
-    config: req.server.config()
+    config
   };
 }