Skip to content
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
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/common/rules/apm_rule_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export const RULE_TYPES_CONFIG: Record<
},
[ApmRuleType.Anomaly]: {
name: i18n.translate('xpack.apm.anomalyAlert.name', {
defaultMessage: 'Anomaly',
defaultMessage: 'APM Anomaly',
}),
actionGroups: [THRESHOLD_MET_GROUP],
defaultActionGroupId: THRESHOLD_MET_GROUP_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export function registerApmRuleTypes(
}),
requiresAppContext: false,
defaultActionMessage: errorCountMessage,
priority: 80,
Copy link
Copy Markdown
Member Author

@maryam-saeidi maryam-saeidi Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This priority will be used to order the rule type list in observability. You can check the priorities assigned to each rule in the issue.

});

observabilityRuleTypeRegistry.register({
Expand Down Expand Up @@ -92,6 +93,7 @@ export function registerApmRuleTypes(
),
requiresAppContext: false,
defaultActionMessage: transactionDurationMessage,
priority: 60,
});

observabilityRuleTypeRegistry.register({
Expand Down Expand Up @@ -124,6 +126,7 @@ export function registerApmRuleTypes(
}),
requiresAppContext: false,
defaultActionMessage: transactionErrorRateMessage,
priority: 70,
});

observabilityRuleTypeRegistry.register({
Expand Down Expand Up @@ -153,5 +156,6 @@ export function registerApmRuleTypes(
}),
requiresAppContext: false,
defaultActionMessage: anomalyMessage,
priority: 90,
});
}
1 change: 1 addition & 0 deletions x-pack/plugins/infra/public/alerting/inventory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,6 @@ export function createInventoryMetricRuleType(): ObservabilityRuleTypeModel<Inve
defaultRecoveryMessage: inventoryDefaultRecoveryMessage,
requiresAppContext: false,
format: formatReason,
priority: 20,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,6 @@ export function createLogThresholdRuleType(
defaultRecoveryMessage: logThresholdDefaultRecoveryMessage,
requiresAppContext: false,
format: createRuleFormatter(logsLocator),
priority: 30,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,6 @@ export function createMetricThresholdRuleType(): ObservabilityRuleTypeModel<Metr
requiresAppContext: false,
format: formatReason,
alertDetailsAppSection: lazy(() => import('./components/alert_details_app_section')),
priority: 10,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React from 'react';
import { render } from '@testing-library/react';
import { CoreStart } from '@kbn/core/public';
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import { ObservabilityPublicPluginsStart } from '../../plugin';
import { RulesPage } from './rules';
import { kibanaStartMock } from '../../utils/kibana_react.mock';
Expand Down Expand Up @@ -104,7 +105,11 @@ describe('RulesPage with all capabilities', () => {
ruleTypeIndex,
});

return render(<RulesPage />);
return render(
<IntlProvider locale="en">
<RulesPage />
</IntlProvider>
);
}

it('should render a page template', async () => {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/observability/public/pages/rules/rules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ export function RulesPage({ activeTab = RULES_TAB_NAME }: RulesPageProps) {
setRefresh(new Date());
return Promise.resolve();
}}
useRuleProducer={true}
hideGrouping
useRuleProducer
/>
)}
</ObservabilityPageTemplate>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,27 @@ export type ObservabilityRuleTypeFormatter = (options: {
export interface ObservabilityRuleTypeModel<Params extends RuleTypeParams = RuleTypeParams>
extends RuleTypeModel<Params> {
format: ObservabilityRuleTypeFormatter;
priority?: number;
}

export function createObservabilityRuleTypeRegistry(ruleTypeRegistry: RuleTypeRegistryContract) {
const formatters: Array<{ typeId: string; fn: ObservabilityRuleTypeFormatter }> = [];
const formatters: Array<{
typeId: string;
priority: number;
fn: ObservabilityRuleTypeFormatter;
}> = [];

return {
register: (type: ObservabilityRuleTypeModel<any>) => {
const { format, ...rest } = type;
formatters.push({ typeId: type.id, fn: format });
const { format, priority, ...rest } = type;
formatters.push({ typeId: type.id, priority: priority || 0, fn: format });
ruleTypeRegistry.register(rest);
},
getFormatter: (typeId: string) => {
return formatters.find((formatter) => formatter.typeId === typeId)?.fn;
},
list: () => formatters.map((formatter) => formatter.typeId),
list: () =>
formatters.sort((a, b) => b.priority - a.priority).map((formatter) => formatter.typeId),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export const registerObservabilityRuleTypes = (
requiresAppContext: false,
defaultActionMessage: sloBurnRateDefaultActionMessage,
defaultRecoveryMessage: sloBurnRateDefaultRecoveryMessage,
priority: 100,
});

if (config.unsafe.thresholdRule.enabled) {
Expand Down Expand Up @@ -124,6 +125,7 @@ export const registerObservabilityRuleTypes = (
alertDetailsAppSection: lazy(
() => import('../components/custom_threshold/components/alert_details_app_section')
),
priority: 110,
});
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,16 @@
*/

import { RuleTypeModel } from '../../types';
import { ruleTypeGroupCompare, ruleTypeCompare } from './rule_type_compare';
import {
RuleTypeGroup,
ruleTypeGroupCompare,
ruleTypeCompare,
ruleTypeUngroupedCompare,
} from './rule_type_compare';
import { IsEnabledResult, IsDisabledResult } from './check_rule_type_enabled';

test('should sort groups by containing enabled rule types first and then by name', async () => {
const ruleTypes: Array<
[
string,
Array<{
id: string;
name: string;
checkEnabledResult: IsEnabledResult | IsDisabledResult;
ruleTypeItem: RuleTypeModel;
}>
]
> = [
const ruleTypes: RuleTypeGroup[] = [
[
'abc',
[
Expand Down Expand Up @@ -113,6 +108,102 @@ test('should sort groups by containing enabled rule types first and then by name
expect(result[2]).toEqual(ruleTypes[0]);
});

describe('ruleTypeUngroupedCompare', () => {
test('should maintain the order of rules', async () => {
const ruleTypes: RuleTypeGroup[] = [
[
'abc',
[
{
id: '1',
name: 'test2',
checkEnabledResult: { isEnabled: false, message: 'gold license' },
ruleTypeItem: {
id: 'ruleTypeItemId1',
iconClass: 'test',
description: 'Alert when testing',
documentationUrl: 'https://localhost.local/docs',
validate: () => {
return { errors: {} };
},
ruleParamsExpression: () => null,
requiresAppContext: false,
},
},
],
],
[
'bcd',
[
{
id: '2',
name: 'abc',
checkEnabledResult: { isEnabled: false, message: 'platinum license' },
ruleTypeItem: {
id: 'ruleTypeItemId2',
iconClass: 'test',
description: 'Alert when testing',
documentationUrl: 'https://localhost.local/docs',
validate: () => {
return { errors: {} };
},
ruleParamsExpression: () => null,
requiresAppContext: false,
},
},
{
id: '3',
name: 'cdf',
checkEnabledResult: { isEnabled: true },
ruleTypeItem: {
id: 'ruleTypeItemId3',
iconClass: 'test',
description: 'Alert when testing',
documentationUrl: 'https://localhost.local/docs',
validate: () => {
return { errors: {} };
},
ruleParamsExpression: () => null,
requiresAppContext: false,
},
},
],
],
[
'cde',
[
{
id: '4',
name: 'cde',
checkEnabledResult: { isEnabled: true },
ruleTypeItem: {
id: 'ruleTypeItemId4',
iconClass: 'test',
description: 'Alert when testing',
documentationUrl: 'https://localhost.local/docs',
validate: () => {
return { errors: {} };
},
ruleParamsExpression: () => null,
requiresAppContext: false,
},
},
],
],
];

const ruleTypesOrder = ['4', '1', '2', '3'];

const result = [...ruleTypes].sort((left, right) =>
ruleTypeUngroupedCompare(left, right, ruleTypesOrder)
);

expect(result[0]).toEqual(ruleTypes[2]);
expect(result[1]).toEqual(ruleTypes[1]);
expect(result[2]).toEqual(ruleTypes[0]);
});
});

test('should sort rule types by enabled first and then by name', async () => {
const ruleTypes: Array<{
id: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,19 @@
import { RuleTypeModel } from '../../types';
import { IsEnabledResult, IsDisabledResult } from './check_rule_type_enabled';

export type RuleTypeGroup = [
string,
Array<{
id: string;
name: string;
checkEnabledResult: IsEnabledResult | IsDisabledResult;
ruleTypeItem: RuleTypeModel;
}>
];

export function ruleTypeGroupCompare(
left: [
string,
Array<{
id: string;
name: string;
checkEnabledResult: IsEnabledResult | IsDisabledResult;
ruleTypeItem: RuleTypeModel;
}>
],
right: [
string,
Array<{
id: string;
name: string;
checkEnabledResult: IsEnabledResult | IsDisabledResult;
ruleTypeItem: RuleTypeModel;
}>
],
left: RuleTypeGroup,
right: RuleTypeGroup,
groupNames: Map<string, string> | undefined
) {
const groupNameA = left[0];
Expand Down Expand Up @@ -54,6 +48,35 @@ export function ruleTypeGroupCompare(
: groupNameA.localeCompare(groupNameB);
}

export function ruleTypeUngroupedCompare(
left: RuleTypeGroup,
right: RuleTypeGroup,
ruleTypes?: string[]
) {
const leftRuleTypesList = left[1];
const rightRuleTypesList = right[1];

const hasEnabledRuleTypeInListLeft =
leftRuleTypesList.find((ruleTypeItem) => ruleTypeItem.checkEnabledResult.isEnabled) !==
undefined;

const hasEnabledRuleTypeInListRight =
rightRuleTypesList.find((ruleTypeItem) => ruleTypeItem.checkEnabledResult.isEnabled) !==
undefined;

if (hasEnabledRuleTypeInListLeft && !hasEnabledRuleTypeInListRight) {
return -1;
}
if (!hasEnabledRuleTypeInListLeft && hasEnabledRuleTypeInListRight) {
return 1;
}

return ruleTypes
? ruleTypes.findIndex((frtA) => leftRuleTypesList.some((aRuleType) => aRuleType.id === frtA)) -
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credit goes to @XavierM

ruleTypes.findIndex((frtB) => rightRuleTypesList.some((bRuleType) => bRuleType.id === frtB))
: 0;
}

export function ruleTypeCompare(
a: {
id: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const RuleAdd = ({
initialValues,
reloadRules,
onSave,
hideGrouping,
hideInterval,
metadata: initialMetadata,
filteredRuleTypes,
Expand Down Expand Up @@ -286,6 +287,7 @@ const RuleAdd = ({
ruleTypeRegistry={ruleTypeRegistry}
metadata={metadata}
filteredRuleTypes={filteredRuleTypes}
hideGrouping={hideGrouping}
hideInterval={hideInterval}
onChangeMetaData={onChangeMetaData}
setConsumer={setSelectedConsumer}
Expand Down
Loading