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

[RAM] Mark disabled alerts as Untracked in both Stack and o11y #164788

Merged
merged 62 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
a801e1e
[RAM] Mark disabled alerts as Untracked in both Stack and o11y
Zacqary Aug 24, 2023
8ac28c3
Add untracked filter to alerts list
Zacqary Aug 24, 2023
1db7232
Fix types
Zacqary Aug 24, 2023
7ef4cd7
Update registered task type test
Zacqary Aug 25, 2023
8488fe9
Fix disable rule event test
Zacqary Aug 25, 2023
a4ac597
Fix jest and typecheck
Zacqary Aug 25, 2023
5ff37f3
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Aug 25, 2023
3cf2e5a
Fix jest
Zacqary Aug 25, 2023
839d858
Fix jest snapshots
Zacqary Aug 25, 2023
d185d41
Fix registered task type test sorting
Zacqary Aug 25, 2023
9f37561
Fix event validator
Zacqary Aug 25, 2023
f4b61ff
Fix alert summary test
Zacqary Aug 25, 2023
d56ab0c
Fix rule type registry jest
Zacqary Aug 25, 2023
dcb5a77
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Aug 28, 2023
99b644f
Clean up typedef and add jest test for untrack
Zacqary Aug 28, 2023
72b9f60
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Aug 29, 2023
2db2f5f
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Aug 30, 2023
90acc70
Fix bad merge
Zacqary Aug 30, 2023
79d6ac2
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 11, 2023
1f243c6
Remove task spawner and use AAD methods to update alerts
Zacqary Sep 11, 2023
84f707f
Remove comment
Zacqary Sep 11, 2023
1de7e21
Type fix
Zacqary Sep 11, 2023
2903a3b
Fix typecheck
Zacqary Sep 11, 2023
87209f5
Fix Jest
Zacqary Sep 11, 2023
870a131
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 11, 2023
ac26389
Iterate over indicesAlertIdMap
Zacqary Sep 13, 2023
ce89b91
Refactor untrack function to just use ruleId
Zacqary Sep 13, 2023
ddb2cd1
Restore alertUuid
Zacqary Sep 13, 2023
bac60b8
Fix typecheck
Zacqary Sep 13, 2023
a18cf49
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 13, 2023
3ba23a2
Add logging for untrack failures
Zacqary Sep 14, 2023
2fd6a11
Move alertsService to rulesClient
Zacqary Sep 14, 2023
a8b37f2
Fix typecheck
Zacqary Sep 14, 2023
36283a5
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 18, 2023
dd412bd
Use autoRecoverAlerts as isLifecycleAlert
Zacqary Sep 18, 2023
dae99ee
Add test for untrackRuleIdsByIndices
Zacqary Sep 18, 2023
e7f3403
Typecheck fix
Zacqary Sep 18, 2023
0624ea6
Handle un-flattened AAD fields
Zacqary Sep 20, 2023
2d6ffd7
untrackRuleIdsByIndices -> setAlertStatusToUntracked
Zacqary Sep 20, 2023
2867b41
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 20, 2023
6259846
Fix lifecycle executor event log to support untracked alerts
Zacqary Sep 20, 2023
e1e74fa
Fix untracked alert table filter query
Zacqary Sep 20, 2023
7ff8935
Fix typecheck
Zacqary Sep 21, 2023
0b4aba7
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 21, 2023
cbb17c1
Fix typecheck
Zacqary Sep 21, 2023
895e9ac
Move alert state clearing into bulkDisable function
Zacqary Sep 25, 2023
ef6457f
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 25, 2023
5b384e6
Fix typecheck
Zacqary Sep 25, 2023
30d4d88
Fix jest
Zacqary Sep 25, 2023
c2ec732
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 25, 2023
f512be7
Fix tests
Zacqary Sep 26, 2023
f3407f1
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 26, 2023
645e1e6
Revert query test change
Zacqary Sep 26, 2023
e6ee727
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 26, 2023
29d3192
Allow bulkDisable to take array of task ids to remove
Zacqary Sep 26, 2023
b94e238
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 26, 2023
38e29a1
Revert "Allow bulkDisable to take array of task ids to remove"
Zacqary Sep 27, 2023
a669d03
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 27, 2023
f3b85df
Revert "Revert "Allow bulkDisable to take array of task ids to remove""
Zacqary Sep 27, 2023
cda79f1
Add isLifecycleAlert check to disable method
Zacqary Sep 27, 2023
8df838e
Merge remote-tracking branch 'upstream/main' into 164059-untracked-di…
Zacqary Sep 27, 2023
c6c1ef1
Fix jest
Zacqary Sep 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import React, { memo } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiBadge, EuiBadgeProps } from '@elastic/eui';
import { AlertStatus, ALERT_STATUS_RECOVERED } from '@kbn/rule-data-utils';
import { AlertStatus, ALERT_STATUS_RECOVERED, ALERT_STATUS_UNTRACKED } from '@kbn/rule-data-utils';

export interface AlertLifecycleStatusBadgeProps {
alertStatus: AlertStatus;
Expand Down Expand Up @@ -37,15 +37,27 @@ const FLAPPING_LABEL = i18n.translate(
}
);

const UNTRACKED_LABEL = i18n.translate(
'alertsUIShared.components.alertLifecycleStatusBadge.untrackedLabel',
{
defaultMessage: 'Untracked',
}
);

interface BadgeProps {
label: string;
color: string;
isDisabled?: boolean;
iconProps?: {
iconType: EuiBadgeProps['iconType'];
};
}

const getBadgeProps = (alertStatus: AlertStatus, flapping: boolean | undefined): BadgeProps => {
if (alertStatus === ALERT_STATUS_UNTRACKED) {
return { label: UNTRACKED_LABEL, color: 'default', isDisabled: true };
}

// Prefer recovered over flapping
if (alertStatus === ALERT_STATUS_RECOVERED) {
return {
Expand Down Expand Up @@ -82,10 +94,15 @@ export const AlertLifecycleStatusBadge = memo((props: AlertLifecycleStatusBadgeP

const castedFlapping = castFlapping(flapping);

const { label, color, iconProps } = getBadgeProps(alertStatus, castedFlapping);
const { label, color, iconProps, isDisabled } = getBadgeProps(alertStatus, castedFlapping);

return (
<EuiBadge data-test-subj="alertLifecycleStatusBadge" color={color} {...iconProps}>
<EuiBadge
data-test-subj="alertLifecycleStatusBadge"
isDisabled={isDisabled}
color={color}
{...iconProps}
>
{label}
</EuiBadge>
);
Expand Down
6 changes: 5 additions & 1 deletion packages/kbn-rule-data-utils/src/alerts_as_data_status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,9 @@

export const ALERT_STATUS_ACTIVE = 'active';
export const ALERT_STATUS_RECOVERED = 'recovered';
export const ALERT_STATUS_UNTRACKED = 'untracked';

export type AlertStatus = typeof ALERT_STATUS_ACTIVE | typeof ALERT_STATUS_RECOVERED;
export type AlertStatus =
| typeof ALERT_STATUS_ACTIVE
| typeof ALERT_STATUS_RECOVERED
| typeof ALERT_STATUS_UNTRACKED;
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/common/alert_summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,5 @@ export interface AlertStatus {
activeStartDate?: string;
flapping: boolean;
maintenanceWindowIds?: string[];
tracked: boolean;
XavierM marked this conversation as resolved.
Show resolved Hide resolved
}
47 changes: 47 additions & 0 deletions x-pack/plugins/alerting/server/alerts_client/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3026,6 +3026,53 @@ describe('Alerts Client', () => {
expect(recoveredAlert.hit).toBeUndefined();
});
});

describe('setAlertStatusToUntracked()', () => {
test('should call updateByQuery on provided ruleIds', async () => {
const alertsClient = new AlertsClient<{}, {}, {}, 'default', 'recovered'>(
alertsClientParams
);

const opts = {
maxAlerts,
ruleLabel: `test: rule-name`,
flappingSettings: DEFAULT_FLAPPING_SETTINGS,
activeAlertsFromState: {},
recoveredAlertsFromState: {},
};
await alertsClient.initializeExecution(opts);

await alertsClient.setAlertStatusToUntracked(['test-index'], ['test-rule']);

expect(clusterClient.updateByQuery).toHaveBeenCalledTimes(1);
});

test('should retry updateByQuery on failure', async () => {
clusterClient.updateByQuery.mockResponseOnce({
total: 10,
updated: 8,
});
const alertsClient = new AlertsClient<{}, {}, {}, 'default', 'recovered'>(
alertsClientParams
);

const opts = {
maxAlerts,
ruleLabel: `test: rule-name`,
flappingSettings: DEFAULT_FLAPPING_SETTINGS,
activeAlertsFromState: {},
recoveredAlertsFromState: {},
};
await alertsClient.initializeExecution(opts);

await alertsClient.setAlertStatusToUntracked(['test-index'], ['test-rule']);

expect(clusterClient.updateByQuery).toHaveBeenCalledTimes(2);
expect(logger.warn).toHaveBeenCalledWith(
'Attempt 1: Failed to untrack 2 of 10; indices test-index, ruleIds test-rule'
);
});
});
});
}
});
61 changes: 60 additions & 1 deletion x-pack/plugins/alerting/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
*/

import { ElasticsearchClient } from '@kbn/core/server';
import { ALERT_RULE_UUID, ALERT_UUID } from '@kbn/rule-data-utils';
import {
ALERT_RULE_UUID,
ALERT_STATUS,
ALERT_STATUS_UNTRACKED,
ALERT_STATUS_ACTIVE,
ALERT_UUID,
} from '@kbn/rule-data-utils';
import { chunk, flatMap, isEmpty, keys } from 'lodash';
import { SearchRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { Alert } from '@kbn/alerts-as-data-utils';
Expand Down Expand Up @@ -198,6 +204,51 @@ export class AlertsClient<
return { hits, total };
}

public async setAlertStatusToUntracked(indices: string[], ruleIds: string[]) {
const esClient = await this.options.elasticsearchClientPromise;
const terms: Array<{ term: Record<string, { value: string }> }> = ruleIds.map((ruleId) => ({
term: {
[ALERT_RULE_UUID]: { value: ruleId },
},
}));
terms.push({
term: {
[ALERT_STATUS]: { value: ALERT_STATUS_ACTIVE },
},
});

try {
// Retry this updateByQuery up to 3 times to make sure the number of documents
// updated equals the number of documents matched
for (let retryCount = 0; retryCount < 3; retryCount++) {
const response = await esClient.updateByQuery({
index: indices,
allow_no_indices: true,
body: {
conflicts: 'proceed',
script: {
source: UNTRACK_UPDATE_PAINLESS_SCRIPT,
lang: 'painless',
},
query: {
bool: {
must: terms,
},
},
},
});
if (response.total === response.updated) break;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we log indices and ruleIds when we have to retry if we have SDH

this.options.logger.warn(
`Attempt ${retryCount + 1}: Failed to untrack ${
(response.total ?? 0) - (response.updated ?? 0)
} of ${response.total}; indices ${indices}, ruleIds ${ruleIds}`
);
}
} catch (err) {
this.options.logger.error(`Error marking ${ruleIds} as untracked - ${err.message}`);
}
}

public report(
alert: ReportedAlert<
AlertData,
Expand Down Expand Up @@ -562,3 +613,11 @@ export class AlertsClient<
return this._isUsingDataStreams;
}
}

const UNTRACK_UPDATE_PAINLESS_SCRIPT = `
// Certain rule types don't flatten their AAD values, apply the ALERT_STATUS key to them directly
if (!ctx._source.containsKey('${ALERT_STATUS}') || ctx._source['${ALERT_STATUS}'].empty) {
ctx._source.${ALERT_STATUS} = '${ALERT_STATUS_UNTRACKED}';
} else {
ctx._source['${ALERT_STATUS}'] = '${ALERT_STATUS_UNTRACKED}'
}`;
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,8 @@ export class LegacyAlertsClient<
}

public async persistAlerts() {}

public async setAlertStatusToUntracked() {
return;
}
}
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/server/alerts_client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export interface IAlertsClient<
alertsToReturn: Record<string, RawAlertInstance>;
recoveredAlertsToReturn: Record<string, RawAlertInstance>;
};
setAlertStatusToUntracked(indices: string[], ruleIds: string[]): Promise<void>;
factory(): PublicAlertFactory<
State,
Context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
kibanaVersion,
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
getAlertIndicesAlias: jest.fn(),
alertsService: null,
maxScheduledPerMinute: 1000,
internalSavedObjectsRepository,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
minimumScheduleInterval: { value: '1m', enforce: false },
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
getAlertIndicesAlias: jest.fn(),
alertsService: null,
};

const getBulkOperationStatusErrorResponse = (statusCode: number) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
minimumScheduleInterval: { value: '1m', enforce: false },
isAuthenticationTypeAPIKey: isAuthenticationTypeApiKeyMock,
getAuthenticationAPIKey: getAuthenticationApiKeyMock,
getAlertIndicesAlias: jest.fn(),
alertsService: null,
};
const paramsModifier = jest.fn();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
minimumScheduleInterval: { value: '1m', enforce: false },
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
getAlertIndicesAlias: jest.fn(),
alertsService: null,
};

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
minimumScheduleInterval: { value: '1m', enforce: false },
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
getAlertIndicesAlias: jest.fn(),
alertsService: null,
};

const getMockAggregationResult = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": true,
"status": "OK",
"tracked": true,
"uuid": undefined,
},
"alert-2": Object {
Expand All @@ -135,6 +136,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": true,
"status": "OK",
"tracked": true,
"uuid": undefined,
},
},
Expand Down Expand Up @@ -241,6 +243,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "OK",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -283,6 +286,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "OK",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -324,6 +328,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "OK",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -366,6 +371,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -408,6 +414,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -450,6 +457,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -490,6 +498,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down Expand Up @@ -534,6 +543,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": true,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
"alert-2": Object {
Expand All @@ -542,6 +552,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": true,
"status": "OK",
"tracked": true,
"uuid": "uuid-2",
},
},
Expand Down Expand Up @@ -593,6 +604,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
"alert-2": Object {
Expand All @@ -601,6 +613,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": false,
"muted": false,
"status": "OK",
"tracked": true,
"uuid": "uuid-2",
},
},
Expand Down Expand Up @@ -639,6 +652,7 @@ describe('alertSummaryFromEventLog', () => {
"flapping": true,
"muted": false,
"status": "Active",
"tracked": true,
"uuid": "uuid-1",
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ export function alertSummaryFromEventLog(params: AlertSummaryFromEventLogParams)
status.activeStartDate = undefined;
status.actionGroupId = undefined;
}

status.tracked = action !== EVENT_LOG_ACTIONS.untrackedInstance;
}

for (const event of executionEvents.reverse()) {
Expand Down Expand Up @@ -169,6 +171,7 @@ function getAlertStatus(
actionGroupId: undefined,
activeStartDate: undefined,
flapping: false,
tracked: true,
};
alerts.set(alertId, status);
return status;
Expand Down
Loading