From 265fed590e72d23ee918b07a8643ddf287f30e2f Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 30 Aug 2021 14:11:42 +0300 Subject: [PATCH] [Data Table] Expensive queries are causing unnecessary load and delays on Elasticsearch (#98903) * [Data Table] Expensive queries are causing unnecessary load and delays on Elasticsearch Part of #93770 * remove extra cycles * fix PR comments * fix finder.close * code cleanup * add namespaces: ['*'], * fix jest Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../server/usage_collector/get_stats.test.ts | 16 ++++-- .../server/usage_collector/get_stats.ts | 56 ++++++++++++------- .../register_usage_collector.test.ts | 15 +++-- .../register_usage_collector.ts | 3 +- 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/src/plugins/vis_type_table/server/usage_collector/get_stats.test.ts b/src/plugins/vis_type_table/server/usage_collector/get_stats.test.ts index 3f8f4289321b5..76f067e3a23d7 100644 --- a/src/plugins/vis_type_table/server/usage_collector/get_stats.test.ts +++ b/src/plugins/vis_type_table/server/usage_collector/get_stats.test.ts @@ -6,8 +6,8 @@ * Side Public License, v 1. */ -import { SavedObjectsClientContract } from 'kibana/server'; import { getStats } from './get_stats'; +import type { SavedObjectsClientContract } from '../../../../core/server'; const mockVisualizations = { saved_objects: [ @@ -42,15 +42,23 @@ const mockVisualizations = { describe('vis_type_table getStats', () => { const mockSoClient = ({ - find: jest.fn().mockResolvedValue(mockVisualizations), + createPointInTimeFinder: jest.fn().mockResolvedValue({ + close: jest.fn(), + find: function* asyncGenerator() { + yield mockVisualizations; + }, + }), } as unknown) as SavedObjectsClientContract; test('Returns stats from saved objects for table vis only', async () => { const result = await getStats(mockSoClient); - expect(mockSoClient.find).toHaveBeenCalledWith({ + + expect(mockSoClient.createPointInTimeFinder).toHaveBeenCalledWith({ type: 'visualization', - perPage: 10000, + perPage: 1000, + namespaces: ['*'], }); + expect(result).toEqual({ total: 4, total_split: 3, diff --git a/src/plugins/vis_type_table/server/usage_collector/get_stats.ts b/src/plugins/vis_type_table/server/usage_collector/get_stats.ts index 6fdb555c86328..ef948c2d7b70b 100644 --- a/src/plugins/vis_type_table/server/usage_collector/get_stats.ts +++ b/src/plugins/vis_type_table/server/usage_collector/get_stats.ts @@ -6,12 +6,14 @@ * Side Public License, v 1. */ -import { ISavedObjectsRepository, SavedObjectsClientContract } from 'kibana/server'; -import { - SavedVisState, - VisualizationSavedObjectAttributes, -} from 'src/plugins/visualizations/common'; -import { TableVisParams, VIS_TYPE_TABLE } from '../../common'; +import { VIS_TYPE_TABLE } from '../../common'; + +import type { + ISavedObjectsRepository, + SavedObjectsClientContract, + SavedObjectsFindResult, +} from '../../../../core/server'; +import type { SavedVisState } from '../../../visualizations/common'; export interface VisTypeTableUsage { /** @@ -44,17 +46,14 @@ export interface VisTypeTableUsage { export async function getStats( soClient: SavedObjectsClientContract | ISavedObjectsRepository ): Promise { - const visualizations = await soClient.find({ + const finder = await soClient.createPointInTimeFinder({ type: 'visualization', - perPage: 10000, + perPage: 1000, + namespaces: ['*'], }); - const tableVisualizations = visualizations.saved_objects - .map>(({ attributes }) => JSON.parse(attributes.visState)) - .filter(({ type }) => type === VIS_TYPE_TABLE); - - const defaultStats = { - total: tableVisualizations.length, + const stats: VisTypeTableUsage = { + total: 0, total_split: 0, split_columns: { total: 0, @@ -66,20 +65,39 @@ export async function getStats( }, }; - return tableVisualizations.reduce((acc, { aggs, params }) => { + const doTelemetry = ({ aggs, params }: SavedVisState) => { + stats.total += 1; + const hasSplitAgg = aggs.find((agg) => agg.schema === 'split'); if (hasSplitAgg) { - acc.total_split += 1; + stats.total_split += 1; const isSplitRow = params.row; const isSplitEnabled = hasSplitAgg.enabled; + const container = isSplitRow ? stats.split_rows : stats.split_columns; - const container = isSplitRow ? acc.split_rows : acc.split_columns; container.total += 1; container.enabled = isSplitEnabled ? container.enabled + 1 : container.enabled; } + }; + + for await (const response of finder.find()) { + (response.saved_objects || []).forEach(({ attributes }: SavedObjectsFindResult) => { + if (attributes?.visState) { + try { + const visState: SavedVisState = JSON.parse(attributes.visState); + + if (visState.type === VIS_TYPE_TABLE) { + doTelemetry(visState); + } + } catch { + // nothing to be here, "so" not valid + } + } + }); + } + await finder.close(); - return acc; - }, defaultStats); + return stats; } diff --git a/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.test.ts b/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.test.ts index e045788897b61..d32435ac45406 100644 --- a/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.test.ts +++ b/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.test.ts @@ -6,20 +6,19 @@ * Side Public License, v 1. */ -jest.mock('./get_stats', () => ({ - getStats: jest.fn().mockResolvedValue({ somestat: 1 }), -})); - import { createUsageCollectionSetupMock, createCollectorFetchContextMock, -} from 'src/plugins/usage_collection/server/mocks'; - +} from '../../../usage_collection/server/mocks'; import { registerVisTypeTableUsageCollector } from './register_usage_collector'; import { getStats } from './get_stats'; +jest.mock('./get_stats', () => ({ + getStats: jest.fn().mockResolvedValue({ somestat: 1 }), +})); + describe('registerVisTypeTableUsageCollector', () => { - it('Usage collector configs fit the shape', () => { + test('Usage collector configs fit the shape', () => { const mockCollectorSet = createUsageCollectionSetupMock(); registerVisTypeTableUsageCollector(mockCollectorSet); expect(mockCollectorSet.makeUsageCollector).toBeCalledTimes(1); @@ -45,7 +44,7 @@ describe('registerVisTypeTableUsageCollector', () => { expect(usageCollectorConfig.isReady()).toBe(true); }); - it('Usage collector config.fetch calls getStats', async () => { + test('Usage collector config.fetch calls getStats', async () => { const mockCollectorSet = createUsageCollectionSetupMock(); registerVisTypeTableUsageCollector(mockCollectorSet); const usageCollector = mockCollectorSet.makeUsageCollector.mock.results[0].value; diff --git a/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.ts b/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.ts index d3d3204e0841c..74044c9ae70c0 100644 --- a/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.ts +++ b/src/plugins/vis_type_table/server/usage_collector/register_usage_collector.ts @@ -6,9 +6,8 @@ * Side Public License, v 1. */ -import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; - import { getStats, VisTypeTableUsage } from './get_stats'; +import type { UsageCollectionSetup } from '../../../usage_collection/server'; export function registerVisTypeTableUsageCollector(collectorSet: UsageCollectionSetup) { const collector = collectorSet.makeUsageCollector({