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

[getSavedObjectsCount] Use soClient instead of .kibana searches #155035

Merged
merged 2 commits into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
rewriteRootLevelAttribute,
} from './validation_utils';
import { aggregationSchemas } from './aggs_types';
import { getRootFields } from '../included_fields';

const aggregationKeys = ['aggs', 'aggregations'];

Expand Down Expand Up @@ -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
Expand All @@ -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}`);
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,71 +6,54 @@
* 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<TBody>(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<SavedObjectsClientContract>;

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',
},
},
},
});
});

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 } } },
});
});

Expand All @@ -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: [
Expand All @@ -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: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<SavedObjectsCounts> {
const body = await esClient.search<void, { types: estypes.AggregationsStringTermsAggregate }>({
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<void, { types: estypes.AggregationsStringTermsAggregate }>({
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 }),
},
},
},
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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 },
Expand All @@ -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 },
Expand All @@ -93,8 +94,7 @@ describe('getKibanaSavedObjectCounts', () => {
});

expect(getSavedObjectsCountsMock).toHaveBeenCalledWith(
esClient,
'.kibana',
soClient,
['dashboard', 'visualization', 'search', 'index-pattern', 'graph-workspace'],
true
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<KibanaSavedObjectCounts> {
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 }])
Expand Down Expand Up @@ -80,10 +79,10 @@ export function registerKibanaUsageCollector(
},
},
},
async fetch({ esClient }) {
async fetch({ soClient }) {
return {
index: kibanaIndex,
...(await getKibanaSavedObjectCounts(esClient, kibanaIndex)),
...(await getKibanaSavedObjectCounts(soClient)),
};
},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
)
);
Expand Down Expand Up @@ -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
);
Expand Down
Loading