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

feat(slo): Create SLO with specified ID #157057

Merged
merged 7 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/kbn-slo-schema/src/rest_specs/slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const createSLOParamsSchema = t.type({
budgetingMethod: budgetingMethodSchema,
objective: objectiveSchema,
}),
t.partial({ settings: optionalSettingsSchema, tags: tagsSchema }),
t.partial({ id: sloIdSchema, settings: optionalSettingsSchema, tags: tagsSchema }),
]),
});

Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/observability/docs/openapi/slo/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/4xx_response'
'409':
description: Conflict - The SLO id already exists
content:
application/json:
schema:
$ref: '#/components/schemas/4xx_response'
servers:
- url: https://localhost:5601
get:
Expand Down Expand Up @@ -690,6 +696,10 @@ components:
- budgetingMethod
- objective
properties:
id:
description: A unique identifier for the SLO. Must be between 8 and 36 chars
type: string
example: my-super-slo-id
name:
description: A name for the SLO.
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ required:
- budgetingMethod
- objective
properties:
id:
description: A unique identifier for the SLO. Must be between 8 and 36 chars
type: string
example: my-super-slo-id
name:
description: A name for the SLO.
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ post:
application/json:
schema:
$ref: '../components/schemas/4xx_response.yaml'
'409':
description: Conflict - The SLO id already exists
content:
application/json:
schema:
$ref: '../components/schemas/4xx_response.yaml'
servers:
- url: https://localhost:5601

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import { SLO } from '../models';
* @param slo {SLO}
*/
export function validateSLO(slo: SLO) {
if (!isValidId(slo.id)) {
throw new IllegalArgumentError('Invalid id');
}

if (!isValidTargetNumber(slo.objective.target)) {
throw new IllegalArgumentError('Invalid objective.target');
}
Expand Down Expand Up @@ -70,6 +74,12 @@ function validateSettings(slo: SLO) {
}
}

function isValidId(id: string): boolean {
const MIN_ID_LENGTH = 8;
const MAX_ID_LENGTH = 36;
return MIN_ID_LENGTH <= id.length && id.length <= MAX_ID_LENGTH;
}

function isValidTargetNumber(value: number): boolean {
return value > 0 && value < 1;
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/observability/server/errors/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export class ObservabilityError extends Error {
}

export class SLONotFound extends ObservabilityError {}
export class SLOIdConflict extends ObservabilityError {}
export class InternalQueryError extends ObservabilityError {}
export class NotSupportedError extends ObservabilityError {}
export class IllegalArgumentError extends ObservabilityError {}
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/observability/server/errors/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@
* 2.0.
*/

import { ObservabilityError, SLONotFound } from './errors';
import { ObservabilityError, SLOIdConflict, SLONotFound } from './errors';

export function getHTTPResponseCode(error: ObservabilityError): number {
if (error instanceof SLONotFound) {
return 404;
}

if (error instanceof SLOIdConflict) {
return 409;
}

return 400;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
*/

import { v4 as uuidv4 } from 'uuid';
import { IBasePath, IUiSettingsClient, SavedObjectsClientContract } from '@kbn/core/server';
import {
IBasePath,
IUiSettingsClient,
SavedObjectsClientContract,
SavedObjectsFindResponse,
} from '@kbn/core/server';
import {
ElasticsearchClientMock,
elasticsearchServiceMock,
Expand All @@ -26,8 +31,8 @@ import {
ALERT_REASON,
} from '@kbn/rule-data-utils';
import { ALERT_ACTION, getRuleExecutor } from './executor';
import { aStoredSLO, createSLO } from '../../../services/slo/fixtures/slo';
import { SLO } from '../../../domain/models';
import { createSLO } from '../../../services/slo/fixtures/slo';
import { SLO, StoredSLO } from '../../../domain/models';
import { SharePluginStart } from '@kbn/share-plugin/server';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import {
Expand All @@ -38,6 +43,9 @@ import {
AlertStates,
} from './types';
import { SLO_ID_FIELD, SLO_REVISION_FIELD } from '../../../../common/field_names/infra_metrics';
import { SLONotFound } from '../../../errors';
import { SO_SLO_TYPE } from '../../../saved_objects';
import { sloSchema } from '@kbn/slo-schema';

const commonEsResponse = {
took: 100,
Expand All @@ -53,6 +61,21 @@ const commonEsResponse = {
},
};

function createFindResponse(sloList: SLO[]): SavedObjectsFindResponse<StoredSLO> {
return {
page: 1,
per_page: 25,
total: sloList.length,
saved_objects: sloList.map((slo) => ({
id: slo.id,
attributes: sloSchema.encode(slo),
type: SO_SLO_TYPE,
references: [],
score: 1,
})),
};
}

const BURN_RATE_THRESHOLD = 2;
const BURN_RATE_ABOVE_THRESHOLD = BURN_RATE_THRESHOLD + 0.01;
const BURN_RATE_BELOW_THRESHOLD = BURN_RATE_THRESHOLD - 0.01;
Expand Down Expand Up @@ -103,12 +126,12 @@ describe('BurnRateRuleExecutor', () => {
});

it('throws when the slo is not found', async () => {
soClientMock.get.mockRejectedValue(new Error('NotFound'));
soClientMock.find.mockRejectedValue(new SLONotFound('SLO [non-existent] not found'));
const executor = getRuleExecutor({ basePath: basePathMock });

await expect(
executor({
params: someRuleParams({ sloId: 'inexistent', burnRateThreshold: BURN_RATE_THRESHOLD }),
params: someRuleParams({ sloId: 'non-existent', burnRateThreshold: BURN_RATE_THRESHOLD }),
startedAt: new Date(),
services: servicesMock,
executionId: 'irrelevant',
Expand All @@ -124,7 +147,7 @@ describe('BurnRateRuleExecutor', () => {

it('returns early when the slo is disabled', async () => {
const slo = createSLO({ objective: { target: 0.9 }, enabled: false });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
const executor = getRuleExecutor({ basePath: basePathMock });

const result = await executor({
Expand All @@ -148,7 +171,7 @@ describe('BurnRateRuleExecutor', () => {

it('does not schedule an alert when both windows burn rates are below the threshold', async () => {
const slo = createSLO({ objective: { target: 0.9 } });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
esClientMock.search.mockResolvedValue(
generateEsResponse(slo, BURN_RATE_BELOW_THRESHOLD, BURN_RATE_BELOW_THRESHOLD)
);
Expand All @@ -173,7 +196,7 @@ describe('BurnRateRuleExecutor', () => {

it('does not schedule an alert when the long window burn rate is below the threshold', async () => {
const slo = createSLO({ objective: { target: 0.9 } });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
esClientMock.search.mockResolvedValue(
generateEsResponse(slo, BURN_RATE_ABOVE_THRESHOLD, BURN_RATE_BELOW_THRESHOLD)
);
Expand All @@ -198,7 +221,7 @@ describe('BurnRateRuleExecutor', () => {

it('does not schedule an alert when the short window burn rate is below the threshold', async () => {
const slo = createSLO({ objective: { target: 0.9 } });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
esClientMock.search.mockResolvedValue(
generateEsResponse(slo, BURN_RATE_BELOW_THRESHOLD, BURN_RATE_ABOVE_THRESHOLD)
);
Expand All @@ -223,7 +246,7 @@ describe('BurnRateRuleExecutor', () => {

it('schedules an alert when both windows burn rate have reached the threshold', async () => {
const slo = createSLO({ objective: { target: 0.9 } });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
esClientMock.search.mockResolvedValue(
generateEsResponse(slo, BURN_RATE_THRESHOLD, BURN_RATE_THRESHOLD)
);
Expand Down Expand Up @@ -274,7 +297,7 @@ describe('BurnRateRuleExecutor', () => {

it('sets the context on the recovered alerts', async () => {
const slo = createSLO({ objective: { target: 0.9 } });
soClientMock.get.mockResolvedValue(aStoredSLO(slo));
soClientMock.find.mockResolvedValueOnce(createFindResponse([slo]));
esClientMock.search.mockResolvedValue(
generateEsResponse(slo, BURN_RATE_BELOW_THRESHOLD, BURN_RATE_ABOVE_THRESHOLD)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ describe('CreateSLO', () => {
enabled: true,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
})
}),
{ throwOnConflict: true }
);
expect(mockTransformManager.install).toHaveBeenCalledWith(
expect.objectContaining({ ...sloParams, id: expect.any(String) })
Expand Down Expand Up @@ -86,7 +87,8 @@ describe('CreateSLO', () => {
enabled: true,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
})
}),
{ throwOnConflict: true }
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class CreateSLO {
validateSLO(slo);

await this.resourceInstaller.ensureCommonResourcesInstalled();
await this.repository.save(slo);
await this.repository.save(slo, { throwOnConflict: true });

let sloTransformId;
try {
Expand Down Expand Up @@ -55,7 +55,7 @@ export class CreateSLO {
const now = new Date();
return {
...params,
id: uuidv1(),
id: params.id ?? uuidv1(),
settings: {
syncDelay: params.settings?.syncDelay ?? new Duration(1, DurationUnit.Minute),
frequency: params.settings?.frequency ?? new Duration(1, DurationUnit.Minute),
Expand Down
Loading