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

…lasticsearch Part of: elastic#93770
alexwizp · Apr 30, 2021 · be14769 · be14769
1 parent 577948b
commit be14769
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 97 deletions.
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
@@ -9,6 +9,7 @@
 import { getStats } from './get_usage_collector';
 import { createCollectorFetchContextMock } from 'src/plugins/usage_collection/server/mocks';
 import { TIME_RANGE_DATA_MODES } from '../../common/timerange_data_modes';
+import { SavedObjectsClientContract } from 'kibana/server';
 
 const mockedSavedObjects = [
   {
@@ -94,23 +95,20 @@ const mockedSavedObjectsByValue = [
 const getMockCollectorFetchContext = (hits?: unknown[], 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 = ({
+    createPointInTimeFinder: jest.fn().mockResolvedValue({
+      find: function* asyncGenerator() {
+        yield savedObjectsByValue;
+      },
+    }),
+  } 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 result = await getStats(mockCollectorFetchContext.soClient);
 
     expect(result).toBeUndefined();
   });
@@ -141,11 +139,7 @@ describe('Timeseries visualization usage collector', () => {
         },
       ]
     );
-    const result = await getStats(
-      mockCollectorFetchContext.esClient,
-      mockCollectorFetchContext.soClient,
-      mockIndex
-    );
+    const result = await getStats(mockCollectorFetchContext.soClient);
 
     expect(result).toBeUndefined();
   });
@@ -155,11 +149,7 @@ describe('Timeseries visualization usage collector', () => {
       mockedSavedObjects,
       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,69 @@
  * Side Public License, v 1.
  */
 
-import { ElasticsearchClient } from 'src/core/server';
-import { SavedObjectsClientContract, ISavedObjectsRepository } from 'kibana/server';
+import {
+  SavedObjectsClientContract,
+  ISavedObjectsRepository,
+  SavedObjectsFindResult,
+} from 'kibana/server';
 import { TIME_RANGE_DATA_MODES } from '../../common/timerange_data_modes';
 import { findByValueEmbeddables } from '../../../dashboard/server';
+import { SavedVisState } from '../../../visualizations/common';
+
+// elasticsearch index.max_result_window default value
+const ES_MAX_RESULT_WINDOW_DEFAULT_VALUE = 1000;
 
 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: ES_MAX_RESULT_WINDOW_DEFAULT_VALUE,
+  });
 
-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
+        }
+      }
+    });
+
+    if (!response.saved_objects.length) {
+      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 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 +79,8 @@ 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 doTelemetryFoVisualizations(soClient, telemetryUseLastValueMode);
+  await 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 { 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
@@ -7,14 +7,9 @@
  */
 
 import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
-import { first } from 'rxjs/operators';
 import { getStats, TimeseriesUsage } from './get_usage_collector';
-import { ConfigObservable } from '../types';
 
-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);