-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Conversation
…per and adding it to persistence rule type
…76-alerts-summary
…oard-detection-rules
…ery alerts should be calculated and alerts stored in task state. Hacking 1h throttle to 5m for testing
…nction to action variable definition
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
@@ -123,7 +132,7 @@ function processAlertsHelper< | |||
updateAlertFlappingHistory(activeAlerts[id], false); | |||
} | |||
} | |||
} else if (existingAlertIds.has(id)) { | |||
} else if (existingAlertIds.has(id) && autoRecoverAlerts) { |
There was a problem hiding this comment.
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)
@@ -197,6 +207,13 @@ export interface RuleType< | |||
cancelAlertsOnRuleTimeout?: boolean; | |||
doesSetRecoveryContext?: boolean; | |||
getSummarizedAlerts?: GetSummarizedAlertsFn; | |||
getRuleUrl?: GetRuleUrlFn<Params>; |
There was a problem hiding this comment.
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
...alerts, | ||
all: { | ||
count: total, | ||
data: [...alerts.new.data, ...alerts.ongoing.data, ...alerts.recovered.data], | ||
}, | ||
}; | ||
|
||
if (summarizedAlerts.all.count > 0) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -32,6 +33,7 @@ interface CreateGetSummarizedAlertsFnOpts { | |||
ruleDataClient: PublicContract<IRuleDataClient>; | |||
useNamespace: boolean; | |||
isLifecycleAlert: boolean; | |||
formatAlert?: (alert: unknown) => unknown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows rule types to pass in custom functions for formatting summarized alerts. This allows detection rules to be formatted exactly as they were before.
}) | ||
.filter((_, idx) => response.body.items[idx].create?.status === 201); | ||
|
||
createdAlerts.forEach((alert) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporting alerts 1-1 back to the framework.
frequency: { | ||
summary: true, | ||
notifyWhen: 'onThrottleInterval', | ||
throttle: throttle === '1h' ? '5m' : throttle, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting 'hourly' to 5 minutes for easier testing.
Add frequency support on legacy action migration
… into poc/onboard-detection-rules
.filter((_, idx) => response.body.items[idx].create?.status === 201); | ||
|
||
createdAlerts.forEach((alert) => | ||
options.services.alertFactory.create(alert._id).scheduleActions('default', {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options.services.alertFactory.create(alert._id).scheduleActions('default', {}) | |
options.services.alertFactory.create(alert._id).scheduleActions('default', { | |
alerts: [alert], | |
results_link: <ruleUrl> | |
}).replaceState({ signals_count: 1}); |
Towards #147379
Summary
POC to remove custom notification scheduling from detection rules in favor of reporting alerts 1-to-1 back to the alerting platform and using the new alert summaries feature.
In this POC:
Not in this POC: