Skip to content

Commit

Permalink
[RAM] Rule List Tags Filtering with Infinite Scroll (#159246)
Browse files Browse the repository at this point in the history
## Summary
Resolves: #150364
Fixes: elastic/sdh-kibana#3806

Fixes a common complaint and bug where we limit the number of tags that
are displayed in the tags dropdown to 50. We now paginate these results
with a max tag of 10,000 (50 per page). It's not a true pagination
because Elasticsearch doesn't support filtering and paginating on
aggregation buckets (bucket selector doesn't work on terms). Since tags
are not nested properties or references, they can only be reached
through terms aggregation.

But at least we don't return 10000 tags from the API.

## How to test:
1. By default we show 50 tags per page, to make testing easier, you may
go to `x-pack/plugins/alerting/server/rules_client/methods/get_tags.ts`
and set `DEFAULT_TAGS_PER_PAGE` to 10 or something
2. Create some rules
3. Create some tags (spread amongst the rules would be even better)
4. Open the tag filter
5. Should be able to search, paginate, and filter rules by the tags

![ezgif
com-video-to-gif](https://user-images.githubusercontent.com/74562234/217985463-ee3ce86f-e614-4690-a48a-74a328ca9cff.gif)

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
JiaweiWu and kibanamachine committed Jun 14, 2023
1 parent 195216f commit 65b8a10
Show file tree
Hide file tree
Showing 22 changed files with 1,257 additions and 450 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const RuleTagFilterSandbox = ({ triggersActionsUi }: SandboxProps) => {
return (
<div style={{ flex: 1 }}>
{triggersActionsUi.getRuleTagFilter({
tags: ['tag1', 'tag2', 'tag3', 'tag4'],
canLoadRules: true,
selectedTags,
onChange: setSelectedTags,
})}
Expand Down
91 changes: 23 additions & 68 deletions x-pack/plugins/alerting/server/routes/get_rule_tags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,24 @@ import { mockHandlerArguments } from './_mock_handler_arguments';
import { rulesClientMock } from '../rules_client.mock';
import { getRuleTagsRoute } from './get_rule_tags';

import {} from '../../common/rule_tags_aggregation';

const rulesClient = rulesClientMock.create();

jest.mock('../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

jest.mock('../../common/rule_tags_aggregation', () => ({
...jest.requireActual('../../common/rule_tags_aggregation'),
formatRuleTagsAggregationResult: jest.fn(),
}));

beforeEach(() => {
jest.resetAllMocks();
rulesClient.getTags.mockResolvedValueOnce({
data: ['a', 'b', 'c'],
page: 1,
perPage: 10,
total: 3,
});
});

const { formatRuleTagsAggregationResult } = jest.requireMock('../../common/rule_tags_aggregation');

describe('getRuleTagsRoute', () => {
it('aggregates rule tags with proper parameters', async () => {
it('gets rule tags with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

Expand All @@ -42,19 +39,13 @@ describe('getRuleTagsRoute', () => {

expect(config.path).toMatchInlineSnapshot(`"/internal/alerting/rules/_tags"`);

const aggregateResult = { ruleTags: ['a', 'b', 'c'] };

formatRuleTagsAggregationResult.mockReturnValueOnce(aggregateResult);

const [context, req, res] = mockHandlerArguments(
{ rulesClient },
{
query: {
filter: 'test',
search: 'search text',
after: {
tags: 'c',
},
search: 'test',
per_page: 10,
page: 1,
},
},
['ok']
Expand All @@ -63,78 +54,42 @@ describe('getRuleTagsRoute', () => {
expect(await handler(context, req, res)).toMatchInlineSnapshot(`
Object {
"body": Object {
"rule_tags": Array [
"data": Array [
"a",
"b",
"c",
],
"page": 1,
"per_page": 10,
"total": 3,
},
}
`);
expect(rulesClient.aggregate).toHaveBeenCalledTimes(1);
expect(rulesClient.aggregate.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"aggs": Object {
"tags": Object {
"composite": Object {
"after": Object {
"tags": "c",
},
"size": 50,
"sources": Array [
Object {
"tags": Object {
"terms": Object {
"field": "alert.attributes.tags",
"order": "asc",
},
},
},
],
},
},
},
"options": Object {
"after": Object {
"tags": "c",
},
"defaultSearchOperator": "AND",
"filter": "test",
"search": "search text",
"searchFields": Array [
"tags",
],
},
},
]
`);
expect(res.ok).toHaveBeenCalledWith({
body: {
rule_tags: ['a', 'b', 'c'],
data: ['a', 'b', 'c'],
page: 1,
per_page: 10,
total: 3,
},
});
});

it('ensures the license allows aggregating rule tags', async () => {
it('ensures the license allows getting rule tags', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

getRuleTagsRoute(router, licenseState);

const [, handler] = router.get.mock.calls[0];

formatRuleTagsAggregationResult.mockReturnValueOnce({ ruleTags: ['a', 'b', 'c', 'd'] });

const [context, req, res] = mockHandlerArguments(
{ rulesClient },
{
query: {
filter: 'test',
search: 'search text',
after: {
tags: 'c',
},
search: 'test',
per_page: 10,
page: 1,
},
}
);
Expand All @@ -144,7 +99,7 @@ describe('getRuleTagsRoute', () => {
expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);
});

it('ensures the license check prevents aggregating rule tags', async () => {
it('ensures the license check prevents getting rule tags', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

Expand Down
50 changes: 13 additions & 37 deletions x-pack/plugins/alerting/server/routes/get_rule_tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,43 +7,29 @@

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import {
RuleTagsAggregationResult,
RuleTagsAggregationFormattedResult,
RuleTagsAggregationOptions,
getRuleTagsAggregation,
formatRuleTagsAggregationResult,
} from '../../common';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { ILicenseState } from '../lib';
import { RewriteResponseCase, RewriteRequestCase, verifyAccessAndContext } from './lib';
import {
DEFAULT_TAGS_PER_PAGE,
GetTagsParams,
GetTagsResult,
} from '../rules_client/methods/get_tags';

const querySchema = schema.object({
filter: schema.maybe(schema.string()),
page: schema.number({ defaultValue: 1, min: 1 }),
per_page: schema.maybe(schema.number({ defaultValue: DEFAULT_TAGS_PER_PAGE, min: 1 })),
search: schema.maybe(schema.string()),
after: schema.maybe(
schema.recordOf(
schema.string(),
schema.nullable(schema.oneOf([schema.string(), schema.number()]))
)
),
max_tags: schema.maybe(schema.number()),
});

const rewriteQueryReq: RewriteRequestCase<RuleTagsAggregationOptions> = ({
max_tags: maxTags,
...rest
}) => ({
const rewriteQueryReq: RewriteRequestCase<GetTagsParams> = ({ per_page: perPage, ...rest }) => ({
...rest,
...(maxTags ? { maxTags } : {}),
perPage,
});

const rewriteBodyRes: RewriteResponseCase<RuleTagsAggregationFormattedResult> = ({
ruleTags,
...rest
}) => ({
const rewriteBodyRes: RewriteResponseCase<GetTagsResult> = ({ perPage, ...rest }) => ({
...rest,
rule_tags: ruleTags,
per_page: perPage,
});

export const getRuleTagsRoute = (
Expand All @@ -62,20 +48,10 @@ export const getRuleTagsRoute = (
const rulesClient = (await context.alerting).getRulesClient();
const options = rewriteQueryReq(req.query);

const aggregateResult = await rulesClient.aggregate<RuleTagsAggregationResult>({
options: {
...options,
defaultSearchOperator: 'AND',
searchFields: ['tags'],
},
aggs: getRuleTagsAggregation({
maxTags: options.maxTags,
after: options.after,
}),
});
const tagsResult = await rulesClient.getTags(options);

return res.ok({
body: rewriteBodyRes(formatRuleTagsAggregationResult(aggregateResult)),
body: rewriteBodyRes(tagsResult),
});
})
)
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/server/rules_client.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type RulesClientMock = jest.Mocked<Schema>;
const createRulesClientMock = () => {
const mocked: RulesClientMock = {
aggregate: jest.fn().mockReturnValue({ ruleExecutionStatus: {}, ruleLastRunOutcome: {} }),
getTags: jest.fn(),
create: jest.fn(),
get: jest.fn(),
resolve: jest.fn(),
Expand Down
122 changes: 122 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/methods/get_tags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import Boom from '@hapi/boom';
import { TypeOf, schema } from '@kbn/config-schema';
import { KueryNode, nodeBuilder, nodeTypes } from '@kbn/es-query';
import { RulesClientContext } from '../types';
import { AlertingAuthorizationEntity } from '../../authorization';
import { alertingAuthorizationFilterOpts } from '../common/constants';
import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events';
import { RawRule } from '../../types';

export const DEFAULT_TAGS_PER_PAGE = 50;
const MAX_TAGS = 10000;

const getTagsParamsSchema = schema.object({
page: schema.number({ defaultValue: 1, min: 1 }),
perPage: schema.maybe(schema.number({ defaultValue: DEFAULT_TAGS_PER_PAGE, min: 1 })),
search: schema.maybe(schema.string()),
});

export type GetTagsParams = TypeOf<typeof getTagsParamsSchema>;

export interface RuleTagsAggregationResult {
tags: {
buckets: Array<{
key: string;
doc_count: number;
}>;
};
}

export interface GetTagsResult {
total: number;
page: number;
perPage: number;
data: string[];
}

export async function getTags(
context: RulesClientContext,
params: GetTagsParams
): Promise<GetTagsResult> {
let validatedParams: GetTagsParams;

try {
validatedParams = getTagsParamsSchema.validate(params);
} catch (error) {
throw Boom.badRequest(`Failed to validate params: ${error.message}`);
}

const { page, perPage = DEFAULT_TAGS_PER_PAGE, search = '' } = validatedParams;

let authorizationTuple;
try {
authorizationTuple = await context.authorization.getFindAuthorizationFilter(
AlertingAuthorizationEntity.Rule,
alertingAuthorizationFilterOpts
);
} catch (error) {
context.auditLogger?.log(
ruleAuditEvent({
action: RuleAuditAction.AGGREGATE,
error,
})
);
throw error;
}

const { filter: authorizationFilter } = authorizationTuple;

const filter =
authorizationFilter && search
? nodeBuilder.and([
nodeBuilder.is('alert.attributes.tags', nodeTypes.wildcard.buildNode(`${search}*`)),
authorizationFilter as KueryNode,
])
: authorizationFilter;

const response = await context.unsecuredSavedObjectsClient.find<
RawRule,
RuleTagsAggregationResult
>({
filter,
type: 'alert',
aggs: {
tags: {
terms: {
field: 'alert.attributes.tags',
order: {
_key: 'asc',
},
size: MAX_TAGS,
},
},
},
});

const filteredTags = (response.aggregations?.tags?.buckets || []).reduce<string[]>(
(result, bucket) => {
if (bucket.key.startsWith(search)) {
result.push(bucket.key);
}
return result;
},
[]
);

const startIndex = (page - 1) * perPage;
const endIndex = startIndex + perPage;
const chunkedTags = filteredTags.slice(startIndex, endIndex);

return {
total: filteredTags.length,
page,
perPage,
data: chunkedTags,
};
}
3 changes: 3 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/rules_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { unmuteInstance } from './methods/unmute_instance';
import { runSoon } from './methods/run_soon';
import { listRuleTypes } from './methods/list_rule_types';
import { getAlertFromRaw, GetAlertFromRawParams } from './lib/get_alert_from_raw';
import { getTags, GetTagsParams } from './methods/get_tags';

export type ConstructorOptions = Omit<
RulesClientContext,
Expand Down Expand Up @@ -165,6 +166,8 @@ export class RulesClient {
return this.context.spaceId;
}

public getTags = (params: GetTagsParams) => getTags(this.context, params);

public getAlertFromRaw = (params: GetAlertFromRawParams) =>
getAlertFromRaw(
this.context,
Expand Down
Loading

0 comments on commit 65b8a10

Please sign in to comment.