Always bump revision on update (#161897)

This commit is contained in:
Kevin Delemme 2023-07-14 09:01:03 -04:00
parent b1cfbbd673
commit 8721978c5b
4 changed files with 97 additions and 171 deletions

View file

@ -5,11 +5,11 @@
* 2.0.
*/
import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
import { rulesClientMock } from '@kbn/alerting-plugin/server/rules_client.mock';
import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
import { ElasticsearchClient } from '@kbn/core/server';
import { elasticsearchServiceMock } from '@kbn/core/server/mocks';
import { getSLOTransformId, SLO_INDEX_TEMPLATE_NAME } from '../../assets/constants';
import { getSLOTransformId, SLO_DESTINATION_INDEX_PATTERN } from '../../assets/constants';
import { DeleteSLO } from './delete_slo';
import { createAPMTransactionErrorRateIndicator, createSLO } from './fixtures/slo';
import { createSLORepositoryMock, createTransformManagerMock } from './mocks';
@ -47,7 +47,7 @@ describe('DeleteSLO', () => {
);
expect(mockEsClient.deleteByQuery).toHaveBeenCalledWith(
expect.objectContaining({
index: `${SLO_INDEX_TEMPLATE_NAME}*`,
index: SLO_DESTINATION_INDEX_PATTERN,
query: {
match: {
'slo.id': slo.id,

View file

@ -7,8 +7,7 @@
import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
import { ElasticsearchClient } from '@kbn/core/server';
import { getSLOTransformId, SLO_INDEX_TEMPLATE_NAME } from '../../assets/constants';
import { getSLOTransformId, SLO_DESTINATION_INDEX_PATTERN } from '../../assets/constants';
import { SLORepository } from './slo_repository';
import { TransformManager } from './transform_manager';
@ -34,7 +33,7 @@ export class DeleteSLO {
private async deleteRollupData(sloId: string): Promise<void> {
await this.esClient.deleteByQuery({
index: `${SLO_INDEX_TEMPLATE_NAME}*`,
index: SLO_DESTINATION_INDEX_PATTERN,
wait_for_completion: false,
query: {
match: {

View file

@ -34,131 +34,94 @@ describe('UpdateSLO', () => {
updateSLO = new UpdateSLO(mockRepository, mockTransformManager, mockEsClient);
});
describe('without breaking changes', () => {
it('updates the SLO saved object without revision bump', async () => {
const slo = createSLO({ indicator: createAPMTransactionErrorRateIndicator() });
mockRepository.findById.mockResolvedValueOnce(slo);
it('updates the settings correctly', async () => {
const slo = createSLO();
mockRepository.findById.mockResolvedValueOnce(slo);
const newName = 'new slo name';
const newTags = ['other', 'tags'];
const response = await updateSLO.execute(slo.id, { name: newName, tags: newTags });
const newSettings = { ...slo.settings, timestamp_field: 'newField' };
await updateSLO.execute(slo.id, { settings: newSettings });
expectTransformManagerNeverCalled();
expect(mockEsClient.deleteByQuery).not.toBeCalled();
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
name: newName,
tags: newTags,
updatedAt: expect.anything(),
})
);
expect(slo.name).not.toEqual(newName);
expect(response.name).toEqual(newName);
expect(response.updatedAt).not.toEqual(slo.updatedAt);
expect(response.revision).toEqual(slo.revision);
expect(response.tags).toEqual(newTags);
expect(slo.tags).not.toEqual(newTags);
});
expectDeletionOfObsoleteSLOData(slo);
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
settings: newSettings,
revision: 2,
updatedAt: expect.anything(),
})
);
expectInstallationOfNewSLOTransform();
});
describe('with breaking changes', () => {
it('consideres settings as a breaking change', async () => {
const slo = createSLO();
mockRepository.findById.mockResolvedValueOnce(slo);
it('updates the budgeting method correctly', async () => {
const slo = createSLO({ budgetingMethod: 'occurrences' });
mockRepository.findById.mockResolvedValueOnce(slo);
const newSettings = { ...slo.settings, timestamp_field: 'newField' };
await updateSLO.execute(slo.id, { settings: newSettings });
expectDeletionOfObsoleteSLOData(slo);
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
settings: newSettings,
revision: 2,
updatedAt: expect.anything(),
})
);
expectInstallationOfNewSLOTransform();
await updateSLO.execute(slo.id, {
budgetingMethod: 'timeslices',
objective: {
target: slo.objective.target,
timesliceTarget: 0.9,
timesliceWindow: oneMinute(),
},
});
it('consideres a budgeting method change as a breaking change', async () => {
const slo = createSLO({ budgetingMethod: 'occurrences' });
mockRepository.findById.mockResolvedValueOnce(slo);
await updateSLO.execute(slo.id, {
budgetingMethod: 'timeslices',
objective: {
target: slo.objective.target,
timesliceTarget: 0.9,
timesliceWindow: oneMinute(),
},
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
it('consideres a timeslice target change as a breaking change', async () => {
const slo = createSLOWithTimeslicesBudgetingMethod();
mockRepository.findById.mockResolvedValueOnce(slo);
await updateSLO.execute(slo.id, {
objective: {
target: slo.objective.target,
timesliceTarget: 0.1,
timesliceWindow: slo.objective.timesliceWindow,
},
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
it('consideres a timeslice window change as a breaking change', async () => {
const slo = createSLOWithTimeslicesBudgetingMethod();
mockRepository.findById.mockResolvedValueOnce(slo);
await updateSLO.execute(slo.id, {
objective: {
target: slo.objective.target,
timesliceTarget: slo.objective.timesliceTarget,
timesliceWindow: fiveMinute(),
},
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
it('removes the obsolete data from the SLO previous revision', async () => {
const slo = createSLO({
indicator: createAPMTransactionErrorRateIndicator({ environment: 'development' }),
});
mockRepository.findById.mockResolvedValueOnce(slo);
const newIndicator = createAPMTransactionErrorRateIndicator({ environment: 'production' });
await updateSLO.execute(slo.id, { indicator: newIndicator });
expectDeletionOfObsoleteSLOData(slo);
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
indicator: newIndicator,
revision: 2,
updatedAt: expect.anything(),
})
);
expectInstallationOfNewSLOTransform();
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
function expectTransformManagerNeverCalled() {
expect(mockTransformManager.stop).not.toBeCalled();
expect(mockTransformManager.uninstall).not.toBeCalled();
expect(mockTransformManager.start).not.toBeCalled();
expect(mockTransformManager.install).not.toBeCalled();
}
it('updates the timeslice target correctly', async () => {
const slo = createSLOWithTimeslicesBudgetingMethod();
mockRepository.findById.mockResolvedValueOnce(slo);
await updateSLO.execute(slo.id, {
objective: {
target: slo.objective.target,
timesliceTarget: 0.1,
timesliceWindow: slo.objective.timesliceWindow,
},
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
it('consideres a timeslice window change as a breaking change', async () => {
const slo = createSLOWithTimeslicesBudgetingMethod();
mockRepository.findById.mockResolvedValueOnce(slo);
await updateSLO.execute(slo.id, {
objective: {
target: slo.objective.target,
timesliceTarget: slo.objective.timesliceTarget,
timesliceWindow: fiveMinute(),
},
});
expectDeletionOfObsoleteSLOData(slo);
expectInstallationOfNewSLOTransform();
});
it('removes the obsolete data from the SLO previous revision', async () => {
const slo = createSLO({
indicator: createAPMTransactionErrorRateIndicator({ environment: 'development' }),
});
mockRepository.findById.mockResolvedValueOnce(slo);
const newIndicator = createAPMTransactionErrorRateIndicator({ environment: 'production' });
await updateSLO.execute(slo.id, { indicator: newIndicator });
expectDeletionOfObsoleteSLOData(slo);
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
indicator: newIndicator,
revision: 2,
updatedAt: expect.anything(),
})
);
expectInstallationOfNewSLOTransform();
});
function expectInstallationOfNewSLOTransform() {
expect(mockTransformManager.start).toBeCalled();

View file

@ -5,15 +5,13 @@
* 2.0.
*/
import deepEqual from 'fast-deep-equal';
import { ElasticsearchClient } from '@kbn/core/server';
import { UpdateSLOParams, UpdateSLOResponse, updateSLOResponseSchema } from '@kbn/slo-schema';
import { getSLOTransformId, SLO_INDEX_TEMPLATE_NAME } from '../../assets/constants';
import { SLORepository } from './slo_repository';
import { TransformManager } from './transform_manager';
import { getSLOTransformId, SLO_DESTINATION_INDEX_PATTERN } from '../../assets/constants';
import { SLO } from '../../domain/models';
import { validateSLO } from '../../domain/services';
import { SLORepository } from './slo_repository';
import { TransformManager } from './transform_manager';
export class UpdateSLO {
constructor(
@ -24,52 +22,18 @@ export class UpdateSLO {
public async execute(sloId: string, params: UpdateSLOParams): Promise<UpdateSLOResponse> {
const originalSlo = await this.repository.findById(sloId);
const { hasBreakingChange, updatedSlo } = this.updateSLO(originalSlo, params);
if (hasBreakingChange) {
await this.deleteObsoleteSLORevisionData(originalSlo);
await this.repository.save(updatedSlo);
await this.transformManager.install(updatedSlo);
await this.transformManager.start(getSLOTransformId(updatedSlo.id, updatedSlo.revision));
} else {
await this.repository.save(updatedSlo);
}
return this.toResponse(updatedSlo);
}
private updateSLO(originalSlo: SLO, params: UpdateSLOParams) {
let hasBreakingChange = false;
const updatedSlo: SLO = Object.assign({}, originalSlo, params, { updatedAt: new Date() });
const updatedSlo: SLO = Object.assign({}, originalSlo, params, {
updatedAt: new Date(),
revision: originalSlo.revision + 1,
});
validateSLO(updatedSlo);
if (!deepEqual(originalSlo.indicator, updatedSlo.indicator)) {
hasBreakingChange = true;
}
await this.deleteObsoleteSLORevisionData(originalSlo);
await this.repository.save(updatedSlo);
await this.transformManager.install(updatedSlo);
await this.transformManager.start(getSLOTransformId(updatedSlo.id, updatedSlo.revision));
if (originalSlo.budgetingMethod !== updatedSlo.budgetingMethod) {
hasBreakingChange = true;
}
if (
originalSlo.budgetingMethod === 'timeslices' &&
updatedSlo.budgetingMethod === 'timeslices' &&
(originalSlo.objective.timesliceTarget !== updatedSlo.objective.timesliceTarget ||
!deepEqual(originalSlo.objective.timesliceWindow, updatedSlo.objective.timesliceWindow))
) {
hasBreakingChange = true;
}
if (!deepEqual(originalSlo.settings, updatedSlo.settings)) {
hasBreakingChange = true;
}
if (hasBreakingChange) {
updatedSlo.revision++;
}
return { hasBreakingChange, updatedSlo };
return this.toResponse(updatedSlo);
}
private async deleteObsoleteSLORevisionData(originalSlo: SLO) {
@ -81,7 +45,7 @@ export class UpdateSLO {
private async deleteRollupData(sloId: string, sloRevision: number): Promise<void> {
await this.esClient.deleteByQuery({
index: `${SLO_INDEX_TEMPLATE_NAME}*`,
index: SLO_DESTINATION_INDEX_PATTERN,
wait_for_completion: false,
query: {
bool: {