From 1c1e45b14f3938c036cfd26c424a0b3837ed5885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Wed, 11 Mar 2020 12:55:31 +0000 Subject: [PATCH 1/2] UI Metrics use findAll to retrieve all Saved Objects --- .../telemetry_application_usage_collector.ts | 24 +----- .../server/collectors/find_all.test.ts | 55 ++++++++++++ .../telemetry/server/collectors/find_all.ts | 41 +++++++++ .../server/collectors/ui_metric/index.test.ts | 86 +++++++++++++++++++ .../telemetry_ui_metric_collector.ts | 37 ++++---- .../core_plugins/telemetry/server/plugin.ts | 2 +- 6 files changed, 207 insertions(+), 38 deletions(-) create mode 100644 src/legacy/core_plugins/telemetry/server/collectors/find_all.test.ts create mode 100644 src/legacy/core_plugins/telemetry/server/collectors/find_all.ts create mode 100644 src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts diff --git a/src/legacy/core_plugins/telemetry/server/collectors/application_usage/telemetry_application_usage_collector.ts b/src/legacy/core_plugins/telemetry/server/collectors/application_usage/telemetry_application_usage_collector.ts index 5047ebc4b0454..4741c86649b89 100644 --- a/src/legacy/core_plugins/telemetry/server/collectors/application_usage/telemetry_application_usage_collector.ts +++ b/src/legacy/core_plugins/telemetry/server/collectors/application_usage/telemetry_application_usage_collector.ts @@ -20,12 +20,8 @@ import moment from 'moment'; import { APPLICATION_USAGE_TYPE } from '../../../common/constants'; import { UsageCollectionSetup } from '../../../../../../plugins/usage_collection/server'; -import { - ISavedObjectsRepository, - SavedObjectAttributes, - SavedObjectsFindOptions, - SavedObject, -} from '../../../../../../core/server'; +import { ISavedObjectsRepository, SavedObjectAttributes } from '../../../../../../core/server'; +import { findAll } from '../find_all'; /** * Roll indices every 24h @@ -61,22 +57,6 @@ interface ApplicationUsageTelemetryReport { }; } -async function findAll( - savedObjectsClient: ISavedObjectsRepository, - opts: SavedObjectsFindOptions -): Promise>> { - const { page = 1, perPage = 100, ...options } = opts; - const { saved_objects: savedObjects, total } = await savedObjectsClient.find({ - ...options, - page, - perPage, - }); - if (page * perPage >= total) { - return savedObjects; - } - return [...savedObjects, ...(await findAll(savedObjectsClient, { ...opts, page: page + 1 }))]; -} - export function registerApplicationUsageCollector( usageCollection: UsageCollectionSetup, getSavedObjectsClient: () => ISavedObjectsRepository | undefined diff --git a/src/legacy/core_plugins/telemetry/server/collectors/find_all.test.ts b/src/legacy/core_plugins/telemetry/server/collectors/find_all.test.ts new file mode 100644 index 0000000000000..012cda395bc6c --- /dev/null +++ b/src/legacy/core_plugins/telemetry/server/collectors/find_all.test.ts @@ -0,0 +1,55 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { savedObjectsRepositoryMock } from '../../../../../core/server/mocks'; + +import { findAll } from './find_all'; + +describe('telemetry_application_usage', () => { + test('when savedObjectClient is initialised, return something', async () => { + const savedObjectClient = savedObjectsRepositoryMock.create(); + savedObjectClient.find.mockImplementation( + async () => + ({ + saved_objects: [], + total: 0, + } as any) + ); + + expect(await findAll(savedObjectClient, { type: 'test-type' })).toStrictEqual([]); + }); + + test('paging in findAll works', async () => { + const savedObjectClient = savedObjectsRepositoryMock.create(); + let total = 201; + const doc = { id: 'test-id', attributes: { test: 1 } }; + savedObjectClient.find.mockImplementation(async opts => { + if ((opts.page || 1) > 2) { + return { saved_objects: [], total } as any; + } + const savedObjects = new Array(opts.perPage).fill(doc); + total = savedObjects.length * 2 + 1; + return { saved_objects: savedObjects, total }; + }); + + expect(await findAll(savedObjectClient, { type: 'test-type' })).toStrictEqual( + new Array(total - 1).fill(doc) + ); + }); +}); diff --git a/src/legacy/core_plugins/telemetry/server/collectors/find_all.ts b/src/legacy/core_plugins/telemetry/server/collectors/find_all.ts new file mode 100644 index 0000000000000..e6363551eba9c --- /dev/null +++ b/src/legacy/core_plugins/telemetry/server/collectors/find_all.ts @@ -0,0 +1,41 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + SavedObjectAttributes, + ISavedObjectsRepository, + SavedObjectsFindOptions, + SavedObject, +} from 'kibana/server'; + +export async function findAll( + savedObjectsClient: ISavedObjectsRepository, + opts: SavedObjectsFindOptions +): Promise>> { + const { page = 1, perPage = 100, ...options } = opts; + const { saved_objects: savedObjects, total } = await savedObjectsClient.find({ + ...options, + page, + perPage, + }); + if (page * perPage >= total) { + return savedObjects; + } + return [...savedObjects, ...(await findAll(savedObjectsClient, { ...opts, page: page + 1 }))]; +} diff --git a/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts new file mode 100644 index 0000000000000..801550b9972bd --- /dev/null +++ b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts @@ -0,0 +1,86 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { UsageCollectionSetup } from '../../../../../../plugins/usage_collection/server'; +import { savedObjectsRepositoryMock } from '../../../../../../core/server/mocks'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { CollectorOptions } from '../../../../../../plugins/usage_collection/server/collector/collector'; + +import { registerUiMetricUsageCollector } from './'; + +describe('telemetry_application_usage', () => { + let collector: CollectorOptions; + + const usageCollectionMock: jest.Mocked = { + makeUsageCollector: jest.fn().mockImplementation(config => (collector = config)), + registerCollector: jest.fn(), + } as any; + + const getUsageCollector = jest.fn(); + const callCluster = jest.fn(); + + beforeAll(() => registerUiMetricUsageCollector(usageCollectionMock, getUsageCollector)); + + test('registered collector is set', () => { + expect(collector).not.toBeUndefined(); + }); + + test('if no savedObjectClient initialised, return undefined', async () => { + expect(await collector.fetch(callCluster)).toBeUndefined(); + }); + + test('when savedObjectClient is initialised, return something', async () => { + const savedObjectClient = savedObjectsRepositoryMock.create(); + savedObjectClient.find.mockImplementation( + async () => + ({ + saved_objects: [], + total: 0, + } as any) + ); + getUsageCollector.mockImplementation(() => savedObjectClient); + + expect(await collector.fetch(callCluster)).toStrictEqual({}); + expect(savedObjectClient.bulkCreate).not.toHaveBeenCalled(); + }); + + test('results grouped by appName', async () => { + const savedObjectClient = savedObjectsRepositoryMock.create(); + savedObjectClient.find.mockImplementation(async () => { + return { + saved_objects: [ + { id: 'testAppName:testKeyName1', attributes: { count: 3 } }, + { id: 'testAppName:testKeyName2', attributes: { count: 5 } }, + { id: 'testAppName2:testKeyName3', attributes: { count: 1 } }, + ], + total: 3, + } as any; + }); + + getUsageCollector.mockImplementation(() => savedObjectClient); + + expect(await collector.fetch(callCluster)).toStrictEqual({ + testAppName: [ + { key: 'testKeyName1', value: 3 }, + { key: 'testKeyName2', value: 5 }, + ], + testAppName2: [{ key: 'testKeyName3', value: 1 }], + }); + }); +}); diff --git a/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/telemetry_ui_metric_collector.ts b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/telemetry_ui_metric_collector.ts index 73157abce8629..a7b6850b0b20a 100644 --- a/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/telemetry_ui_metric_collector.ts +++ b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/telemetry_ui_metric_collector.ts @@ -17,24 +17,33 @@ * under the License. */ +import { ISavedObjectsRepository, SavedObjectAttributes } from 'kibana/server'; import { UI_METRIC_USAGE_TYPE } from '../../../common/constants'; import { UsageCollectionSetup } from '../../../../../../plugins/usage_collection/server'; +import { findAll } from '../find_all'; -export function registerUiMetricUsageCollector(usageCollection: UsageCollectionSetup, server: any) { +interface UIMetricsSavedObjects extends SavedObjectAttributes { + count: number; +} + +export function registerUiMetricUsageCollector( + usageCollection: UsageCollectionSetup, + getSavedObjectsClient: () => ISavedObjectsRepository | undefined +) { const collector = usageCollection.makeUsageCollector({ type: UI_METRIC_USAGE_TYPE, fetch: async () => { - const { SavedObjectsClient, getSavedObjectsRepository } = server.savedObjects; - const { callWithInternalUser } = server.plugins.elasticsearch.getCluster('admin'); - const internalRepository = getSavedObjectsRepository(callWithInternalUser); - const savedObjectsClient = new SavedObjectsClient(internalRepository); + const savedObjectsClient = getSavedObjectsClient(); + if (typeof savedObjectsClient === 'undefined') { + return; + } - const { saved_objects: rawUiMetrics } = await savedObjectsClient.find({ + const rawUiMetrics = await findAll(savedObjectsClient, { type: 'ui-metric', fields: ['count'], }); - const uiMetricsByAppName = rawUiMetrics.reduce((accum: any, rawUiMetric: any) => { + const uiMetricsByAppName = rawUiMetrics.reduce((accum, rawUiMetric) => { const { id, attributes: { count }, @@ -42,18 +51,16 @@ export function registerUiMetricUsageCollector(usageCollection: UsageCollectionS const [appName, metricType] = id.split(':'); - if (!accum[appName]) { - accum[appName] = []; - } - const pair = { key: metricType, value: count }; - accum[appName].push(pair); - return accum; - }, {}); + return { + ...accum, + [appName]: [...(accum[appName] || []), pair], + }; + }, {} as Record>); return uiMetricsByAppName; }, - isReady: () => true, + isReady: () => typeof getSavedObjectsClient() !== 'undefined', }); usageCollection.registerCollector(collector); diff --git a/src/legacy/core_plugins/telemetry/server/plugin.ts b/src/legacy/core_plugins/telemetry/server/plugin.ts index d859c0cfd4678..0b9f0526988c8 100644 --- a/src/legacy/core_plugins/telemetry/server/plugin.ts +++ b/src/legacy/core_plugins/telemetry/server/plugin.ts @@ -59,7 +59,7 @@ export class TelemetryPlugin { registerTelemetryPluginUsageCollector(usageCollection, server); registerLocalizationUsageCollector(usageCollection, server); registerTelemetryUsageCollector(usageCollection, server); - registerUiMetricUsageCollector(usageCollection, server); + registerUiMetricUsageCollector(usageCollection, getSavedObjectsClient); registerManagementUsageCollector(usageCollection, server); registerApplicationUsageCollector(usageCollection, getSavedObjectsClient); } From 25f753c5ff89c832cb8bc8f8c5845e35aa22ca82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Thu, 12 Mar 2020 09:42:53 +0000 Subject: [PATCH 2/2] Rename test description Co-Authored-By: Christiane (Tina) Heiligers --- .../telemetry/server/collectors/ui_metric/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts index 801550b9972bd..ddb58a7d09bbd 100644 --- a/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts +++ b/src/legacy/core_plugins/telemetry/server/collectors/ui_metric/index.test.ts @@ -24,7 +24,7 @@ import { CollectorOptions } from '../../../../../../plugins/usage_collection/ser import { registerUiMetricUsageCollector } from './'; -describe('telemetry_application_usage', () => { +describe('telemetry_ui_metric', () => { let collector: CollectorOptions; const usageCollectionMock: jest.Mocked = {