Skip to content

Commit

Permalink
[ResponseOps][Alerts] Add null-value bucket detection to alerts group…
Browse files Browse the repository at this point in the history
… aggregations endpoint (#190305)

## Summary

- Adds null-value bucket detection to server-side alerts aggregations
and marks those groups with a `--` key and `isNullGroup = true`.
- Improves alerts grouping types with default aggregations.
- Improves documentation

## To verify

1. Temporarily merge
[#189958](#189958) into this
branch
2. Create a rule that fires alerts in Observability > Alerts (i.e.
Custom Threshold, ES Query, ...)
3. Once you start to see some alerts in the Alerts page, toggle the
grouped alerts view using the dropdown at the top-right of the table
(`Group alerts by: ...`), selecting a custom field that doesn't have a
value in alert documents (to find one, open the alert flyout and look at
the fields table)
4. Check that the group based on the empty field shows `--` as a title
5. Check that the alerts table in the expanded group panel is filtered
correctly

### References

Refs [#189958](#189958)

### Checklist

Delete any items that are not applicable to this PR.

- [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
  • Loading branch information
umbopepato committed Aug 19, 2024
1 parent 965b0a6 commit 0299a7a
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 31 deletions.
6 changes: 5 additions & 1 deletion packages/kbn-alerts-grouping/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,9 @@
*/

export { AlertsGrouping } from './src/components/alerts_grouping';
export { type AlertsGroupingProps } from './src/types';
export {
type AlertsGroupingProps,
type BaseAlertsGroupAggregations,
type AlertsGroupAggregationBucket,
} from './src/types';
export { useAlertsGroupingState } from './src/contexts/alerts_grouping_context';
43 changes: 32 additions & 11 deletions packages/kbn-alerts-grouping/src/components/alerts_grouping.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { i18n } from '@kbn/i18n';
import { useAlertsDataView } from '@kbn/alerts-ui-shared/src/common/hooks/use_alerts_data_view';
import useLocalStorage from 'react-use/lib/useLocalStorage';
import { AlertsGroupingLevel, AlertsGroupingLevelProps } from './alerts_grouping_level';
import { AlertsGroupingProps } from '../types';
import type { AlertsGroupingProps, BaseAlertsGroupAggregations } from '../types';
import {
AlertsGroupingContextProvider,
useAlertsGroupingState,
Expand All @@ -40,7 +40,10 @@ const NextLevel = ({
parentGroupingFilter,
groupingFilters,
getLevel,
}: Pick<AlertsGroupingLevelProps, 'children' | 'parentGroupingFilter'> & {
}: Pick<
AlertsGroupingLevelProps<BaseAlertsGroupAggregations>,
'children' | 'parentGroupingFilter'
> & {
level: number;
selectedGroups: string[];
groupingFilters: Filter[];
Expand All @@ -56,7 +59,9 @@ const NextLevel = ({
return children(nextGroupingFilters)!;
};

const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
const AlertsGroupingInternal = <T extends BaseAlertsGroupAggregations>(
props: AlertsGroupingProps<T>
) => {
const {
groupingId,
services,
Expand Down Expand Up @@ -230,6 +235,8 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
return getLevel(0, selectedGroups[0]);
};

const typedMemo: <T>(c: T) => T = memo;

/**
* A coordinator component to show multiple alert tables grouped by one or more fields
*
Expand All @@ -243,7 +250,7 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
*
*
* return (
* <AlertsGrouping
* <AlertsGrouping<YourAggregationsType>
* featureIds={[...]}
* globalQuery={{ query: ..., language: 'kql' }}
* globalFilters={...}
Expand Down Expand Up @@ -274,11 +281,25 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
* </AlertsGrouping>
* );
* ```
*
* To define your aggregations result type, extend the `BaseAlertsGroupAggregations` type:
*
* ```ts
* import { BaseAlertsGroupAggregations } from '@kbn/alerts-grouping';
*
* interface YourAggregationsType extends BaseAlertsGroupAggregations {
* // Your custom aggregations here
* }
* ```
*
* Check {@link useGetAlertsGroupAggregationsQuery} for more info on alerts aggregations.
*/
export const AlertsGrouping = memo((props: AlertsGroupingProps) => {
return (
<AlertsGroupingContextProvider>
<AlertsGroupingInternal {...props} />
</AlertsGroupingContextProvider>
);
});
export const AlertsGrouping = typedMemo(
<T extends BaseAlertsGroupAggregations>(props: AlertsGroupingProps<T>) => {
return (
<AlertsGroupingContextProvider>
<AlertsGroupingInternal {...props} />
</AlertsGroupingContextProvider>
);
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import {
useGetAlertsGroupAggregationsQuery,
UseGetAlertsGroupAggregationsQueryProps,
} from '@kbn/alerts-ui-shared';
import { AlertsGroupingProps } from '../types';
import { AlertsGroupingProps, BaseAlertsGroupAggregations } from '../types';

export interface AlertsGroupingLevelProps<T extends Record<string, unknown> = {}>
extends AlertsGroupingProps<T> {
export interface AlertsGroupingLevelProps<
T extends BaseAlertsGroupAggregations = BaseAlertsGroupAggregations
> extends AlertsGroupingProps<T> {
getGrouping: (
props: Omit<DynamicGroupingProps<T>, 'groupSelector' | 'pagination'>
) => ReactElement;
Expand All @@ -40,8 +41,9 @@ const DEFAULT_FILTERS: Filter[] = [];
/**
* Renders an alerts grouping level
*/
export const AlertsGroupingLevel = memo(
<T extends Record<string, unknown> = {}>({
const typedMemo: <T>(c: T) => T = memo;
export const AlertsGroupingLevel = typedMemo(
<T extends BaseAlertsGroupAggregations>({
featureIds,
defaultFilters = DEFAULT_FILTERS,
from,
Expand Down
27 changes: 26 additions & 1 deletion packages/kbn-alerts-grouping/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export interface AlertsGroupingState {
[groupingId: string]: GroupModel;
}

export interface AlertsGroupingProps<T extends Record<string, unknown> = {}> {
export interface AlertsGroupingProps<
T extends BaseAlertsGroupAggregations = BaseAlertsGroupAggregations
> {
/**
* The leaf component that will be rendered in the grouping panels
*/
Expand Down Expand Up @@ -96,3 +98,26 @@ export interface AlertsGroupingProps<T extends Record<string, unknown> = {}> {
http: HttpSetup;
};
}

export interface AlertsGroupAggregationBucket {
key: string;
doc_count: number;
isNullGroup?: boolean;
unitsCount?: {
value: number;
};
}

export interface BaseAlertsGroupAggregations {
groupByFields: {
doc_count_error_upper_bound: number;
sum_other_doc_count: number;
buckets: AlertsGroupAggregationBucket[];
};
groupsCount: {
value: number;
};
unitsCount: {
value: number;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export interface UseGetAlertsGroupAggregationsQueryProps {
*
* The provided `aggregations` are applied within `groupByFields`. Here the `groupByField` runtime
* field can be used to perform grouping-based aggregations.
* `groupByField` buckets computed over a field with a null/absent value are marked with the
* `isNullGroup` flag set to true and their key is set to the `--` string.
*
* Applies alerting RBAC through featureIds.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,18 @@ import {
} from '@kbn/rule-data-utils';

import {
AggregateName,
AggregationsAggregate,
AggregationsMultiBucketAggregateBase,
InlineScript,
MappingRuntimeFields,
QueryDslQueryContainer,
SortCombinations,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { RuleTypeParams, PluginStartContract as AlertingStart } from '@kbn/alerting-plugin/server';
} from '@elastic/elasticsearch/lib/api/types';
import type {
RuleTypeParams,
PluginStartContract as AlertingStart,
} from '@kbn/alerting-plugin/server';
import {
ReadOperations,
AlertingAuthorization,
Expand Down Expand Up @@ -279,7 +285,7 @@ export class AlertsClient {
/**
* Searches alerts by id or query and audits the results
*/
private async searchAlerts({
private async searchAlerts<TAggregations = Record<AggregateName, AggregationsAggregate>>({
id,
query,
aggs,
Expand Down Expand Up @@ -335,7 +341,7 @@ export class AlertsClient {
};
}

const result = await this.esClient.search<ParsedTechnicalFields>({
const result = await this.esClient.search<ParsedTechnicalFields, TAggregations>({
index: index ?? '.alerts-*',
ignore_unavailable: true,
body: queryBody,
Expand Down Expand Up @@ -975,7 +981,10 @@ export class AlertsClient {
}
}

public async find<Params extends RuleTypeParams = never>({
public async find<
Params extends RuleTypeParams = never,
TAggregations = Record<AggregateName, AggregationsAggregate>
>({
aggs,
featureIds,
index,
Expand Down Expand Up @@ -1007,7 +1016,7 @@ export class AlertsClient {
}
}

const alertsSearchResponse = await this.searchAlerts({
const alertsSearchResponse = await this.searchAlerts<TAggregations>({
query,
aggs,
_source,
Expand Down Expand Up @@ -1036,7 +1045,7 @@ export class AlertsClient {
/**
* Performs a `find` query to extract aggregations on alert groups
*/
public getGroupAggregations({
public async getGroupAggregations({
featureIds,
groupByField,
aggregations,
Expand Down Expand Up @@ -1086,7 +1095,10 @@ export class AlertsClient {
`The number of documents is too high. Paginating through more than ${MAX_PAGINATED_ALERTS} documents is not possible.`
);
}
return this.find({
const searchResult = await this.find<
never,
{ groupByFields: AggregationsMultiBucketAggregateBase<{ key: string }> }
>({
featureIds,
aggs: {
groupByFields: {
Expand Down Expand Up @@ -1139,6 +1151,20 @@ export class AlertsClient {
size: 0,
_source: false,
});
// Replace artificial uuid values with '--' in null-value buckets and mark them with `isNullGroup = true`
const groupsAggregation = searchResult.aggregations?.groupByFields;
if (groupsAggregation) {
const buckets = Array.isArray(groupsAggregation?.buckets)
? groupsAggregation.buckets
: Object.values(groupsAggregation?.buckets ?? {});
buckets.forEach((bucket) => {
if (bucket.key === uniqueValue) {
bucket.key = '--';
(bucket as { isNullGroup?: boolean }).isNullGroup = true;
}
});
}
return searchResult;
}

public async getAuthorizedAlertsIndices(featureIds: string[]): Promise<string[] | undefined> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ beforeEach(() => {
describe('getGroupAggregations()', () => {
test('calls find() with the correct params', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
alertsClient.find = jest.fn();
alertsClient.find = jest.fn().mockResolvedValue({ aggregations: {} });

const featureIds = [AlertConsumers.STACK_ALERTS];
const groupByField = 'kibana.alert.rule.name';
Expand Down Expand Up @@ -141,27 +141,57 @@ describe('getGroupAggregations()', () => {
});
});

test('replaces the key of null-value buckets and marks them with the `isNullGroup` flag', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
alertsClient.find = jest.fn().mockResolvedValue({
aggregations: {
groupByFields: {
buckets: [
{
key: 'unique-value',
doc_count: 1,
},
],
},
},
});

const result = await alertsClient.getGroupAggregations({
featureIds: [AlertConsumers.STACK_ALERTS],
groupByField: 'kibana.alert.rule.name',
aggregations: {},
filters: [],
pageIndex: 0,
pageSize: DEFAULT_ALERTS_GROUP_BY_FIELD_SIZE,
});

const firstBucket = (result.aggregations as any).groupByFields.buckets[0];

expect(firstBucket.isNullGroup).toBe(true);
expect(firstBucket.key).toEqual('--');
});

test('rejects with invalid pagination options', async () => {
const alertsClient = new AlertsClient(alertsClientParams);

expect(() =>
await expect(() =>
alertsClient.getGroupAggregations({
featureIds: ['apm', 'infrastructure', 'logs', 'observability', 'slo', 'uptime'],
groupByField: 'kibana.alert.rule.name',
pageIndex: 101,
pageSize: 50,
})
).toThrowErrorMatchingInlineSnapshot(
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The provided pageIndex value is too high. The maximum allowed pageIndex value is 100."`
);
expect(() =>
await expect(() =>
alertsClient.getGroupAggregations({
featureIds: ['apm', 'infrastructure', 'logs', 'observability', 'slo', 'uptime'],
groupByField: 'kibana.alert.rule.name',
pageIndex: 10,
pageSize: 5000,
})
).toThrowErrorMatchingInlineSnapshot(
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The number of documents is too high. Paginating through more than 10000 documents is not possible."`
);
});
Expand Down

0 comments on commit 0299a7a

Please sign in to comment.