[SLOs] Try async creation of resources !! (#192836)

## Summary

Try async as much as possible while creating SLO !!

Also UI form won't wait now for creating burn rate rule, it will be
created async loading state in toast !!
### Before


![image](https://github.com/user-attachments/assets/dc73ce58-3fc4-475f-a6ae-71ac98a6432c)

### After


![image](https://github.com/user-attachments/assets/4439bb8b-6768-4d8b-b208-1bad28f17ae5)
This commit is contained in:
Shahzad 2024-10-01 17:33:29 +02:00 committed by GitHub
parent c8c74399a0
commit 9e117c3aa2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 223 additions and 142 deletions

View file

@ -5,6 +5,7 @@
* 2.0.
*/
import React from 'react';
import { useMutation } from '@tanstack/react-query';
import { i18n } from '@kbn/i18n';
import { BASE_ALERTING_API_PATH, RuleTypeParams } from '@kbn/alerting-plugin/common';
@ -13,18 +14,23 @@ import type {
CreateRuleRequestBody,
CreateRuleResponse,
} from '@kbn/alerting-plugin/common/routes/rule/apis/create';
import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner } from '@elastic/eui';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { useKibana } from '../utils/kibana_react';
export function useCreateRule<Params extends RuleTypeParams = never>() {
const {
http,
i18n: i18nStart,
notifications: { toasts },
theme,
} = useKibana().services;
const createRule = useMutation<
return useMutation<
CreateRuleResponse<Params>,
Error,
{ rule: CreateRuleRequestBody<Params> }
{ rule: CreateRuleRequestBody<Params> },
{ loadingToastId?: string }
>(
['createRule'],
({ rule }) => {
@ -39,6 +45,24 @@ export function useCreateRule<Params extends RuleTypeParams = never>() {
}
},
{
onMutate: async () => {
const loadingToast = toasts.addInfo({
title: toMountPoint(
<EuiFlexGroup justifyContent="center" alignItems="center">
<EuiFlexItem grow={false}>
{i18n.translate('xpack.slo.rules.createRule.loadingNotification.descriptionText', {
defaultMessage: 'Creating burn rate rule ...',
})}
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiLoadingSpinner size="s" />
</EuiFlexItem>
</EuiFlexGroup>,
{ i18n: i18nStart, theme }
),
});
return { loadingToastId: loadingToast.id };
},
onError: (_err) => {
toasts.addDanger(
i18n.translate('xpack.slo.rules.createRule.errorNotification.descriptionText', {
@ -54,8 +78,11 @@ export function useCreateRule<Params extends RuleTypeParams = never>() {
})
);
},
onSettled: (_d, _err, _res, ctx) => {
if (ctx?.loadingToastId) {
toasts.remove(ctx?.loadingToastId);
}
},
}
);
return createRule;
}

View file

@ -55,7 +55,7 @@ export function useCreateSlo() {
<RedirectAppLinks coreStart={services} data-test-subj="observabilityMainContainer">
<FormattedMessage
id="xpack.slo.slo.create.successNotification"
defaultMessage="Successfully created {name}. {editSLO}"
defaultMessage='Successfully created SLO: "{name}". {editSLO}'
values={{
name: slo.name,
editSLO: (

View file

@ -11,7 +11,7 @@ import type { GetSLOResponse } from '@kbn/slo-schema';
import React, { useCallback, useMemo } from 'react';
import { useFormContext } from 'react-hook-form';
import { InPortal } from 'react-reverse-portal';
import { useCreateRule } from '../../../hooks/use_create_rule';
import { useCreateRule } from '../../../hooks/use_create_burn_rate_rule';
import { useKibana } from '../../../utils/kibana_react';
import { sloEditFormFooterPortal } from '../shared_flyout/slo_add_form_flyout';
import { paths } from '../../../../common/locators/paths';
@ -32,8 +32,6 @@ export interface Props {
onSave?: () => void;
}
export const maxWidth = 775;
export function SloEditFormFooter({ slo, onSave }: Props) {
const {
application: { navigateToUrl },
@ -45,7 +43,7 @@ export function SloEditFormFooter({ slo, onSave }: Props) {
const { mutateAsync: createSlo, isLoading: isCreateSloLoading } = useCreateSlo();
const { mutateAsync: updateSlo, isLoading: isUpdateSloLoading } = useUpdateSlo();
const { mutateAsync: createBurnRateRule, isLoading: isCreateBurnRateRuleLoading } =
const { mutate: createBurnRateRule, isLoading: isCreateBurnRateRuleLoading } =
useCreateRule<BurnRateRuleParams>();
const navigate = useCallback(
@ -70,7 +68,7 @@ export function SloEditFormFooter({ slo, onSave }: Props) {
} else {
const processedValues = transformCreateSLOFormToCreateSLOInput(values);
const resp = await createSlo({ slo: processedValues });
await createBurnRateRule({
createBurnRateRule({
rule: createBurnRateRuleRequestBody({ ...processedValues, id: resp.id }),
});
if (onSave) {

View file

@ -23,7 +23,7 @@ import { useFetchApmSuggestions } from '../../hooks/use_fetch_apm_suggestions';
import { useFetchIndices } from '../../hooks/use_fetch_indices';
import { useFetchSloDetails } from '../../hooks/use_fetch_slo_details';
import { usePermissions } from '../../hooks/use_permissions';
import { useCreateRule } from '../../hooks/use_create_rule';
import { useCreateRule } from '../../hooks/use_create_burn_rate_rule';
import { useUpdateSlo } from '../../hooks/use_update_slo';
import { useKibana } from '../../utils/kibana_react';
import { kibanaStartMock } from '../../utils/kibana_react.mock';
@ -45,7 +45,7 @@ jest.mock('../../hooks/use_create_slo');
jest.mock('../../hooks/use_update_slo');
jest.mock('../../hooks/use_fetch_apm_suggestions');
jest.mock('../../hooks/use_permissions');
jest.mock('../../hooks/use_create_rule');
jest.mock('../../hooks/use_create_burn_rate_rule');
const mockUseKibanaReturnValue = kibanaStartMock.startContract();

View file

@ -29,6 +29,8 @@ import {
updateSLOParamsSchema,
} from '@kbn/slo-schema';
import { getOverviewParamsSchema } from '@kbn/slo-schema/src/rest_specs/routes/get_overview';
import { KibanaRequest } from '@kbn/core-http-server';
import { RegisterRoutesDependencies } from '../register_routes';
import { GetSLOsOverview } from '../../services/get_slos_overview';
import type { IndicatorTypes } from '../../domain/models';
import {
@ -91,6 +93,11 @@ const assertPlatinumLicense = async (context: SloRequestHandlerContext) => {
}
};
const getSpaceId = async (deps: RegisterRoutesDependencies, request: KibanaRequest) => {
const spaces = await deps.getSpacesStart();
return (await spaces?.spacesService?.getActiveSpace(request))?.id ?? 'default';
};
const createSLORoute = createSloServerRoute({
endpoint: 'POST /api/observability/slos 2023-10-31',
options: {
@ -101,10 +108,7 @@ const createSLORoute = createSloServerRoute({
handler: async ({ context, params, logger, dependencies, request }) => {
await assertPlatinumLicense(context);
const spaces = await dependencies.getSpacesStart();
const dataViews = await dependencies.getDataViewsStart();
const spaceId = (await spaces?.spacesService?.getActiveSpace(request))?.id ?? 'default';
const core = await context.core;
const scopedClusterClient = core.elasticsearch.client;
const esClient = core.elasticsearch.client.asCurrentUser;
@ -112,7 +116,10 @@ const createSLORoute = createSloServerRoute({
const soClient = core.savedObjects.client;
const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
const dataViewsService = await dataViews.dataViewsServiceFactory(soClient, esClient);
const [spaceId, dataViewsService] = await Promise.all([
getSpaceId(dependencies, request),
dataViews.dataViewsServiceFactory(soClient, esClient),
]);
const transformManager = new DefaultTransformManager(
transformGenerators,
scopedClusterClient,
@ -125,7 +132,6 @@ const createSLORoute = createSloServerRoute({
scopedClusterClient,
logger
);
const createSLO = new CreateSLO(
esClient,
scopedClusterClient,
@ -137,9 +143,7 @@ const createSLORoute = createSloServerRoute({
basePath
);
const response = await createSLO.execute(params.body);
return response;
return await createSLO.execute(params.body);
},
});

View file

@ -65,7 +65,7 @@ describe('CreateSLO', () => {
const response = await createSLO.execute(sloParams);
expect(mockRepository.save).toHaveBeenCalledWith(
expect(mockRepository.create).toHaveBeenCalledWith(
expect.objectContaining({
...sloParams,
id: 'unique-id',
@ -80,17 +80,14 @@ describe('CreateSLO', () => {
version: 2,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
}),
{ throwOnConflict: true }
})
);
expect(mockTransformManager.install).toHaveBeenCalled();
expect(mockTransformManager.start).toHaveBeenCalled();
expect(
mockScopedClusterClient.asSecondaryAuthUser.ingest.putPipeline.mock.calls[0]
).toMatchSnapshot();
expect(mockSummaryTransformManager.install).toHaveBeenCalled();
expect(mockSummaryTransformManager.start).toHaveBeenCalled();
expect(mockEsClient.index.mock.calls[0]).toMatchSnapshot();
expect(response).toEqual(expect.objectContaining({ id: 'unique-id' }));
@ -108,7 +105,7 @@ describe('CreateSLO', () => {
await createSLO.execute(sloParams);
expect(mockRepository.save).toHaveBeenCalledWith(
expect(mockRepository.create).toHaveBeenCalledWith(
expect.objectContaining({
...sloParams,
id: expect.any(String),
@ -122,8 +119,7 @@ describe('CreateSLO', () => {
enabled: true,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
}),
{ throwOnConflict: true }
})
);
});
@ -141,7 +137,7 @@ describe('CreateSLO', () => {
await createSLO.execute(sloParams);
expect(mockRepository.save).toHaveBeenCalledWith(
expect(mockRepository.create).toHaveBeenCalledWith(
expect.objectContaining({
...sloParams,
id: expect.any(String),
@ -155,8 +151,7 @@ describe('CreateSLO', () => {
enabled: true,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
}),
{ throwOnConflict: true }
})
);
});
});
@ -173,16 +168,16 @@ describe('CreateSLO', () => {
expect(mockRepository.deleteById).toHaveBeenCalled();
expect(
mockScopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline
).toHaveBeenCalledTimes(1);
).toHaveBeenCalledTimes(2);
expect(mockSummaryTransformManager.stop).not.toHaveBeenCalled();
expect(mockSummaryTransformManager.uninstall).not.toHaveBeenCalled();
expect(mockTransformManager.stop).not.toHaveBeenCalled();
expect(mockTransformManager.uninstall).not.toHaveBeenCalled();
expect(mockSummaryTransformManager.stop).toHaveBeenCalledTimes(0);
expect(mockSummaryTransformManager.uninstall).toHaveBeenCalledTimes(1);
expect(mockTransformManager.stop).toHaveBeenCalledTimes(0);
expect(mockTransformManager.uninstall).toHaveBeenCalledTimes(1);
});
it('rollbacks completed operations when summary transform start fails', async () => {
mockSummaryTransformManager.start.mockRejectedValue(
it('rollbacks completed operations when summary transform install fails', async () => {
mockSummaryTransformManager.install.mockRejectedValue(
new Error('Summary transform install error')
);
const sloParams = createSLOParams({ indicator: createAPMTransactionErrorRateIndicator() });
@ -192,7 +187,7 @@ describe('CreateSLO', () => {
);
expect(mockRepository.deleteById).toHaveBeenCalled();
expect(mockTransformManager.stop).toHaveBeenCalled();
expect(mockTransformManager.stop).not.toHaveBeenCalled();
expect(mockTransformManager.uninstall).toHaveBeenCalled();
expect(
mockScopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline
@ -211,12 +206,12 @@ describe('CreateSLO', () => {
);
expect(mockRepository.deleteById).toHaveBeenCalled();
expect(mockTransformManager.stop).toHaveBeenCalled();
expect(mockTransformManager.stop).not.toHaveBeenCalled();
expect(mockTransformManager.uninstall).toHaveBeenCalled();
expect(
mockScopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline
).toHaveBeenCalledTimes(2);
expect(mockSummaryTransformManager.stop).toHaveBeenCalled();
expect(mockSummaryTransformManager.stop).not.toHaveBeenCalled();
expect(mockSummaryTransformManager.uninstall).toHaveBeenCalled();
});
});

View file

@ -10,6 +10,7 @@ import { ElasticsearchClient, IBasePath, Logger } from '@kbn/core/server';
import { ALL_VALUE, CreateSLOParams, CreateSLOResponse } from '@kbn/slo-schema';
import { asyncForEach } from '@kbn/std';
import { v4 as uuidv4 } from 'uuid';
import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
import {
getSLOPipelineId,
getSLOSummaryPipelineId,
@ -22,7 +23,7 @@ import { getSLOPipelineTemplate } from '../assets/ingest_templates/slo_pipeline_
import { getSLOSummaryPipelineTemplate } from '../assets/ingest_templates/slo_summary_pipeline_template';
import { Duration, DurationUnit, SLODefinition } from '../domain/models';
import { validateSLO } from '../domain/services';
import { SecurityException } from '../errors';
import { SecurityException, SLOIdConflict } from '../errors';
import { retryTransientEsErrors } from '../utils/retry';
import { SLORepository } from './slo_repository';
import { createTempSummaryDocument } from './summary_transform_generator/helpers/create_temp_summary';
@ -47,62 +48,58 @@ export class CreateSLO {
const rollbackOperations = [];
await this.repository.save(slo, { throwOnConflict: true });
rollbackOperations.push(() => this.repository.deleteById(slo.id));
const sloAlreadyExists = await this.repository.checkIfSLOExists(slo);
if (sloAlreadyExists) {
throw new SLOIdConflict(`SLO [${slo.id}] already exists`);
}
const createPromise = this.repository.create(slo);
rollbackOperations.push(() => this.repository.deleteById(slo.id, true));
const rollupTransformId = getSLOTransformId(slo.id, slo.revision);
const summaryTransformId = getSLOSummaryTransformId(slo.id, slo.revision);
try {
await retryTransientEsErrors(
() =>
this.scopedClusterClient.asSecondaryAuthUser.ingest.putPipeline(
getSLOPipelineTemplate(slo)
),
{ logger: this.logger }
);
rollbackOperations.push(() =>
this.scopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline(
{ id: getSLOPipelineId(slo.id, slo.revision) },
{ ignore: [404] }
)
);
const sloPipelinePromise = this.createPipeline(getSLOPipelineTemplate(slo));
rollbackOperations.push(() => this.deletePipeline(getSLOPipelineId(slo.id, slo.revision)));
await this.transformManager.install(slo);
const rollupTransformPromise = this.transformManager.install(slo);
rollbackOperations.push(() => this.transformManager.uninstall(rollupTransformId));
await this.transformManager.start(rollupTransformId);
rollbackOperations.push(() => this.transformManager.stop(rollupTransformId));
await retryTransientEsErrors(
() =>
this.scopedClusterClient.asSecondaryAuthUser.ingest.putPipeline(
getSLOSummaryPipelineTemplate(slo, this.spaceId, this.basePath)
),
{ logger: this.logger }
const summaryPipelinePromise = this.createPipeline(
getSLOSummaryPipelineTemplate(slo, this.spaceId, this.basePath)
);
rollbackOperations.push(() =>
this.scopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline(
{ id: getSLOSummaryPipelineId(slo.id, slo.revision) },
{ ignore: [404] }
)
this.deletePipeline(getSLOSummaryPipelineId(slo.id, slo.revision))
);
await this.summaryTransformManager.install(slo);
const summaryTransformPromise = this.summaryTransformManager.install(slo);
rollbackOperations.push(() => this.summaryTransformManager.uninstall(summaryTransformId));
await this.summaryTransformManager.start(summaryTransformId);
const tempDocPromise = this.createTempSummaryDocument(slo);
rollbackOperations.push(() => this.deleteTempSummaryDocument(slo));
await Promise.all([
createPromise,
sloPipelinePromise,
rollupTransformPromise,
summaryPipelinePromise,
summaryTransformPromise,
tempDocPromise,
]);
rollbackOperations.push(() => this.transformManager.stop(rollupTransformId));
rollbackOperations.push(() => this.summaryTransformManager.stop(summaryTransformId));
await retryTransientEsErrors(
() =>
this.esClient.index({
index: SLO_SUMMARY_TEMP_INDEX_NAME,
id: `slo-${slo.id}`,
document: createTempSummaryDocument(slo, this.spaceId, this.basePath),
refresh: true,
}),
{ logger: this.logger }
);
// transforms can only be started after the pipeline is created
await Promise.all([
this.transformManager.start(rollupTransformId),
this.summaryTransformManager.start(summaryTransformId),
]);
} catch (err) {
this.logger.error(
`Cannot install the SLO [id: ${slo.id}, revision: ${slo.revision}]. Rolling back.`
@ -126,6 +123,45 @@ export class CreateSLO {
return this.toResponse(slo);
}
async createTempSummaryDocument(slo: SLODefinition) {
return await retryTransientEsErrors(
() =>
this.esClient.index({
index: SLO_SUMMARY_TEMP_INDEX_NAME,
id: `slo-${slo.id}`,
document: createTempSummaryDocument(slo, this.spaceId, this.basePath),
refresh: true,
}),
{ logger: this.logger }
);
}
async deleteTempSummaryDocument(slo: SLODefinition) {
return await retryTransientEsErrors(
() =>
this.esClient.delete({
index: SLO_SUMMARY_TEMP_INDEX_NAME,
id: `slo-${slo.id}`,
refresh: true,
}),
{ logger: this.logger }
);
}
async createPipeline(params: IngestPutPipelineRequest) {
return await retryTransientEsErrors(
() => this.scopedClusterClient.asSecondaryAuthUser.ingest.putPipeline(params),
{ logger: this.logger }
);
}
async deletePipeline(id: string) {
return this.scopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline(
{ id },
{ ignore: [404] }
);
}
public async inspect(params: CreateSLOParams): Promise<{
slo: CreateSLOParams;
rollUpPipeline: Record<string, any>;

View file

@ -38,7 +38,7 @@ describe('ManageSLO', () => {
expect(mockTransformManager.start).not.toHaveBeenCalled();
expect(mockSummaryTransformManager.start).not.toHaveBeenCalled();
expect(mockRepository.save).not.toHaveBeenCalled();
expect(mockRepository.create).not.toHaveBeenCalled();
});
it('enables the slo when disabled', async () => {
@ -49,7 +49,9 @@ describe('ManageSLO', () => {
expect(mockTransformManager.start).toMatchSnapshot();
expect(mockSummaryTransformManager.start).toMatchSnapshot();
expect(mockRepository.save).toHaveBeenCalledWith(expect.objectContaining({ enabled: true }));
expect(mockRepository.update).toHaveBeenCalledWith(
expect.objectContaining({ enabled: true })
);
});
});
@ -62,7 +64,7 @@ describe('ManageSLO', () => {
expect(mockTransformManager.stop).not.toHaveBeenCalled();
expect(mockSummaryTransformManager.stop).not.toHaveBeenCalled();
expect(mockRepository.save).not.toHaveBeenCalled();
expect(mockRepository.update).not.toHaveBeenCalled();
});
it('disables the slo when enabled', async () => {
@ -73,7 +75,9 @@ describe('ManageSLO', () => {
expect(mockTransformManager.stop).toMatchSnapshot();
expect(mockSummaryTransformManager.stop).toMatchSnapshot();
expect(mockRepository.save).toHaveBeenCalledWith(expect.objectContaining({ enabled: false }));
expect(mockRepository.update).toHaveBeenCalledWith(
expect.objectContaining({ enabled: false })
);
});
});
});

View file

@ -26,7 +26,7 @@ export class ManageSLO {
await this.transformManager.start(getSLOTransformId(slo.id, slo.revision));
slo.enabled = true;
slo.updatedAt = new Date();
await this.repository.save(slo);
await this.repository.update(slo);
}
async disable(sloId: string) {
@ -39,6 +39,6 @@ export class ManageSLO {
await this.transformManager.stop(getSLOTransformId(slo.id, slo.revision));
slo.enabled = false;
slo.updatedAt = new Date();
await this.repository.save(slo);
await this.repository.update(slo);
}
}

View file

@ -42,11 +42,13 @@ const createSummaryTransformManagerMock = (): jest.Mocked<TransformManager> => {
const createSLORepositoryMock = (): jest.Mocked<SLORepository> => {
return {
save: jest.fn(),
create: jest.fn(),
update: jest.fn(),
findById: jest.fn(),
findAllByIds: jest.fn(),
deleteById: jest.fn(),
search: jest.fn(),
checkIfSLOExists: jest.fn(),
};
};

View file

@ -63,7 +63,7 @@ describe('ResetSLO', () => {
it('resets all associated resources', async () => {
const slo = createSLO({ id: 'irrelevant', version: 1 });
mockRepository.findById.mockResolvedValueOnce(slo);
mockRepository.save.mockImplementation((v) => Promise.resolve(v));
mockRepository.update.mockImplementation((v) => Promise.resolve(v));
await resetSLO.execute(slo.id);
@ -87,7 +87,7 @@ describe('ResetSLO', () => {
expect(mockEsClient.index).toMatchSnapshot();
expect(mockRepository.save).toHaveBeenCalledWith({
expect(mockRepository.update).toHaveBeenCalledWith({
...slo,
version: SLO_MODEL_VERSION,
updatedAt: expect.anything(),

View file

@ -104,7 +104,7 @@ export class ResetSLO {
throw err;
}
const updatedSlo = await this.repository.save({
const updatedSlo = await this.repository.update({
...slo,
version: SLO_MODEL_VERSION,
updatedAt: new Date(),

View file

@ -11,7 +11,7 @@ import { MockedLogger } from '@kbn/logging-mocks';
import { sloDefinitionSchema } from '@kbn/slo-schema';
import { SLO_MODEL_VERSION } from '../../common/constants';
import { SLODefinition, StoredSLODefinition } from '../domain/models';
import { SLOIdConflict, SLONotFound } from '../errors';
import { SLONotFound } from '../errors';
import { SO_SLO_TYPE } from '../saved_objects';
import { aStoredSLO, createAPMTransactionDurationIndicator, createSLO } from './fixtures/slo';
import { KibanaSavedObjectsSLORepository } from './slo_repository';
@ -82,43 +82,45 @@ describe('KibanaSavedObjectsSLORepository', () => {
});
describe('saving an SLO', () => {
it('checking existing id for slo', async () => {
const slo = createSLO({ id: 'my-id' });
soClientMock.find.mockResolvedValueOnce(soFindResponse([]));
soClientMock.create.mockResolvedValueOnce(aStoredSLO(slo));
const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
await repository.checkIfSLOExists(slo);
expect(soClientMock.find).toHaveBeenCalledWith({
type: SO_SLO_TYPE,
perPage: 0,
filter: `slo.attributes.id:(${slo.id})`,
});
});
it('saves the new SLO', async () => {
const slo = createSLO({ id: 'my-id' });
soClientMock.find.mockResolvedValueOnce(soFindResponse([]));
soClientMock.create.mockResolvedValueOnce(aStoredSLO(slo));
const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
const savedSLO = await repository.save(slo);
const savedSLO = await repository.create(slo);
expect(savedSLO).toEqual(slo);
expect(soClientMock.find).toHaveBeenCalledWith({
type: SO_SLO_TYPE,
page: 1,
perPage: 1,
filter: `slo.attributes.id:(${slo.id})`,
});
expect(soClientMock.create).toHaveBeenCalledWith(
SO_SLO_TYPE,
sloDefinitionSchema.encode(slo),
{
id: undefined,
overwrite: true,
}
sloDefinitionSchema.encode(slo)
);
});
it('throws when the SLO id already exists and "throwOnConflict" is true', async () => {
it('checks when the SLO id already exists', async () => {
const slo = createSLO({ id: 'my-id' });
soClientMock.find.mockResolvedValueOnce(soFindResponse([slo]));
const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
await expect(repository.save(slo, { throwOnConflict: true })).rejects.toThrowError(
new SLOIdConflict(`SLO [my-id] already exists`)
);
await expect(await repository.checkIfSLOExists(slo)).toEqual(true);
expect(soClientMock.find).toHaveBeenCalledWith({
type: SO_SLO_TYPE,
page: 1,
perPage: 1,
perPage: 0,
filter: `slo.attributes.id:(${slo.id})`,
});
});
@ -129,15 +131,10 @@ describe('KibanaSavedObjectsSLORepository', () => {
soClientMock.create.mockResolvedValueOnce(aStoredSLO(slo));
const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
const savedSLO = await repository.save(slo);
const savedSLO = await repository.update(slo);
expect(savedSLO).toEqual(slo);
expect(soClientMock.find).toHaveBeenCalledWith({
type: SO_SLO_TYPE,
page: 1,
perPage: 1,
filter: `slo.attributes.id:(${slo.id})`,
});
expect(soClientMock.create).toHaveBeenCalledWith(
SO_SLO_TYPE,
sloDefinitionSchema.encode(slo),

View file

@ -11,14 +11,16 @@ import { ALL_VALUE, Paginated, Pagination, sloDefinitionSchema } from '@kbn/slo-
import { isLeft } from 'fp-ts/lib/Either';
import { SLO_MODEL_VERSION } from '../../common/constants';
import { SLODefinition, StoredSLODefinition } from '../domain/models';
import { SLOIdConflict, SLONotFound } from '../errors';
import { SLONotFound } from '../errors';
import { SO_SLO_TYPE } from '../saved_objects';
export interface SLORepository {
save(slo: SLODefinition, options?: { throwOnConflict: boolean }): Promise<SLODefinition>;
checkIfSLOExists(slo: SLODefinition): Promise<boolean>;
create(slo: SLODefinition): Promise<SLODefinition>;
update(slo: SLODefinition): Promise<SLODefinition>;
findAllByIds(ids: string[]): Promise<SLODefinition[]>;
findById(id: string): Promise<SLODefinition>;
deleteById(id: string): Promise<void>;
deleteById(id: string, ignoreNotFound?: boolean): Promise<void>;
search(
search: string,
pagination: Pagination,
@ -29,19 +31,30 @@ export interface SLORepository {
export class KibanaSavedObjectsSLORepository implements SLORepository {
constructor(private soClient: SavedObjectsClientContract, private logger: Logger) {}
async save(slo: SLODefinition, options = { throwOnConflict: false }): Promise<SLODefinition> {
let existingSavedObjectId;
async checkIfSLOExists(slo: SLODefinition) {
const findResponse = await this.soClient.find<StoredSLODefinition>({
type: SO_SLO_TYPE,
perPage: 0,
filter: `slo.attributes.id:(${slo.id})`,
});
return findResponse.total > 0;
}
async create(slo: SLODefinition): Promise<SLODefinition> {
await this.soClient.create<StoredSLODefinition>(SO_SLO_TYPE, toStoredSLO(slo));
return slo;
}
async update(slo: SLODefinition): Promise<SLODefinition> {
let existingSavedObjectId: string | undefined;
const findResponse = await this.soClient.find<StoredSLODefinition>({
type: SO_SLO_TYPE,
page: 1,
perPage: 1,
filter: `slo.attributes.id:(${slo.id})`,
});
if (findResponse.total === 1) {
if (options.throwOnConflict) {
throw new SLOIdConflict(`SLO [${slo.id}] already exists`);
}
existingSavedObjectId = findResponse.saved_objects[0].id;
}
@ -73,7 +86,7 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
return slo;
}
async deleteById(id: string): Promise<void> {
async deleteById(id: string, ignoreNotFound = false): Promise<void> {
const response = await this.soClient.find<StoredSLODefinition>({
type: SO_SLO_TYPE,
page: 1,
@ -82,6 +95,9 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
});
if (response.total === 0) {
if (ignoreNotFound) {
return;
}
throw new SLONotFound(`SLO [${id}] not found`);
}

View file

@ -204,7 +204,7 @@ describe('UpdateSLO', () => {
await updateSLO.execute(slo.id, { settings: newSettings });
expectDeletionOfOriginalSLOResources(slo);
expect(mockRepository.save).toHaveBeenCalledWith(
expect(mockRepository.update).toHaveBeenCalledWith(
expect.objectContaining({
...slo,
settings: newSettings,
@ -316,7 +316,7 @@ describe('UpdateSLO', () => {
updateSLO.execute(originalSlo.id, { indicator: newIndicator })
).rejects.toThrowError('Transform install error');
expect(mockRepository.save).toHaveBeenCalledWith(originalSlo);
expect(mockRepository.update).toHaveBeenCalledWith(originalSlo);
expect(
mockScopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline
).toHaveBeenCalledTimes(1); // for the sli only
@ -343,7 +343,7 @@ describe('UpdateSLO', () => {
updateSLO.execute(originalSlo.id, { indicator: newIndicator })
).rejects.toThrowError('summary transform start error');
expect(mockRepository.save).toHaveBeenCalledWith(originalSlo);
expect(mockRepository.update).toHaveBeenCalledWith(originalSlo);
expect(mockSummaryTransformManager.uninstall).toHaveBeenCalled();
expect(mockScopedClusterClient.asSecondaryAuthUser.ingest.deletePipeline).toHaveBeenCalled();
expect(mockTransformManager.stop).toHaveBeenCalled();

View file

@ -70,8 +70,8 @@ export class UpdateSLO {
const rollbackOperations = [];
await this.repository.save(updatedSlo);
rollbackOperations.push(() => this.repository.save(originalSlo));
await this.repository.update(updatedSlo);
rollbackOperations.push(() => this.repository.update(originalSlo));
if (!requireRevisionBump) {
// At this point, we still need to update the sli and summary pipeline to include the changes (id and revision in the rollup index) and (name, desc, tags, ...) in the summary index

View file

@ -55,14 +55,16 @@ export default function ({ getService }: FtrProviderContext) {
const { id } = response.body;
const savedObject = await kibanaServer.savedObjects.find({
type: SO_SLO_TYPE,
await retry.tryForTime(10000, async () => {
const savedObject = await kibanaServer.savedObjects.find({
type: SO_SLO_TYPE,
});
expect(savedObject.saved_objects.length).eql(1);
expect(savedObject.saved_objects[0].attributes.id).eql(id);
});
expect(savedObject.saved_objects.length).eql(1);
expect(savedObject.saved_objects[0].attributes.id).eql(id);
await retry.tryForTime(300 * 1000, async () => {
// expect summary and rollup data to exist
const sloSummaryResponse = await sloEsClient.getSLOSummaryDataById(id);

View file

@ -14,7 +14,7 @@ import { loadTestData } from './helper/load_test_data';
import { sloData } from './fixtures/create_slo';
export default function ({ getService }: FtrProviderContext) {
describe('Update SLOs', function () {
describe('UpdateSLOs', function () {
this.tags('skipCloud');
const supertestAPI = getService('supertest');