From 7009f2ba72425eef9330dd6020066f0cfb1facca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Mon, 17 Apr 2023 14:49:02 +0200 Subject: [PATCH 1/2] [`getSavedObjectsCount`] Use `soClient` instead of `.kibana` searches --- .../src/lib/aggregations/validation.test.ts | 17 +++ .../src/lib/aggregations/validation.ts | 8 ++ .../get_saved_object_counts.test.ts | 116 ++++++------------ .../get_saved_object_counts.ts | 44 +++---- .../kibana_usage_collector.test.ts | 12 +- .../kibana_usage_collector.ts | 11 +- .../saved_objects_count_collector.test.ts | 7 +- .../saved_objects_count_collector.ts | 5 +- .../kibana_usage_collection/server/plugin.ts | 2 +- 9 files changed, 94 insertions(+), 128 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.test.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.test.ts index 0120ecf75c7971..0b5d750e57040e 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.test.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.test.ts @@ -522,4 +522,21 @@ describe('validateAndConvertAggregations', () => { '"[aggName.cardinality.field] Invalid attribute path: alert.alert.actions.group"' ); }); + + it('allows aggregations for root fields', () => { + const aggregations: AggsMap = { + types: { + terms: { + field: 'type', + }, + }, + }; + expect(validateAndConvertAggregations(['foo'], aggregations, mockMappings)).toEqual({ + types: { + terms: { + field: 'type', + }, + }, + }); + }); }); diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.ts index b3a6bbae5e9561..c673f73d8d844a 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/aggregations/validation.ts @@ -18,6 +18,7 @@ import { rewriteRootLevelAttribute, } from './validation_utils'; import { aggregationSchemas } from './aggs_types'; +import { getRootFields } from '../included_fields'; const aggregationKeys = ['aggs', 'aggregations']; @@ -226,6 +227,10 @@ const isAttributeValue = (fieldName: string, fieldValue: unknown): boolean => { return attributeFields.includes(fieldName) && typeof fieldValue === 'string'; }; +const isRootField = (fieldName: string): boolean => { + return getRootFields().includes(fieldName); +}; + const validateAndRewriteAttributePath = ( attributePath: string, { allowedTypes, indexMapping, currentPath }: ValidationContext @@ -236,5 +241,8 @@ const validateAndRewriteAttributePath = ( if (isObjectTypeAttribute(attributePath, indexMapping, allowedTypes)) { return rewriteObjectTypeAttribute(attributePath); } + if (isRootField(attributePath)) { + return attributePath; + } throw new Error(`[${currentPath.join('.')}] Invalid attribute path: ${attributePath}`); }; diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.test.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.test.ts index 173453e9e24207..f83200f56f27ce 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.test.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.test.ts @@ -6,48 +6,41 @@ * Side Public License, v 1. */ -import { elasticsearchServiceMock } from '@kbn/core/server/mocks'; +import type { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; +import { createCollectorFetchContextMock } from '@kbn/usage-collection-plugin/server/mocks'; import { getSavedObjectsCounts } from './get_saved_object_counts'; -function mockGetSavedObjectsCounts(params: TBody) { - const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; - // @ts-expect-error arbitrary type - esClient.search.mockResponse(params); - return esClient; -} +const soEmptyResponse = { total: 0, saved_objects: [], per_page: 0, page: 1 }; describe('getSavedObjectsCounts', () => { + const fetchContextMock = createCollectorFetchContextMock(); + const soClient = fetchContextMock.soClient as jest.Mocked; + + beforeEach(() => { + soClient.find.mockReset(); + }); + test('should not fail if no body returned', async () => { - const esClient = mockGetSavedObjectsCounts({}); + soClient.find.mockResolvedValueOnce(soEmptyResponse); - const results = await getSavedObjectsCounts(esClient, '.kibana', ['type-a']); + const results = await getSavedObjectsCounts(soClient, ['type-a']); // Make sure ES.search is triggered (we'll test the actual params in other specific tests) - expect(esClient.search).toHaveBeenCalledTimes(1); + expect(soClient.find).toHaveBeenCalledTimes(1); expect(results).toStrictEqual({ total: 0, per_type: [], non_expected_types: [], others: 0 }); }); test('should match all and request the `missing` bucket (size + 1) when `exclusive === false`', async () => { - const esClient = mockGetSavedObjectsCounts({}); - await getSavedObjectsCounts(esClient, '.kibana', ['type-a', 'type_2']); - expect(esClient.search).toHaveBeenCalledWith({ - index: '.kibana', - ignore_unavailable: true, - filter_path: [ - 'aggregations.types.buckets', - 'aggregations.types.sum_other_doc_count', - 'hits.total', - ], - body: { - size: 0, - track_total_hits: true, - query: { match_all: {} }, - aggs: { - types: { - terms: { - field: 'type', - size: 3, - missing: 'missing_so_type', - }, + soClient.find.mockResolvedValueOnce(soEmptyResponse); + await getSavedObjectsCounts(soClient, ['type-a', 'type_2']); + expect(soClient.find).toHaveBeenCalledWith({ + type: ['type-a', 'type_2'], + perPage: 0, + aggs: { + types: { + terms: { + field: 'type', + size: 3, + missing: 'missing_so_type', }, }, }, @@ -55,22 +48,12 @@ describe('getSavedObjectsCounts', () => { }); test('should apply the terms query and aggregation with the size matching the length of the list when `exclusive === true`', async () => { - const esClient = mockGetSavedObjectsCounts({}); - await getSavedObjectsCounts(esClient, '.kibana', ['type_one', 'type_two'], true); - expect(esClient.search).toHaveBeenCalledWith({ - index: '.kibana', - ignore_unavailable: true, - filter_path: [ - 'aggregations.types.buckets', - 'aggregations.types.sum_other_doc_count', - 'hits.total', - ], - body: { - size: 0, - track_total_hits: true, - query: { terms: { type: ['type_one', 'type_two'] } }, - aggs: { types: { terms: { field: 'type', size: 2 } } }, - }, + soClient.find.mockResolvedValueOnce(soEmptyResponse); + await getSavedObjectsCounts(soClient, ['type_one', 'type_two'], true); + expect(soClient.find).toHaveBeenCalledWith({ + type: ['type_one', 'type_two'], + perPage: 0, + aggs: { types: { terms: { field: 'type', size: 2 } } }, }); }); @@ -80,39 +63,13 @@ describe('getSavedObjectsCounts', () => { { key: 'type-two', doc_count: 2 }, ]; - const esClient = mockGetSavedObjectsCounts({ - hits: { total: { value: 13 } }, - aggregations: { types: { buckets, sum_other_doc_count: 10 } }, - }); - - const results = await getSavedObjectsCounts(esClient, '.kibana', [ - 'type_one', - 'type-two', - 'type-3', - ]); - expect(results).toStrictEqual({ + soClient.find.mockResolvedValueOnce({ + ...soEmptyResponse, total: 13, - per_type: [ - { key: 'type_one', doc_count: 1 }, - { key: 'type-two', doc_count: 2 }, - ], - non_expected_types: [], - others: 10, - }); - }); - - test('supports ES returning total as a number (just in case)', async () => { - const buckets = [ - { key: 'type_one', doc_count: 1 }, - { key: 'type-two', doc_count: 2 }, - ]; - - const esClient = mockGetSavedObjectsCounts({ - hits: { total: 13 }, aggregations: { types: { buckets, sum_other_doc_count: 10 } }, }); - const results = await getSavedObjectsCounts(esClient, '.kibana', ['type_one', 'type-two']); + const results = await getSavedObjectsCounts(soClient, ['type_one', 'type-two', 'type-3']); expect(results).toStrictEqual({ total: 13, per_type: [ @@ -132,12 +89,13 @@ describe('getSavedObjectsCounts', () => { { key: 'type-four', doc_count: 2 }, ]; - const esClient = mockGetSavedObjectsCounts({ - hits: { total: { value: 13 } }, + soClient.find.mockResolvedValueOnce({ + ...soEmptyResponse, + total: 13, aggregations: { types: { buckets, sum_other_doc_count: 6 } }, }); - const results = await getSavedObjectsCounts(esClient, '.kibana', ['type_one', 'type-two']); + const results = await getSavedObjectsCounts(soClient, ['type_one', 'type-two']); expect(results).toStrictEqual({ total: 13, per_type: [ diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.ts index 812933eeb3e699..cdca68e5d60065 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/get_saved_object_counts/get_saved_object_counts.ts @@ -7,7 +7,7 @@ */ import { estypes } from '@elastic/elasticsearch'; -import { ElasticsearchClient } from '@kbn/core/server'; +import type { SavedObjectsClientContract } from '@kbn/core/server'; const MISSING_TYPE_KEY = 'missing_so_type'; @@ -39,40 +39,28 @@ export interface SavedObjectsCounts { * It also returns a break-down of the document count for all the built-in SOs in Kibana (or the types specified in `soTypes`). * Finally, it completes the information with an `others` counter, that indicates the number of documents that do not match the SO type breakdown. * - * @param esClient The {@link ElasticsearchClient} to use when performing the aggregation. - * @param kibanaIndex The index where SOs are stored. Typically '.kibana'. + * @param soClient The {@link SavedObjectsClientContract} to use when performing the aggregation. * @param soTypes The SO types we want to know about. * @param exclusive If `true`, the results will only contain the breakdown for the specified `soTypes`. Otherwise, it'll also return `missing` and `others` bucket. * @returns {@link SavedObjectsCounts} */ export async function getSavedObjectsCounts( - esClient: ElasticsearchClient, - kibanaIndex: string, // Typically '.kibana'. We might need a way to obtain it from the SavedObjects client (or the SavedObjects client to provide a way to run aggregations?) + soClient: SavedObjectsClientContract, soTypes: string[], exclusive: boolean = false ): Promise { - const body = await esClient.search({ - index: kibanaIndex, - ignore_unavailable: true, - filter_path: [ - 'aggregations.types.buckets', - 'aggregations.types.sum_other_doc_count', - 'hits.total', - ], - body: { - size: 0, - track_total_hits: true, - query: exclusive ? { terms: { type: soTypes } } : { match_all: {} }, - aggs: { - types: { - terms: { - field: 'type', - // If `exclusive == true`, we only care about the strict length of the provided SO types. - // Otherwise, we want to account for the `missing` bucket (size and missing option). - ...(exclusive - ? { size: soTypes.length } - : { missing: MISSING_TYPE_KEY, size: soTypes.length + 1 }), - }, + const body = await soClient.find({ + type: soTypes, + perPage: 0, + aggs: { + types: { + terms: { + field: 'type', + // If `exclusive == true`, we only care about the strict length of the provided SO types. + // Otherwise, we want to account for the `missing` bucket (size and missing option). + ...(exclusive + ? { size: soTypes.length } + : { missing: MISSING_TYPE_KEY, size: soTypes.length + 1 }), }, }, }, @@ -93,7 +81,7 @@ export async function getSavedObjectsCounts( }); return { - total: (typeof body.hits?.total === 'number' ? body.hits?.total : body.hits?.total?.value) ?? 0, + total: body.total, per_type: perType, non_expected_types: nonExpectedTypes, others: body.aggregations?.types?.sum_other_doc_count ?? 0, diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.test.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.test.ts index 65003ff99c22ae..cdf2ca35d6ecc0 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.test.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.test.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { loggingSystemMock, elasticsearchServiceMock } from '@kbn/core/server/mocks'; +import { loggingSystemMock } from '@kbn/core/server/mocks'; import { Collector, createCollectorFetchContextMock, @@ -52,7 +52,8 @@ describe('kibana_usage', () => { }); describe('getKibanaSavedObjectCounts', () => { - const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + const fetchContextMock = createCollectorFetchContextMock(); + const soClient = fetchContextMock.soClient; test('Get all the saved objects equal to 0 because no results were found', async () => { getSavedObjectsCountsMock.mockResolvedValueOnce({ @@ -61,7 +62,7 @@ describe('getKibanaSavedObjectCounts', () => { non_expected_types: [], others: 0, }); - const results = await getKibanaSavedObjectCounts(esClient, '.kibana'); + const results = await getKibanaSavedObjectCounts(soClient); expect(results).toStrictEqual({ dashboard: { total: 0 }, visualization: { total: 0 }, @@ -83,7 +84,7 @@ describe('getKibanaSavedObjectCounts', () => { others: 0, }); - const results = await getKibanaSavedObjectCounts(esClient, '.kibana'); + const results = await getKibanaSavedObjectCounts(soClient); expect(results).toStrictEqual({ dashboard: { total: 1 }, visualization: { total: 0 }, @@ -93,8 +94,7 @@ describe('getKibanaSavedObjectCounts', () => { }); expect(getSavedObjectsCountsMock).toHaveBeenCalledWith( - esClient, - '.kibana', + soClient, ['dashboard', 'visualization', 'search', 'index-pattern', 'graph-workspace'], true ); diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.ts index dbff86fb23ad7a..9a128888f05d53 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/kibana_usage_collector.ts @@ -6,9 +6,9 @@ * Side Public License, v 1. */ -import type { ElasticsearchClient } from '@kbn/core/server'; import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; import { snakeCase } from 'lodash'; +import { SavedObjectsClientContract } from '@kbn/core/server'; import { getSavedObjectsCounts } from './get_saved_object_counts'; interface KibanaSavedObjectCounts { @@ -26,10 +26,9 @@ interface KibanaUsage extends KibanaSavedObjectCounts { const TYPES = ['dashboard', 'visualization', 'search', 'index-pattern', 'graph-workspace']; export async function getKibanaSavedObjectCounts( - esClient: ElasticsearchClient, - kibanaIndex: string + soClient: SavedObjectsClientContract ): Promise { - const { per_type: buckets } = await getSavedObjectsCounts(esClient, kibanaIndex, TYPES, true); + const { per_type: buckets } = await getSavedObjectsCounts(soClient, TYPES, true); const allZeros = Object.fromEntries( TYPES.map((type) => [snakeCase(type), { total: 0 }]) @@ -80,10 +79,10 @@ export function registerKibanaUsageCollector( }, }, }, - async fetch({ esClient }) { + async fetch({ soClient }) { return { index: kibanaIndex, - ...(await getKibanaSavedObjectCounts(esClient, kibanaIndex)), + ...(await getKibanaSavedObjectCounts(soClient)), }; }, }) diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.test.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.test.ts index 25ac79f88e5239..53168be3c34b67 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.test.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.test.ts @@ -17,10 +17,8 @@ describe('saved_objects_count_collector', () => { const usageCollectionMock = createUsageCollectionSetupMock(); const fetchContextMock = createCollectorFetchContextMock(); - const kibanaIndex = '.kibana-tests'; - beforeAll(() => - registerSavedObjectsCountUsageCollector(usageCollectionMock, kibanaIndex, () => + registerSavedObjectsCountUsageCollector(usageCollectionMock, () => Promise.resolve(['type_one', 'type_two', 'type-three', 'type-four']) ) ); @@ -81,8 +79,7 @@ describe('saved_objects_count_collector', () => { }); expect(getSavedObjectsCountsMock).toHaveBeenCalledWith( - fetchContextMock.esClient, - kibanaIndex, + fetchContextMock.soClient, ['type_one', 'type_two', 'type-three', 'type-four'], false ); diff --git a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.ts b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.ts index 376a036a0f24a9..49bfb7819389d1 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/saved_objects_counts/saved_objects_count_collector.ts @@ -23,7 +23,6 @@ interface SavedObjectsCountUsage { export function registerSavedObjectsCountUsageCollector( usageCollection: UsageCollectionSetup, - kibanaIndex: string, getAllSavedObjectTypes: () => Promise ) { usageCollection.registerCollector( @@ -68,14 +67,14 @@ export function registerSavedObjectsCountUsageCollector( }, }, }, - async fetch({ esClient }) { + async fetch({ soClient }) { const allRegisteredSOTypes = await getAllSavedObjectTypes(); const { total, per_type: buckets, non_expected_types: nonRegisteredTypes, others, - } = await getSavedObjectsCounts(esClient, kibanaIndex, allRegisteredSOTypes, false); + } = await getSavedObjectsCounts(soClient, allRegisteredSOTypes, false); return { total, by_type: buckets.map(({ key: type, doc_count: count }) => ({ type, count })), diff --git a/src/plugins/kibana_usage_collection/server/plugin.ts b/src/plugins/kibana_usage_collection/server/plugin.ts index 784ab3ef4af62f..8787d085c692e2 100644 --- a/src/plugins/kibana_usage_collection/server/plugin.ts +++ b/src/plugins/kibana_usage_collection/server/plugin.ts @@ -148,7 +148,7 @@ export class KibanaUsageCollectionPlugin implements Plugin { .getAllTypes() .map(({ name }) => name); }; - registerSavedObjectsCountUsageCollector(usageCollection, kibanaIndex, getAllSavedObjectTypes); + registerSavedObjectsCountUsageCollector(usageCollection, getAllSavedObjectTypes); registerManagementUsageCollector(usageCollection, getUiSettingsClient); registerUiMetricUsageCollector(usageCollection, registerType, getSavedObjectsClient); registerApplicationUsageCollector( From 00c3789e1aa91ef1c527ca525a518d4762b3d91f Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 17 Apr 2023 13:01:35 +0000 Subject: [PATCH 2/2] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- src/plugins/kibana_usage_collection/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/kibana_usage_collection/tsconfig.json b/src/plugins/kibana_usage_collection/tsconfig.json index 271032000d510d..d56cb860cd463a 100644 --- a/src/plugins/kibana_usage_collection/tsconfig.json +++ b/src/plugins/kibana_usage_collection/tsconfig.json @@ -18,6 +18,7 @@ "@kbn/logging", "@kbn/core-test-helpers-kbn-server", "@kbn/core-usage-data-server", + "@kbn/core-saved-objects-api-server", ], "exclude": [ "target/**/*",