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

[ES Query] Make rule created in Discover visible in Observability #171364

Merged
merged 26 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9dfe8d6
[ES Query] Make rule created in Discover visible in Observability
Zacqary Nov 15, 2023
09de96c
Remove console
Zacqary Nov 15, 2023
a283015
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Nov 15, 2023
567e1a0
Add default consumer of logs
Zacqary Nov 16, 2023
69420e7
Update Jest
Zacqary Nov 16, 2023
25a5876
Merge branch '170497-discover-esquery-scopeselect' of https://github.…
Zacqary Nov 16, 2023
27630dd
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Nov 16, 2023
9a8f809
Fix jest
Zacqary Nov 16, 2023
091ff3e
Merge branch '170497-discover-esquery-scopeselect' of https://github.…
Zacqary Nov 17, 2023
8269b03
Merge remote-tracking branch 'upstream/main' into 170497-discover-esq…
Zacqary Nov 20, 2023
db09cd4
Revert rule form jest
Zacqary Nov 20, 2023
c450c2d
Update functional test permission
Zacqary Nov 20, 2023
c3bb46b
Fix stack rule scope visibility
Zacqary Nov 20, 2023
c3906c0
Remove selectable consumer from non-multi ruletypes
Zacqary Nov 20, 2023
4ee21b3
Fix jest, have consumer selection fall back to first value on null
Zacqary Nov 21, 2023
dfa37ef
Add stackRules back to odiscover alert flyout
Zacqary Nov 22, 2023
d9a5463
Merge remote-tracking branch 'upstream/main' into 170497-discover-esq…
Zacqary Nov 22, 2023
33502b4
Add comment
Zacqary Nov 22, 2023
26e9969
Fix Jest
Zacqary Nov 22, 2023
1c9a672
Revert bad merge in rule_form
Zacqary Nov 30, 2023
12c7a13
Merge remote-tracking branch 'upstream/main' into 170497-discover-esq…
Zacqary Nov 30, 2023
5f648dd
Add observability consumer to discover form for serverless
Zacqary Nov 30, 2023
a826a7b
Fix bad merge on rule_add
Zacqary Nov 30, 2023
040d0de
Add observability consumer to view in app registration
Zacqary Nov 30, 2023
e48be57
Default to stack rules if available
Zacqary Dec 1, 2023
729f036
Merge remote-tracking branch 'upstream/main' into 170497-discover-esq…
Zacqary Dec 1, 2023
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
8 changes: 8 additions & 0 deletions packages/kbn-rule-data-utils/src/rule_types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { AlertConsumers } from '../alerts_as_data_rbac';
import { STACK_ALERTS_FEATURE_ID } from './stack_rules';

export * from './stack_rules';
export * from './o11y_rules';

export type RuleCreationValidConsumer =
| typeof AlertConsumers.LOGS
| typeof AlertConsumers.INFRASTRUCTURE
| typeof AlertConsumers.OBSERVABILITY
| typeof STACK_ALERTS_FEATURE_ID;
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,24 @@ import { FormattedMessage } from '@kbn/i18n-react';
import type { DataView } from '@kbn/data-plugin/common';
import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { STACK_ALERTS_FEATURE_ID } from '@kbn/rule-data-utils';
import {
AlertConsumers,
ES_QUERY_ID,
RuleCreationValidConsumer,
STACK_ALERTS_FEATURE_ID,
} from '@kbn/rule-data-utils';
import { DiscoverStateContainer } from '../../services/discover_state';
import { DiscoverServices } from '../../../../build_services';

const container = document.createElement('div');
let isOpen = false;

const ALERT_TYPE_ID = '.es-query';
const EsQueryValidConsumer: RuleCreationValidConsumer[] = [
AlertConsumers.INFRASTRUCTURE,
AlertConsumers.LOGS,
AlertConsumers.OBSERVABILITY,
STACK_ALERTS_FEATURE_ID,
];

interface AlertsPopoverProps {
onClose: () => void;
Expand Down Expand Up @@ -98,7 +108,7 @@ export function AlertsPopover({

return triggersActionsUi?.getAddRuleFlyout({
metadata: discoverMetadata,
consumer: STACK_ALERTS_FEATURE_ID,
consumer: 'alerts',
XavierM marked this conversation as resolved.
Show resolved Hide resolved
onClose: (_, metadata) => {
onFinishFlyoutInteraction(metadata as EsQueryAlertMetaData);
onClose();
Expand All @@ -107,8 +117,12 @@ export function AlertsPopover({
onFinishFlyoutInteraction(metadata as EsQueryAlertMetaData);
},
canChangeTrigger: false,
ruleTypeId: ALERT_TYPE_ID,
ruleTypeId: ES_QUERY_ID,
initialValues: { params: getParams() },
validConsumers: EsQueryValidConsumer,
useRuleProducer: true,
// Default to the Logs consumer if it's available. This should fall back to Stack Alerts if it's not.
initialSelectedConsumer: AlertConsumers.LOGS,
});
}, [alertFlyoutVisible, triggersActionsUi, discoverMetadata, getParams, onClose, stateContainer]);

Expand Down
6 changes: 2 additions & 4 deletions src/plugins/discover/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,12 @@
"@kbn/react-kibana-context-render",
"@kbn/unified-data-table",
"@kbn/no-data-page-plugin",
"@kbn/rule-data-utils",
"@kbn/global-search-plugin",
"@kbn/resizable-layout",
"@kbn/unsaved-changes-badge",
"@kbn/rule-data-utils",
"@kbn/core-chrome-browser",
"@kbn/core-plugins-server"
],
"exclude": [
"target/**/*"
]
"exclude": ["target/**/*"]
}
2 changes: 2 additions & 0 deletions x-pack/plugins/observability/public/pages/rules/rules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useLoadRuleTypes } from '@kbn/triggers-actions-ui-plugin/public';
import { ALERTS_FEATURE_ID } from '@kbn/alerting-plugin/common';
import { useBreadcrumbs } from '@kbn/observability-shared-plugin/public';

import { AlertConsumers } from '@kbn/rule-data-utils';
import { RULES_LOGS_PATH, RULES_PATH } from '../../../common/locators/paths';
import { useKibana } from '../../utils/kibana_react';
import { usePluginContext } from '../../hooks/use_plugin_context';
Expand Down Expand Up @@ -144,6 +145,7 @@ export function RulesPage({ activeTab = RULES_TAB_NAME }: RulesPageProps) {
consumer={ALERTS_FEATURE_ID}
filteredRuleTypes={filteredRuleTypes}
validConsumers={observabilityRuleCreationValidConsumers}
initialSelectedConsumer={AlertConsumers.LOGS}
onClose={() => {
setAddRuleFlyoutVisibility(false);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,12 @@ function registerNavigation(alerting: AlertingSetup) {
return;
}
);
alerting.registerNavigation(
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can thank the failing tests for this one

'observability',
ES_QUERY_ID,
(rule: SanitizedRule<EsQueryRuleParams<SearchType.searchSource>>) => {
if (isSearchSourceRule(rule.params)) return `/app/discover#/viewAlert/${rule.id}`;
return;
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -340,87 +340,6 @@ describe('rule_add', () => {
});
});

it('should NOT allow to save the rule if the consumer is not set', async () => {
XavierM marked this conversation as resolved.
Show resolved Hide resolved
(triggersActionsUiConfig as jest.Mock).mockResolvedValue({
minimumScheduleInterval: { value: '1m', enforce: false },
});
const onClose = jest.fn();
await setup({
initialValues: {
name: 'Simple rule',
consumer: 'alerts',
ruleTypeId: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
tags: ['uptime', 'logs'],
schedule: {
interval: '1h',
},
},
onClose,
ruleTypesOverwrite: [
{
id: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
name: 'Threshold Rule',
actionGroups: [
{
id: 'testActionGroup',
name: 'Test Action Group',
},
],
enabledInLicense: true,
defaultActionGroupId: 'threshold.fired',
minimumLicenseRequired: 'basic',
recoveryActionGroup: { id: 'recovered', name: 'Recovered' },
producer: ALERTS_FEATURE_ID,
authorizedConsumers: {
alerts: { read: true, all: true },
apm: { read: true, all: true },
discover: { read: true, all: true },
infrastructure: { read: true, all: true },
logs: { read: true, all: true },
ml: { read: true, all: true },
monitoring: { read: true, all: true },
siem: { read: true, all: true },
// Setting SLO all to false, this shouldn't show up
slo: { read: true, all: false },
stackAlerts: { read: true, all: true },
uptime: { read: true, all: true },
},
actionVariables: {
context: [],
state: [],
params: [],
},
},
],
ruleTypeModelOverwrite: {
id: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
iconClass: 'test',
description: 'test',
documentationUrl: null,
validate: (): ValidationResult => {
return { errors: {} };
},
ruleParamsExpression: TestExpression,
requiresAppContext: false,
},
validConsumers: [AlertConsumers.INFRASTRUCTURE, AlertConsumers.LOGS],
});

await act(async () => {
await nextTick();
wrapper.update();
});

wrapper.find('[data-test-subj="saveRuleButton"]').last().simulate('click');

await act(async () => {
await nextTick();
wrapper.update();
});

expect(createRule).toBeCalledTimes(0);
});

it('should set consumer automatically if only 1 authorized consumer exists', async () => {
(triggersActionsUiConfig as jest.Mock).mockResolvedValue({
minimumScheduleInterval: { value: '1m', enforce: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { HealthContextProvider } from '../../context/health_context';
import { useKibana } from '../../../common/lib/kibana';
import { hasRuleChanged, haveRuleParamsChanged } from './has_rule_changed';
import { getRuleWithInvalidatedFields } from '../../lib/value_validators';
import { DEFAULT_RULE_INTERVAL } from '../../constants';
import { DEFAULT_RULE_INTERVAL, MULTI_CONSUMER_RULE_TYPE_IDS } from '../../constants';
import { triggersActionsUiConfig } from '../../../common/lib/config_api';
import { getInitialInterval } from './get_initial_interval';
import { ToastWithCircuitBreakerContent } from '../../components/toast_with_circuit_breaker_content';
Expand Down Expand Up @@ -65,6 +65,7 @@ const RuleAdd = ({
filteredRuleTypes,
validConsumers,
useRuleProducer,
initialSelectedConsumer,
...props
}: RuleAddProps) => {
const onSaveHandler = onSave ?? reloadRules;
Expand All @@ -84,7 +85,6 @@ const RuleAdd = ({
...(initialValues ? initialValues : {}),
};
}, [ruleTypeId, consumer, initialValues]);

const [{ rule }, dispatch] = useReducer(ruleReducer as InitialRuleReducer, {
rule: initialRule,
});
Expand All @@ -97,9 +97,14 @@ const RuleAdd = ({
props.ruleTypeIndex
);
const [changedFromDefaultInterval, setChangedFromDefaultInterval] = useState<boolean>(false);

const selectableConsumer = useMemo(
() => rule.ruleTypeId && MULTI_CONSUMER_RULE_TYPE_IDS.includes(rule.ruleTypeId),
[rule]
);
const [selectedConsumer, setSelectedConsumer] = useState<
RuleCreationValidConsumer | null | undefined
>();
>(selectableConsumer ? initialSelectedConsumer : null);

const setRule = (value: InitialRule) => {
dispatch({ command: { type: 'setRule' }, payload: { key: 'rule', value } });
Expand Down Expand Up @@ -218,12 +223,14 @@ const RuleAdd = ({
getRuleErrors(
{
...rule,
...(selectedConsumer !== undefined ? { consumer: selectedConsumer } : {}),
...(selectableConsumer && selectedConsumer !== undefined
? { consumer: selectedConsumer }
: {}),
} as Rule,
ruleType,
config
),
[rule, selectedConsumer, ruleType, config]
[rule, selectedConsumer, selectableConsumer, ruleType, config]
);

// Confirm before saving if user is able to add actions but hasn't added any to this rule
Expand All @@ -235,7 +242,7 @@ const RuleAdd = ({
http,
rule: {
...rule,
...(selectedConsumer ? { consumer: selectedConsumer } : {}),
...(selectableConsumer && selectedConsumer ? { consumer: selectedConsumer } : {}),
} as RuleUpdates,
});
toasts.addSuccess(
Expand Down Expand Up @@ -298,14 +305,14 @@ const RuleAdd = ({
}
)}
validConsumers={validConsumers}
selectedConsumer={selectedConsumer}
actionTypeRegistry={actionTypeRegistry}
ruleTypeRegistry={ruleTypeRegistry}
metadata={metadata}
filteredRuleTypes={filteredRuleTypes}
hideGrouping={hideGrouping}
hideInterval={hideInterval}
onChangeMetaData={onChangeMetaData}
selectedConsumer={selectedConsumer}
setConsumer={setSelectedConsumer}
useRuleProducer={useRuleProducer}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ describe('rule_form', () => {
expect(mockSetConsumer).toHaveBeenLastCalledWith('infrastructure');
});

it('should be able to select multiple consumer', async () => {
it('should render multiple consumers in the dropdown and select the first one in the list if no default is specified', async () => {
await setup({
initialRuleOverwrite: {
name: 'Simple rule',
Expand Down Expand Up @@ -660,7 +660,7 @@ describe('rule_form', () => {
wrapper.update();
});

expect(mockSetConsumer).toHaveBeenLastCalledWith(null);
expect(mockSetConsumer).toHaveBeenLastCalledWith('infrastructure');
});

it('should not display the consumer select for invalid rule types', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export const RuleForm = ({
setHasActionsDisabled,
setHasActionsWithBrokenConnector,
setConsumer = NOOP,
selectedConsumer,
operation,
ruleTypeRegistry,
actionTypeRegistry,
Expand All @@ -177,7 +178,6 @@ export const RuleForm = ({
hideGrouping = false,
hideInterval,
connectorFeatureId = AlertingConnectorFeatureId,
selectedConsumer,
validConsumers,
onChangeMetaData,
useRuleProducer,
Expand Down Expand Up @@ -253,11 +253,11 @@ export const RuleForm = ({
validConsumers,
})
)
.filter((item) => {
return rule.consumer === ALERTS_FEATURE_ID
.filter((item) =>
rule.consumer === ALERTS_FEATURE_ID
? !item.ruleTypeModel.requiresAppContext
: item.ruleType!.producer === rule.consumer;
});
: item.ruleType!.producer === rule.consumer
);

const availableRuleTypesResult = getAvailableRuleTypes(ruleTypes);
setAvailableRuleTypes(availableRuleTypesResult);
Expand Down Expand Up @@ -427,6 +427,7 @@ export const RuleForm = ({
const selectedRuleType = availableRuleTypes.find(
({ ruleType: availableRuleType }) => availableRuleType.id === rule.ruleTypeId
);

if (!selectedRuleType?.ruleType?.authorizedConsumers) {
return [];
}
Expand Down Expand Up @@ -812,6 +813,7 @@ export const RuleForm = ({
consumers={authorizedConsumers}
onChange={setConsumer}
errors={errors}
selectedConsumer={selectedConsumer}
/>
</EuiFlexItem>
</>
Expand Down
Loading