feat(slo): Handle updating SLO (elastic#142273)

x-pack/plugins/observability/server/assets/constants.ts

@@ -12,4 +12,5 @@ export const SLO_RESOURCES_VERSION = 1;
 export const SLO_INGEST_PIPELINE_NAME = `${SLO_INDEX_TEMPLATE_NAME}.monthly`;
 export const SLO_DESTINATION_INDEX_NAME = `${SLO_INDEX_TEMPLATE_NAME}-v${SLO_RESOURCES_VERSION}`;
 
-export const getSLOTransformId = (sloId: string) => `slo-${sloId}`;
+export const getSLOTransformId = (sloId: string, sloRevision: number) =>
+  `slo-${sloId}-${sloRevision}`;

x-pack/plugins/observability/server/routes/slo/route.ts

@@ -12,22 +12,23 @@ import {
   DefaultTransformManager,
   KibanaSavedObjectsSLORepository,
   GetSLO,
+  UpdateSLO,
 } from '../../services/slo';
-
 import {
   ApmTransactionDurationTransformGenerator,
   ApmTransactionErrorRateTransformGenerator,
   TransformGenerator,
 } from '../../services/slo/transform_generators';
-import { SLITypes } from '../../types/models';
+import { IndicatorTypes } from '../../types/models';
 import {
   createSLOParamsSchema,
   deleteSLOParamsSchema,
   getSLOParamsSchema,
+  updateSLOParamsSchema,
 } from '../../types/rest_specs';
 import { createObservabilityServerRoute } from '../create_observability_server_route';
 
-const transformGenerators: Record<SLITypes, TransformGenerator> = {
+const transformGenerators: Record<IndicatorTypes, TransformGenerator> = {
   'slo.apm.transaction_duration': new ApmTransactionDurationTransformGenerator(),
   'slo.apm.transaction_error_rate': new ApmTransactionErrorRateTransformGenerator(),
 };
@@ -53,6 +54,26 @@ const createSLORoute = createObservabilityServerRoute({
   },
 });
 
+const updateSLORoute = createObservabilityServerRoute({
+  endpoint: 'PUT /api/observability/slos/{id}',
+  options: {
+    tags: [],
+  },
+  params: updateSLOParamsSchema,
+  handler: async ({ context, params, logger }) => {
+    const esClient = (await context.core).elasticsearch.client.asCurrentUser;
+    const soClient = (await context.core).savedObjects.client;
+
+    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
+    const updateSLO = new UpdateSLO(repository, transformManager, esClient);
+
+    const response = await updateSLO.execute(params.path.id, params.body);
+
+    return response;
+  },
+});
+
 const deleteSLORoute = createObservabilityServerRoute({
   endpoint: 'DELETE /api/observability/slos/{id}',
   options: {
@@ -89,4 +110,9 @@ const getSLORoute = createObservabilityServerRoute({
   },
 });
 
-export const slosRouteRepository = { ...createSLORoute, ...getSLORoute, ...deleteSLORoute };
+export const slosRouteRepository = {
+  ...createSLORoute,
+  ...updateSLORoute,
+  ...getSLORoute,
+  ...deleteSLORoute,
+};

x-pack/plugins/observability/server/services/slo/create_slo.ts

@@ -20,8 +20,8 @@ export class CreateSLO {
     private transformManager: TransformManager
   ) {}
 
-  public async execute(sloParams: CreateSLOParams): Promise<CreateSLOResponse> {
-    const slo = this.toSLO(sloParams);
+  public async execute(params: CreateSLOParams): Promise<CreateSLOResponse> {
+    const slo = this.toSLO(params);
 
     await this.resourceInstaller.ensureCommonResourcesInstalled();
     await this.repository.save(slo);
@@ -48,10 +48,14 @@ export class CreateSLO {
     return this.toResponse(slo);
   }
 
-  private toSLO(sloParams: CreateSLOParams): SLO {
+  private toSLO(params: CreateSLOParams): SLO {
+    const now = new Date();
     return {
-      ...sloParams,
+      ...params,
       id: uuid.v1(),
+      revision: 1,
+      created_at: now,
+      updated_at: now,
     };
   }
 

x-pack/plugins/observability/server/services/slo/delete_slo.test.ts

@@ -30,12 +30,17 @@ describe('DeleteSLO', () => {
   describe('happy path', () => {
     it('removes the transform, the roll up data and the SLO from the repository', async () => {
       const slo = createSLO(createAPMTransactionErrorRateIndicator());
+      mockRepository.findById.mockResolvedValueOnce(slo);
 
       await deleteSLO.execute(slo.id);
 
       expect(mockRepository.findById).toHaveBeenCalledWith(slo.id);
-      expect(mockTransformManager.stop).toHaveBeenCalledWith(getSLOTransformId(slo.id));
-      expect(mockTransformManager.uninstall).toHaveBeenCalledWith(getSLOTransformId(slo.id));
+      expect(mockTransformManager.stop).toHaveBeenCalledWith(
+        getSLOTransformId(slo.id, slo.revision)
+      );
+      expect(mockTransformManager.uninstall).toHaveBeenCalledWith(
+        getSLOTransformId(slo.id, slo.revision)
+      );
       expect(mockEsClient.deleteByQuery).toHaveBeenCalledWith(
         expect.objectContaining({
           index: `${SLO_INDEX_TEMPLATE_NAME}*`,

x-pack/plugins/observability/server/services/slo/delete_slo.ts

@@ -19,15 +19,14 @@ export class DeleteSLO {
   ) {}
 
   public async execute(sloId: string): Promise<void> {
-    // ensure the slo exists on the request's space.
-    await this.repository.findById(sloId);
+    const slo = await this.repository.findById(sloId);
 
-    const sloTransformId = getSLOTransformId(sloId);
+    const sloTransformId = getSLOTransformId(slo.id, slo.revision);
     await this.transformManager.stop(sloTransformId);
     await this.transformManager.uninstall(sloTransformId);
 
-    await this.deleteRollupData(sloId);
-    await this.repository.deleteById(sloId);
+    await this.deleteRollupData(slo.id);
+    await this.repository.deleteById(slo.id);
   }
 
   private async deleteRollupData(sloId: string): Promise<void> {

x-pack/plugins/observability/server/services/slo/fixtures/slo.ts

@@ -6,10 +6,15 @@
  */
 
 import uuid from 'uuid';
-import { SLI, SLO } from '../../../types/models';
+import {
+  APMTransactionDurationIndicator,
+  APMTransactionErrorRateIndicator,
+  Indicator,
+  SLO,
+} from '../../../types/models';
 import { CreateSLOParams } from '../../../types/rest_specs';
 
-const commonSLO: Omit<CreateSLOParams, 'indicator'> = {
+const defaultSLO: Omit<SLO, 'indicator' | 'id' | 'created_at' | 'updated_at'> = {
   name: 'irrelevant',
   description: 'irrelevant',
   time_window: {
@@ -20,20 +25,29 @@ const commonSLO: Omit<CreateSLOParams, 'indicator'> = {
   objective: {
     target: 0.999,
   },
+  revision: 1,
 };
 
-export const createSLOParams = (indicator: SLI): CreateSLOParams => ({
-  ...commonSLO,
+export const createSLOParams = (indicator: Indicator): CreateSLOParams => ({
+  ...defaultSLO,
   indicator,
 });
 
-export const createSLO = (indicator: SLI): SLO => ({
-  ...commonSLO,
-  id: uuid.v1(),
-  indicator,
-});
+export const createSLO = (indicator: Indicator): SLO => {
+  const now = new Date();
+  return {
+    ...defaultSLO,
+    id: uuid.v1(),
+    indicator,
+    revision: 1,
+    created_at: now,
+    updated_at: now,
+  };
+};
 
-export const createAPMTransactionErrorRateIndicator = (params = {}): SLI => ({
+export const createAPMTransactionErrorRateIndicator = (
+  params: Partial<APMTransactionErrorRateIndicator['params']> = {}
+): Indicator => ({
   type: 'slo.apm.transaction_error_rate',
   params: {
     environment: 'irrelevant',
@@ -45,7 +59,9 @@ export const createAPMTransactionErrorRateIndicator = (params = {}): SLI => ({
   },
 });
 
-export const createAPMTransactionDurationIndicator = (params = {}): SLI => ({
+export const createAPMTransactionDurationIndicator = (
+  params: Partial<APMTransactionDurationIndicator['params']> = {}
+): Indicator => ({
   type: 'slo.apm.transaction_duration',
   params: {
     environment: 'irrelevant',

x-pack/plugins/observability/server/services/slo/get_slo.test.ts

@@ -49,6 +49,9 @@ describe('GetSLO', () => {
           duration: '7d',
           is_rolling: true,
         },
+        created_at: slo.created_at,
+        updated_at: slo.updated_at,
+        revision: slo.revision,
       });
     });
   });

x-pack/plugins/observability/server/services/slo/get_slo.ts

@@ -26,6 +26,9 @@ export class GetSLO {
       time_window: slo.time_window,
       budgeting_method: slo.budgeting_method,
       objective: slo.objective,
+      revision: slo.revision,
+      created_at: slo.created_at,
+      updated_at: slo.updated_at,
     };
   }
 }

x-pack/plugins/observability/server/services/slo/index.ts

@@ -11,3 +11,4 @@ export * from './transform_manager';
 export * from './create_slo';
 export * from './delete_slo';
 export * from './get_slo';
+export * from './update_slo';

x-pack/plugins/observability/server/services/slo/slo_repository.test.ts

@@ -20,11 +20,7 @@ const SOME_SLO = createSLO(createAPMTransactionDurationIndicator());
 function aStoredSLO(slo: SLO): SavedObject<StoredSLO> {
   return {
     id: slo.id,
-    attributes: {
-      ...slo,
-      updated_at: new Date().toISOString(),
-      created_at: new Date().toISOString(),
-    },
+    attributes: slo,
     type: SO_SLO_TYPE,
     references: [],
   };
@@ -73,7 +69,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
         updated_at: expect.anything(),
         created_at: expect.anything(),
       }),
-      { id: SOME_SLO.id }
+      { id: SOME_SLO.id, overwrite: true }
     );
   });
 

x-pack/plugins/observability/server/services/slo/slo_repository.ts

@@ -22,24 +22,18 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
   constructor(private soClient: SavedObjectsClientContract) {}
 
   async save(slo: SLO): Promise<SLO> {
-    const now = new Date().toISOString();
-    const savedSLO = await this.soClient.create<StoredSLO>(
-      SO_SLO_TYPE,
-      {
-        ...slo,
-        created_at: now,
-        updated_at: now,
-      },
-      { id: slo.id }
-    );
+    const savedSLO = await this.soClient.create<StoredSLO>(SO_SLO_TYPE, slo, {
+      id: slo.id,
+      overwrite: true,
+    });
 
-    return toSLOModel(savedSLO.attributes);
+    return savedSLO.attributes;
   }
 
   async findById(id: string): Promise<SLO> {
     try {
       const slo = await this.soClient.get<StoredSLO>(SO_SLO_TYPE, id);
-      return toSLOModel(slo.attributes);
+      return slo.attributes;
     } catch (err) {
       if (SavedObjectsErrorHelpers.isNotFoundError(err)) {
         throw new SLONotFound(`SLO [${id}] not found`);
@@ -59,15 +53,3 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
     }
   }
 }
-
-function toSLOModel(slo: StoredSLO): SLO {
-  return {
-    id: slo.id,
-    name: slo.name,
-    description: slo.description,
-    indicator: slo.indicator,
-    time_window: slo.time_window,
-    budgeting_method: slo.budgeting_method,
-    objective: slo.objective,
-  };
-}

x-pack/plugins/observability/server/services/slo/transform_generators/apm_transaction_duration.test.ts

@@ -19,10 +19,13 @@ describe('APM Transaction Duration Transform Generator', () => {
       transform_id: expect.any(String),
       source: { runtime_mappings: { 'slo.id': { script: { source: expect.any(String) } } } },
     });
-    expect(transform.transform_id).toEqual(`slo-${anSLO.id}`);
+    expect(transform.transform_id).toEqual(`slo-${anSLO.id}-${anSLO.revision}`);
     expect(transform.source.runtime_mappings!['slo.id']).toMatchObject({
       script: { source: `emit('${anSLO.id}')` },
     });
+    expect(transform.source.runtime_mappings!['slo.revision']).toMatchObject({
+      script: { source: `emit(${anSLO.revision})` },
+    });
   });
 
   it("does not include the query filter when params are '*'", async () => {