Skip to content

Commit

Permalink
[8.11] [SLO] Fix SLO burn rate rule to call alertWithLifecycle befo…
Browse files Browse the repository at this point in the history 
…re `getAlertUuid` (#169004) (#169026)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[SLO] Fix SLO burn rate rule to call `alertWithLifecycle` before
`getAlertUuid` (#169004)](#169004)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Chris
Cowan","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-16T19:55:39Z","message":"[SLO]
Fix SLO burn rate rule to call `alertWithLifecycle` before
`getAlertUuid` (#169004)\n\n## Summary\r\n\r\nThis PR fixes #168962 by
calling `alertWithLifecycle` before\r\n`getAlertUuid`. When
`getAlertUuid` is called first it will generate a\r\nUUID which is not
associated with the actual alert. When you call\r\n`alertWithLifecycle`,
it generates the Alert then stores the `alertUuid`\r\nthat maps the
`alertId` to the `alertUuid`; `getAlertUuid` tries to\r\nrecall this
association but falls back to generating a UUID. I had to\r\nchange the
way the test worked by modifying mock's behavior to match
the\r\nimplementation; once I had a test that failed similarly to what
we see\r\nin the real world, I fixed the issue by changing the
implementation to\r\ncall `alertWithLifecycle`
first\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"48bb5ae6797fd58f5c4076df583b32060572ca66","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
Actionable
Observability","backport:prev-minor","v8.12.0"],"number":169004,"url":"https://github.com/elastic/kibana/pull/169004","mergeCommit":{"message":"[SLO]
Fix SLO burn rate rule to call `alertWithLifecycle` before
`getAlertUuid` (#169004)\n\n## Summary\r\n\r\nThis PR fixes #168962 by
calling `alertWithLifecycle` before\r\n`getAlertUuid`. When
`getAlertUuid` is called first it will generate a\r\nUUID which is not
associated with the actual alert. When you call\r\n`alertWithLifecycle`,
it generates the Alert then stores the `alertUuid`\r\nthat maps the
`alertId` to the `alertUuid`; `getAlertUuid` tries to\r\nrecall this
association but falls back to generating a UUID. I had to\r\nchange the
way the test worked by modifying mock's behavior to match
the\r\nimplementation; once I had a test that failed similarly to what
we see\r\nin the real world, I fixed the issue by changing the
implementation to\r\ncall `alertWithLifecycle`
first\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"48bb5ae6797fd58f5c4076df583b32060572ca66"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169004","number":169004,"mergeCommit":{"message":"[SLO]
Fix SLO burn rate rule to call `alertWithLifecycle` before
`getAlertUuid` (#169004)\n\n## Summary\r\n\r\nThis PR fixes #168962 by
calling `alertWithLifecycle` before\r\n`getAlertUuid`. When
`getAlertUuid` is called first it will generate a\r\nUUID which is not
associated with the actual alert. When you call\r\n`alertWithLifecycle`,
it generates the Alert then stores the `alertUuid`\r\nthat maps the
`alertId` to the `alertUuid`; `getAlertUuid` tries to\r\nrecall this
association but falls back to generating a UUID. I had to\r\nchange the
way the test worked by modifying mock's behavior to match
the\r\nimplementation; once I had a test that failed similarly to what
we see\r\nin the real world, I fixed the issue by changing the
implementation to\r\ncall `alertWithLifecycle`
first\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"48bb5ae6797fd58f5c4076df583b32060572ca66"}}]}]
BACKPORT-->

Co-authored-by: Chris Cowan <[email protected]>
  • Loading branch information
@kibanamachine @simianhacker
kibanamachine and simianhacker authored Oct 16, 2023
1 parent 1dedb1e commit eb2a11f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 25 deletions.
24 changes: 12 additions & 12 deletions x-pack/plugins/observability/server/lib/rules/slo_burn_rate/executor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ describe('BurnRateRuleExecutor', () => {
let esClientMock: ElasticsearchClientMock;
let soClientMock: jest.Mocked<SavedObjectsClientContract>;
let loggerMock: jest.Mocked<MockedLogger>;
let alertUuidMap: Map<string, string>;
let alertMock: Partial<Alert>;
const alertUuid = 'mockedAlertUuid';
const basePathMock = { publicBaseUrl: 'https://kibana.dev' } as IBasePath;
const alertsLocatorMock = {
Expand All @@ -124,9 +126,17 @@ describe('BurnRateRuleExecutor', () => {
LifecycleAlertServices<BurnRateAlertState, BurnRateAlertContext, BurnRateAllowedActionGroups>;

beforeEach(() => {
alertUuidMap = new Map<string, string>();
alertMock = {
scheduleActions: jest.fn(),
replaceState: jest.fn(),
};
esClientMock = elasticsearchServiceMock.createElasticsearchClient();
soClientMock = savedObjectsClientMock.create();
alertWithLifecycleMock = jest.fn();
alertWithLifecycleMock = jest.fn().mockImplementation(({ id }) => {
alertUuidMap.set(id, alertUuid);
return alertMock as any;
});
alertFactoryMock = {
create: jest.fn(),
done: jest.fn(),
Expand All @@ -144,7 +154,7 @@ describe('BurnRateRuleExecutor', () => {
shouldWriteAlerts: jest.fn(),
shouldStopExecution: jest.fn(),
getAlertStartedDate: jest.fn(),
getAlertUuid: jest.fn().mockImplementation(() => alertUuid),
getAlertUuid: jest.fn().mockImplementation((id) => alertUuidMap.get(id) || 'bad-uuid'),
getAlertByAlertUuid: jest.fn(),
share: {} as SharePluginStart,
dataViews: dataViewPluginMocks.createStartContract(),
Expand Down Expand Up @@ -312,11 +322,6 @@ describe('BurnRateRuleExecutor', () => {
esClientMock.search.mockResolvedValueOnce(
generateEsResponse(ruleParams, [], { instanceId: 'bar' })
);
const alertMock: Partial<Alert> = {
scheduleActions: jest.fn(),
replaceState: jest.fn(),
};
alertWithLifecycleMock.mockImplementation(() => alertMock as any);
alertFactoryMock.done.mockReturnValueOnce({ getRecoveredAlerts: () => [] });

const executor = getRuleExecutor({
Expand Down Expand Up @@ -417,11 +422,6 @@ describe('BurnRateRuleExecutor', () => {
esClientMock.search.mockResolvedValueOnce(
generateEsResponse(ruleParams, [], { instanceId: 'bar' })
);
const alertMock: Partial<Alert> = {
scheduleActions: jest.fn(),
replaceState: jest.fn(),
};
alertWithLifecycleMock.mockImplementation(() => alertMock as any);
alertFactoryMock.done.mockReturnValueOnce({ getRecoveredAlerts: () => [] });

const executor = getRuleExecutor({ basePath: basePathMock });
Expand Down
24 changes: 11 additions & 13 deletions x-pack/plugins/observability/server/lib/rules/slo_burn_rate/executor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ export const getRuleExecutor = ({
);

const alertId = instanceId;
const alert = alertWithLifecycle({
id: alertId,
fields: {
[ALERT_REASON]: reason,
[ALERT_EVALUATION_THRESHOLD]: windowDef.burnRateThreshold,
[ALERT_EVALUATION_VALUE]: Math.min(longWindowBurnRate, shortWindowBurnRate),
[SLO_ID_FIELD]: slo.id,
[SLO_REVISION_FIELD]: slo.revision,
[SLO_INSTANCE_ID_FIELD]: instanceId,
},
});
const indexedStartedAt = getAlertStartedDate(alertId) ?? startedAt.toISOString();
const alertUuid = getAlertUuid(alertId);
const alertDetailsUrl = await getAlertUrl(
Expand All @@ -143,19 +154,6 @@ export const getRuleExecutor = ({
sloInstanceId: instanceId,
};

const alert = alertWithLifecycle({
id: alertId,

fields: {
[ALERT_REASON]: reason,
[ALERT_EVALUATION_THRESHOLD]: windowDef.burnRateThreshold,
[ALERT_EVALUATION_VALUE]: Math.min(longWindowBurnRate, shortWindowBurnRate),
[SLO_ID_FIELD]: slo.id,
[SLO_REVISION_FIELD]: slo.revision,
[SLO_INSTANCE_ID_FIELD]: instanceId,
},
});

alert.scheduleActions(windowDef.actionGroup, context);
alert.replaceState({ alertState: AlertStates.ALERT });
}
Expand Down

0 comments on commit eb2a11f

Please sign in to comment.