Skip to content

Commit

Permalink
[Data Table] Expensive queries are causing unnecessary load and delay…
Browse files Browse the repository at this point in the history
…s 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>
  • Loading branch information
alexwizp and kibanamachine authored Aug 30, 2021
1 parent 1932540 commit 265fed5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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,
Expand Down
56 changes: 37 additions & 19 deletions src/plugins/vis_type_table/server/usage_collector/get_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -44,17 +46,14 @@ export interface VisTypeTableUsage {
export async function getStats(
soClient: SavedObjectsClientContract | ISavedObjectsRepository
): Promise<VisTypeTableUsage | undefined> {
const visualizations = await soClient.find<VisualizationSavedObjectAttributes>({
const finder = await soClient.createPointInTimeFinder({
type: 'visualization',
perPage: 10000,
perPage: 1000,
namespaces: ['*'],
});

const tableVisualizations = visualizations.saved_objects
.map<SavedVisState<TableVisParams>>(({ 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,
Expand All @@ -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<any>) => {
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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<VisTypeTableUsage | undefined>({
Expand Down

0 comments on commit 265fed5

Please sign in to comment.