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

[POC] [Response Ops] Onboard detection rules to use alerting framework summaries. #147539

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
600af4f
Get and inject the alerts summary data in to actions
ersin-erdal Dec 12, 2022
e64eb64
Fix throttling action bug
ersin-erdal Dec 12, 2022
20e2ed1
Fix throttling action bug
ersin-erdal Dec 12, 2022
0525af6
Fix throttling action bug
ersin-erdal Dec 12, 2022
4204595
fix hasThrottle check
ersin-erdal Dec 12, 2022
7d21829
fix typos
ersin-erdal Dec 13, 2022
3d137a1
change summary action execution order, move action functions to helper
ersin-erdal Dec 13, 2022
7a8b83c
Merge branch 'main' into 143376-alerts-summary
ersin-erdal Dec 13, 2022
eda6aa3
Initial commit to set frequency within each rule action
ymao1 Dec 13, 2022
441427f
Commenting out schedule actions inside create security rule type wrap…
ymao1 Dec 14, 2022
f267e88
add integration tests
ersin-erdal Dec 14, 2022
3728778
Merge remote-tracking branch 'origin/143376-alerts-summary' into 1433…
ersin-erdal Dec 14, 2022
c0fdb1c
Merge branch 'main' of https://github.com/elastic/kibana into poc/onb…
ymao1 Dec 14, 2022
81b1a67
Merge branch 'pr/147360' into poc/onboard-detection-rules
ymao1 Dec 14, 2022
8281156
add getSummarizedAlerts check
ersin-erdal Dec 14, 2022
25213b6
skip scheduling summary action when there is no alerts
ersin-erdal Dec 14, 2022
526f376
Adding autoRecoverAlerts flag to rule type to determine whether recov…
ymao1 Dec 14, 2022
66e1d4a
merge
ersin-erdal Dec 14, 2022
d7b8c43
refactor integration tests
ersin-erdal Dec 14, 2022
b289217
Moving security context variables to server. Adding summaryBuilder fu…
ymao1 Dec 14, 2022
b1c0cc9
Merge branch 'pr/147360' into poc/onboard-detection-rules
ymao1 Dec 14, 2022
e732c7b
Merging in main
ymao1 Dec 15, 2022
24881f1
Aliasing context variables
ymao1 Dec 15, 2022
82fd509
Cleanup
ymao1 Dec 15, 2022
9bd69a0
Fixing result_link
ymao1 Dec 16, 2022
63393ed
Merging in main
ymao1 Dec 20, 2022
acd919b
Formatting alerts
ymao1 Dec 20, 2022
8320dd6
Setting throttle interval always
ymao1 Dec 20, 2022
4d86684
Add frequency support on legacy action migration
e40pud Jan 9, 2023
6a27c94
Merge pull request #11 from e40pud/poc/onboard-detection-rules-frequency
ymao1 Jan 9, 2023
650d275
Merging in main
ymao1 Feb 9, 2023
dea0cdd
Merge branch 'poc/onboard-detection-rules' of github.com:ymao1/kibana…
ymao1 Feb 9, 2023
9406c5b
Merging in main
ymao1 Feb 15, 2023
cf9be1e
Cleanup
ymao1 Feb 15, 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
19 changes: 19 additions & 0 deletions x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,25 @@ export interface AlertsHealth {
};
}

export interface SummarizedAlertsWithAll {
new: {
count: number;
data: unknown[];
};
ongoing: {
count: number;
data: unknown[];
};
recovered: {
count: number;
data: unknown[];
};
all: {
count: number;
data: unknown[];
};
}

export interface ActionVariable {
name: string;
description: string;
Expand Down
13 changes: 11 additions & 2 deletions x-pack/plugins/alerting/server/lib/process_alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface ProcessAlertsOpts<
previouslyRecoveredAlerts: Record<string, Alert<State, Context>>;
hasReachedAlertLimit: boolean;
alertLimit: number;
autoRecoverAlerts: boolean;
// flag used to determine whether or not we want to push the flapping state on to the flappingHistory array
setFlapping: boolean;
}
Expand Down Expand Up @@ -47,6 +48,7 @@ export function processAlerts<
previouslyRecoveredAlerts,
hasReachedAlertLimit,
alertLimit,
autoRecoverAlerts,
setFlapping,
}: ProcessAlertsOpts<State, Context>): ProcessAlertsResult<
State,
Expand All @@ -62,7 +64,13 @@ export function processAlerts<
alertLimit,
setFlapping
)
: processAlertsHelper(alerts, existingAlerts, previouslyRecoveredAlerts, setFlapping);
: processAlertsHelper(
alerts,
existingAlerts,
previouslyRecoveredAlerts,
autoRecoverAlerts,
setFlapping
);
}

function processAlertsHelper<
Expand All @@ -74,6 +82,7 @@ function processAlertsHelper<
alerts: Record<string, Alert<State, Context>>,
existingAlerts: Record<string, Alert<State, Context>>,
previouslyRecoveredAlerts: Record<string, Alert<State, Context>>,
autoRecoverAlerts: boolean,
setFlapping: boolean
): ProcessAlertsResult<State, Context, ActionGroupIds, RecoveryActionGroupId> {
const existingAlertIds = new Set(Object.keys(existingAlerts));
Expand Down Expand Up @@ -123,7 +132,7 @@ function processAlertsHelper<
updateAlertFlappingHistory(activeAlerts[id], false);
}
}
} else if (existingAlertIds.has(id)) {
} else if (existingAlertIds.has(id) && autoRecoverAlerts) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skips this step for persistent alert rule types (slight time optimization when running a rule)

recoveredAlerts[id] = alerts[id];
currentRecoveredAlerts[id] = alerts[id];

Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ export class AlertingPlugin {
ruleType.cancelAlertsOnRuleTimeout =
ruleType.cancelAlertsOnRuleTimeout ?? this.config.cancelAlertsOnRuleTimeout;
ruleType.doesSetRecoveryContext = ruleType.doesSetRecoveryContext ?? false;
ruleType.autoRecoverAlerts =
ruleType.autoRecoverAlerts === undefined ? true : ruleType.autoRecoverAlerts;
ruleTypeRegistry.register(ruleType);
},
getSecurityHealth: async () => {
Expand Down
61 changes: 55 additions & 6 deletions x-pack/plugins/alerting/server/task_runner/execution_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ExecuteOptions as EnqueueExecutionOptions } from '@kbn/actions-plugin/s
import { ActionsClient } from '@kbn/actions-plugin/server/actions_client';
import { chunk } from 'lodash';
import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger';
import { parseDuration, RawRule, ThrottledActions } from '../types';
import { GetRuleUrlFnOpts, parseDuration, RawRule, ThrottledActions } from '../types';
import { RuleRunMetricsStore } from '../lib/rule_run_metrics_store';
import { injectActionParams } from './inject_action_params';
import { ExecutionHandlerOptions, RuleTaskInstance } from './types';
Expand Down Expand Up @@ -201,7 +201,7 @@ export class ExecutionHandler<
if (isSummaryActionPerRuleRun(action) && !this.hasAlerts(alerts)) {
continue;
}
const summarizedAlerts = await this.getSummarizedAlerts({
const { startMs, endMs, summarizedAlerts } = await this.getSummarizedAlerts({
action,
spaceId,
ruleId,
Expand All @@ -222,7 +222,13 @@ export class ExecutionHandler<
actionsPlugin,
actionTypeId,
kibanaBaseUrl: this.taskRunnerContext.kibanaBaseUrl,
ruleUrl: this.buildRuleUrl(spaceId),
ruleUrl: this.buildRuleUrl({
id: ruleId,
spaceId,
params: this.rule.params,
startMs,
endMs,
}),
}),
}),
};
Expand Down Expand Up @@ -267,7 +273,11 @@ export class ExecutionHandler<
kibanaBaseUrl: this.taskRunnerContext.kibanaBaseUrl,
alertParams: this.rule.params,
actionParams: action.params,
ruleUrl: this.buildRuleUrl(spaceId),
ruleUrl: this.buildRuleUrl({
id: ruleId,
spaceId,
params: this.rule.params,
}),
flapping: executableAlert.getFlapping(),
}),
}),
Expand Down Expand Up @@ -401,7 +411,24 @@ export class ExecutionHandler<
return alert.getScheduledActionOptions()?.actionGroup || this.ruleType.recoveryActionGroup.id;
}

private buildRuleUrl(spaceId: string): string | undefined {
private buildRuleUrl({
id,
params,
spaceId,
startMs,
endMs,
}: GetRuleUrlFnOpts<Params>): string | undefined {
// Use the rule type's getRuleUrl callback if defined
// This does not necessarily require `kibanaBaseUrl` to be defined
const ruleTypeUrl = this.ruleType.getRuleUrl
? this.ruleType?.getRuleUrl({ id, params, spaceId, startMs, endMs })
: null;

if (ruleTypeUrl) {
return ruleTypeUrl;
}

// Fallback to generic rule urle
if (!this.taskRunnerContext.kibanaBaseUrl) {
return;
}
Expand Down Expand Up @@ -543,13 +570,35 @@ export class ExecutionHandler<
const alerts = await this.ruleType.getSummarizedAlerts!(options);

const total = alerts.new.count + alerts.ongoing.count + alerts.recovered.count;
return {
const summarizedAlerts = {
...alerts,
all: {
count: total,
data: [...alerts.new.data, ...alerts.ongoing.data, ...alerts.recovered.data],
},
};

if (summarizedAlerts.all.count > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returns the time bounds for summarized alerts that can be used for building rule URLs. This ensures that the time bounds used to load alerts in the UI matches the time bounds for the alert summary so the alert counts match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calculating it from the summarized alerts instead of using existing time bounds because when we query for alerts per rule execution UUID, we don't have existing time bounds.

// get the time bounds for this alert array
const timestampMillis: number[] = summarizedAlerts.all.data
.map((alert: unknown) => {
const timestamp = (alert as { '@timestamp': string })['@timestamp'];
if (timestamp) {
return new Date(timestamp).valueOf();
}
return null;
})
.filter((timeInMillis: number | null) => null != timeInMillis)
.sort() as number[];

return {
startMs: timestampMillis[0],
endMs: timestampMillis[timestampMillis.length - 1],
summarizedAlerts,
};
}

return { summarizedAlerts };
}

private async actionRunOrAddToBulk({
Expand Down
23 changes: 17 additions & 6 deletions x-pack/plugins/alerting/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
RuleTypeState,
parseDuration,
WithoutReservedActionGroups,
RawAlertInstance,
} from '../../common';
import { NormalizedRuleType, UntypedNormalizedRuleType } from '../rule_type_registry';
import { getEsErrorMessage } from '../lib/errors';
Expand Down Expand Up @@ -415,6 +416,8 @@ export class TaskRunner<
previouslyRecoveredAlerts: originalRecoveredAlerts,
hasReachedAlertLimit,
alertLimit: this.maxAlerts,
autoRecoverAlerts:
this.ruleType.autoRecoverAlerts !== undefined ? this.ruleType.autoRecoverAlerts : true,
setFlapping: true,
});

Expand Down Expand Up @@ -479,12 +482,20 @@ export class TaskRunner<
}
});

const { alertsToReturn, recoveredAlertsToReturn } = determineAlertsToReturn<
State,
Context,
ActionGroupIds,
RecoveryActionGroupId
>(activeAlerts, recoveredAlerts);
let alertsToReturn: Record<string, RawAlertInstance> = {};
let recoveredAlertsToReturn: Record<string, RawAlertInstance> = {};

// Only serialize alerts into task state if we're auto-recovering, otherwise
// we don't need to keep this information around.
if (this.ruleType.autoRecoverAlerts) {
const { alertsToReturn: alerts, recoveredAlertsToReturn: recovered } =
determineAlertsToReturn<State, Context, ActionGroupIds, RecoveryActionGroupId>(
activeAlerts,
recoveredAlerts
);
alertsToReturn = alerts;
recoveredAlertsToReturn = recovered;
}

return {
metrics: ruleRunMetricsStore.getMetrics(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
AlertInstanceContext,
RuleTypeParams,
SanitizedRule,
SummarizedAlertsWithAll,
} from '../types';

interface TransformActionParamsOptions {
Expand All @@ -35,25 +36,6 @@ interface TransformActionParamsOptions {
flapping: boolean;
}

interface SummarizedAlertsWithAll {
new: {
count: number;
data: unknown[];
};
ongoing: {
count: number;
data: unknown[];
};
recovered: {
count: number;
data: unknown[];
};
all: {
count: number;
data: unknown[];
};
}

export function transformActionParams({
actionsPlugin,
alertId,
Expand Down Expand Up @@ -140,6 +122,15 @@ export function transformSummaryActionParams({
const variables = {
kibanaBaseUrl,
date: new Date().toISOString(),
// For backwards compatibility with security solutions rules
context: {
alerts: alerts.new.data,
results_link: ruleUrl,
rule: rule.params,
},
state: {
signals_count: alerts.new.count,
},
rule: {
params: rule.params,
id: rule.id,
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,16 @@ export interface SummarizedAlerts {
}
export type GetSummarizedAlertsFn = (opts: GetSummarizedAlertsFnOpts) => Promise<SummarizedAlerts>;

export interface GetRuleUrlFnOpts<Params extends RuleTypeParams> {
id: string;
params: Params;
spaceId: string;
startMs?: number;
endMs?: number;
}
export type GetRuleUrlFn<Params extends RuleTypeParams> = (
opts: GetRuleUrlFnOpts<Params>
) => string | null;
export interface RuleType<
Params extends RuleTypeParams = never,
ExtractedParams extends RuleTypeParams = never,
Expand Down Expand Up @@ -197,6 +207,13 @@ export interface RuleType<
cancelAlertsOnRuleTimeout?: boolean;
doesSetRecoveryContext?: boolean;
getSummarizedAlerts?: GetSummarizedAlertsFn;
getRuleUrl?: GetRuleUrlFn<Params>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

allows rule types to specify custom function for building rule URLs


/**
* Determines whether framework should
* automatically make recovery determination. Defaults to true.
*/
autoRecoverAlerts?: boolean;
}
export type UntypedRuleType = RuleType<
RuleTypeParams,
Expand Down
Loading