[8.12] fix(slo): handle invalid stored SLO (#175125) (#175442)

# Backport

This will backport the following commits from `main` to `8.12`:
- [fix(slo): handle invalid stored SLO
(#175125)](https://github.com/elastic/kibana/pull/175125)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Kevin
Delemme","email":"kevin.delemme@elastic.co"},"sourceCommit":{"committedDate":"2024-01-24T14:30:40Z","message":"fix(slo):
handle invalid stored SLO
(#175125)","sha":"ed419896b75bf5b20a0c86ab7ade35016796e5bf","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:fix","backport:prev-minor","Feature:SLO","Team:obs-ux-management","v8.12.1","v8.13.0"],"number":175125,"url":"https://github.com/elastic/kibana/pull/175125","mergeCommit":{"message":"fix(slo):
handle invalid stored SLO
(#175125)","sha":"ed419896b75bf5b20a0c86ab7ade35016796e5bf"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/175125","number":175125,"mergeCommit":{"message":"fix(slo):
handle invalid stored SLO
(#175125)","sha":"ed419896b75bf5b20a0c86ab7ade35016796e5bf"}}]}]
BACKPORT-->

x-pack/plugins/observability/server/lib/rules/slo_burn_rate/executor.ts

@@ -61,6 +61,7 @@ export const getRuleExecutor = ({
   async function executor({
     services,
     params,
+    logger,
     startedAt,
     spaceId,
     getTimeRange,
@@ -82,7 +83,7 @@ export const getRuleExecutor = ({
       getAlertUuid,
     } = services;
 
-    const sloRepository = new KibanaSavedObjectsSLORepository(soClient);
+    const sloRepository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const slo = await sloRepository.findById(params.sloId);
 
     if (!slo.enabled) {

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

@@ -91,7 +91,7 @@ const createSLORoute = createObservabilityServerRoute({
 
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
     const soClient = (await context.core).savedObjects.client;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
     const summaryTransformManager = new DefaultSummaryTransformManager(
       new DefaultSummaryTransformGenerator(),
@@ -129,7 +129,7 @@ const updateSLORoute = createObservabilityServerRoute({
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
     const soClient = (await context.core).savedObjects.client;
 
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
     const summaryTransformManager = new DefaultSummaryTransformManager(
       new DefaultSummaryTransformGenerator(),
@@ -172,7 +172,7 @@ const deleteSLORoute = createObservabilityServerRoute({
     const soClient = (await context.core).savedObjects.client;
     const rulesClient = getRulesClientWithRequest(request);
 
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
 
     const summaryTransformManager = new DefaultSummaryTransformManager(
@@ -200,12 +200,12 @@ const getSLORoute = createObservabilityServerRoute({
     access: 'public',
   },
   params: getSLOParamsSchema,
-  handler: async ({ context, params }) => {
+  handler: async ({ context, params, logger }) => {
     await assertPlatinumLicense(context);
 
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const summaryClient = new DefaultSummaryClient(esClient);
     const getSLO = new GetSLO(repository, summaryClient);
 
@@ -228,7 +228,7 @@ const enableSLORoute = createObservabilityServerRoute({
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
 
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
     const summaryTransformManager = new DefaultSummaryTransformManager(
       new DefaultSummaryTransformGenerator(),
@@ -257,7 +257,7 @@ const disableSLORoute = createObservabilityServerRoute({
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
 
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
     const summaryTransformManager = new DefaultSummaryTransformManager(
       new DefaultSummaryTransformGenerator(),
@@ -288,7 +288,7 @@ const resetSLORoute = createObservabilityServerRoute({
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
 
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);
     const summaryTransformManager = new DefaultSummaryTransformManager(
       new DefaultSummaryTransformGenerator(),
@@ -326,7 +326,7 @@ const findSLORoute = createObservabilityServerRoute({
 
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const summarySearchClient = new DefaultSummarySearchClient(esClient, logger, spaceId);
     const findSLO = new FindSLO(repository, summarySearchClient);
 
@@ -358,11 +358,11 @@ const findSloDefinitionsRoute = createObservabilityServerRoute({
     tags: ['access:slo_read'],
   },
   params: findSloDefinitionsParamsSchema,
-  handler: async ({ context, params }) => {
+  handler: async ({ context, params, logger }) => {
     await assertPlatinumLicense(context);
 
     const soClient = (await context.core).savedObjects.client;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const findSloDefinitions = new FindSLODefinitions(repository);
 
     const response = await findSloDefinitions.execute(params?.query ?? {});
@@ -377,12 +377,12 @@ const fetchHistoricalSummary = createObservabilityServerRoute({
     tags: ['access:slo_read'],
   },
   params: fetchHistoricalSummaryParamsSchema,
-  handler: async ({ context, params }) => {
+  handler: async ({ context, params, logger }) => {
     await assertPlatinumLicense(context);
 
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const historicalSummaryClient = new DefaultHistoricalSummaryClient(esClient);
 
     const fetchSummaryData = new FetchHistoricalSummary(repository, historicalSummaryClient);
@@ -400,12 +400,12 @@ const getSLOInstancesRoute = createObservabilityServerRoute({
     access: 'internal',
   },
   params: getSLOInstancesParamsSchema,
-  handler: async ({ context, params }) => {
+  handler: async ({ context, params, logger }) => {
     await assertPlatinumLicense(context);
 
     const soClient = (await context.core).savedObjects.client;
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
-    const repository = new KibanaSavedObjectsSLORepository(soClient);
+    const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
 
     const getSLOInstances = new GetSLOInstances(repository, esClient);
 
@@ -445,7 +445,7 @@ const getSloBurnRates = createObservabilityServerRoute({
     access: 'internal',
   },
   params: getSLOBurnRatesParamsSchema,
-  handler: async ({ context, params }) => {
+  handler: async ({ context, params, logger }) => {
     await assertPlatinumLicense(context);
 
     const esClient = (await context.core).elasticsearch.client.asCurrentUser;
@@ -457,6 +457,7 @@ const getSloBurnRates = createObservabilityServerRoute({
       {
         soClient,
         esClient,
+        logger,
       }
     );
     return { burnRates };

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

@@ -5,16 +5,18 @@
  * 2.0.
  */
 
-import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
 import { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
-import { KibanaSavedObjectsSLORepository } from './slo_repository';
-import { DefaultSLIClient } from './sli_client';
+import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
+import { Logger } from '@kbn/core/server';
 import { Duration } from '../../domain/models';
-import { computeSLI, computeBurnRate } from '../../domain/services';
+import { computeBurnRate, computeSLI } from '../../domain/services';
+import { DefaultSLIClient } from './sli_client';
+import { KibanaSavedObjectsSLORepository } from './slo_repository';
 
 interface Services {
   soClient: SavedObjectsClientContract;
   esClient: ElasticsearchClient;
+  logger: Logger;
 }
 
 interface LookbackWindow {
@@ -28,9 +30,9 @@ export async function getBurnRates(
   windows: LookbackWindow[],
   services: Services
 ) {
-  const { soClient, esClient } = services;
+  const { soClient, esClient, logger } = services;
 
-  const repository = new KibanaSavedObjectsSLORepository(soClient);
+  const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
   const sliClient = new DefaultSLIClient(esClient);
   const slo = await repository.findById(sloId);
 

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

@@ -6,7 +6,8 @@
  */
 
 import { SavedObjectsClientContract, SavedObjectsFindResponse } from '@kbn/core/server';
-import { savedObjectsClientMock } from '@kbn/core/server/mocks';
+import { loggingSystemMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
+import { MockedLogger } from '@kbn/logging-mocks';
 import { sloSchema } from '@kbn/slo-schema';
 import { SLO_MODEL_VERSION } from '../../../common/slo/constants';
 import { SLO, StoredSLO } from '../../domain/models';
@@ -17,33 +18,53 @@ import { KibanaSavedObjectsSLORepository } from './slo_repository';
 
 const SOME_SLO = createSLO({ indicator: createAPMTransactionDurationIndicator() });
 const ANOTHER_SLO = createSLO();
+const INVALID_SLO_ID = 'invalid-slo-id';
 
-function soFindResponse(sloList: SLO[]): SavedObjectsFindResponse<StoredSLO> {
+function soFindResponse(
+  sloList: SLO[],
+  includeInvalidStoredSLO: boolean = false
+): 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,
-    })),
+    total: includeInvalidStoredSLO ? sloList.length + 1 : sloList.length,
+    // @ts-ignore invalid SLO is not following shape of StoredSLO
+    saved_objects: [
+      ...sloList.map((slo) => ({
+        id: slo.id,
+        attributes: sloSchema.encode(slo),
+        type: SO_SLO_TYPE,
+        references: [],
+        score: 1,
+      })),
+      ...(includeInvalidStoredSLO
+        ? [
+            {
+              id: 'invalid-so-id',
+              type: SO_SLO_TYPE,
+              references: [],
+              score: 1,
+              attributes: { id: INVALID_SLO_ID, name: 'invalid' },
+            },
+          ]
+        : []),
+    ],
   };
 }
 
 describe('KibanaSavedObjectsSLORepository', () => {
+  let loggerMock: jest.Mocked<MockedLogger>;
   let soClientMock: jest.Mocked<SavedObjectsClientContract>;
 
   beforeEach(() => {
+    loggerMock = loggingSystemMock.createLogger();
     soClientMock = savedObjectsClientMock.create();
   });
 
   describe('validation', () => {
     it('findById throws when an SLO is not found', async () => {
       soClientMock.find.mockResolvedValueOnce(soFindResponse([]));
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
 
       await expect(repository.findById('inexistant-slo-id')).rejects.toThrowError(
         new SLONotFound('SLO [inexistant-slo-id] not found')
@@ -52,7 +73,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
 
     it('deleteById throws when an SLO is not found', async () => {
       soClientMock.find.mockResolvedValueOnce(soFindResponse([]));
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
 
       await expect(repository.deleteById('inexistant-slo-id')).rejects.toThrowError(
         new SLONotFound('SLO [inexistant-slo-id] not found')
@@ -65,7 +86,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
       const slo = createSLO({ id: 'my-id' });
       soClientMock.find.mockResolvedValueOnce(soFindResponse([]));
       soClientMock.create.mockResolvedValueOnce(aStoredSLO(slo));
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
 
       const savedSLO = await repository.save(slo);
 
@@ -85,7 +106,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
     it('throws when the SLO id already exists and "throwOnConflict" is true', async () => {
       const slo = createSLO({ id: 'my-id' });
       soClientMock.find.mockResolvedValueOnce(soFindResponse([slo]));
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
 
       await expect(repository.save(slo, { throwOnConflict: true })).rejects.toThrowError(
         new SLOIdConflict(`SLO [my-id] already exists`)
@@ -102,7 +123,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
       const slo = createSLO({ id: 'my-id' });
       soClientMock.find.mockResolvedValueOnce(soFindResponse([slo]));
       soClientMock.create.mockResolvedValueOnce(aStoredSLO(slo));
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
 
       const savedSLO = await repository.save(slo);
 
@@ -120,38 +141,67 @@ describe('KibanaSavedObjectsSLORepository', () => {
     });
   });
 
-  it('finds an existing SLO', async () => {
-    const repository = new KibanaSavedObjectsSLORepository(soClientMock);
-    soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO]));
+  describe('Find SLO', () => {
+    it('finds an existing SLO', async () => {
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
+      soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO]));
 
-    const foundSLO = await repository.findById(SOME_SLO.id);
+      const foundSLO = await repository.findById(SOME_SLO.id);
 
-    expect(foundSLO).toEqual(SOME_SLO);
-    expect(soClientMock.find).toHaveBeenCalledWith({
-      type: SO_SLO_TYPE,
-      page: 1,
-      perPage: 1,
-      filter: `slo.attributes.id:(${SOME_SLO.id})`,
+      expect(foundSLO).toEqual(SOME_SLO);
+      expect(soClientMock.find).toHaveBeenCalledWith({
+        type: SO_SLO_TYPE,
+        page: 1,
+        perPage: 1,
+        filter: `slo.attributes.id:(${SOME_SLO.id})`,
+      });
+    });
+
+    it('throws and logs error on invalid stored SLO', async () => {
+      const INCLUDE_INVALID_STORED_SLO = true;
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
+      soClientMock.find.mockResolvedValueOnce(soFindResponse([], INCLUDE_INVALID_STORED_SLO));
+
+      await expect(repository.findById(INVALID_SLO_ID)).rejects.toThrowError(
+        new Error('Invalid stored SLO')
+      );
+
+      expect(loggerMock.error).toHaveBeenCalled();
     });
   });
 
-  it('finds all SLOs by ids', async () => {
-    const repository = new KibanaSavedObjectsSLORepository(soClientMock);
-    soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO, ANOTHER_SLO]));
+  describe('Find all SLO by ids', () => {
+    it('returns the SLOs', async () => {
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
+      soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO, ANOTHER_SLO]));
 
-    const results = await repository.findAllByIds([SOME_SLO.id, ANOTHER_SLO.id]);
+      const results = await repository.findAllByIds([SOME_SLO.id, ANOTHER_SLO.id]);
 
-    expect(results).toEqual([SOME_SLO, ANOTHER_SLO]);
-    expect(soClientMock.find).toHaveBeenCalledWith({
-      type: SO_SLO_TYPE,
-      page: 1,
-      perPage: 2,
-      filter: `slo.attributes.id:(${SOME_SLO.id} or ${ANOTHER_SLO.id})`,
+      expect(results).toEqual([SOME_SLO, ANOTHER_SLO]);
+      expect(soClientMock.find).toHaveBeenCalledWith({
+        type: SO_SLO_TYPE,
+        page: 1,
+        perPage: 2,
+        filter: `slo.attributes.id:(${SOME_SLO.id} or ${ANOTHER_SLO.id})`,
+      });
+    });
+
+    it('handles invalid stored SLO by logging error', async () => {
+      const INCLUDE_INVALID_STORED_SLO = true;
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
+      soClientMock.find.mockResolvedValueOnce(
+        soFindResponse([SOME_SLO, ANOTHER_SLO], INCLUDE_INVALID_STORED_SLO)
+      );
+
+      const results = await repository.findAllByIds([SOME_SLO.id, INVALID_SLO_ID, ANOTHER_SLO.id]);
+
+      expect(loggerMock.error).toHaveBeenCalled();
+      expect(results).toEqual([SOME_SLO, ANOTHER_SLO]);
     });
   });
 
   it('deletes an SLO', async () => {
-    const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+    const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
     soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO]));
 
     await repository.deleteById(SOME_SLO.id);
@@ -167,7 +217,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
 
   describe('search', () => {
     it('searches by name', async () => {
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
       soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO, ANOTHER_SLO]));
 
       const results = await repository.search(SOME_SLO.name, { page: 1, perPage: 100 });
@@ -183,7 +233,7 @@ describe('KibanaSavedObjectsSLORepository', () => {
     });
 
     it('searches only the outdated ones', async () => {
-      const repository = new KibanaSavedObjectsSLORepository(soClientMock);
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
       soClientMock.find.mockResolvedValueOnce(soFindResponse([SOME_SLO, ANOTHER_SLO]));
 
       const results = await repository.search(
@@ -202,5 +252,18 @@ describe('KibanaSavedObjectsSLORepository', () => {
         filter: `slo.attributes.version < ${SLO_MODEL_VERSION}`,
       });
     });
+
+    it('handles invalid stored SLO by logging error', async () => {
+      const INCLUDE_INVALID_STORED_SLO = true;
+      const repository = new KibanaSavedObjectsSLORepository(soClientMock, loggerMock);
+      soClientMock.find.mockResolvedValueOnce(
+        soFindResponse([SOME_SLO, ANOTHER_SLO], INCLUDE_INVALID_STORED_SLO)
+      );
+
+      const results = await repository.search('*', { page: 1, perPage: 100 });
+
+      expect(loggerMock.error).toHaveBeenCalled();
+      expect(results.results).toEqual([SOME_SLO, ANOTHER_SLO]);
+    });
   });
 });

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

@@ -6,11 +6,9 @@
  */
 
 import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
-import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server';
-import { Paginated, Pagination, sloSchema } from '@kbn/slo-schema';
-import { fold } from 'fp-ts/lib/Either';
-import { pipe } from 'fp-ts/lib/pipeable';
-import * as t from 'io-ts';
+import { Logger } from '@kbn/core/server';
+import { ALL_VALUE, Paginated, Pagination, sloSchema } from '@kbn/slo-schema';
+import { isLeft } from 'fp-ts/lib/Either';
 import { SLO_MODEL_VERSION } from '../../../common/slo/constants';
 import { SLO, StoredSLO } from '../../domain/models';
 import { SLOIdConflict, SLONotFound } from '../../errors';
@@ -29,7 +27,7 @@ export interface SLORepository {
 }
 
 export class KibanaSavedObjectsSLORepository implements SLORepository {
-  constructor(private soClient: SavedObjectsClientContract) {}
+  constructor(private soClient: SavedObjectsClientContract, private logger: Logger) {}
 
   async save(slo: SLO, options = { throwOnConflict: false }): Promise<SLO> {
     let existingSavedObjectId;
@@ -47,12 +45,12 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
       existingSavedObjectId = findResponse.saved_objects[0].id;
     }
 
-    const savedSLO = await this.soClient.create<StoredSLO>(SO_SLO_TYPE, toStoredSLO(slo), {
+    await this.soClient.create<StoredSLO>(SO_SLO_TYPE, toStoredSLO(slo), {
       id: existingSavedObjectId,
       overwrite: true,
     });
 
-    return toSLO(savedSLO.attributes);
+    return slo;
   }
 
   async findById(id: string): Promise<SLO> {
@@ -67,7 +65,12 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
       throw new SLONotFound(`SLO [${id}] not found`);
     }
 
-    return toSLO(response.saved_objects[0].attributes);
+    const slo = this.toSLO(response.saved_objects[0].attributes);
+    if (slo === undefined) {
+      throw new Error('Invalid stored SLO');
+    }
+
+    return slo;
   }
 
   async deleteById(id: string): Promise<void> {
@@ -88,20 +91,16 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
   async findAllByIds(ids: string[]): Promise<SLO[]> {
     if (ids.length === 0) return [];
 
-    try {
-      const response = await this.soClient.find<StoredSLO>({
-        type: SO_SLO_TYPE,
-        page: 1,
-        perPage: ids.length,
-        filter: `slo.attributes.id:(${ids.join(' or ')})`,
-      });
-      return response.saved_objects.map((slo) => toSLO(slo.attributes));
-    } catch (err) {
-      if (SavedObjectsErrorHelpers.isNotFoundError(err)) {
-        throw new SLONotFound(`SLOs [${ids.join(',')}] not found`);
-      }
-      throw err;
-    }
+    const response = await this.soClient.find<StoredSLO>({
+      type: SO_SLO_TYPE,
+      page: 1,
+      perPage: ids.length,
+      filter: `slo.attributes.id:(${ids.join(' or ')})`,
+    });
+
+    return response.saved_objects
+      .map((slo) => this.toSLO(slo.attributes))
+      .filter((slo) => slo !== undefined) as SLO[];
   }
 
   async search(
@@ -124,26 +123,32 @@ export class KibanaSavedObjectsSLORepository implements SLORepository {
       total: response.total,
       perPage: response.per_page,
       page: response.page,
-      results: response.saved_objects.map((slo) => toSLO(slo.attributes)),
+      results: response.saved_objects
+        .map((savedObject) => this.toSLO(savedObject.attributes))
+        .filter((slo) => slo !== undefined) as SLO[],
     };
   }
+
+  toSLO(storedSLO: StoredSLO): SLO | undefined {
+    const result = sloSchema.decode({
+      ...storedSLO,
+      // groupBy was added in 8.10.0
+      groupBy: storedSLO.groupBy ?? ALL_VALUE,
+      // version was added in 8.12.0. This is a safeguard against SO migration issue.
+      // if not present, we considered the version to be 1, e.g. not migrated.
+      // We would need to call the _reset api on this SLO.
+      version: storedSLO.version ?? 1,
+    });
+
+    if (isLeft(result)) {
+      this.logger.error(`Invalid stored SLO with id [${storedSLO.id}]`);
+      return undefined;
+    }
+
+    return result.right;
+  }
 }
 
 function toStoredSLO(slo: SLO): StoredSLO {
   return sloSchema.encode(slo);
 }
-
-function toSLO(storedSLO: StoredSLO): SLO {
-  return pipe(
-    sloSchema.decode({
-      ...storedSLO,
-      // version was added in 8.12.0. This is a safeguard against SO migration issue.
-      // if not present, we considered the version to be 1, e.g. not migrated.
-      // We would need to call the _reset api on this SLO.
-      version: storedSLO.version ?? 1,
-    }),
-    fold(() => {
-      throw new Error('Invalid Stored SLO');
-    }, t.identity)
-  );
-}