[8.10] backport SLO fixes (#168231)

elastic · Oct 10, 2023 · afa98f7 · afa98f7
1 parent dead5ec
commit afa98f7
Show file tree

Hide file tree

Showing 37 changed files with 263 additions and 98 deletions.
diff --git a/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts
@@ -5,13 +5,16 @@
  * 2.0.
  */
 
+import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public';
 import { i18n } from '@kbn/i18n';
 import type { CreateSLOInput, CreateSLOResponse, FindSLOResponse } from '@kbn/slo-schema';
 import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
 import { v1 as uuidv1 } from 'uuid';
 import { useKibana } from '../../utils/kibana_react';
 import { sloKeys } from './query_key_factory';
 
+type ServerError = IHttpFetchError<ResponseErrorBody>;
+
 export function useCloneSlo() {
   const {
     http,
@@ -21,7 +24,7 @@ export function useCloneSlo() {
 
   return useMutation<
     CreateSLOResponse,
-    string,
+    ServerError,
     { slo: CreateSLOInput; originalSloId?: string },
     { previousData?: FindSLOResponse; queryKey?: QueryKey }
   >(
@@ -58,16 +61,17 @@ export function useCloneSlo() {
         return { queryKey, previousData };
       },
       // If the mutation fails, use the context returned from onMutate to roll back
-      onError: (_err, { slo }, context) => {
+      onError: (error, { slo }, context) => {
         if (context?.previousData && context?.queryKey) {
           queryClient.setQueryData(context.queryKey, context.previousData);
         }
-        toasts.addDanger(
-          i18n.translate('xpack.observability.slo.clone.errorNotification', {
+
+        toasts.addError(new Error(error.body?.message ?? error.message), {
+          title: i18n.translate('xpack.observability.slo.clone.errorNotification', {
             defaultMessage: 'Failed to clone {name}',
             values: { name: slo.name },
-          })
-        );
+          }),
+        });
       },
       onSuccess: (_data, { slo }) => {
         toasts.addSuccess(

diff --git a/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public';
 import { i18n } from '@kbn/i18n';
 import { encode } from '@kbn/rison';
 import type { CreateSLOInput, CreateSLOResponse, FindSLOResponse } from '@kbn/slo-schema';
@@ -14,6 +15,8 @@ import { paths } from '../../../common/locators/paths';
 import { useKibana } from '../../utils/kibana_react';
 import { sloKeys } from './query_key_factory';
 
+type ServerError = IHttpFetchError<ResponseErrorBody>;
+
 export function useCreateSlo() {
   const {
     application: { navigateToUrl },
@@ -24,7 +27,7 @@ export function useCreateSlo() {
 
   return useMutation<
     CreateSLOResponse,
-    string,
+    ServerError,
     { slo: CreateSLOInput },
     { previousData?: FindSLOResponse; queryKey?: QueryKey }
   >(
@@ -72,7 +75,7 @@ export function useCreateSlo() {
           queryClient.setQueryData(context.queryKey, context.previousData);
         }
 
-        toasts.addError(new Error(String(error)), {
+        toasts.addError(new Error(error.body?.message ?? error.message), {
           title: i18n.translate('xpack.observability.slo.create.errorNotification', {
             defaultMessage: 'Something went wrong while creating {name}',
             values: { name: slo.name },

diff --git a/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts
@@ -8,9 +8,12 @@
 import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
 import { i18n } from '@kbn/i18n';
 import { FindSLOResponse } from '@kbn/slo-schema';
+import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public';
 import { useKibana } from '../../utils/kibana_react';
 import { sloKeys } from './query_key_factory';
 
+type ServerError = IHttpFetchError<ResponseErrorBody>;
+
 export function useDeleteSlo() {
   const {
     http,
@@ -20,7 +23,7 @@ export function useDeleteSlo() {
 
   return useMutation<
     string,
-    string,
+    ServerError,
     { id: string; name: string },
     { previousData?: FindSLOResponse; queryKey?: QueryKey }
   >(
@@ -56,17 +59,17 @@ export function useDeleteSlo() {
         return { previousData, queryKey };
       },
       // If the mutation fails, use the context returned from onMutate to roll back
-      onError: (_err, { name }, context) => {
+      onError: (error, { name }, context) => {
         if (context?.previousData && context?.queryKey) {
           queryClient.setQueryData(context.queryKey, context.previousData);
         }
 
-        toasts.addDanger(
-          i18n.translate('xpack.observability.slo.slo.delete.errorNotification', {
+        toasts.addError(new Error(error.body?.message ?? error.message), {
+          title: i18n.translate('xpack.observability.slo.slo.delete.errorNotification', {
             defaultMessage: 'Failed to delete {name}',
             values: { name },
-          })
-        );
+          }),
+        });
       },
       onSuccess: (_data, { name }) => {
         toasts.addSuccess(

diff --git a/x-pack/plugins/observability/public/hooks/slo/use_get_preview_data.ts b/x-pack/plugins/observability/public/hooks/slo/use_get_preview_data.ts
@@ -42,7 +42,7 @@ export function useGetPreviewData(isValid: boolean, indicator: Indicator): UseGe
           }
         );
 
-        return response;
+        return Array.isArray(response) ? response : [];
       },
       retry: false,
       refetchOnWindowFocus: false,

diff --git a/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts
@@ -5,12 +5,15 @@
  * 2.0.
  */
 
+import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public';
 import { i18n } from '@kbn/i18n';
 import type { FindSLOResponse, UpdateSLOInput, UpdateSLOResponse } from '@kbn/slo-schema';
 import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
 import { useKibana } from '../../utils/kibana_react';
 import { sloKeys } from './query_key_factory';
 
+type ServerError = IHttpFetchError<ResponseErrorBody>;
+
 export function useUpdateSlo() {
   const {
     http,
@@ -20,7 +23,7 @@ export function useUpdateSlo() {
 
   return useMutation<
     UpdateSLOResponse,
-    string,
+    ServerError,
     { sloId: string; slo: UpdateSLOInput },
     { previousData?: FindSLOResponse; queryKey?: QueryKey }
   >(
@@ -69,7 +72,7 @@ export function useUpdateSlo() {
           queryClient.setQueryData(context.queryKey, context.previousData);
         }
 
-        toasts.addError(new Error(String(error)), {
+        toasts.addError(new Error(error.body?.message ?? error.message), {
           title: i18n.translate('xpack.observability.slo.update.errorNotification', {
             defaultMessage: 'Something went wrong when updating {name}',
             values: { name },

diff --git a/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx b/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx
@@ -21,7 +21,7 @@ export function SloList({ autoRefresh }: Props) {
   const [query, setQuery] = useState('');
   const [sort, setSort] = useState<SortField | undefined>('status');
 
-  const { isInitialLoading, isLoading, isRefetching, isError, sloList, refetch } = useFetchSloList({
+  const { isLoading, isRefetching, isError, sloList } = useFetchSloList({
     page: activePage + 1,
     kqlQuery: query,
     sortBy: sort,
@@ -38,34 +38,23 @@ export function SloList({ autoRefresh }: Props) {
 
   const handlePageClick = (pageNumber: number) => {
     setActivePage(pageNumber);
-    refetch();
   };
 
   const handleChangeQuery = (newQuery: string) => {
     setActivePage(0);
     setQuery(newQuery);
-    refetch();
   };
 
   const handleChangeSort = (newSort: SortField | undefined) => {
     setActivePage(0);
     setSort(newSort);
-    refetch();
   };
 
   return (
     <EuiFlexGroup direction="column" gutterSize="m" data-test-subj="sloList">
       <EuiFlexItem grow>
         <SloListSearchFilterSortBar
-          loading={
-            isInitialLoading ||
-            isLoading ||
-            isRefetching ||
-            isCreatingSlo ||
-            isCloningSlo ||
-            isUpdatingSlo ||
-            isDeletingSlo
-          }
+          loading={isLoading || isCreatingSlo || isCloningSlo || isUpdatingSlo || isDeletingSlo}
           onChangeQuery={handleChangeQuery}
           onChangeSort={handleChangeSort}
         />

diff --git a/...ck/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx b/...ck/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx
@@ -16,6 +16,7 @@ import {
   EuiSelectableOption,
 } from '@elastic/eui';
 import { EuiSelectableOptionCheckedType } from '@elastic/eui/src/components/selectable/selectable_option';
+import { Query } from '@kbn/es-query';
 import { i18n } from '@kbn/i18n';
 import { QueryStringInput } from '@kbn/unified-search-plugin/public';
 import React, { useState } from 'react';
@@ -103,9 +104,13 @@ export function SloListSearchFilterSortBar({
             unifiedSearch,
           }}
           disableAutoFocus
-          onSubmit={() => onChangeQuery(query)}
+          onSubmit={(value: Query) => {
+            setQuery(String(value.query));
+            onChangeQuery(String(value.query));
+          }}
           disableLanguageSwitcher
           isDisabled={loading}
+          autoSubmit
           indexPatterns={dataView ? [dataView] : []}
           placeholder={i18n.translate('xpack.observability.slo.list.search', {
             defaultMessage: 'Search your SLOs...',

diff --git a/x-pack/plugins/observability/server/assets/constants.ts b/x-pack/plugins/observability/server/assets/constants.ts
@@ -6,6 +6,7 @@
  */
 
 export const SLO_RESOURCES_VERSION = 2;
+export const SLO_SUMMARY_TRANSFORMS_VERSION = 3;
 
 export const SLO_COMPONENT_TEMPLATE_MAPPINGS_NAME = '.slo-observability.sli-mappings';
 export const SLO_COMPONENT_TEMPLATE_SETTINGS_NAME = '.slo-observability.sli-settings';

diff --git a/x-pack/plugins/observability/server/assets/transform_templates/slo_transform_template.ts b/x-pack/plugins/observability/server/assets/transform_templates/slo_transform_template.ts
@@ -36,6 +36,7 @@ export const getSLOTransformTemplate = (
   dest: destination,
   settings: {
     deduce_mappings: false,
+    unattended: true,
   },
   sync: {
     time: {

diff --git a/x-pack/plugins/observability/server/errors/errors.ts b/x-pack/plugins/observability/server/errors/errors.ts
@@ -25,3 +25,5 @@ export class InternalQueryError extends ObservabilityError {}
 export class NotSupportedError extends ObservabilityError {}
 export class IllegalArgumentError extends ObservabilityError {}
 export class InvalidTransformError extends ObservabilityError {}
+
+export class SecurityException extends ObservabilityError {}
diff --git a/x-pack/plugins/observability/server/errors/handler.ts b/x-pack/plugins/observability/server/errors/handler.ts
@@ -9,6 +9,7 @@ import {
   CompositeSLOIdConflict,
   CompositeSLONotFound,
   ObservabilityError,
+  SecurityException,
   SLOIdConflict,
   SLONotFound,
 } from './errors';
@@ -22,5 +23,9 @@ export function getHTTPResponseCode(error: ObservabilityError): number {
     return 409;
   }
 
+  if (error instanceof SecurityException) {
+    return 403;
+  }
+
   return 400;
 }
diff --git a/x-pack/plugins/observability/server/services/slo/create_slo.test.ts b/x-pack/plugins/observability/server/services/slo/create_slo.test.ts
@@ -55,6 +55,7 @@ describe('CreateSLO', () => {
       expect(mockTransformManager.install).toHaveBeenCalledWith(
         expect.objectContaining({ ...sloParams, id: 'unique-id' })
       );
+      expect(mockTransformManager.preview).toHaveBeenCalledWith('slo-transform-id');
       expect(mockTransformManager.start).toHaveBeenCalledWith('slo-transform-id');
       expect(response).toEqual(expect.objectContaining({ id: 'unique-id' }));
       expect(esClientMock.index.mock.calls[0]).toMatchSnapshot();
@@ -100,6 +101,16 @@ describe('CreateSLO', () => {
       expect(mockRepository.deleteById).toBeCalled();
     });
 
+    it('removes the transform and deletes the SLO when transform preview fails', async () => {
+      mockTransformManager.install.mockResolvedValue('slo-transform-id');
+      mockTransformManager.preview.mockRejectedValue(new Error('Transform preview error'));
+      const sloParams = createSLOParams({ indicator: createAPMTransactionErrorRateIndicator() });
+
+      await expect(createSLO.execute(sloParams)).rejects.toThrowError('Transform preview error');
+      expect(mockTransformManager.uninstall).toBeCalledWith('slo-transform-id');
+      expect(mockRepository.deleteById).toBeCalled();
+    });
+
     it('removes the transform and deletes the SLO when transform start fails', async () => {
       mockTransformManager.install.mockResolvedValue('slo-transform-id');
       mockTransformManager.start.mockRejectedValue(new Error('Transform start error'));

diff --git a/x-pack/plugins/observability/server/services/slo/create_slo.ts b/x-pack/plugins/observability/server/services/slo/create_slo.ts
@@ -36,6 +36,7 @@ export class CreateSLO {
     }
 
     try {
+      await this.transformManager.preview(sloTransformId);
       await this.transformManager.start(sloTransformId);
     } catch (err) {
       await Promise.all([

diff --git a/x-pack/plugins/observability/server/services/slo/get_preview_data.ts b/x-pack/plugins/observability/server/services/slo/get_preview_data.ts
@@ -16,12 +16,13 @@ import {
   KQLCustomIndicator,
   MetricCustomIndicator,
 } from '@kbn/slo-schema';
+import { assertNever } from '@kbn/std';
 import { APMTransactionDurationIndicator } from '../../domain/models';
 import { computeSLI } from '../../domain/services';
 import { InvalidQueryError } from '../../errors';
 import {
-  GetHistogramIndicatorAggregation,
   GetCustomMetricIndicatorAggregation,
+  GetHistogramIndicatorAggregation,
 } from './aggregations';
 
 export class GetPreviewData {
@@ -299,19 +300,24 @@ export class GetPreviewData {
   }
 
   public async execute(params: GetPreviewDataParams): Promise<GetPreviewDataResponse> {
-    switch (params.indicator.type) {
-      case 'sli.apm.transactionDuration':
-        return this.getAPMTransactionDurationPreviewData(params.indicator);
-      case 'sli.apm.transactionErrorRate':
-        return this.getAPMTransactionErrorPreviewData(params.indicator);
-      case 'sli.kql.custom':
-        return this.getCustomKQLPreviewData(params.indicator);
-      case 'sli.histogram.custom':
-        return this.getHistogramPreviewData(params.indicator);
-      case 'sli.metric.custom':
-        return this.getCustomMetricPreviewData(params.indicator);
-      default:
-        return [];
+    try {
+      const type = params.indicator.type;
+      switch (type) {
+        case 'sli.apm.transactionDuration':
+          return this.getAPMTransactionDurationPreviewData(params.indicator);
+        case 'sli.apm.transactionErrorRate':
+          return this.getAPMTransactionErrorPreviewData(params.indicator);
+        case 'sli.kql.custom':
+          return this.getCustomKQLPreviewData(params.indicator);
+        case 'sli.histogram.custom':
+          return this.getHistogramPreviewData(params.indicator);
+        case 'sli.metric.custom':
+          return this.getCustomMetricPreviewData(params.indicator);
+        default:
+          assertNever(type);
+      }
+    } catch (err) {
+      return [];
     }
   }
 }

diff --git a/x-pack/plugins/observability/server/services/slo/historical_summary_client.ts b/x-pack/plugins/observability/server/services/slo/historical_summary_client.ts
@@ -279,6 +279,7 @@ function generateSearchQuery(
               window: timeWindowDurationInDays * bucketsPerDay,
               shift: 1,
               script: 'MovingFunctions.sum(values)',
+              gap_policy: 'insert_zeros',
             },
           },
           cumulative_total: {
@@ -287,6 +288,7 @@ function generateSearchQuery(
               window: timeWindowDurationInDays * bucketsPerDay,
               shift: 1,
               script: 'MovingFunctions.sum(values)',
+              gap_policy: 'insert_zeros',
             },
           },
         },

diff --git a/x-pack/plugins/observability/server/services/slo/mocks/index.ts b/x-pack/plugins/observability/server/services/slo/mocks/index.ts
@@ -28,6 +28,7 @@ const createSummaryTransformInstallerMock = (): jest.Mocked<SummaryTransformInst
 const createTransformManagerMock = (): jest.Mocked<TransformManager> => {
   return {
     install: jest.fn(),
+    preview: jest.fn(),
     uninstall: jest.fn(),
     start: jest.fn(),
     stop: jest.fn(),