fix(slo): introduce cursor pagination in Find SLO API (#203712)

elastic · Jan 8, 2025 · 464d361 · 464d361
1 parent a8e1bf4
commit 464d361
Show file tree

Hide file tree

Showing 14 changed files with 323 additions and 84 deletions.
diff --git a/x-pack/platform/packages/shared/kbn-slo-schema/src/rest_specs/routes/find.test.ts b/x-pack/platform/packages/shared/kbn-slo-schema/src/rest_specs/routes/find.test.ts
@@ -0,0 +1,46 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { merge } from 'lodash';
+import { findSLOParamsSchema } from './find';
+
+const BASE_REQUEST = {
+  query: {
+    filters: 'irrelevant',
+    kqlQuery: 'irrelevant',
+    page: '1',
+    perPage: '25',
+    sortBy: 'error_budget_consumed',
+    sortDirection: 'asc',
+    hideStale: true,
+  },
+};
+
+describe('FindSLO schema validation', () => {
+  it.each(['not_an_array', 42, [], [42, 'ok']])(
+    'returns an error when searchAfter is not a valid JSON array (%s)',
+    (searchAfter) => {
+      const request = merge(BASE_REQUEST, {
+        query: {
+          searchAfter,
+        },
+      });
+      const result = findSLOParamsSchema.decode(request);
+      expect(result._tag === 'Left').toBe(true);
+    }
+  );
+
+  it('parses searchAfter correctly', () => {
+    const request = merge(BASE_REQUEST, {
+      query: {
+        searchAfter: JSON.stringify([1, 'ok']),
+      },
+    });
+    const result = findSLOParamsSchema.decode(request);
+    expect(result._tag === 'Right').toBe(true);
+  });
+});
diff --git a/x-pack/platform/packages/shared/kbn-slo-schema/src/rest_specs/routes/find.ts b/x-pack/platform/packages/shared/kbn-slo-schema/src/rest_specs/routes/find.ts
@@ -4,8 +4,9 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
-import * as t from 'io-ts';
 import { toBooleanRt } from '@kbn/io-ts-utils';
+import { either, isRight } from 'fp-ts/lib/Either';
+import * as t from 'io-ts';
 import { sloWithDataResponseSchema } from '../slo';
 
 const sortDirectionSchema = t.union([t.literal('asc'), t.literal('desc')]);
@@ -19,24 +20,64 @@ const sortBySchema = t.union([
   t.literal('burn_rate_1d'),
 ]);
 
+const searchAfterArraySchema = t.array(t.union([t.string, t.number]));
+type SearchAfterArray = t.TypeOf<typeof searchAfterArraySchema>;
+
+const searchAfterSchema = new t.Type<SearchAfterArray, string, unknown>(
+  'SearchAfter',
+  (input: unknown): input is SearchAfterArray =>
+    Array.isArray(input) &&
+    input.length > 0 &&
+    input.every((item) => typeof item === 'string' || typeof item === 'number'),
+  (input: unknown, context: t.Context) =>
+    either.chain(t.string.validate(input, context), (value: string) => {
+      try {
+        const parsedValue = JSON.parse(value);
+        const decoded = searchAfterArraySchema.decode(parsedValue);
+        if (isRight(decoded)) {
+          return t.success(decoded.right);
+        }
+        return t.failure(
+          input,
+          context,
+          'Invalid searchAfter value, must be a JSON array of strings or numbers'
+        );
+      } catch (err) {
+        return t.failure(
+          input,
+          context,
+          'Invalid searchAfter value, must be a JSON array of strings or numbers'
+        );
+      }
+    }),
+  (input: SearchAfterArray): string => JSON.stringify(input)
+);
+
 const findSLOParamsSchema = t.partial({
   query: t.partial({
     filters: t.string,
     kqlQuery: t.string,
+    // Used for page pagination
     page: t.string,
     perPage: t.string,
     sortBy: sortBySchema,
     sortDirection: sortDirectionSchema,
     hideStale: toBooleanRt,
+    // Used for cursor pagination, searchAfter is a JSON array
+    searchAfter: searchAfterSchema,
+    size: t.string,
   }),
 });
 
-const findSLOResponseSchema = t.type({
-  page: t.number,
-  perPage: t.number,
-  total: t.number,
-  results: t.array(sloWithDataResponseSchema),
-});
+const findSLOResponseSchema = t.intersection([
+  t.type({
+    page: t.number,
+    perPage: t.number,
+    total: t.number,
+    results: t.array(sloWithDataResponseSchema),
+  }),
+  t.partial({ searchAfter: searchAfterArraySchema, size: t.number }),
+]);
 
 type FindSLOParams = t.TypeOf<typeof findSLOParamsSchema.props.query>;
 type FindSLOResponse = t.OutputOf<typeof findSLOResponseSchema>;

diff --git a/x-pack/solutions/observability/plugins/slo/server/routes/slo/route.ts b/x-pack/solutions/observability/plugins/slo/server/routes/slo/route.ts
@@ -57,7 +57,7 @@ import { ManageSLO } from '../../services/manage_slo';
 import { ResetSLO } from '../../services/reset_slo';
 import { SloDefinitionClient } from '../../services/slo_definition_client';
 import { getSloSettings, storeSloSettings } from '../../services/slo_settings';
-import { DefaultSummarySearchClient } from '../../services/summary_search_client';
+import { DefaultSummarySearchClient } from '../../services/summary_search_client/summary_search_client';
 import { DefaultSummaryTransformGenerator } from '../../services/summary_transform_generator/summary_transform_generator';
 import { createTransformGenerators } from '../../services/transform_generators';
 import { createSloServerRoute } from '../create_slo_server_route';

diff --git a/x-pack/solutions/observability/plugins/slo/server/services/find_slo.test.ts b/x-pack/solutions/observability/plugins/slo/server/services/find_slo.test.ts
@@ -12,7 +12,7 @@ import { FindSLO } from './find_slo';
 import { createSLO } from './fixtures/slo';
 import { createSLORepositoryMock, createSummarySearchClientMock } from './mocks';
 import { SLORepository } from './slo_repository';
-import { SummaryResult, SummarySearchClient } from './summary_search_client';
+import type { SummaryResult, SummarySearchClient } from './summary_search_client/types';
 
 describe('FindSLO', () => {
   let mockRepository: jest.Mocked<SLORepository>;
@@ -151,16 +151,27 @@ describe('FindSLO', () => {
   });
 
   describe('validation', () => {
-    it("throws an error when 'perPage > 5000'", async () => {
+    beforeEach(() => {
       const slo = createSLO();
       mockSummarySearchClient.search.mockResolvedValueOnce(summarySearchResult(slo));
       mockRepository.findAllByIds.mockResolvedValueOnce([slo]);
+    });
 
+    it("throws an error when 'perPage' > 5000", async () => {
       await expect(findSLO.execute({ perPage: '5000' })).resolves.not.toThrow();
       await expect(findSLO.execute({ perPage: '5001' })).rejects.toThrowError(
         'perPage limit set to 5000'
       );
     });
+
+    describe('Cursor Pagination', () => {
+      it("throws an error when 'size' > 5000", async () => {
+        await expect(findSLO.execute({ size: '5000' })).resolves.not.toThrow();
+        await expect(findSLO.execute({ size: '5001' })).rejects.toThrowError(
+          'size limit set to 5000'
+        );
+      });
+    });
   });
 });
 

diff --git a/x-pack/solutions/observability/plugins/slo/server/services/find_slo.ts b/x-pack/solutions/observability/plugins/slo/server/services/find_slo.ts
@@ -5,16 +5,22 @@
  * 2.0.
  */
 
-import { FindSLOParams, FindSLOResponse, findSLOResponseSchema, Pagination } from '@kbn/slo-schema';
+import { FindSLOParams, FindSLOResponse, findSLOResponseSchema } from '@kbn/slo-schema';
 import { keyBy } from 'lodash';
 import { SLODefinition } from '../domain/models';
 import { IllegalArgumentError } from '../errors';
 import { SLORepository } from './slo_repository';
-import { Sort, SummaryResult, SummarySearchClient } from './summary_search_client';
+import type {
+  Pagination,
+  Sort,
+  SummaryResult,
+  SummarySearchClient,
+} from './summary_search_client/types';
 
 const DEFAULT_PAGE = 1;
 const DEFAULT_PER_PAGE = 25;
-const MAX_PER_PAGE = 5000;
+const DEFAULT_SIZE = 100;
+const MAX_PER_PAGE_OR_SIZE = 5000;
 
 export class FindSLO {
   constructor(
@@ -38,8 +44,10 @@ export class FindSLO {
     );
 
     return findSLOResponseSchema.encode({
-      page: summaryResults.page,
-      perPage: summaryResults.perPage,
+      page: 'page' in summaryResults ? summaryResults.page : DEFAULT_PAGE,
+      perPage: 'perPage' in summaryResults ? summaryResults.perPage : DEFAULT_PER_PAGE,
+      size: 'size' in summaryResults ? summaryResults.size : undefined,
+      searchAfter: 'searchAfter' in summaryResults ? summaryResults.searchAfter : undefined,
       total: summaryResults.total,
       results: mergeSloWithSummary(localSloDefinitions, summaryResults.results),
     });
@@ -78,16 +86,29 @@ function mergeSloWithSummary(
 }
 
 function toPagination(params: FindSLOParams): Pagination {
+  const isCursorBased = !!params.searchAfter || !!params.size;
+
+  if (isCursorBased) {
+    const size = Number(params.size);
+    if (!isNaN(size) && size > MAX_PER_PAGE_OR_SIZE) {
+      throw new IllegalArgumentError('size limit set to 5000');
+    }
+
+    return {
+      searchAfter: params.searchAfter,
+      size: !isNaN(size) && size > 0 ? size : DEFAULT_SIZE,
+    };
+  }
+
   const page = Number(params.page);
   const perPage = Number(params.perPage);
-
-  if (!isNaN(perPage) && perPage > MAX_PER_PAGE) {
-    throw new IllegalArgumentError(`perPage limit set to ${MAX_PER_PAGE}`);
+  if (!isNaN(perPage) && perPage > MAX_PER_PAGE_OR_SIZE) {
+    throw new IllegalArgumentError('perPage limit set to 5000');
   }
 
   return {
     page: !isNaN(page) && page >= 1 ? page : DEFAULT_PAGE,
-    perPage: !isNaN(perPage) && perPage >= 0 ? perPage : DEFAULT_PER_PAGE,
+    perPage: !isNaN(perPage) && perPage > 0 ? perPage : DEFAULT_PER_PAGE,
   };
 }
 

diff --git a/...k/solutions/observability/plugins/slo/server/services/fixtures/summary_search_document.ts b/...k/solutions/observability/plugins/slo/server/services/fixtures/summary_search_document.ts
@@ -25,7 +25,7 @@ export const aSummaryDocument = (
 
 export const aHitFromSummaryIndex = (_source: any) => {
   return {
-    _index: '.slo-observability.summary-v2',
+    _index: '.slo-observability.summary-v3.3',
     _id: uuidv4(),
     _score: 1,
     _source,
@@ -34,7 +34,7 @@ export const aHitFromSummaryIndex = (_source: any) => {
 
 export const aHitFromTempSummaryIndex = (_source: any) => {
   return {
-    _index: '.slo-observability.summary-v2.temp',
+    _index: '.slo-observability.summary-v3.3.temp',
     _id: uuidv4(),
     _score: 1,
     _source,

diff --git a/x-pack/solutions/observability/plugins/slo/server/services/index.ts b/x-pack/solutions/observability/plugins/slo/server/services/index.ts
@@ -22,3 +22,4 @@ export * from './summary_client';
 export * from './get_slo_groupings';
 export * from './find_slo_groups';
 export * from './get_slo_health';
+export * from './summary_search_client/summary_search_client';
diff --git a/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts b/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts
@@ -9,7 +9,7 @@ import { ResourceInstaller } from '../resource_installer';
 import { BurnRatesClient } from '../burn_rates_client';
 import { SLORepository } from '../slo_repository';
 import { SummaryClient } from '../summary_client';
-import { SummarySearchClient } from '../summary_search_client';
+import { SummarySearchClient } from '../summary_search_client/types';
 import { TransformManager } from '../transform_manager';
 
 const createResourceInstallerMock = (): jest.Mocked<ResourceInstaller> => {

diff --git a/...hots__/summary_search_client.test.ts.snap → ...hots__/summary_search_client.test.ts.snap b/...hots__/summary_search_client.test.ts.snap → ...hots__/summary_search_client.test.ts.snap
diff --git a/...er/services/summary_search_client.test.ts → ...arch_client/summary_search_client.test.ts b/...er/services/summary_search_client.test.ts → ...arch_client/summary_search_client.test.ts
@@ -8,13 +8,14 @@
 import { ElasticsearchClientMock, elasticsearchServiceMock } from '@kbn/core/server/mocks';
 import { loggerMock } from '@kbn/logging-mocks';
 import { Pagination } from '@kbn/slo-schema/src/models/pagination';
-import { createSLO } from './fixtures/slo';
+import { createSLO } from '../fixtures/slo';
 import {
   aHitFromSummaryIndex,
   aHitFromTempSummaryIndex,
   aSummaryDocument,
-} from './fixtures/summary_search_document';
-import { DefaultSummarySearchClient, Sort, SummarySearchClient } from './summary_search_client';
+} from '../fixtures/summary_search_document';
+import { DefaultSummarySearchClient } from './summary_search_client';
+import type { Sort, SummarySearchClient } from './types';
 
 const defaultSort: Sort = {
   field: 'sli_value',
@@ -169,6 +170,12 @@ describe('Summary Search Client', () => {
           sliValue: {
             order: 'asc',
           },
+          'slo.id': {
+            order: 'asc',
+          },
+          'slo.instanceId': {
+            order: 'asc',
+          },
         },
         track_total_hits: true,
       },
@@ -202,6 +209,12 @@ describe('Summary Search Client', () => {
           sliValue: {
             order: 'asc',
           },
+          'slo.id': {
+            order: 'asc',
+          },
+          'slo.instanceId': {
+            order: 'asc',
+          },
         },
         track_total_hits: true,
       },
@@ -229,7 +242,16 @@ describe('Summary Search Client', () => {
           },
         },
         size: 40,
-        sort: { isTempDoc: { order: 'asc' }, sliValue: { order: 'asc' } },
+        sort: {
+          isTempDoc: { order: 'asc' },
+          sliValue: { order: 'asc' },
+          'slo.id': {
+            order: 'asc',
+          },
+          'slo.instanceId': {
+            order: 'asc',
+          },
+        },
         track_total_hits: true,
       },
     ]);