Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Data Table] Expensive queries are causing unnecessary load and delays on Elasticsearch #98903

Merged
merged 13 commits into from
Aug 30, 2021
Merged
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