Skip to content

Commit

Permalink
[Logs Explorer] Add ability to create alerts within the Explorer app (e…
Browse files Browse the repository at this point in the history
…lastic#175777)

## Summary
Users in Logs Explorer should be able to define rules that will create
alerts when the number of logs passes a certain threshold. As part of
this issue, support for the new custom threshold rule will be added in
the header.

## Screenshot

<img width="1266" alt="Screenshot 2024-01-29 at 12 48 28"
src="https://github.com/elastic/kibana/assets/190132/0a141fa9-726c-4a15-a085-f124d39071f2">

## Notes for reviewers

Apologies for the noise across multiple plugins but I tried to update
the `RuleAddProps` type since it offered no type safety making it
incredibly difficult and error prone to understand what properties are
required for the selected rule type.

The lack of type safety in the exposed alerting/rules related public
APIs caused various bugs during integration, each dependant on specific
conditions and requiring a lot of manual testing. I have tried to fix as
many of these bugs as possible but had to base it mainly on trial and
error due to the lack of correct typing with too many optional (but in
reality required) properties.

An example of this are filter badges in the universal search component.
These were not displayed correctly due to missing props on the
`FilterMeta` interface which are all marked as optional but somehow
assumed to be there by the UI components that render them.

Another issue was caused by implicit service dependencies with no
validation in place by consuming components. An example of this is the
`triggersActionsUi.getAddRuleFlyout` method which requires
`unifiedSearch`, `dataViews`, `dataViewEditor` and `lens` services in
React context but for the majority silently fails causing bugs only
under specific conditions or when trying to carry out specific actions.
Integration is made even more difficult since these can differ between
different rule types. It would be great if these are made either
explicit or if validation is put in place to warn developers of
integration issues.

There is also an existing bug in that filters displayed by the universal
search component provide the ability to edit the filter but when
attempting to do so and clicking "Update filter" to confirm nothing
happens despite the `SearchBar.onFiltersUpdated` being defined. I have
tested this behaviour in other integrations which all have the same bugs
so am treating these as existing issues.

## Acceptance criteria
- Add an `Alerts` item to the header that will include two options:
  - `Create rule` that will open the custom threshold rule flyout
- `Manage rules` that will link to the observability rules management
page (`app/observability/alerts/rules`)
- Set default configuration that will be used in the flyout
  - The Ad Hoc data view that is created as part of the selector
  - The query from the KQL bar
  - Role visibility should be hidden

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Maryam Saeidi <[email protected]>
  • Loading branch information
3 people authored Feb 9, 2024
1 parent 5ab0240 commit 239b957
Show file tree
Hide file tree
Showing 34 changed files with 650 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
RuleCreationValidConsumer,
STACK_ALERTS_FEATURE_ID,
} from '@kbn/rule-data-utils';
import { RuleTypeMetaData } from '@kbn/alerting-plugin/common';
import { DiscoverStateContainer } from '../../services/discover_state';
import { DiscoverServices } from '../../../../build_services';

Expand All @@ -42,7 +43,7 @@ interface AlertsPopoverProps {
isPlainRecord?: boolean;
}

interface EsQueryAlertMetaData {
interface EsQueryAlertMetaData extends RuleTypeMetaData {
isManagementPage?: boolean;
adHocDataViewList: DataView[];
}
Expand Down Expand Up @@ -110,11 +111,11 @@ export function AlertsPopover({
metadata: discoverMetadata,
consumer: 'alerts',
onClose: (_, metadata) => {
onFinishFlyoutInteraction(metadata as EsQueryAlertMetaData);
onFinishFlyoutInteraction(metadata!);
onClose();
},
onSave: async (metadata) => {
onFinishFlyoutInteraction(metadata as EsQueryAlertMetaData);
onFinishFlyoutInteraction(metadata!);
},
canChangeTrigger: false,
ruleTypeId: ES_QUERY_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function FilterBadge({
`}
>
<EuiTextBlockTruncate lines={10}>
{!hideAlias && filter.meta.alias !== null ? (
{filter.meta.alias && !hideAlias ? (
<>
<span className={marginLeftLabelCss(euiTheme)}>
{prefix}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type { ActionVariable } from '@kbn/alerting-types';

export type RuleTypeState = Record<string, unknown>;
export type RuleTypeParams = Record<string, unknown>;
export type RuleTypeMetaData = Record<string, unknown>;

// rule type defined alert fields to persist in alerts index
export type RuleAlertData = Record<string, unknown>;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React, { useCallback, useMemo } from 'react';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { ApmRuleType } from '@kbn/rule-data-utils';
import type { RuleTypeParams } from '@kbn/alerting-plugin/common';
import { APM_SERVER_FEATURE_ID } from '../../../../../common/rules/apm_rule_types';
import { getInitialAlertValues } from '../../utils/get_initial_alert_values';
import { ApmPluginStartDeps } from '../../../../plugin';
Expand Down Expand Up @@ -35,7 +36,7 @@ export function AlertingFlyout(props: Props) {
const { start, end } = useTimeRange({ rangeFrom, rangeTo, optional: true });

const environment =
'environment' in query ? query.environment : ENVIRONMENT_ALL.value;
'environment' in query ? query.environment! : ENVIRONMENT_ALL.value;
const transactionType =
'transactionType' in query ? query.transactionType : undefined;
const transactionName =
Expand All @@ -53,7 +54,10 @@ export function AlertingFlyout(props: Props) {
const addAlertFlyout = useMemo(
() =>
ruleType &&
services.triggersActionsUi.getAddRuleFlyout({
services.triggersActionsUi.getAddRuleFlyout<
RuleTypeParams,
AlertMetadata
>({
consumer: APM_SERVER_FEATURE_ID,
onClose: onCloseAddFlyout,
ruleTypeId: ruleType,
Expand All @@ -67,7 +71,7 @@ export function AlertingFlyout(props: Props) {
errorGroupingKey,
start,
end,
} as AlertMetadata,
},
useRuleProducer: true,
}),
/* eslint-disable-next-line react-hooks/exhaustive-deps */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
*/

import { TIME_UNITS } from '@kbn/triggers-actions-ui-plugin/public';
import type { RuleTypeMetaData } from '@kbn/alerting-plugin/common';

import moment from 'moment';

export interface AlertMetadata {
export interface AlertMetadata extends RuleTypeMetaData {
environment: string;
serviceName?: string;
transactionType?: string;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { Query } from '@kbn/es-query';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import React, { useEffect, useState } from 'react';
Expand Down Expand Up @@ -54,7 +53,6 @@ import { LogRateAnalysis } from './log_rate_analysis';
import { Groups } from './groups';
import { Tags } from './tags';
import { RuleConditionChart } from '../rule_condition_chart/rule_condition_chart';
import { getFilterQuery } from './helpers/get_filter_query';

// TODO Use a generic props for app sections https://github.com/elastic/kibana/issues/152690
export type CustomThresholdRule = Rule<CustomThresholdRuleTypeParams>;
Expand Down Expand Up @@ -118,7 +116,6 @@ export default function AlertDetailsAppSection({
const { euiTheme } = useEuiTheme();
const hasLogRateAnalysisLicense = hasAtLeast('platinum');
const [dataView, setDataView] = useState<DataView>();
const [filterQuery, setFilterQuery] = useState<string>('');
const [, setDataViewError] = useState<Error>();
const ruleParams = rule.params as RuleTypeParams & AlertParams;
const chartProps = {
Expand Down Expand Up @@ -204,11 +201,6 @@ export default function AlertDetailsAppSection({
setAlertSummaryFields(alertSummaryFields);
}, [groups, tags, rule, ruleLink, setAlertSummaryFields]);

useEffect(() => {
const query = `${(ruleParams.searchConfiguration?.query as Query)?.query as string}`;
setFilterQuery(getFilterQuery(query, groups));
}, [groups, ruleParams.searchConfiguration]);

useEffect(() => {
const initDataView = async () => {
const ruleSearchConfiguration = ruleParams.searchConfiguration;
Expand Down Expand Up @@ -271,7 +263,7 @@ export default function AlertDetailsAppSection({
<RuleConditionChart
metricExpression={criterion}
dataView={dataView}
filterQuery={filterQuery}
searchConfiguration={ruleParams.searchConfiguration}
groupBy={ruleParams.groupBy}
annotations={annotations}
timeRange={timeRange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React from 'react';
import { SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { act } from 'react-dom/test-utils';
import { DataView } from '@kbn/data-views-plugin/common';
import { mountWithIntl, nextTick } from '@kbn/test-jest-helpers';
Expand Down Expand Up @@ -35,7 +36,7 @@ describe('Rule condition chart', () => {
<RuleConditionChart
metricExpression={expression}
dataView={dataView}
filterQuery={''}
searchConfiguration={{} as SerializedSearchSourceFields}
groupBy={[]}
error={{}}
timeRange={{ from: 'now-15m', to: 'now' }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { useState, useEffect } from 'react';
import { SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { EuiEmptyPrompt, useEuiTheme } from '@elastic/eui';
import { Query } from '@kbn/es-query';
import { FillStyle, SeriesType } from '@kbn/lens-plugin/public';
import { DataView } from '@kbn/data-views-plugin/common';
import { FormattedMessage } from '@kbn/i18n-react';
Expand Down Expand Up @@ -38,19 +41,24 @@ import {

interface RuleConditionChartProps {
metricExpression: MetricExpression;
searchConfiguration: SerializedSearchSourceFields;
dataView?: DataView;
filterQuery?: string;
groupBy?: string | string[];
error?: IErrorObject;
timeRange: TimeRange;
annotations?: EventAnnotationConfig[];
seriesType?: SeriesType;
}

const defaultQuery: Query = {
language: 'kuery',
query: '',
};

export function RuleConditionChart({
metricExpression,
searchConfiguration,
dataView,
filterQuery,
groupBy,
error,
annotations,
Expand Down Expand Up @@ -283,7 +291,7 @@ export function RuleConditionChart({
comparator,
dataView,
equation,
filterQuery,
searchConfiguration,
formula,
formulaAsync.value,
groupBy,
Expand Down Expand Up @@ -337,10 +345,8 @@ export function RuleConditionChart({
timeRange={timeRange}
attributes={attributes}
disableTriggers={true}
query={{
language: 'kuery',
query: filterQuery || '',
}}
query={(searchConfiguration.query as Query) || defaultQuery}
filters={searchConfiguration.filter}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,14 +405,24 @@ export default function Expressions(props: Props) {
indexPatterns={dataView ? [dataView] : undefined}
showQueryInput={true}
showQueryMenu={false}
showFilterBar={false}
showFilterBar={!!ruleParams.searchConfiguration?.filter}
showDatePicker={false}
showSubmitButton={false}
displayStyle="inPage"
onQueryChange={debouncedOnFilterChange}
onQuerySubmit={onFilterChange}
dataTestSubj="thresholdRuleUnifiedSearchBar"
query={ruleParams.searchConfiguration?.query as Query}
filters={ruleParams.searchConfiguration?.filter}
onFiltersUpdated={(filter) => {
// Since rule params will be sent to the API as is, and we only need meta and query parameters to be
// saved in the rule's saved object, we filter extra fields here (such as $state).
const filters = filter.map(({ meta, query }) => ({ meta, query }));
setRuleParams('searchConfiguration', {
...ruleParams.searchConfiguration,
filter: filters,
});
}}
/>
{errors.filterQuery && (
<EuiFormErrorText data-test-subj="thresholdRuleDataViewErrorNoTimestamp">
Expand Down Expand Up @@ -454,7 +464,7 @@ export default function Expressions(props: Props) {
<PreviewChart
metricExpression={e}
dataView={dataView}
filterQuery={(ruleParams.searchConfiguration?.query as Query)?.query as string}
searchConfiguration={ruleParams.searchConfiguration}
groupBy={ruleParams.groupBy}
error={(errors[idx] as IErrorObject) || emptyError}
timeRange={{ from: `now-${(timeSize ?? 1) * 20}${timeUnit}`, to: 'now' }}
Expand Down
Loading

0 comments on commit 239b957

Please sign in to comment.