From 57dcc8b737afe8959f44dfdfe6dd51521e3f0cea Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 3 May 2021 13:50:57 +0300 Subject: [PATCH] [Visualizations] Expensive queries are causing unnecessary load and delays on Elasticsearch Part of: #93770 --- .../get_usage_collector.test.ts | 125 +++++++----------- .../usage_collector/get_usage_collector.ts | 104 +++++++-------- .../register_visualizations_collector.test.ts | 30 ++--- .../register_visualizations_collector.ts | 15 +-- 4 files changed, 111 insertions(+), 163 deletions(-) diff --git a/src/plugins/visualizations/server/usage_collector/get_usage_collector.test.ts b/src/plugins/visualizations/server/usage_collector/get_usage_collector.test.ts index 325b52eaec368..39e181e81daca 100644 --- a/src/plugins/visualizations/server/usage_collector/get_usage_collector.test.ts +++ b/src/plugins/visualizations/server/usage_collector/get_usage_collector.test.ts @@ -7,126 +7,97 @@ */ import moment from 'moment'; -import { ElasticsearchClient } from 'src/core/server'; import { getStats } from './get_usage_collector'; +import type { SavedObjectsClientContract } from '../../../../core/server'; const defaultMockSavedObjects = [ { - _id: 'visualization:coolviz-123', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "shell_beads"}' }, - updated_at: moment().subtract(7, 'days').startOf('day').toString(), - }, + id: 'visualization:coolviz-123', + attributes: { visState: '{"type": "shell_beads"}' }, + updated_at: moment().subtract(7, 'days').startOf('day').toString(), }, ]; const enlargedMockSavedObjects = [ // default space { - _id: 'visualization:coolviz-123', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "cave_painting"}' }, - updated_at: moment().subtract(7, 'days').startOf('day').toString(), - }, + id: 'visualization:coolviz-123', + attributes: { visState: '{"type": "cave_painting"}' }, + updated_at: moment().subtract(7, 'days').startOf('day').toString(), }, { - _id: 'visualization:coolviz-456', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "printing_press"}' }, - updated_at: moment().subtract(20, 'days').startOf('day').toString(), - }, + id: 'visualization:coolviz-456', + attributes: { visState: '{"type": "printing_press"}' }, + updated_at: moment().subtract(20, 'days').startOf('day').toString(), }, { - _id: 'meat:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "floppy_disk"}' }, - updated_at: moment().subtract(2, 'months').startOf('day').toString(), - }, + id: 'meat:visualization:coolviz-789', + attributes: { visState: '{"type": "floppy_disk"}' }, + updated_at: moment().subtract(2, 'months').startOf('day').toString(), }, // meat space { - _id: 'meat:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "cave_painting"}' }, - updated_at: moment().subtract(89, 'days').startOf('day').toString(), - }, + id: 'meat:visualization:coolviz-789', + attributes: { visState: '{"type": "cave_painting"}' }, + updated_at: moment().subtract(89, 'days').startOf('day').toString(), }, { - _id: 'meat:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "cuneiform"}' }, - updated_at: moment().subtract(5, 'months').startOf('day').toString(), - }, + id: 'meat:visualization:coolviz-789', + attributes: { visState: '{"type": "cuneiform"}' }, + updated_at: moment().subtract(5, 'months').startOf('day').toString(), }, { - _id: 'meat:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "cuneiform"}' }, - updated_at: moment().subtract(2, 'days').startOf('day').toString(), - }, + id: 'meat:visualization:coolviz-789', + attributes: { visState: '{"type": "cuneiform"}' }, + updated_at: moment().subtract(2, 'days').startOf('day').toString(), }, { - _id: 'meat:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "floppy_disk"}' }, - updated_at: moment().subtract(7, 'days').startOf('day').toString(), - }, + id: 'meat:visualization:coolviz-789', + attributes: { visState: '{"type": "floppy_disk"}' }, + updated_at: moment().subtract(7, 'days').startOf('day').toString(), }, // cyber space { - _id: 'cyber:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "floppy_disk"}' }, - updated_at: moment().subtract(7, 'months').startOf('day').toString(), - }, + id: 'cyber:visualization:coolviz-789', + attributes: { visState: '{"type": "floppy_disk"}' }, + updated_at: moment().subtract(7, 'months').startOf('day').toString(), }, { - _id: 'cyber:visualization:coolviz-789', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "floppy_disk"}' }, - updated_at: moment().subtract(3, 'days').startOf('day').toString(), - }, + id: 'cyber:visualization:coolviz-789', + attributes: { visState: '{"type": "floppy_disk"}' }, + updated_at: moment().subtract(3, 'days').startOf('day').toString(), }, { - _id: 'cyber:visualization:coolviz-123', - _source: { - type: 'visualization', - visualization: { visState: '{"type": "cave_painting"}' }, - updated_at: moment().subtract(15, 'days').startOf('day').toString(), - }, + id: 'cyber:visualization:coolviz-123', + attributes: { visState: '{"type": "cave_painting"}' }, + updated_at: moment().subtract(15, 'days').startOf('day').toString(), }, ]; describe('Visualizations usage collector', () => { - const mockIndex = ''; - - const getMockCallCluster = (hits: unknown[]) => - ({ - search: () => Promise.resolve({ body: { hits: { hits } } }) as unknown, - } as ElasticsearchClient); + const getMockCallCluster = (savedObjects: unknown[]) => + (({ + createPointInTimeFinder: jest.fn().mockResolvedValue({ + close: jest.fn(), + find: function* asyncGenerator() { + yield { saved_objects: savedObjects }; + }, + }), + } as unknown) as SavedObjectsClientContract); test('Returns undefined when no results found (undefined)', async () => { - const result = await getStats(getMockCallCluster(undefined as any), mockIndex); + const result = await getStats(getMockCallCluster(undefined as any)); + expect(result).toBeUndefined(); }); test('Returns undefined when no results found (0 results)', async () => { - const result = await getStats(getMockCallCluster([]), mockIndex); + const result = await getStats(getMockCallCluster([])); expect(result).toBeUndefined(); }); test('Summarizes visualizations response data', async () => { - const result = await getStats(getMockCallCluster(defaultMockSavedObjects), mockIndex); + const result = await getStats(getMockCallCluster(defaultMockSavedObjects)); expect(result).toMatchObject({ shell_beads: { @@ -181,7 +152,7 @@ describe('Visualizations usage collector', () => { }, }; - const result = await getStats(getMockCallCluster(enlargedMockSavedObjects), mockIndex); + const result = await getStats(getMockCallCluster(enlargedMockSavedObjects)); expect(result).toMatchObject(expectedStats); }); diff --git a/src/plugins/visualizations/server/usage_collector/get_usage_collector.ts b/src/plugins/visualizations/server/usage_collector/get_usage_collector.ts index 2cd715b7b02c8..9db76d815688d 100644 --- a/src/plugins/visualizations/server/usage_collector/get_usage_collector.ts +++ b/src/plugins/visualizations/server/usage_collector/get_usage_collector.ts @@ -6,12 +6,11 @@ * Side Public License, v 1. */ -import { countBy, get, groupBy, mapValues, max, min, values } from 'lodash'; -import { ElasticsearchClient } from 'kibana/server'; -import type { estypes } from '@elastic/elasticsearch'; - +import { countBy, groupBy, mapValues, max, min, values } from 'lodash'; import { getPastDays } from './get_past_days'; -type ESResponse = estypes.SearchResponse<{ visualization: { visState: string } }>; + +import type { SavedObjectsClientContract, SavedObjectsFindResult } from '../../../../core/server'; +import type { SavedVisState } from '../../../visualizations/common'; interface VisSummary { type: string; @@ -35,61 +34,52 @@ export interface VisualizationUsage { * Parse the response data into telemetry payload */ export async function getStats( - esClient: ElasticsearchClient, - index: string + soClient: SavedObjectsClientContract ): Promise { - const searchParams = { - size: 10000, // elasticsearch index.max_result_window default value - index, - ignoreUnavailable: true, - filterPath: [ - 'hits.hits._id', - 'hits.hits._source.visualization', - 'hits.hits._source.updated_at', - ], - body: { - query: { - bool: { filter: { term: { type: 'visualization' } } }, - }, - }, - }; - const { body: esResponse } = await esClient.search(searchParams); - const size = get(esResponse, 'hits.hits.length', 0); - if (size < 1) { - return; - } - - // `map` to get the raw types - const visSummaries: VisSummary[] = esResponse.hits.hits.map((hit) => { - const spacePhrases = hit._id.toString().split(':'); - const lastUpdated: string = get(hit, '_source.updated_at'); - const space = spacePhrases.length === 3 ? spacePhrases[0] : 'default'; // if in a custom space, the format of a saved object ID is space:type:id - const visualization = get(hit, '_source.visualization', { visState: '{}' }); - const visState: { type?: string } = JSON.parse(visualization.visState); - return { - type: visState.type || '_na_', - space, - past_days: getPastDays(lastUpdated), - }; + const finder = await soClient.createPointInTimeFinder({ + type: 'visualization', + perPage: 1000, }); - // organize stats per type - const visTypes = groupBy(visSummaries, 'type'); + const visSummaries: VisSummary[] = []; - // get the final result - return mapValues(visTypes, (curr) => { - const total = curr.length; - const spacesBreakdown = countBy(curr, 'space'); - const spaceCounts: number[] = values(spacesBreakdown); + for await (const response of finder.find()) { + (response.saved_objects || []).forEach((so: SavedObjectsFindResult) => { + if (so.attributes?.visState) { + const visState: SavedVisState = JSON.parse(so.attributes.visState); + const spacePhrases = so.id.toString().split(':'); + // if in a custom space, the format of a saved object ID is space:type:id + const space = spacePhrases.length === 3 ? spacePhrases[0] : 'default'; - return { - total, - spaces_min: min(spaceCounts), - spaces_max: max(spaceCounts), - spaces_avg: total / spaceCounts.length, - saved_7_days_total: curr.filter((c) => c.past_days <= 7).length, - saved_30_days_total: curr.filter((c) => c.past_days <= 30).length, - saved_90_days_total: curr.filter((c) => c.past_days <= 90).length, - }; - }); + visSummaries.push({ + type: visState.type || '_na_', + space, + past_days: getPastDays(so.updated_at!), + }); + } + }); + } + await finder.close(); + + if (visSummaries.length) { + // organize stats per type + const visTypes = groupBy(visSummaries, 'type'); + + // get the final result + return mapValues(visTypes, (curr) => { + const total = curr.length; + const spacesBreakdown = countBy(curr, 'space'); + const spaceCounts: number[] = values(spacesBreakdown); + + return { + total, + spaces_min: min(spaceCounts), + spaces_max: max(spaceCounts), + spaces_avg: total / spaceCounts.length, + saved_7_days_total: curr.filter((c) => c.past_days <= 7).length, + saved_30_days_total: curr.filter((c) => c.past_days <= 30).length, + saved_90_days_total: curr.filter((c) => c.past_days <= 90).length, + }; + }); + } } diff --git a/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.test.ts b/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.test.ts index a3617631f734b..e8f9df8516632 100644 --- a/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.test.ts +++ b/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.test.ts @@ -5,28 +5,24 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ - -import { of } from 'rxjs'; +import { + createUsageCollectionSetupMock, + createCollectorFetchContextMock, +} from '../../../usage_collection/server/mocks'; 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 { registerVisualizationsCollector } from './register_visualizations_collector'; describe('registerVisualizationsCollector', () => { - const mockIndex = 'mock_index'; - const mockConfig = of({ kibana: { index: mockIndex } }); - - it('makes a usage collector and registers it`', () => { + test('makes a usage collector and registers it`', () => { const mockCollectorSet = createUsageCollectionSetupMock(); - registerVisualizationsCollector(mockCollectorSet, mockConfig); + registerVisualizationsCollector(mockCollectorSet); expect(mockCollectorSet.makeUsageCollector).toBeCalledTimes(1); expect(mockCollectorSet.registerCollector).toBeCalledTimes(1); }); - it('makeUsageCollector configs fit the shape', () => { + test('makeUsageCollector configs fit the shape', () => { const mockCollectorSet = createUsageCollectionSetupMock(); - registerVisualizationsCollector(mockCollectorSet, mockConfig); + registerVisualizationsCollector(mockCollectorSet); expect(mockCollectorSet.makeUsageCollector).toHaveBeenCalledWith({ type: 'visualization_types', isReady: expect.any(Function), @@ -37,21 +33,21 @@ describe('registerVisualizationsCollector', () => { expect(usageCollectorConfig.isReady()).toBe(true); }); - it('makeUsageCollector config.isReady returns true', () => { + test('makeUsageCollector config.isReady returns true', () => { const mockCollectorSet = createUsageCollectionSetupMock(); - registerVisualizationsCollector(mockCollectorSet, mockConfig); + registerVisualizationsCollector(mockCollectorSet); const usageCollectorConfig = mockCollectorSet.makeUsageCollector.mock.calls[0][0]; expect(usageCollectorConfig.isReady()).toBe(true); }); - it('makeUsageCollector config.fetch calls getStats', async () => { + test('makeUsageCollector config.fetch calls getStats', async () => { const mockCollectorSet = createUsageCollectionSetupMock(); - registerVisualizationsCollector(mockCollectorSet, mockConfig); + registerVisualizationsCollector(mockCollectorSet); const usageCollector = mockCollectorSet.makeUsageCollector.mock.results[0].value; const mockCollectorFetchContext = createCollectorFetchContextMock(); const fetchResult = await usageCollector.fetch(mockCollectorFetchContext); expect(mockGetStats).toBeCalledTimes(1); - expect(mockGetStats).toBeCalledWith(mockCollectorFetchContext.esClient, mockIndex); + expect(mockGetStats).toBeCalledWith(mockCollectorFetchContext.soClient); expect(fetchResult).toBe(mockStats); }); }); diff --git a/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.ts b/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.ts index 1836af66233bd..b7ca4268c9a89 100644 --- a/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.ts +++ b/src/plugins/visualizations/server/usage_collector/register_visualizations_collector.ts @@ -6,16 +6,10 @@ * Side Public License, v 1. */ -import { Observable } from 'rxjs'; -import { first } from 'rxjs/operators'; -import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; - import { getStats, VisualizationUsage } from './get_usage_collector'; +import type { UsageCollectionSetup } from '../../../usage_collection/server'; -export function registerVisualizationsCollector( - collectorSet: UsageCollectionSetup, - config: Observable<{ kibana: { index: string } }> -) { +export function registerVisualizationsCollector(collectorSet: UsageCollectionSetup) { const collector = collectorSet.makeUsageCollector({ type: 'visualization_types', isReady: () => true, @@ -30,10 +24,7 @@ export function registerVisualizationsCollector( saved_90_days_total: { type: 'long' }, }, }, - fetch: async ({ esClient }) => { - const index = (await config.pipe(first()).toPromise()).kibana.index; - return await getStats(esClient, index); - }, + fetch: async ({ soClient }) => await getStats(soClient), }); collectorSet.registerCollector(collector); }