[TSVB] Expensive queries are causing unnecessary load and delays on E…

…lasticsearch (elastic#98914) * [TSVB] Expensive queries are causing unnecessary load and delays on Elasticsearch Part of: elastic#93770 * remove globalConfig * fix tests * fix finder.close * cleanup code * run queries concurrently * add namespaces: ['*'], Co-authored-by: Kibana Machine <[email protected]>
alexwizp · Aug 30, 2021 · cca0719 · cca0719
1 parent 709bc13
commit cca0719
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 135 deletions.
diff --git a/src/plugins/vis_type_timeseries/server/plugin.ts b/src/plugins/vis_type_timeseries/server/plugin.ts
@@ -120,7 +120,7 @@ export class VisTypeTimeseriesPlugin implements Plugin<VisTypeTimeseriesSetup> {
     fieldsRoutes(router, framework);
 
     if (plugins.usageCollection) {
-      registerTimeseriesUsageCollector(plugins.usageCollection, globalConfig$);
+      registerTimeseriesUsageCollector(plugins.usageCollection);
     }
 
     return {

diff --git a/src/plugins/vis_type_timeseries/server/usage_collector/get_usage_collector.test.ts b/src/plugins/vis_type_timeseries/server/usage_collector/get_usage_collector.test.ts
@@ -7,15 +7,14 @@
  */
 
 import { getStats } from './get_usage_collector';
-import { createCollectorFetchContextMock } from 'src/plugins/usage_collection/server/mocks';
+import { createCollectorFetchContextMock } from '../../../usage_collection/server/mocks';
+import type { SavedObjectsClientContract, SavedObjectsFindResponse } from '../../../../core/server';
 import { TIME_RANGE_DATA_MODES } from '../../common/enums';
 
-const mockedSavedObjects = [
-  {
-    _id: 'visualization:timeseries-123',
-    _source: {
-      type: 'visualization',
-      visualization: {
+const mockedSavedObject = {
+  saved_objects: [
+    {
+      attributes: {
         visState: JSON.stringify({
           type: 'metrics',
           title: 'TSVB visualization 1',
@@ -25,12 +24,8 @@ const mockedSavedObjects = [
         }),
       },
     },
-  },
-  {
-    _id: 'visualization:timeseries-321',
-    _source: {
-      type: 'visualization',
-      visualization: {
+    {
+      attributes: {
         visState: JSON.stringify({
           type: 'metrics',
           title: 'TSVB visualization 2',
@@ -40,12 +35,8 @@ const mockedSavedObjects = [
         }),
       },
     },
-  },
-  {
-    _id: 'visualization:timeseries-456',
-    _source: {
-      type: 'visualization',
-      visualization: {
+    {
+      attributes: {
         visState: JSON.stringify({
           type: 'metrics',
           title: 'TSVB visualization 3',
@@ -55,8 +46,8 @@ const mockedSavedObjects = [
         }),
       },
     },
-  },
-];
+  ],
+} as SavedObjectsFindResponse;
 
 const mockedSavedObjectsByValue = [
   {
@@ -91,42 +82,43 @@ const mockedSavedObjectsByValue = [
   },
 ];
 
-const getMockCollectorFetchContext = (hits?: unknown[], savedObjectsByValue: unknown[] = []) => {
+const getMockCollectorFetchContext = (
+  savedObjects: SavedObjectsFindResponse,
+  savedObjectsByValue: unknown[] = []
+) => {
   const fetchParamsMock = createCollectorFetchContextMock();
 
-  fetchParamsMock.esClient.search = jest.fn().mockResolvedValue({ body: { hits: { hits } } });
-  fetchParamsMock.soClient.find = jest.fn().mockResolvedValue({
-    saved_objects: savedObjectsByValue,
-  });
+  fetchParamsMock.soClient = ({
+    find: jest.fn().mockResolvedValue({
+      saved_objects: savedObjectsByValue,
+    }),
+    createPointInTimeFinder: jest.fn().mockResolvedValue({
+      close: jest.fn(),
+      find: function* asyncGenerator() {
+        yield savedObjects;
+      },
+    }),
+  } as unknown) as SavedObjectsClientContract;
   return fetchParamsMock;
 };
 
 describe('Timeseries visualization usage collector', () => {
-  const mockIndex = 'mock_index';
-
   test('Returns undefined when no results found (undefined)', async () => {
-    const mockCollectorFetchContext = getMockCollectorFetchContext([], []);
-    const result = await getStats(
-      mockCollectorFetchContext.esClient,
-      mockCollectorFetchContext.soClient,
-      mockIndex
+    const mockCollectorFetchContext = getMockCollectorFetchContext(
+      ({ saved_objects: [] } as unknown) as SavedObjectsFindResponse,
+      []
     );
+    const result = await getStats(mockCollectorFetchContext.soClient);
 
     expect(result).toBeUndefined();
   });
 
   test('Returns undefined when no timeseries saved objects found', async () => {
-    const mockCollectorFetchContext = getMockCollectorFetchContext(
-      [
+    const mockCollectorFetchContext = getMockCollectorFetchContext({
+      saved_objects: [
         {
-          _id: 'visualization:myvis-123',
-          _source: {
-            type: 'visualization',
-            visualization: { visState: '{"type": "area"}' },
-          },
+          attributes: { visState: '{"type": "area"}' },
         },
-      ],
-      [
         {
           attributes: {
             panelsJSON: JSON.stringify({
@@ -139,27 +131,20 @@ describe('Timeseries visualization usage collector', () => {
             }),
           },
         },
-      ]
-    );
-    const result = await getStats(
-      mockCollectorFetchContext.esClient,
-      mockCollectorFetchContext.soClient,
-      mockIndex
-    );
+      ],
+    } as SavedObjectsFindResponse);
+
+    const result = await getStats(mockCollectorFetchContext.soClient);
 
     expect(result).toBeUndefined();
   });
 
   test('Summarizes visualizations response data', async () => {
     const mockCollectorFetchContext = getMockCollectorFetchContext(
-      mockedSavedObjects,
+      mockedSavedObject,
       mockedSavedObjectsByValue
     );
-    const result = await getStats(
-      mockCollectorFetchContext.esClient,
-      mockCollectorFetchContext.soClient,
-      mockIndex
-    );
+    const result = await getStats(mockCollectorFetchContext.soClient);
 
     expect(result).toMatchObject({
       timeseries_use_last_value_mode_total: 3,

diff --git a/src/plugins/vis_type_timeseries/server/usage_collector/get_usage_collector.ts b/src/plugins/vis_type_timeseries/server/usage_collector/get_usage_collector.ts
@@ -6,49 +6,65 @@
  * Side Public License, v 1.
  */
 
-import { ElasticsearchClient } from 'src/core/server';
-import { SavedObjectsClientContract, ISavedObjectsRepository } from 'kibana/server';
 import { TIME_RANGE_DATA_MODES } from '../../common/enums';
 import { findByValueEmbeddables } from '../../../dashboard/server';
 
+import type {
+  SavedObjectsClientContract,
+  ISavedObjectsRepository,
+  SavedObjectsFindResult,
+} from '../../../../core/server';
+import type { SavedVisState } from '../../../visualizations/common';
+
 export interface TimeseriesUsage {
   timeseries_use_last_value_mode_total: number;
 }
 
-interface VisState {
-  type?: string;
-  params?: any;
-}
+const doTelemetryFoVisualizations = async (
+  soClient: SavedObjectsClientContract | ISavedObjectsRepository,
+  telemetryUseLastValueMode: (savedVis: SavedVisState) => void
+) => {
+  const finder = await soClient.createPointInTimeFinder({
+    type: 'visualization',
+    perPage: 1000,
+    namespaces: ['*'],
+  });
 
-export const getStats = async (
-  esClient: ElasticsearchClient,
+  for await (const response of finder.find()) {
+    (response.saved_objects || []).forEach(({ attributes }: SavedObjectsFindResult<any>) => {
+      if (attributes?.visState) {
+        try {
+          const visState: SavedVisState = JSON.parse(attributes.visState);
+
+          telemetryUseLastValueMode(visState);
+        } catch {
+          // nothing to be here, "so" not valid
+        }
+      }
+    });
+  }
+  await finder.close();
+};
+
+const doTelemetryForByValueVisualizations = async (
   soClient: SavedObjectsClientContract | ISavedObjectsRepository,
-  index: string
+  telemetryUseLastValueMode: (savedVis: SavedVisState) => void
+) => {
+  const byValueVisualizations = await findByValueEmbeddables(soClient, 'visualization');
+
+  for (const item of byValueVisualizations) {
+    telemetryUseLastValueMode((item.savedVis as unknown) as SavedVisState);
+  }
+};
+
+export const getStats = async (
+  soClient: SavedObjectsClientContract | ISavedObjectsRepository
 ): Promise<TimeseriesUsage | undefined> => {
   const timeseriesUsage = {
     timeseries_use_last_value_mode_total: 0,
   };
 
-  const searchParams = {
-    size: 10000,
-    index,
-    ignoreUnavailable: true,
-    filterPath: ['hits.hits._id', 'hits.hits._source.visualization'],
-    body: {
-      query: {
-        bool: {
-          filter: { term: { type: 'visualization' } },
-        },
-      },
-    },
-  };
-
-  const { body: esResponse } = await esClient.search<{
-    visualization: { visState: string };
-    updated_at: string;
-  }>(searchParams);
-
-  function telemetryUseLastValueMode(visState: VisState) {
+  function telemetryUseLastValueMode(visState: SavedVisState) {
     if (
       visState.type === 'metrics' &&
       visState.params.type !== 'timeseries' &&
@@ -59,28 +75,10 @@ export const getStats = async (
     }
   }
 
-  if (esResponse?.hits?.hits?.length) {
-    for (const hit of esResponse.hits.hits) {
-      if (hit._source && 'visualization' in hit._source) {
-        const { visualization } = hit._source!;
-
-        let visState: VisState = {};
-        try {
-          visState = JSON.parse(visualization?.visState ?? '{}');
-        } catch (e) {
-          // invalid visState
-        }
-
-        telemetryUseLastValueMode(visState);
-      }
-    }
-  }
-
-  const byValueVisualizations = await findByValueEmbeddables(soClient, 'visualization');
-
-  for (const item of byValueVisualizations) {
-    telemetryUseLastValueMode(item.savedVis as VisState);
-  }
+  await Promise.all([
+    doTelemetryFoVisualizations(soClient, telemetryUseLastValueMode),
+    doTelemetryForByValueVisualizations(soClient, telemetryUseLastValueMode),
+  ]);
 
   return timeseriesUsage.timeseries_use_last_value_mode_total ? timeseriesUsage : undefined;
 };
diff --git a/src/plugins/vis_type_timeseries/server/usage_collector/register_timeseries_collector.test.ts b/src/plugins/vis_type_timeseries/server/usage_collector/register_timeseries_collector.test.ts
@@ -6,27 +6,22 @@
  * Side Public License, v 1.
  */
 
-import { of } from 'rxjs';
 import { mockStats, mockGetStats } from './get_usage_collector.mock';
-import { createUsageCollectionSetupMock } from 'src/plugins/usage_collection/server/mocks';
-import { createCollectorFetchContextMock } from 'src/plugins/usage_collection/server/mocks';
+import { createUsageCollectionSetupMock } from '../../../usage_collection/server/mocks';
+import { createCollectorFetchContextMock } from '../../../usage_collection/server/mocks';
 import { registerTimeseriesUsageCollector } from './register_timeseries_collector';
-import { ConfigObservable } from '../types';
 
 describe('registerTimeseriesUsageCollector', () => {
-  const mockIndex = 'mock_index';
-  const mockConfig = of({ kibana: { index: mockIndex } }) as ConfigObservable;
-
   it('makes a usage collector and registers it`', () => {
     const mockCollectorSet = createUsageCollectionSetupMock();
-    registerTimeseriesUsageCollector(mockCollectorSet, mockConfig);
+    registerTimeseriesUsageCollector(mockCollectorSet);
     expect(mockCollectorSet.makeUsageCollector).toBeCalledTimes(1);
     expect(mockCollectorSet.registerCollector).toBeCalledTimes(1);
   });
 
   it('makeUsageCollector configs fit the shape', () => {
     const mockCollectorSet = createUsageCollectionSetupMock();
-    registerTimeseriesUsageCollector(mockCollectorSet, mockConfig);
+    registerTimeseriesUsageCollector(mockCollectorSet);
     expect(mockCollectorSet.makeUsageCollector).toHaveBeenCalledWith({
       type: 'vis_type_timeseries',
       isReady: expect.any(Function),
@@ -39,23 +34,19 @@ describe('registerTimeseriesUsageCollector', () => {
 
   it('makeUsageCollector config.isReady returns true', () => {
     const mockCollectorSet = createUsageCollectionSetupMock();
-    registerTimeseriesUsageCollector(mockCollectorSet, mockConfig);
+    registerTimeseriesUsageCollector(mockCollectorSet);
     const usageCollectorConfig = mockCollectorSet.makeUsageCollector.mock.calls[0][0];
     expect(usageCollectorConfig.isReady()).toBe(true);
   });
 
   it('makeUsageCollector config.fetch calls getStats', async () => {
     const mockCollectorSet = createUsageCollectionSetupMock();
-    registerTimeseriesUsageCollector(mockCollectorSet, mockConfig);
+    registerTimeseriesUsageCollector(mockCollectorSet);
     const usageCollector = mockCollectorSet.makeUsageCollector.mock.results[0].value;
     const mockedCollectorFetchContext = createCollectorFetchContextMock();
     const fetchResult = await usageCollector.fetch(mockedCollectorFetchContext);
     expect(mockGetStats).toBeCalledTimes(1);
-    expect(mockGetStats).toBeCalledWith(
-      mockedCollectorFetchContext.esClient,
-      mockedCollectorFetchContext.soClient,
-      mockIndex
-    );
+    expect(mockGetStats).toBeCalledWith(mockedCollectorFetchContext.soClient);
     expect(fetchResult).toBe(mockStats);
   });
 });
diff --git a/src/plugins/vis_type_timeseries/server/usage_collector/register_timeseries_collector.ts b/src/plugins/vis_type_timeseries/server/usage_collector/register_timeseries_collector.ts
@@ -6,15 +6,10 @@
  * Side Public License, v 1.
  */
 
-import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
-import { first } from 'rxjs/operators';
 import { getStats, TimeseriesUsage } from './get_usage_collector';
-import { ConfigObservable } from '../types';
+import type { UsageCollectionSetup } from '../../../usage_collection/server';
 
-export function registerTimeseriesUsageCollector(
-  collectorSet: UsageCollectionSetup,
-  config: ConfigObservable
-) {
+export function registerTimeseriesUsageCollector(collectorSet: UsageCollectionSetup) {
   const collector = collectorSet.makeUsageCollector<TimeseriesUsage | undefined>({
     type: 'vis_type_timeseries',
     isReady: () => true,
@@ -24,11 +19,7 @@ export function registerTimeseriesUsageCollector(
         _meta: { description: 'Number of TSVB visualizations using "last value" as a time range' },
       },
     },
-    fetch: async ({ esClient, soClient }) => {
-      const { index } = (await config.pipe(first()).toPromise()).kibana;
-
-      return await getStats(esClient, soClient, index);
-    },
+    fetch: async ({ soClient }) => await getStats(soClient),
   });
 
   collectorSet.registerCollector(collector);