Skip to content

Commit

Permalink
[ResponseOps][Rules] Use rule form instead of rule flyout in observab…
Browse files Browse the repository at this point in the history
…ility solution (elastic#206774)

## Summary

Resolves elastic#195574

This PR updates observability solution to use new rule form to `create`
and `edit` rules same as `stack management > rules` page.
It removes usage of rule flyout form o11y solution. Also updated
functional tests.

### 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


### How to test
- Create a rule in o11y, verify it works as expected
- Edit rule in o11y via different options (from rule details page, rule
list table, alert details page etc.) verify it works as expected
- Verify the same in serverless o11y project

### Release Note
Use rule form to create or edit rules in observability.

---------

Co-authored-by: Maryam Saeidi <[email protected]>
  • Loading branch information
js-jankisalvi and maryam-saeidi authored Jan 24, 2025
1 parent 1ca4d96 commit dd6376d
Show file tree
Hide file tree
Showing 12 changed files with 469 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface CreateRuleFormProps {
shouldUseRuleProducer?: boolean;
canShowConsumerSelection?: boolean;
showMustacheAutocompleteSwitch?: boolean;
isServerless?: boolean;
onCancel?: () => void;
onSubmit?: (ruleId: string) => void;
}
Expand All @@ -60,6 +61,7 @@ export const CreateRuleForm = (props: CreateRuleFormProps) => {
shouldUseRuleProducer = false,
canShowConsumerSelection = true,
showMustacheAutocompleteSwitch = false,
isServerless = false,
onCancel,
onSubmit,
} = props;
Expand Down Expand Up @@ -195,6 +197,7 @@ export const CreateRuleForm = (props: CreateRuleFormProps) => {
validConsumers,
ruleType,
ruleTypes,
isServerless,
}),
}}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import React, { useMemo } from 'react';
import { EuiEmptyPrompt, EuiText } from '@elastic/eui';
import { QueryClientProvider, QueryClient } from '@tanstack/react-query';
import { type RuleCreationValidConsumer } from '@kbn/rule-data-utils';
import { useParams } from 'react-router-dom';
import { CreateRuleForm } from './create_rule_form';
import { EditRuleForm } from './edit_rule_form';
Expand All @@ -27,10 +28,20 @@ export interface RuleFormProps {
plugins: RuleFormPlugins;
onCancel?: () => void;
onSubmit?: (ruleId: string) => void;
validConsumers?: RuleCreationValidConsumer[];
multiConsumerSelection?: RuleCreationValidConsumer | null;
isServerless?: boolean;
}

export const RuleForm = (props: RuleFormProps) => {
const { plugins: _plugins, onCancel, onSubmit } = props;
const {
plugins: _plugins,
onCancel,
onSubmit,
validConsumers,
multiConsumerSelection,
isServerless,
} = props;
const { id, ruleTypeId } = useParams<{
id?: string;
ruleTypeId?: string;
Expand Down Expand Up @@ -80,6 +91,9 @@ export const RuleForm = (props: RuleFormProps) => {
plugins={plugins}
onCancel={onCancel}
onSubmit={onSubmit}
validConsumers={validConsumers}
multiConsumerSelection={multiConsumerSelection}
isServerless={isServerless}
/>
);
}
Expand Down Expand Up @@ -112,6 +126,9 @@ export const RuleForm = (props: RuleFormProps) => {
actionTypeRegistry,
id,
ruleTypeId,
validConsumers,
multiConsumerSelection,
isServerless,
onCancel,
onSubmit,
]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
/*
* 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { RuleTypeWithDescription } from '@kbn/alerts-ui-shared';
import { getInitialMultiConsumer } from './get_initial_multi_consumer';

describe('getInitialMultiConsumer', () => {
const ruleType = {
id: '.es-query',
name: 'Test',
actionGroups: [
{
id: 'testActionGroup',
name: 'Test Action Group',
},
{
id: 'recovered',
name: 'Recovered',
},
],
defaultActionGroupId: 'testActionGroup',
minimumLicenseRequired: 'basic',
recoveryActionGroup: {
id: 'recovered',
name: 'Recovered',
},
producer: 'logs',
authorizedConsumers: {
alerting: { read: true, all: true },
test: { read: true, all: true },
stackAlerts: { read: true, all: true },
logs: { read: true, all: true },
},
actionVariables: {
params: [],
state: [],
},
enabledInLicense: true,
category: 'test',
} as RuleTypeWithDescription;

const ruleTypes = [
{
id: '.es-query',
name: 'Test',
actionGroups: [
{
id: 'testActionGroup',
name: 'Test Action Group',
},
{
id: 'recovered',
name: 'Recovered',
},
],
defaultActionGroupId: 'testActionGroup',
minimumLicenseRequired: 'basic',
recoveryActionGroup: {
id: 'recovered',
},
producer: 'logs',
authorizedConsumers: {
alerting: { read: true, all: true },
test: { read: true, all: true },
stackAlerts: { read: true, all: true },
logs: { read: true, all: true },
},
actionVariables: {
params: [],
state: [],
},
enabledInLicense: true,
},
{
enabledInLicense: true,
recoveryActionGroup: {
id: 'recovered',
name: 'Recovered',
},
actionGroups: [],
defaultActionGroupId: 'threshold met',
minimumLicenseRequired: 'basic',
authorizedConsumers: {
stackAlerts: {
read: true,
all: true,
},
},
actionVariables: {
params: [],
state: [],
},
id: '.index-threshold',
name: 'Index threshold',
category: 'management',
producer: 'stackAlerts',
},
] as RuleTypeWithDescription[];

test('should return null when rule type id does not match', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['logs', 'observability'],
ruleType: {
...ruleType,
id: 'test',
},
ruleTypes,
isServerless: false,
});

expect(res).toBe(null);
});

test('should return null when no valid consumers', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: [],
ruleType,
ruleTypes,
isServerless: false,
});

expect(res).toBe(null);
});

test('should return same valid consumer when only one valid consumer', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['alerts'],
ruleType,
ruleTypes,
isServerless: false,
});

expect(res).toBe('alerts');
});

test('should not return observability consumer for non serverless', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['logs', 'infrastructure', 'observability'],
ruleType,
ruleTypes,
isServerless: false,
});

expect(res).toBe('logs');
});

test('should return observability consumer for serverless', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['logs', 'infrastructure', 'observability'],
ruleType,
ruleTypes,
isServerless: true,
});

expect(res).toBe('observability');
});

test('should return null when there is no authorized consumers', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['alerts', 'infrastructure'],
ruleType: {
...ruleType,
authorizedConsumers: {},
},
ruleTypes,
isServerless: false,
});

expect(res).toBe(null);
});

test('should return null when multiConsumerSelection is null', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: null,
validConsumers: ['stackAlerts', 'logs'],
ruleType: {
...ruleType,
authorizedConsumers: {
stackAlerts: { read: true, all: true },
},
},
ruleTypes,
isServerless: false,
});

expect(res).toBe(null);
});

test('should return valid multi consumer correctly', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: 'logs',
validConsumers: ['stackAlerts', 'logs'],
ruleType: {
...ruleType,
authorizedConsumers: {
stackAlerts: { read: true, all: true },
},
},
ruleTypes,
isServerless: false,
});

expect(res).toBe('logs');
});

test('should return stackAlerts correctly', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: 'alerts',
validConsumers: ['stackAlerts', 'logs'],
ruleType: {
...ruleType,
authorizedConsumers: {},
},
ruleTypes,
isServerless: false,
});

expect(res).toBe('stackAlerts');
});

test('should return null valid consumer correctly', () => {
const res = getInitialMultiConsumer({
multiConsumerSelection: 'alerts',
validConsumers: ['infrastructure', 'logs'],
ruleType: {
...ruleType,
authorizedConsumers: {},
},
ruleTypes: [],
isServerless: false,
});

expect(res).toBe(null);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ export const getInitialMultiConsumer = ({
validConsumers,
ruleType,
ruleTypes,
isServerless,
}: {
multiConsumerSelection?: RuleCreationValidConsumer | null;
validConsumers: RuleCreationValidConsumer[];
ruleType: RuleTypeWithDescription;
ruleTypes: RuleTypeWithDescription[];
isServerless?: boolean;
}): RuleCreationValidConsumer | null => {
// If rule type doesn't support multi-consumer or no valid consumers exists,
// return nothing
Expand All @@ -52,8 +54,8 @@ export const getInitialMultiConsumer = ({
return validConsumers[0];
}

// If o11y is in the valid consumers, just use that
if (validConsumers.includes(AlertConsumers.OBSERVABILITY)) {
// If o11y is in the valid consumers and it is serverless, just use that
if (isServerless && validConsumers.includes(AlertConsumers.OBSERVABILITY)) {
return AlertConsumers.OBSERVABILITY;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const EXPLORATORY_VIEW_PATH = '/exploratory-view' as const; // has been m
export const RULES_PATH = '/alerts/rules' as const;
export const RULES_LOGS_PATH = '/alerts/rules/logs' as const;
export const RULE_DETAIL_PATH = '/alerts/rules/:ruleId' as const;
export const CREATE_RULE_PATH = '/alerts/rules/create/:ruleTypeId' as const;
export const CASES_PATH = '/cases' as const;
export const ANNOTATIONS_PATH = '/annotations' as const;
export const SETTINGS_PATH = '/slos/settings' as const;
Expand All @@ -37,6 +38,8 @@ export const paths = {
rules: `${OBSERVABILITY_BASE_PATH}${RULES_PATH}`,
ruleDetails: (ruleId: string) =>
`${OBSERVABILITY_BASE_PATH}${RULES_PATH}/${encodeURIComponent(ruleId)}`,
createRule: (ruleTypeId: string) =>
`${OBSERVABILITY_BASE_PATH}${RULES_PATH}/create/${encodeURIComponent(ruleTypeId)}`,
},
};

Expand Down
Loading

0 comments on commit dd6376d

Please sign in to comment.