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

[TSVB] Expensive queries are causing unnecessary load and delays on Elasticsearch #98914

Merged
merged 11 commits into from
Aug 30, 2021
2 changes: 1 addition & 1 deletion src/plugins/vis_type_timeseries/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class VisTypeTimeseriesPlugin implements Plugin<VisTypeTimeseriesSetup> {
fieldsRoutes(router, framework);

if (plugins.usageCollection) {
registerTimeseriesUsageCollector(plugins.usageCollection, globalConfig$);
registerTimeseriesUsageCollector(plugins.usageCollection);
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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();
});
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' &&
Expand All @@ -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);
alexwizp marked this conversation as resolved.
Show resolved Hide resolved

return timeseriesUsage.timeseries_use_last_value_mode_total ? timeseriesUsage : undefined;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down