From a8335fa57e51c476a717d78170e026b088ac86b6 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 19 Oct 2023 21:42:32 +0200 Subject: [PATCH 1/8] [Discover] Custom grid toolbar (#166744) - Closes https://github.com/elastic/kibana/issues/167245 ## Summary This PR customizes the grid toolbar for Discover. The controls will have the "ToolbarButton" look. Screenshot 2023-10-17 at 11 23 16 Screenshot 2023-10-17 at 11 23 00 Screenshot 2023-10-17 at 11 23 44 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Davis McPhee --- .../src/panels_resizable.tsx | 18 +- packages/kbn-unified-data-table/index.ts | 6 +- .../src/components/data_table.scss | 4 + .../src/components/data_table.test.tsx | 52 +++- .../src/components/data_table.tsx | 48 ++- .../data_table_document_selection.tsx | 18 +- .../context/context_app_content.test.tsx | 1 + .../context/context_app_content.tsx | 8 +- .../document_explorer_callout.tsx | 1 + .../document_explorer_update_callout.tsx | 1 + .../layout/discover_documents.test.tsx | 5 + .../components/layout/discover_documents.tsx | 287 +++++++++++------- .../components/layout/discover_layout.scss | 14 - .../layout/discover_main_content.test.tsx | 9 +- .../layout/discover_main_content.tsx | 62 ++-- .../layout/discover_resizable_layout.tsx | 1 - .../render_custom_toolbar.test.tsx.snap | 237 +++++++++++++++ .../discover_grid/discover_grid.tsx | 20 ++ .../public/components/discover_grid/index.ts | 9 + .../discover_grid/render_custom_toolbar.scss | 61 ++++ .../render_custom_toolbar.test.tsx | 63 ++++ .../discover_grid/render_custom_toolbar.tsx | 129 ++++++++ .../components/doc_table/_doc_table.scss | 1 + .../view_mode_toggle/view_mode_toggle.tsx | 17 +- .../saved_search_embeddable_base.tsx | 35 ++- .../saved_search_embeddable_component.tsx | 1 - .../public/embeddable/saved_search_grid.tsx | 30 +- .../embeddable/_saved_search_embeddable.ts | 5 + .../apps/discover/group2/_data_grid.ts | 4 + .../discover/group2/_data_grid_context.ts | 4 + .../apps/discover/group3/_view_mode_toggle.ts | 130 ++++++++ test/functional/apps/discover/group3/index.ts | 1 + .../translations/translations/fr-FR.json | 1 - .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 35 files changed, 1069 insertions(+), 216 deletions(-) create mode 100644 src/plugins/discover/public/components/discover_grid/__snapshots__/render_custom_toolbar.test.tsx.snap create mode 100644 src/plugins/discover/public/components/discover_grid/discover_grid.tsx create mode 100644 src/plugins/discover/public/components/discover_grid/index.ts create mode 100644 src/plugins/discover/public/components/discover_grid/render_custom_toolbar.scss create mode 100644 src/plugins/discover/public/components/discover_grid/render_custom_toolbar.test.tsx create mode 100644 src/plugins/discover/public/components/discover_grid/render_custom_toolbar.tsx create mode 100644 test/functional/apps/discover/group3/_view_mode_toggle.ts diff --git a/packages/kbn-resizable-layout/src/panels_resizable.tsx b/packages/kbn-resizable-layout/src/panels_resizable.tsx index 968e5203047fe..300c9130b8100 100644 --- a/packages/kbn-resizable-layout/src/panels_resizable.tsx +++ b/packages/kbn-resizable-layout/src/panels_resizable.tsx @@ -64,6 +64,17 @@ export const PanelsResizable = ({ () => setResizeWithPortalsHackIsResizing(false), [] ); + const baseButtonCss = css` + background-color: transparent !important; + gap: 0 !important; + + &:not(:hover):not(:focus) { + &:before, + &:after { + width: 0; + } + } + `; const defaultButtonCss = css` z-index: 3; `; @@ -207,9 +218,10 @@ export const PanelsResizable = ({ { expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` Object { - "additionalControls": , + "additionalControls": null, "showColumnSelector": false, "showDisplaySelector": Object { "additionalDisplaySettings": { expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` Object { - "additionalControls": , + "additionalControls": null, "showColumnSelector": false, "showDisplaySelector": Object { "allowDensity": false, @@ -360,7 +360,7 @@ describe('UnifiedDataTable', () => { expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` Object { - "additionalControls": , + "additionalControls": null, "showColumnSelector": false, "showDisplaySelector": undefined, "showFullScreenSelector": true, @@ -511,6 +511,52 @@ describe('UnifiedDataTable', () => { }); }); + describe('renderCustomToolbar', () => { + it('should render a custom toolbar', async () => { + let toolbarParams: Record = {}; + let gridParams: Record = {}; + const renderCustomToolbarMock = jest.fn((props) => { + toolbarParams = props.toolbarProps; + gridParams = props.gridProps; + return
Custom layout
; + }); + const component = await getComponent({ + ...getProps(), + renderCustomToolbar: renderCustomToolbarMock, + }); + + // custom toolbar should be rendered + expect(findTestSubject(component, 'custom-toolbar').exists()).toBe(true); + + expect(renderCustomToolbarMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + toolbarProps: expect.objectContaining({ + hasRoomForGridControls: true, + }), + gridProps: expect.objectContaining({ + additionalControls: null, + }), + }) + ); + + // the default eui controls should be available for custom rendering + expect(toolbarParams?.columnSortingControl).toBeTruthy(); + expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); + expect(gridParams?.additionalControls).toBe(null); + + // additional controls become available after selecting a document + act(() => { + component + .find('[data-gridcell-column-id="select"] .euiCheckbox__input') + .first() + .simulate('change'); + }); + + expect(toolbarParams?.keyboardShortcutsControl).toBeTruthy(); + expect(gridParams?.additionalControls).toBeTruthy(); + }); + }); + describe('gridStyleOverride', () => { it('should render the grid with the default style if no gridStyleOverride is provided', async () => { const component = await getComponent({ diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 22a625f479e3b..4ce88e52e6c8d 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -27,8 +27,11 @@ import { EuiDataGridControlColumn, EuiDataGridCustomBodyProps, EuiDataGridCellValueElementProps, + EuiDataGridCustomToolbarProps, + EuiDataGridToolBarVisibilityOptions, EuiDataGridToolBarVisibilityDisplaySelectorOptions, EuiDataGridStyle, + EuiDataGridProps, } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import { @@ -66,6 +69,17 @@ import { import { UnifiedDataTableFooter } from './data_table_footer'; import { UnifiedDataTableAdditionalDisplaySettings } from './data_table_additional_display_settings'; +export interface UnifiedDataTableRenderCustomToolbarProps { + toolbarProps: EuiDataGridCustomToolbarProps; + gridProps: { + additionalControls?: EuiDataGridToolBarVisibilityOptions['additionalControls']; + }; +} + +export type UnifiedDataTableRenderCustomToolbar = ( + props: UnifiedDataTableRenderCustomToolbarProps +) => React.ReactElement; + export type SortOrder = [string, string]; export enum DataLoadingState { @@ -288,6 +302,12 @@ export interface UnifiedDataTableProps { * It receives #EuiDataGridCustomBodyProps as its only argument. */ renderCustomGridBody?: (args: EuiDataGridCustomBodyProps) => React.ReactNode; + /** + * Optional render for the grid toolbar + * @param toolbarProps + * @param gridProps + */ + renderCustomToolbar?: UnifiedDataTableRenderCustomToolbar; /** * An optional list of the EuiDataGridControlColumn type for setting trailing control columns standard for EuiDataGrid. */ @@ -360,6 +380,7 @@ export const UnifiedDataTable = ({ onFieldEdited, services, renderCustomGridBody, + renderCustomToolbar, trailingControlColumns, totalHits, onFetchMoreRecords, @@ -709,8 +730,12 @@ export const UnifiedDataTable = ({ : internalControlColumns; }, [canSetExpandedDoc, externalControlColumns, controlColumnIds]); - const additionalControls = useMemo( - () => ( + const additionalControls = useMemo(() => { + if (!externalAdditionalControls && !usedSelectedDocs.length) { + return null; + } + + return ( <> {usedSelectedDocs.length ? ( - ), - [usedSelectedDocs, isFilterActive, rows, externalAdditionalControls] + ); + }, [usedSelectedDocs, isFilterActive, rows, externalAdditionalControls]); + + const renderCustomToolbarFn: EuiDataGridProps['renderCustomToolbar'] | undefined = useMemo( + () => + renderCustomToolbar + ? (toolbarProps) => + renderCustomToolbar({ + toolbarProps, + gridProps: { + additionalControls, + }, + }) + : undefined, + [renderCustomToolbar, additionalControls] ); const showDisplaySelector = useMemo(() => { @@ -852,10 +890,12 @@ export const UnifiedDataTable = ({ inMemory={inMemory} gridStyle={gridStyleOverride ?? GRID_STYLE} renderCustomGridBody={renderCustomGridBody} + renderCustomToolbar={renderCustomToolbarFn} trailingControlColumns={trailingControlColumns} /> {loadingState !== DataLoadingState.loading && + !usedSelectedDocs.length && // hide footer when showing selected documents isPaginationEnabled && ( // we hide the footer for Surrounding Documents page - + + + + + + {selectedDocs.length} + + } > diff --git a/src/plugins/discover/public/application/context/context_app_content.test.tsx b/src/plugins/discover/public/application/context/context_app_content.test.tsx index f6809d63c035d..f6c87772ec913 100644 --- a/src/plugins/discover/public/application/context/context_app_content.test.tsx +++ b/src/plugins/discover/public/application/context/context_app_content.test.tsx @@ -104,5 +104,6 @@ describe('ContextAppContent test', () => { it('should render discover grid correctly', async () => { const component = await mountComponent({ isLegacy: false }); expect(component.find(UnifiedDataTable).length).toBe(1); + expect(findTestSubject(component, 'dscGridToolbar').exists()).toBe(true); }); }); diff --git a/src/plugins/discover/public/application/context/context_app_content.tsx b/src/plugins/discover/public/application/context/context_app_content.tsx index 81ca3e6f81b66..5b10995fad145 100644 --- a/src/plugins/discover/public/application/context/context_app_content.tsx +++ b/src/plugins/discover/public/application/context/context_app_content.tsx @@ -26,8 +26,9 @@ import { ROW_HEIGHT_OPTION, SHOW_MULTIFIELDS, } from '@kbn/discover-utils'; -import { DataLoadingState, UnifiedDataTable } from '@kbn/unified-data-table'; +import { DataLoadingState } from '@kbn/unified-data-table'; import { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; +import { DiscoverGrid } from '../../components/discover_grid'; import { getDefaultRowsPerPage } from '../../../common/constants'; import { LoadingStatus } from './services/context_query_state'; import { ActionBar } from './components/action_bar/action_bar'; @@ -37,7 +38,6 @@ import { MAX_CONTEXT_SIZE, MIN_CONTEXT_SIZE } from './services/constants'; import { DocTableContext } from '../../components/doc_table/doc_table_context'; import { useDiscoverServices } from '../../hooks/use_discover_services'; import { DiscoverGridFlyout } from '../../components/discover_grid_flyout'; -import { DISCOVER_TOUR_STEP_ANCHOR_IDS } from '../../components/discover_tour'; export interface ContextAppContentProps { columns: string[]; @@ -66,7 +66,7 @@ export function clamp(value: number) { return Math.max(Math.min(MAX_CONTEXT_SIZE, value), MIN_CONTEXT_SIZE); } -const DiscoverGridMemoized = React.memo(UnifiedDataTable); +const DiscoverGridMemoized = React.memo(DiscoverGrid); const DocTableContextMemoized = React.memo(DocTableContext); const ActionBarMemoized = React.memo(ActionBar); @@ -191,7 +191,6 @@ export function ContextAppContent({ diff --git a/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_callout.tsx b/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_callout.tsx index a18389806f8f9..9822f0081fd30 100644 --- a/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_callout.tsx +++ b/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_callout.tsx @@ -60,6 +60,7 @@ export const DocumentExplorerCallout = () => { return ( } iconType="search" diff --git a/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_update_callout.tsx b/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_update_callout.tsx index 617790c28907b..82ed197d2977b 100644 --- a/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_update_callout.tsx +++ b/src/plugins/discover/public/application/main/components/document_explorer_callout/document_explorer_update_callout.tsx @@ -51,6 +51,7 @@ export const DocumentExplorerUpdateCallout = () => { return ( } iconType="tableDensityNormal" diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx index fdbd153122cd1..187c2b4719249 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { BehaviorSubject } from 'rxjs'; +import { findTestSubject } from '@elastic/eui/lib/test'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { setHeaderActionMenuMounter } from '../../../../kibana_services'; import { DataDocuments$ } from '../../services/discover_data_state_container'; @@ -40,6 +41,7 @@ async function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { stateContainer.dataState.data$.documents$ = documents$; const props = { + viewModeToggle:
test
, dataView: dataViewMock, onAddFilter: jest.fn(), stateContainer, @@ -76,6 +78,9 @@ describe('Discover documents layout', () => { const component = await mountComponent(FetchStatus.COMPLETE, esHitsMock); expect(component.find('.dscDocuments__loading').exists()).toBeFalsy(); expect(component.find('.dscTable').exists()).toBeTruthy(); + expect(findTestSubject(component, 'dscGridToolbar').exists()).toBe(true); + expect(findTestSubject(component, 'dscGridToolbarBottom').exists()).toBe(true); + expect(findTestSubject(component, 'viewModeToggle').exists()).toBe(true); }); test('should set rounded width to state on resize column', () => { diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 60367b83d02ed..774d47d577a6d 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -23,7 +23,6 @@ import type { DataTableRecord } from '@kbn/discover-utils/types'; import { SearchResponseWarnings } from '@kbn/search-response-warnings'; import { DataLoadingState, - UnifiedDataTable, useColumns, type DataTableColumnTypes, getTextBasedColumnTypes, @@ -38,7 +37,10 @@ import { SHOW_MULTIFIELDS, SORT_DEFAULT_ORDER_SETTING, } from '@kbn/discover-utils'; +import { i18n } from '@kbn/i18n'; +import useObservable from 'react-use/lib/useObservable'; import type { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; +import { DiscoverGrid } from '../../../../components/discover_grid'; import { getDefaultRowsPerPage } from '../../../../../common/constants'; import { useInternalStateSelector } from '../../services/discover_internal_state_container'; import { useAppStateSelector } from '../../services/discover_app_state_container'; @@ -60,8 +62,11 @@ import { getAllowedSampleSize, } from '../../../../utils/get_allowed_sample_size'; import { DiscoverGridFlyout } from '../../../../components/discover_grid_flyout'; +import { getRenderCustomToolbarWithElements } from '../../../../components/discover_grid/render_custom_toolbar'; import { useSavedSearchInitial } from '../../services/discover_state_provider'; import { useFetchMoreRecords } from './use_fetch_more_records'; +import { ErrorCallout } from '../../../../components/common/error_callout'; +import { SelectedVSAvailableCallout } from './selected_vs_available_callout'; const containerStyles = css` position: relative; @@ -74,7 +79,7 @@ const progressStyle = css` const TOUR_STEPS = { expandButton: DISCOVER_TOUR_STEP_ANCHOR_IDS.expandDocument }; const DocTableInfiniteMemoized = React.memo(DocTableInfinite); -const DiscoverGridMemoized = React.memo(UnifiedDataTable); +const DiscoverGridMemoized = React.memo(DiscoverGrid); // export needs for testing export const onResize = ( @@ -92,11 +97,13 @@ export const onResize = ( }; function DiscoverDocumentsComponent({ + viewModeToggle, dataView, onAddFilter, stateContainer, onFieldEdited, }: { + viewModeToggle: React.ReactElement | undefined; dataView: DataView; onAddFilter?: DocViewFilterFn; stateContainer: DiscoverStateContainer; @@ -249,6 +256,86 @@ function DiscoverDocumentsComponent({ [dataView, onAddColumn, onAddFilter, onRemoveColumn, query, savedSearch.id, setExpandedDoc] ); + const dataState = useDataState(stateContainer.dataState.data$.main$); + const documents = useObservable(stateContainer.dataState.data$.documents$); + + const callouts = useMemo( + () => ( + <> + {dataState.error && ( + + )} + + {!!documentState.interceptedWarnings?.length && ( + + )} + + ), + [ + dataState.error, + isTextBasedQuery, + currentColumns, + documents?.textBasedQueryColumns, + documentState.interceptedWarnings, + ] + ); + + const gridAnnouncementCallout = useMemo(() => { + if (hideAnnouncements || isLegacy) { + return null; + } + + return !isTextBasedQuery ? ( + + + + ) : null; + }, [hideAnnouncements, isLegacy, isTextBasedQuery]); + + const loadingIndicator = useMemo( + () => + isDataLoading ? ( + + ) : null, + [isDataLoading] + ); + + const renderCustomToolbar = useMemo( + () => + getRenderCustomToolbarWithElements({ + leftSide: viewModeToggle, + bottomSection: ( + <> + {callouts} + {gridAnnouncementCallout} + {loadingIndicator} + + ), + }), + [viewModeToggle, callouts, gridAnnouncementCallout, loadingIndicator] + ); + if (isDataViewLoading || (isEmptyDataResult && isDataLoading)) { return (
@@ -262,111 +349,103 @@ function DiscoverDocumentsComponent({ } return ( - - -

- -

- - {!!documentState.interceptedWarnings?.length && ( - - )} - {isLegacy && rows && rows.length > 0 && ( - <> - {!hideAnnouncements && } - - - )} - {!isLegacy && ( + <> + {isLegacy && ( <> - {!hideAnnouncements && !isTextBasedQuery && ( - - - - )} -
- - - -
+ {viewModeToggle} + {callouts} )} - {isDataLoading && ( - - )} - + + +

+ +

+ + {isLegacy && ( + <> + {rows && rows.length > 0 && ( + <> + {!hideAnnouncements && } + + + )} + {loadingIndicator} + + )} + {!isLegacy && ( + <> +
+ + + +
+ + )} + + ); } diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.scss b/src/plugins/discover/public/application/main/components/layout/discover_layout.scss index d3978fe1c1d82..55972b4f7f629 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.scss +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.scss @@ -32,16 +32,6 @@ discover-app { height: 100%; } -.dscSidebarResizeButton { - background-color: transparent !important; - - &:not(:hover):not(:focus) { - &:before, &:after { - width: 0; - } - } -} - .dscPageContent__wrapper { overflow: hidden; // Ensures horizontal scroll of table display: flex; @@ -53,10 +43,6 @@ discover-app { position: relative; overflow: hidden; height: 100%; - - .euiDataGrid__controls { - border-top: none; - } } .dscPageContent--centered { diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx index 2901b49de1586..b12208cc76299 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx @@ -125,12 +125,17 @@ describe('Discover main content component', () => { describe('DocumentViewModeToggle', () => { it('should show DocumentViewModeToggle when isPlainRecord is false', async () => { const component = await mountComponent(); - expect(component.find(DocumentViewModeToggle).exists()).toBe(true); + expect(component.find(DiscoverDocuments).prop('viewModeToggle')).toBeDefined(); }); it('should not show DocumentViewModeToggle when isPlainRecord is true', async () => { const component = await mountComponent({ isPlainRecord: true }); - expect(component.find(DocumentViewModeToggle).exists()).toBe(false); + expect(component.find(DiscoverDocuments).prop('viewModeToggle')).toBeUndefined(); + }); + + it('should show DocumentViewModeToggle for Field Statistics', async () => { + const component = await mountComponent({ viewMode: VIEW_MODE.AGGREGATED_LEVEL }); + expect(component.find(DocumentViewModeToggle).exists()).toBe(true); }); }); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx index e241a52b1d259..8b6ff5880d3dc 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx @@ -6,10 +6,9 @@ * Side Public License, v 1. */ -import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiHorizontalRule } from '@elastic/eui'; import { DragDrop, type DropType, DropOverlayWrapper } from '@kbn/dom-drag-drop'; -import useObservable from 'react-use/lib/useObservable'; -import React, { useCallback } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { DataView } from '@kbn/data-views-plugin/common'; import { METRIC_TYPE } from '@kbn/analytics'; import { i18n } from '@kbn/i18n'; @@ -21,9 +20,7 @@ import { DiscoverStateContainer } from '../../services/discover_state'; import { FieldStatisticsTab } from '../field_stats_table'; import { DiscoverDocuments } from './discover_documents'; import { DOCUMENTS_VIEW_CLICK, FIELD_STATISTICS_VIEW_CLICK } from '../field_stats_table/constants'; -import { ErrorCallout } from '../../../../components/common/error_callout'; -import { useDataState } from '../../hooks/use_data_state'; -import { SelectedVSAvailableCallout } from './selected_vs_available_callout'; +import { useAppStateSelector } from '../../services/discover_app_state_container'; const DROP_PROPS = { value: { @@ -76,10 +73,16 @@ export const DiscoverMainContent = ({ [trackUiMetric, stateContainer] ); - const dataState = useDataState(stateContainer.dataState.data$.main$); - const documents = useObservable(stateContainer.dataState.data$.documents$); const isDropAllowed = Boolean(onDropFieldToTable); + const viewModeToggle = useMemo(() => { + return !isPlainRecord ? ( + + ) : undefined; + }, [viewMode, setDiscoverViewMode, isPlainRecord]); + + const showChart = useAppStateSelector((state) => !state.hideChart); + return ( - - {!isPlainRecord && ( - - )} - - {dataState.error && ( - - )} - - + {showChart && } {viewMode === VIEW_MODE.DOCUMENT_LEVEL ? ( ) : ( - + <> + {viewModeToggle} + + )} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_resizable_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_resizable_layout.tsx index 32491a38d86fd..e0859617f0057 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_resizable_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_resizable_layout.tsx @@ -71,7 +71,6 @@ export const DiscoverResizableLayout = ({ minFlexPanelSize={minMainPanelWidth} fixedPanel={} flexPanel={} - resizeButtonClassName="dscSidebarResizeButton" data-test-subj="discoverLayout" onFixedPanelSizeChange={setSidebarWidth} /> diff --git a/src/plugins/discover/public/components/discover_grid/__snapshots__/render_custom_toolbar.test.tsx.snap b/src/plugins/discover/public/components/discover_grid/__snapshots__/render_custom_toolbar.test.tsx.snap new file mode 100644 index 0000000000000..9f2af58f1dccb --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/__snapshots__/render_custom_toolbar.test.tsx.snap @@ -0,0 +1,237 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`renderCustomToolbar should render correctly for smaller screens 1`] = ` + + + + + + + + +
+
+ keyboard +
+
+ display +
+
+ fullScreen +
+
+ + + + + +`; + +exports[`renderCustomToolbar should render correctly with an element 1`] = ` + + + +
+ left +
+ + + + + +
+ additional +
+ + +
+ column +
+ + +
+ columnSorting +
+ + + +
+
+ keyboard +
+
+ display +
+
+ fullScreen +
+
+ + + + +
+
+ bottom +
+
+ +`; + +exports[`renderCustomToolbar should render successfully 1`] = ` + + + + + + +
+ column +
+ + +
+ columnSorting +
+ + +
+ additional +
+ + + + + + + +
+
+ keyboard +
+
+ display +
+
+ fullScreen +
+
+ + + + + +`; diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx new file mode 100644 index 0000000000000..fe2586e32f895 --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx @@ -0,0 +1,20 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { UnifiedDataTable, type UnifiedDataTableProps } from '@kbn/unified-data-table'; +import { renderCustomToolbar } from './render_custom_toolbar'; + +/** + * Customized version of the UnifiedDataTable + * @param props + * @constructor + */ +export const DiscoverGrid: React.FC = (props) => { + return ; +}; diff --git a/src/plugins/discover/public/components/discover_grid/index.ts b/src/plugins/discover/public/components/discover_grid/index.ts new file mode 100644 index 0000000000000..b9057e101ee9e --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/index.ts @@ -0,0 +1,9 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { DiscoverGrid } from './discover_grid'; diff --git a/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.scss b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.scss new file mode 100644 index 0000000000000..e24f7f8c6fcb1 --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.scss @@ -0,0 +1,61 @@ +.dscGridToolbar { + padding: $euiSizeS $euiSizeS $euiSizeXS; +} + +.dscGridToolbarControlButton .euiDataGrid__controlBtn { + block-size: $euiSizeXL; + border: $euiBorderThin; + border-radius: $euiBorderRadiusSmall; + + // making the icons larger than the default size + & svg { + inline-size: $euiSize; + block-size: $euiSize; + } + + // cancel default background and font changes + &.euiDataGrid__controlBtn--active { + font-weight: $euiFontWeightMedium; + } + &:active, &:focus { + background: transparent; + } + + // add toolbar control animation + transition: transform $euiAnimSpeedNormal ease-in-out; + &:hover { + transform: translateY(-1px); + } + &:active { + transform: translateY(0); + } +} + +.dscGridToolbarControlGroup { + box-shadow: inset 0 0 0 $euiBorderWidthThin $euiBorderColor; + border-radius: $euiBorderRadiusSmall; + display: inline-flex; + align-items: stretch; + flex-direction: row; +} + +.dscGridToolbarControlIconButton .euiButtonIcon { + inline-size: $euiSizeXL; + block-size: $euiSizeXL; + + // cancel default behaviour + &:hover, &:active, &:focus { + background: transparent; + animation: none !important; + transform: none !important; + } + + .dscGridToolbarControlIconButton + & { + border-inline-start: $euiBorderThin; + border-radius: 0; + } +} + +.dscGridToolbarBottom { + position: relative; // for placing a loading indicator correctly +} diff --git a/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.test.tsx b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.test.tsx new file mode 100644 index 0000000000000..3b8a4bb9457f9 --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.test.tsx @@ -0,0 +1,63 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { renderCustomToolbar, getRenderCustomToolbarWithElements } from './render_custom_toolbar'; + +describe('renderCustomToolbar', () => { + it('should render successfully', () => { + expect( + renderCustomToolbar({ + toolbarProps: { + hasRoomForGridControls: true, + columnControl: 'column', + columnSortingControl: 'columnSorting', + displayControl: 'display', + fullScreenControl: 'fullScreen', + keyboardShortcutsControl: 'keyboard', + }, + gridProps: { additionalControls: 'additional' }, + }) + ).toMatchSnapshot(); + }); + + it('should render correctly for smaller screens', () => { + expect( + renderCustomToolbar({ + toolbarProps: { + hasRoomForGridControls: false, + columnControl: 'column', + columnSortingControl: 'columnSorting', + displayControl: 'display', + fullScreenControl: 'fullScreen', + keyboardShortcutsControl: 'keyboard', + }, + gridProps: { additionalControls: 'additional' }, + }) + ).toMatchSnapshot(); + }); + + it('should render correctly with an element', () => { + expect( + getRenderCustomToolbarWithElements({ + leftSide:
left
, + bottomSection:
bottom
, + })({ + toolbarProps: { + hasRoomForGridControls: true, + columnControl: 'column', + columnSortingControl: 'columnSorting', + displayControl: 'display', + fullScreenControl: 'fullScreen', + keyboardShortcutsControl: 'keyboard', + }, + gridProps: { additionalControls: 'additional' }, + }) + ).toMatchSnapshot(); + }); +}); diff --git a/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.tsx b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.tsx new file mode 100644 index 0000000000000..2baeb4c287a16 --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/render_custom_toolbar.tsx @@ -0,0 +1,129 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import type { + UnifiedDataTableRenderCustomToolbarProps, + UnifiedDataTableRenderCustomToolbar, +} from '@kbn/unified-data-table'; +import './render_custom_toolbar.scss'; + +interface RenderCustomToolbarProps extends UnifiedDataTableRenderCustomToolbarProps { + leftSide?: React.ReactElement; + bottomSection?: React.ReactElement; +} + +export const renderCustomToolbar = (props: RenderCustomToolbarProps): React.ReactElement => { + const { + leftSide, + bottomSection, + toolbarProps: { + hasRoomForGridControls, + columnControl, + columnSortingControl, + fullScreenControl, + keyboardShortcutsControl, + displayControl, + }, + gridProps: { additionalControls }, + } = props; + + const buttons = hasRoomForGridControls ? ( + <> + {leftSide && additionalControls && ( + +
{additionalControls}
+ + )} + {columnControl && ( + +
{columnControl}
+ + )} + {columnSortingControl && ( + +
{columnSortingControl}
+ + )} + {!leftSide && additionalControls && ( + +
{additionalControls}
+ + )} + + ) : null; + + return ( + <> + + + {leftSide || ( + + {buttons} + + )} + + + + {Boolean(leftSide) && buttons} + {(keyboardShortcutsControl || displayControl || fullScreenControl) && ( + +
+ {keyboardShortcutsControl && ( +
+ {keyboardShortcutsControl} +
+ )} + {displayControl && ( +
{displayControl}
+ )} + {fullScreenControl && ( +
{fullScreenControl}
+ )} +
+ + )} + + + + {bottomSection ? ( +
+ {bottomSection} +
+ ) : null} + + ); +}; + +/** + * Render custom element on the left side and all controls to the right + */ +export const getRenderCustomToolbarWithElements = ({ + leftSide, + bottomSection, +}: { + leftSide?: React.ReactElement; + bottomSection?: React.ReactElement; +}): UnifiedDataTableRenderCustomToolbar => { + const reservedSpace = <>; + return (props) => + renderCustomToolbar({ + ...props, + leftSide: leftSide || reservedSpace, + bottomSection, + }); +}; diff --git a/src/plugins/discover/public/components/doc_table/_doc_table.scss b/src/plugins/discover/public/components/doc_table/_doc_table.scss index 4553cdc05fdad..8a9b629a9694b 100644 --- a/src/plugins/discover/public/components/doc_table/_doc_table.scss +++ b/src/plugins/discover/public/components/doc_table/_doc_table.scss @@ -4,6 +4,7 @@ // stylelint-disable selector-no-qualifying-type .kbnDocTableWrapper { @include euiScrollBar; + @include euiOverflowShadow; overflow: auto; display: flex; flex: 1 1 100%; diff --git a/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx b/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx index 63fcbcc40db37..79c9213e76395 100644 --- a/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx +++ b/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx @@ -6,11 +6,11 @@ * Side Public License, v 1. */ -import React from 'react'; +import React, { useMemo } from 'react'; import { EuiTab, EuiTabs, useEuiTheme } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; import { css } from '@emotion/react'; -import { SHOW_FIELD_STATISTICS } from '@kbn/discover-utils'; +import { DOC_TABLE_LEGACY, SHOW_FIELD_STATISTICS } from '@kbn/discover-utils'; import { VIEW_MODE } from '../../../common/constants'; import { useDiscoverServices } from '../../hooks/use_discover_services'; @@ -23,9 +23,16 @@ export const DocumentViewModeToggle = ({ }) => { const { euiTheme } = useEuiTheme(); const { uiSettings } = useDiscoverServices(); + const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); + const includesNormalTabsStyle = viewMode === VIEW_MODE.AGGREGATED_LEVEL || isLegacy; + const tabsPadding = includesNormalTabsStyle ? euiTheme.size.s : 0; const tabsCss = css` - padding: 0 ${euiTheme.size.s}; + padding: ${tabsPadding} ${tabsPadding} 0 ${tabsPadding}; + + .euiTab__content { + line-height: ${euiTheme.size.xl}; + } `; const showViewModeToggle = uiSettings.get(SHOW_FIELD_STATISTICS) ?? false; @@ -35,11 +42,10 @@ export const DocumentViewModeToggle = ({ } return ( - + setDiscoverViewMode(VIEW_MODE.DOCUMENT_LEVEL)} - className="dscViewModeToggle__tab" data-test-subj="dscViewModeDocumentButton" > @@ -47,7 +53,6 @@ export const DocumentViewModeToggle = ({ setDiscoverViewMode(VIEW_MODE.AGGREGATED_LEVEL)} - className="dscViewModeToggle__tab" data-test-subj="dscViewModeFieldStatsButton" > data-test-subj={dataTestSubj} > {isLoading && } - - - {Boolean(prepend) && {prepend}} - {!!totalHitCount && ( - - - - )} - - + {Boolean(prepend || totalHitCount) && ( + + + {Boolean(prepend) && {prepend}} + + {!!totalHitCount && ( + + + + )} + + + )} {children} diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx index 43085e3c0902e..b9bd2112f6ee6 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx @@ -47,7 +47,6 @@ export function SavedSearchEmbeddableComponent({ sampleSizeState={fetchedSampleSize} loadingState={searchProps.isLoading ? DataLoadingState.loading : DataLoadingState.loaded} showFullScreenButton={false} - showColumnTokens query={query} className="unifiedDataTable" /> diff --git a/src/plugins/discover/public/embeddable/saved_search_grid.tsx b/src/plugins/discover/public/embeddable/saved_search_grid.tsx index 1cadf5414f8a3..9b0653bd63352 100644 --- a/src/plugins/discover/public/embeddable/saved_search_grid.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_grid.tsx @@ -5,21 +5,22 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import type { DataTableRecord } from '@kbn/discover-utils/types'; import { AggregateQuery, Query } from '@kbn/es-query'; import type { SearchResponseInterceptedWarning } from '@kbn/search-response-warnings'; +import { MAX_DOC_FIELDS_DISPLAYED, ROW_HEIGHT_OPTION, SHOW_MULTIFIELDS } from '@kbn/discover-utils'; import { - DataLoadingState as DiscoverGridLoadingState, - UnifiedDataTable, + type UnifiedDataTableProps, type DataTableColumnTypes, + DataLoadingState as DiscoverGridLoadingState, } from '@kbn/unified-data-table'; -import type { UnifiedDataTableProps } from '@kbn/unified-data-table'; +import { DiscoverGrid } from '../components/discover_grid'; import './saved_search_grid.scss'; -import { MAX_DOC_FIELDS_DISPLAYED, ROW_HEIGHT_OPTION, SHOW_MULTIFIELDS } from '@kbn/discover-utils'; import { DiscoverGridFlyout } from '../components/discover_grid_flyout'; import { SavedSearchEmbeddableBase } from './saved_search_embeddable_base'; -import { DISCOVER_TOUR_STEP_ANCHOR_IDS } from '../components/discover_tour'; +import { getRenderCustomToolbarWithElements } from '../components/discover_grid/render_custom_toolbar'; +import { TotalDocuments } from '../application/main/components/total_documents/total_documents'; export interface DiscoverGridEmbeddableProps extends Omit { @@ -32,7 +33,7 @@ export interface DiscoverGridEmbeddableProps savedSearchId?: string; } -export const DiscoverGridMemoized = React.memo(UnifiedDataTable); +export const DiscoverGridMemoized = React.memo(DiscoverGrid); export function DiscoverGridEmbeddable(props: DiscoverGridEmbeddableProps) { const { interceptedWarnings, ...gridProps } = props; @@ -71,9 +72,20 @@ export function DiscoverGridEmbeddable(props: DiscoverGridEmbeddableProps) { ] ); + const renderCustomToolbar = useMemo( + () => + getRenderCustomToolbarWithElements({ + leftSide: + typeof props.totalHitCount === 'number' ? ( + + ) : undefined, + }), + [props.totalHitCount] + ); + return ( ); diff --git a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts index 2fb99d9ebb43f..03f40c5b6ebcf 100644 --- a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts +++ b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts @@ -145,5 +145,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await addSearchEmbeddableToDashboard(); await testSubjects.missingOrFail('dataGridFullScreenButton'); }); + + it('should show the the grid toolbar', async () => { + await addSearchEmbeddableToDashboard(); + await testSubjects.existOrFail('dscGridToolbar'); + }); }); } diff --git a/test/functional/apps/discover/group2/_data_grid.ts b/test/functional/apps/discover/group2/_data_grid.ts index 58052ce6665ca..d869044613873 100644 --- a/test/functional/apps/discover/group2/_data_grid.ts +++ b/test/functional/apps/discover/group2/_data_grid.ts @@ -46,5 +46,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.unifiedFieldList.clickFieldListItemRemove('agent'); expect(await getTitles()).to.be('@timestamp Document'); }); + + it('should show the the grid toolbar', async () => { + await testSubjects.existOrFail('dscGridToolbar'); + }); }); } diff --git a/test/functional/apps/discover/group2/_data_grid_context.ts b/test/functional/apps/discover/group2/_data_grid_context.ts index ae5030f226b82..e8e218625687b 100644 --- a/test/functional/apps/discover/group2/_data_grid_context.ts +++ b/test/functional/apps/discover/group2/_data_grid_context.ts @@ -97,6 +97,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(disabledFilterCounter).to.be(TEST_FILTER_COLUMN_NAMES.length); }); + it('should show the the grid toolbar', async () => { + await testSubjects.existOrFail('dscGridToolbar'); + }); + it('navigates to context view from embeddable', async () => { await PageObjects.common.navigateToApp('discover'); await PageObjects.discover.saveSearch('my search'); diff --git a/test/functional/apps/discover/group3/_view_mode_toggle.ts b/test/functional/apps/discover/group3/_view_mode_toggle.ts new file mode 100644 index 0000000000000..c47aad66c9a01 --- /dev/null +++ b/test/functional/apps/discover/group3/_view_mode_toggle.ts @@ -0,0 +1,130 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const PageObjects = getPageObjects([ + 'common', + 'discover', + 'timePicker', + 'dashboard', + 'unifiedFieldList', + 'header', + ]); + const esArchiver = getService('esArchiver'); + const retry = getService('retry'); + const testSubjects = getService('testSubjects'); + const queryBar = getService('queryBar'); + const kibanaServer = getService('kibanaServer'); + const security = getService('security'); + const defaultSettings = { + defaultIndex: 'logstash-*', + }; + + describe('discover view mode toggle', function () { + before(async () => { + await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); + }); + + after(async () => { + await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); + await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.savedObjects.cleanStandardList(); + }); + + [true, false].forEach((useLegacyTable) => { + describe(`isLegacy: ${useLegacyTable}`, function () { + before(async function () { + await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); + await kibanaServer.uiSettings.update({ + ...defaultSettings, + 'doc_table:legacy': useLegacyTable, + }); + await PageObjects.common.navigateToApp('discover'); + await PageObjects.discover.waitUntilSearchingHasFinished(); + }); + + after(async () => { + await kibanaServer.uiSettings.replace({}); + }); + + it('should show Documents tab', async () => { + await testSubjects.existOrFail('dscViewModeToggle'); + + if (!useLegacyTable) { + await testSubjects.existOrFail('dscGridToolbar'); + } + + const documentsTab = await testSubjects.find('dscViewModeDocumentButton'); + expect(await documentsTab.getAttribute('aria-selected')).to.be('true'); + }); + + it('should show Document Explorer info callout', async () => { + await testSubjects.existOrFail( + useLegacyTable ? 'dscDocumentExplorerLegacyCallout' : 'dscDocumentExplorerTourCallout' + ); + }); + + it('should show an error callout', async () => { + await queryBar.setQuery('@message::'); // invalid + await queryBar.submitQuery(); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await testSubjects.existOrFail('discoverMainError'); + + await queryBar.clearQuery(); + await queryBar.submitQuery(); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await testSubjects.missingOrFail('discoverMainError'); + }); + + it('should show Field Statistics tab', async () => { + await testSubjects.click('dscViewModeFieldStatsButton'); + + await retry.try(async () => { + const fieldStatsTab = await testSubjects.find('dscViewModeFieldStatsButton'); + expect(await fieldStatsTab.getAttribute('aria-selected')).to.be('true'); + }); + + await testSubjects.existOrFail('dscViewModeToggle'); + }); + + it('should not show view mode toggle for text-based searches', async () => { + await testSubjects.click('dscViewModeDocumentButton'); + + await retry.try(async () => { + const documentsTab = await testSubjects.find('dscViewModeDocumentButton'); + expect(await documentsTab.getAttribute('aria-selected')).to.be('true'); + }); + + await testSubjects.existOrFail('dscViewModeToggle'); + + await PageObjects.discover.selectTextBaseLang(); + + await testSubjects.missingOrFail('dscViewModeToggle'); + + if (!useLegacyTable) { + await testSubjects.existOrFail('dscGridToolbar'); + } + }); + + it('should show text-based columns callout', async () => { + await testSubjects.missingOrFail('dscSelectedColumnsCallout'); + await PageObjects.unifiedFieldList.clickFieldListItemAdd('extension'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await testSubjects.existOrFail('dscSelectedColumnsCallout'); + }); + }); + }); + }); +} diff --git a/test/functional/apps/discover/group3/index.ts b/test/functional/apps/discover/group3/index.ts index 9af02c006b14b..5827c1e7ed805 100644 --- a/test/functional/apps/discover/group3/index.ts +++ b/test/functional/apps/discover/group3/index.ts @@ -24,5 +24,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./_sidebar')); loadTestFile(require.resolve('./_request_counts')); loadTestFile(require.resolve('./_doc_viewer')); + loadTestFile(require.resolve('./_view_mode_toggle')); }); } diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index f12c565dc7293..2437e516212c7 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -2445,7 +2445,6 @@ "unifiedDataTable.tableHeader.timeFieldIconTooltipAriaLabel": "{timeFieldName} – Ce champ représente l'heure à laquelle les événements se sont produits.", "unifiedDataTable.searchGenerationWithDescription": "Tableau généré par la recherche {searchTitle}", "unifiedDataTable.searchGenerationWithDescriptionGrid": "Tableau généré par la recherche {searchTitle} ({searchDescription})", - "unifiedDataTable.selectedDocumentsNumber": "{nr} documents sélectionnés", "unifiedDataTable.clearSelection": "Effacer la sélection", "unifiedDataTable.controlColumnHeader": "Colonne de commande", "unifiedDataTable.copyToClipboardJSON": "Copier les documents dans le presse-papiers (JSON)", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index a08fadcf5bebc..cbbb8cacf8835 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -2460,7 +2460,6 @@ "unifiedDataTable.tableHeader.timeFieldIconTooltipAriaLabel": "{timeFieldName} - このフィールドはイベントの発生時刻を表します。", "unifiedDataTable.searchGenerationWithDescription": "検索{searchTitle}で生成されたテーブル", "unifiedDataTable.searchGenerationWithDescriptionGrid": "検索{searchTitle}で生成されたテーブル（{searchDescription}）", - "unifiedDataTable.selectedDocumentsNumber": "{nr}個のドキュメントが選択されました", "unifiedDataTable.clearSelection": "選択した項目をクリア", "unifiedDataTable.controlColumnHeader": "列の制御", "unifiedDataTable.copyToClipboardJSON": "ドキュメントをクリップボードにコピー（JSON）", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index cdcbd17d43d3f..b47064e16298a 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -2460,7 +2460,6 @@ "unifiedDataTable.tableHeader.timeFieldIconTooltipAriaLabel": "{timeFieldName} - 此字段表示事件发生的时间。", "unifiedDataTable.searchGenerationWithDescription": "搜索 {searchTitle} 生成的表", "unifiedDataTable.searchGenerationWithDescriptionGrid": "搜索 {searchTitle} 生成的表（{searchDescription}）", - "unifiedDataTable.selectedDocumentsNumber": "{nr} 个文档已选择", "unifiedDataTable.clearSelection": "清除所选内容", "unifiedDataTable.controlColumnHeader": "控制列", "unifiedDataTable.copyToClipboardJSON": "将文档复制到剪贴板 (JSON)", From 961b0285f27e6ddab6338df2a19b3197a274fb1a Mon Sep 17 00:00:00 2001 From: Thom Heymann <190132+thomheymann@users.noreply.github.com> Date: Thu, 19 Oct 2023 20:59:49 +0100 Subject: [PATCH 2/8] Report CSP violations using EBT (#159489) Original POC issue: #153584 Part of issue: [#162974](https://github.com/elastic/kibana/issues/162974) ## Summary This PR adds the ability to capture CSP violations using EBT: - Adds `POST /internal/security/analytics/_record_violations` endpoint. - Adds `report-sample` directive to default CSP header. - Adds Hapi `override` option to route config. (Explanation here https://github.com/elastic/kibana/pull/159489/files#diff-6cc0985273a2c3186d12e916aed324c437e996143e47898765ba95d7fbef0375R145-R166) ## Testing 1. Update `kibana.dev.yml` config: ```yml logging.appenders: console: type: console layout: type: json logging.loggers: - name: analytics level: all appenders: [console] server.customResponseHeaders.Reporting-Endpoints: violations-endpoint="https://localhost:5601/xyz/internal/security/analytics/_record_violations" server.customResponseHeaders.Content-Security-Policy-Report-Only: style-src none; report-to violations-endpoint csp.report_to: [violations-endpoint] ``` 2. Run ES with SSL: `yarn es snapshot --license trial --ssl` 4. Run Kibana with SSL: `yarn start --ssl` 5. Quit Chrome if already running 6. Open Chrome using: `/Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --ignore-certificate-errors --ignore-urlfetcher-cert-requests --short-reporting-delay "https://localhost:5601/"` 7. Observe telemetry events being reported in the Kibana process: ```json { "@timestamp": "2023-06-12T15:00:17.794+01:00", "message": "Report event \"security_csp_violation\"", "ebt_event": { "timestamp": "2023-06-12T14:00:17.794Z", "event_type": "security_csp_violation", "context": {}, "properties": { "created": "1686578417796", "url": "https://localhost:5601/dev/login?next=%2Fdev%2F", "user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.0.0 Safari/537.36", "blockedURL": "inline", "columnNumber": 11, "disposition": "report", "documentURL": "https://localhost:5601/dev/login?next=%2Fdev%2F", "effectiveDirective": "style-src-elem", "lineNumber": 353, "originalPolicy": "style-src none; report-to violations-endpoint", "referrer": "", "sample": "", "sourceFile": "https://localhost:5601/dev/9007199254740991/bundles/plugin/security/1.0.0/security.chunk.0.js", "statusCode": 200 } } } ``` --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../src/csp/csp_config.test.ts | 22 +- .../src/csp/csp_directives.test.ts | 8 +- .../src/csp/csp_directives.ts | 6 +- .../src/http_server.ts | 7 +- .../http/core-http-server/src/router/route.ts | 5 + .../integration_tests/http/lifecycle.test.ts | 2 +- .../http_resources_service.test.ts | 4 +- .../analytics/analytics_service.mock.ts | 2 + .../analytics/analytics_service.test.ts | 62 ++++- .../server/analytics/analytics_service.ts | 236 ++++++++++++++++++ .../security/server/routes/analytics/index.ts | 2 + .../analytics/record_violations.test.ts | 133 ++++++++++ .../routes/analytics/record_violations.ts | 198 +++++++++++++++ x-pack/plugins/security/tsconfig.json | 1 + .../platform_security/response_headers.ts | 2 +- 15 files changed, 661 insertions(+), 29 deletions(-) create mode 100644 x-pack/plugins/security/server/routes/analytics/record_violations.test.ts create mode 100644 x-pack/plugins/security/server/routes/analytics/record_violations.ts diff --git a/packages/core/http/core-http-server-internal/src/csp/csp_config.test.ts b/packages/core/http/core-http-server-internal/src/csp/csp_config.test.ts index da3a21e5816af..ef4358ef42f15 100644 --- a/packages/core/http/core-http-server-internal/src/csp/csp_config.test.ts +++ b/packages/core/http/core-http-server-internal/src/csp/csp_config.test.ts @@ -34,7 +34,7 @@ describe('CspConfig', () => { CspConfig { "disableEmbedding": false, "disableUnsafeEval": true, - "header": "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", + "header": "script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'", "strict": true, "warnLegacyBrowsers": true, } @@ -65,7 +65,7 @@ describe('CspConfig', () => { worker_src: ['foo', 'bar'], }); expect(config.header).toEqual( - `script-src 'self'; worker-src blob: 'self' foo bar; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob: foo bar; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); @@ -76,7 +76,7 @@ describe('CspConfig', () => { }); expect(config.header).toEqual( - `script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self' foo bar` + `script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline' foo bar` ); }); @@ -87,7 +87,7 @@ describe('CspConfig', () => { }); expect(config.header).toEqual( - `script-src 'self' foo bar; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self' foo bar; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); @@ -99,7 +99,7 @@ describe('CspConfig', () => { style_src: ['style', 'dolly'], }); expect(config.header).toEqual( - `script-src 'self' script foo; worker-src blob: 'self' worker bar; style-src 'unsafe-inline' 'self' style dolly` + `script-src 'report-sample' 'self' script foo; worker-src 'report-sample' 'self' blob: worker bar; style-src 'report-sample' 'self' 'unsafe-inline' style dolly` ); }); @@ -111,7 +111,7 @@ describe('CspConfig', () => { style_src: ['style'], }); expect(config.header).toEqual( - `script-src 'self' script; worker-src blob: 'self' worker; style-src 'unsafe-inline' 'self' style` + `script-src 'report-sample' 'self' script; worker-src 'report-sample' 'self' blob: worker; style-src 'report-sample' 'self' 'unsafe-inline' style` ); }); @@ -124,7 +124,7 @@ describe('CspConfig', () => { }); expect(config.header).toEqual( - `script-src 'self' foo bar; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self' foo bar; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); @@ -136,7 +136,7 @@ describe('CspConfig', () => { }); expect(config.header).toEqual( - `script-src 'self' 'unsafe-eval' foo bar; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self' 'unsafe-eval' foo bar; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); @@ -147,7 +147,7 @@ describe('CspConfig', () => { }); expect(config.header).toEqual( - `script-src 'self' foo bar; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self' foo bar; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); }); @@ -160,7 +160,7 @@ describe('CspConfig', () => { expect(config.disableEmbedding).toEqual(disableEmbedding); expect(config.disableEmbedding).not.toEqual(CspConfig.DEFAULT.disableEmbedding); expect(config.header).toMatchInlineSnapshot( - `"script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self'"` + `"script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'; frame-ancestors 'self'"` ); }); @@ -173,7 +173,7 @@ describe('CspConfig', () => { expect(config.disableEmbedding).toEqual(disableEmbedding); expect(config.disableEmbedding).not.toEqual(CspConfig.DEFAULT.disableEmbedding); expect(config.header).toMatchInlineSnapshot( - `"script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self'"` + `"script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'; frame-ancestors 'self'"` ); }); }); diff --git a/packages/core/http/core-http-server-internal/src/csp/csp_directives.test.ts b/packages/core/http/core-http-server-internal/src/csp/csp_directives.test.ts index dccc4d43516fe..e250a6b1fea44 100644 --- a/packages/core/http/core-http-server-internal/src/csp/csp_directives.test.ts +++ b/packages/core/http/core-http-server-internal/src/csp/csp_directives.test.ts @@ -78,7 +78,7 @@ describe('CspDirectives', () => { const config = cspConfig.schema.validate({}); const directives = CspDirectives.fromConfig(config); expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'"` + `"script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'"` ); }); @@ -91,7 +91,7 @@ describe('CspDirectives', () => { const directives = CspDirectives.fromConfig(config); expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"script-src 'self' baz; worker-src blob: 'self' foo; style-src 'unsafe-inline' 'self' bar dolly"` + `"script-src 'report-sample' 'self' baz; worker-src 'report-sample' 'self' blob: foo; style-src 'report-sample' 'self' 'unsafe-inline' bar dolly"` ); }); @@ -108,7 +108,7 @@ describe('CspDirectives', () => { }); const directives = CspDirectives.fromConfig(config); expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; connect-src 'self' connect-src; default-src 'self' default-src; font-src 'self' font-src; frame-src 'self' frame-src; img-src 'self' img-src; frame-ancestors 'self' frame-ancestors; report-uri report-uri; report-to report-to"` + `"script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'; connect-src 'self' connect-src; default-src 'self' default-src; font-src 'self' font-src; frame-src 'self' frame-src; img-src 'self' img-src; frame-ancestors 'self' frame-ancestors; report-uri report-uri; report-to report-to"` ); }); @@ -118,7 +118,7 @@ describe('CspDirectives', () => { }); const directives = CspDirectives.fromConfig(config); expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"script-src 'self' 'unsafe-hashes'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'"` + `"script-src 'report-sample' 'self' 'unsafe-hashes'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'"` ); }); }); diff --git a/packages/core/http/core-http-server-internal/src/csp/csp_directives.ts b/packages/core/http/core-http-server-internal/src/csp/csp_directives.ts index e4eefefef1ef8..28ec58f8db900 100644 --- a/packages/core/http/core-http-server-internal/src/csp/csp_directives.ts +++ b/packages/core/http/core-http-server-internal/src/csp/csp_directives.ts @@ -25,9 +25,9 @@ export type CspDirectiveName = * The default directives rules that are always applied */ export const defaultRules: Partial> = { - 'script-src': [`'self'`], - 'worker-src': [`blob:`, `'self'`], - 'style-src': [`'unsafe-inline'`, `'self'`], + 'script-src': [`'report-sample'`, `'self'`], + 'worker-src': [`'report-sample'`, `'self'`, `blob:`], + 'style-src': [`'report-sample'`, `'self'`, `'unsafe-inline'`], }; /** diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts index f12367419341f..dbd0279cae9ec 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.ts @@ -605,7 +605,7 @@ export class HttpServer { // Hapi does not allow payload validation to be specified for 'head' or 'get' requests const validate = isSafeMethod(route.method) ? undefined : { payload: true }; const { authRequired, tags, body = {}, timeout } = route.options; - const { accepts: allow, maxBytes, output, parse } = body; + const { accepts: allow, override, maxBytes, output, parse } = body; const kibanaRouteOptions: KibanaRouteOptions = { xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method), @@ -632,9 +632,12 @@ export class HttpServer { // (All NP routes are already required to specify their own validation in order to access the payload) validate, // @ts-expect-error Types are outdated and doesn't allow `payload.multipart` to be `true` - payload: [allow, maxBytes, output, parse, timeout?.payload].some((x) => x !== undefined) + payload: [allow, override, maxBytes, output, parse, timeout?.payload].some( + (x) => x !== undefined + ) ? { allow, + override, maxBytes, output, parse, diff --git a/packages/core/http/core-http-server/src/router/route.ts b/packages/core/http/core-http-server/src/router/route.ts index 349ad2e392453..62ec27dbc12b0 100644 --- a/packages/core/http/core-http-server/src/router/route.ts +++ b/packages/core/http/core-http-server/src/router/route.ts @@ -63,6 +63,11 @@ export interface RouteConfigOptionsBody { */ accepts?: RouteContentType | RouteContentType[] | string | string[]; + /** + * A mime type string overriding the 'Content-Type' header value received. + */ + override?: string; + /** * Limits the size of incoming payloads to the specified byte count. Allowing very large payloads may cause the server to run out of memory. * diff --git a/src/core/server/integration_tests/http/lifecycle.test.ts b/src/core/server/integration_tests/http/lifecycle.test.ts index 239350747c7fd..7e056fcd52613 100644 --- a/src/core/server/integration_tests/http/lifecycle.test.ts +++ b/src/core/server/integration_tests/http/lifecycle.test.ts @@ -1494,7 +1494,7 @@ describe('runs with default preResponse handlers', () => { expect(response.header.foo).toBe('bar'); expect(response.header['kbn-name']).toBe('kibana'); expect(response.header['content-security-policy']).toBe( - `script-src 'self' 'unsafe-eval'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + `script-src 'report-sample' 'self' 'unsafe-eval'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` ); }); }); diff --git a/src/core/server/integration_tests/http_resources/http_resources_service.test.ts b/src/core/server/integration_tests/http_resources/http_resources_service.test.ts index 0c967c69f3e8f..c158c4739bc82 100644 --- a/src/core/server/integration_tests/http_resources/http_resources_service.test.ts +++ b/src/core/server/integration_tests/http_resources/http_resources_service.test.ts @@ -20,8 +20,8 @@ function applyTestsWithDisableUnsafeEvalSetTo(disableUnsafeEval: boolean) { describe(`with disableUnsafeEval=${disableUnsafeEval}`, () => { let root: ReturnType; const defaultCspRules = disableUnsafeEval - ? `script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` - : `script-src 'self' 'unsafe-eval'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`; + ? `script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'` + : `script-src 'report-sample' 'self' 'unsafe-eval'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'`; beforeEach(async () => { root = createRoot({ csp: { disableUnsafeEval }, diff --git a/x-pack/plugins/security/server/analytics/analytics_service.mock.ts b/x-pack/plugins/security/server/analytics/analytics_service.mock.ts index 9b8b3cbee4a60..93a0d62e9dee7 100644 --- a/x-pack/plugins/security/server/analytics/analytics_service.mock.ts +++ b/x-pack/plugins/security/server/analytics/analytics_service.mock.ts @@ -10,5 +10,7 @@ import type { AnalyticsServiceSetup } from './analytics_service'; export const analyticsServiceMock = { createSetup: (): jest.Mocked => ({ reportAuthenticationTypeEvent: jest.fn(), + reportCSPViolation: jest.fn(), + reportPermissionsPolicyViolation: jest.fn(), }), }; diff --git a/x-pack/plugins/security/server/analytics/analytics_service.test.ts b/x-pack/plugins/security/server/analytics/analytics_service.test.ts index 75a662e03f557..2d42784707f9b 100644 --- a/x-pack/plugins/security/server/analytics/analytics_service.test.ts +++ b/x-pack/plugins/security/server/analytics/analytics_service.test.ts @@ -8,7 +8,13 @@ import type { Logger } from '@kbn/core/server'; import { coreMock, loggingSystemMock } from '@kbn/core/server/mocks'; -import { AnalyticsService, AUTHENTICATION_TYPE_EVENT_TYPE } from './analytics_service'; +import type { CSPViolationEvent, PermissionsPolicyViolationEvent } from './analytics_service'; +import { + AnalyticsService, + AUTHENTICATION_TYPE_EVENT_TYPE, + CSP_VIOLATION_EVENT_TYPE, + PERMISSIONS_POLICY_VIOLATION_EVENT_TYPE, +} from './analytics_service'; describe('AnalyticsService', () => { let service: AnalyticsService; @@ -28,10 +34,14 @@ describe('AnalyticsService', () => { it('returns proper contract', () => { const setupParams = getSetupParams(); - expect(service.setup(setupParams)).toEqual({ - reportAuthenticationTypeEvent: expect.any(Function), - }); - expect(setupParams.analytics.registerEventType).toHaveBeenCalledTimes(1); + expect(service.setup(setupParams)).toMatchInlineSnapshot(` + Object { + "reportAuthenticationTypeEvent": [Function], + "reportCSPViolation": [Function], + "reportPermissionsPolicyViolation": [Function], + } + `); + expect(setupParams.analytics.registerEventType).toHaveBeenCalledTimes(3); expect(setupParams.analytics.registerEventType).toHaveBeenCalledWith({ eventType: AUTHENTICATION_TYPE_EVENT_TYPE, schema: expect.objectContaining({ @@ -40,6 +50,16 @@ describe('AnalyticsService', () => { http_authentication_scheme: expect.anything(), }), }); + expect(setupParams.analytics.registerEventType).toHaveBeenCalledWith( + expect.objectContaining({ + eventType: CSP_VIOLATION_EVENT_TYPE, + }) + ); + expect(setupParams.analytics.registerEventType).toHaveBeenCalledWith( + expect.objectContaining({ + eventType: PERMISSIONS_POLICY_VIOLATION_EVENT_TYPE, + }) + ); }); describe('#reportAuthenticationTypeEvent', () => { @@ -64,5 +84,37 @@ describe('AnalyticsService', () => { ); }); }); + + describe('#reportCSPViolation', () => { + it('properly reports CSP violation event', async () => { + const setupParams = getSetupParams(); + const analyticsSetup = service.setup(setupParams); + + const event = {} as CSPViolationEvent; + analyticsSetup.reportCSPViolation(event); + + expect(setupParams.analytics.reportEvent).toHaveBeenCalledTimes(1); + expect(setupParams.analytics.reportEvent).toHaveBeenCalledWith( + 'security_csp_violation', + event + ); + }); + }); + + describe('#reportPermissionsPolicyViolation', () => { + it('properly reports Permissions policy violation event', async () => { + const setupParams = getSetupParams(); + const analyticsSetup = service.setup(setupParams); + + const event = {} as PermissionsPolicyViolationEvent; + analyticsSetup.reportPermissionsPolicyViolation(event); + + expect(setupParams.analytics.reportEvent).toHaveBeenCalledTimes(1); + expect(setupParams.analytics.reportEvent).toHaveBeenCalledWith( + 'security_permissions_policy_violation', + event + ); + }); + }); }); }); diff --git a/x-pack/plugins/security/server/analytics/analytics_service.ts b/x-pack/plugins/security/server/analytics/analytics_service.ts index 54e69ba7461ce..98d316d8a8d5f 100644 --- a/x-pack/plugins/security/server/analytics/analytics_service.ts +++ b/x-pack/plugins/security/server/analytics/analytics_service.ts @@ -5,9 +5,17 @@ * 2.0. */ +import type { EventTypeOpts } from '@kbn/analytics-client'; import type { AnalyticsServiceSetup as CoreAnalyticsServiceSetup, Logger } from '@kbn/core/server'; +import type { + CSPViolationReport, + PermissionsPolicyViolationReport, +} from '../routes/analytics/record_violations'; + export const AUTHENTICATION_TYPE_EVENT_TYPE = 'security_authentication_type'; +export const CSP_VIOLATION_EVENT_TYPE = 'security_csp_violation'; +export const PERMISSIONS_POLICY_VIOLATION_EVENT_TYPE = 'security_permissions_policy_violation'; export interface AnalyticsServiceSetupParams { analytics: CoreAnalyticsServiceSetup; @@ -19,8 +27,30 @@ export interface AnalyticsServiceSetup { * @param event Instance of the AuthenticationTypeEvent. */ reportAuthenticationTypeEvent(event: AuthenticationTypeAnalyticsEvent): void; + + /** + * Registers CSP violation sent by the user's browser using Reporting API. + * @param event Instance of the AuthenticationTypeEvent. + */ + reportCSPViolation(event: CSPViolationEvent): void; + + /** + * Registers CSP violation sent by the user's browser using Reporting API. + * @param event Instance of the AuthenticationTypeEvent. + */ + reportPermissionsPolicyViolation(event: PermissionsPolicyViolationEvent): void; } +/** + * Interface that represents how CSP violations are stored as EBT events. + */ +export type CSPViolationEvent = FlattenReport; + +/** + * Interface that represents how permissions policy violations are stored as EBT events. + */ +export type PermissionsPolicyViolationEvent = FlattenReport; + /** * Describes the shape of the authentication type event. */ @@ -42,6 +72,200 @@ export interface AuthenticationTypeAnalyticsEvent { httpAuthenticationScheme?: string; } +/** + * Properties that all Reporting API schemas share. + */ +interface CommonReportFields { + type: string; + age?: number; + body: {}; +} + +/** + * Helper type that transforms any Reporting API schema into its corresponding EBT schema: + * + * - Removes `type` property since events are identified by their `eventType` in EBT. + * - Replaces `age` property with `created` timestamp so that we capture a fully qualified date. + * - Spreads `body` property to keep the resulting EBT schema flat. + */ +type FlattenReport = { created: string } & Omit< + T, + keyof CommonReportFields +> & + T['body']; + +/** + * Describes the shape of the CSP violation event type. + */ +const cspViolation: EventTypeOpts = { + eventType: CSP_VIOLATION_EVENT_TYPE, + schema: { + created: { + type: 'keyword', + _meta: { + description: 'Timestamp when the violation was captured by the user agent.', + optional: false, + }, + }, + url: { + type: 'keyword', + _meta: { + description: '"url" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + user_agent: { + type: 'text', + _meta: { + description: '"user_agent" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + documentURL: { + type: 'text', + _meta: { + description: '"documentURL" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + referrer: { + type: 'text', + _meta: { + description: '"referrer" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + blockedURL: { + type: 'text', + _meta: { + description: '"blockedURL" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + effectiveDirective: { + type: 'text', + _meta: { + description: '"effectiveDirective" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + originalPolicy: { + type: 'text', + _meta: { + description: '"originalPolicy" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + sourceFile: { + type: 'text', + _meta: { + description: '"sourceFile" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + sample: { + type: 'text', + _meta: { + description: '"sample" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + disposition: { + type: 'text', + _meta: { + description: '"disposition" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + statusCode: { + type: 'integer', + _meta: { + description: '"statusCode" field of W3 Reporting API CSP violation report.', + optional: false, + }, + }, + lineNumber: { + type: 'long', + _meta: { + description: '"lineNumber" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + columnNumber: { + type: 'long', + _meta: { + description: '"columnNumber" field of W3 Reporting API CSP violation report.', + optional: true, + }, + }, + }, +}; + +/** + * Describes the shape of the CSP violation event type. + */ +const permissionsPolicyViolation: EventTypeOpts = { + eventType: PERMISSIONS_POLICY_VIOLATION_EVENT_TYPE, + schema: { + created: { + type: 'keyword', + _meta: { + description: 'Timestamp when the violation was captured by the user agent.', + optional: false, + }, + }, + url: { + type: 'keyword', + _meta: { + description: '"url" field of Reporting API permissions policy violation report.', + optional: false, + }, + }, + user_agent: { + type: 'text', + _meta: { + description: '"user_agent" field of Reporting API permissions policy violation report.', + optional: true, + }, + }, + featureId: { + type: 'text', + _meta: { + description: '"featureId" field of Reporting API permissions policy violation report.', + optional: false, + }, + }, + sourceFile: { + type: 'text', + _meta: { + description: '"sourceFile" field of Reporting API permissions policy violation report.', + optional: true, + }, + }, + lineNumber: { + type: 'long', + _meta: { + description: '"lineNumber" field of Reporting API permissions policy violation report.', + optional: true, + }, + }, + columnNumber: { + type: 'long', + _meta: { + description: '"columnNumber" field of Reporting API permissions policy violation report.', + optional: true, + }, + }, + disposition: { + type: 'text', + _meta: { + description: '"disposition" field of Reporting API permissions policy violation report.', + optional: false, + }, + }, + }, +}; + /** * Service that interacts with the Core's analytics module to collect usage of * the various Security plugin features (e.g. type of the authentication used). @@ -81,6 +305,12 @@ export class AnalyticsService { }, }); + this.logger.debug(`Registering ${cspViolation.eventType} event type.`); + analytics.registerEventType(cspViolation); + + this.logger.debug(`Registering ${permissionsPolicyViolation.eventType} event type.`); + analytics.registerEventType(permissionsPolicyViolation); + return { reportAuthenticationTypeEvent(event: AuthenticationTypeAnalyticsEvent) { analytics.reportEvent(AUTHENTICATION_TYPE_EVENT_TYPE, { @@ -89,6 +319,12 @@ export class AnalyticsService { http_authentication_scheme: event.httpAuthenticationScheme?.toLowerCase(), }); }, + reportCSPViolation(event: CSPViolationEvent) { + analytics.reportEvent(CSP_VIOLATION_EVENT_TYPE, event); + }, + reportPermissionsPolicyViolation(event: PermissionsPolicyViolationEvent) { + analytics.reportEvent(PERMISSIONS_POLICY_VIOLATION_EVENT_TYPE, event); + }, }; } } diff --git a/x-pack/plugins/security/server/routes/analytics/index.ts b/x-pack/plugins/security/server/routes/analytics/index.ts index a8152117a2873..9b401ffd2ee28 100644 --- a/x-pack/plugins/security/server/routes/analytics/index.ts +++ b/x-pack/plugins/security/server/routes/analytics/index.ts @@ -6,8 +6,10 @@ */ import { defineRecordAnalyticsOnAuthTypeRoutes } from './authentication_type'; +import { defineRecordViolations } from './record_violations'; import type { RouteDefinitionParams } from '..'; export function defineAnalyticsRoutes(params: RouteDefinitionParams) { defineRecordAnalyticsOnAuthTypeRoutes(params); + defineRecordViolations(params); } diff --git a/x-pack/plugins/security/server/routes/analytics/record_violations.test.ts b/x-pack/plugins/security/server/routes/analytics/record_violations.test.ts new file mode 100644 index 0000000000000..97fe293205187 --- /dev/null +++ b/x-pack/plugins/security/server/routes/analytics/record_violations.test.ts @@ -0,0 +1,133 @@ +/* + * 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 type { RequestHandler } from '@kbn/core/server'; +import { kibanaResponseFactory } from '@kbn/core/server'; +import { httpServerMock } from '@kbn/core/server/mocks'; +import type { DeeplyMockedKeys } from '@kbn/utility-types-jest'; + +import { + type CSPViolationReport, + defineRecordViolations, + type PermissionsPolicyViolationReport, +} from './record_violations'; +import type { RouteDefinitionParams } from '..'; +import type { SecurityRequestHandlerContext } from '../../types'; +import { routeDefinitionParamsMock } from '../index.mock'; + +jest.useFakeTimers().setSystemTime(new Date('2023-10-23')); + +function getMockContext( + licenseCheckResult: { state: string; message?: string } = { state: 'valid' } +) { + return { + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, + } as unknown as SecurityRequestHandlerContext; +} + +describe('POST /internal/security/analytics/_record_violations', () => { + let routeHandler: RequestHandler; + let routeParamsMock: DeeplyMockedKeys; + + beforeEach(() => { + routeParamsMock = routeDefinitionParamsMock.create(); + defineRecordViolations(routeParamsMock); + + const [, recordViolationsRouteHandler] = routeParamsMock.router.post.mock.calls.find( + ([{ path }]) => path === '/internal/security/analytics/_record_violations' + )!; + routeHandler = recordViolationsRouteHandler; + }); + + describe('CSP violations', () => { + const cspViolation: CSPViolationReport = { + type: 'csp-violation', + url: 'http://localhost:5601/app/home', + age: 99, + user_agent: 'jest', + body: { + blockedURL: 'inline', + disposition: 'report', + documentURL: 'http://localhost:5601/app/home', + effectiveDirective: 'style-src-elem', + originalPolicy: 'style-src none; report-to violations-endpoint', + statusCode: 200, + }, + }; + + it('reports CSP violation if user is authenticated', async () => { + const request = httpServerMock.createKibanaRequest({ + body: [cspViolation], + auth: { isAuthenticated: true }, + }); + const response = await routeHandler(getMockContext(), request, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(routeParamsMock.analyticsService.reportCSPViolation).toHaveBeenCalledWith({ + url: cspViolation.url, + user_agent: cspViolation.user_agent, + created: '1698019200099', + ...cspViolation.body, + }); + }); + + it('does not report CSP violation if user is not authenticated', async () => { + const request = httpServerMock.createKibanaRequest({ + body: [cspViolation], + auth: { isAuthenticated: false }, + }); + const response = await routeHandler(getMockContext(), request, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(routeParamsMock.analyticsService.reportCSPViolation).not.toHaveBeenCalled(); + }); + }); + + describe('Permissions Policy violations', () => { + const permissionsPolicyViolation: PermissionsPolicyViolationReport = { + type: 'permissions-policy-violation', + url: 'http://localhost:5601/app/home', + age: 99, + user_agent: 'jest', + body: { + disposition: 'report', + featureId: 'camera', + }, + }; + + it('reports permissions policy violation if user is authenticated', async () => { + const request = httpServerMock.createKibanaRequest({ + body: [permissionsPolicyViolation], + auth: { isAuthenticated: true }, + }); + const response = await routeHandler(getMockContext(), request, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect( + routeParamsMock.analyticsService.reportPermissionsPolicyViolation + ).toHaveBeenCalledWith({ + url: permissionsPolicyViolation.url, + user_agent: permissionsPolicyViolation.user_agent, + created: '1698019200099', + ...permissionsPolicyViolation.body, + }); + }); + + it('does not report permissions policy violation if user is not authenticated', async () => { + const request = httpServerMock.createKibanaRequest({ + body: [permissionsPolicyViolation], + auth: { isAuthenticated: false }, + }); + const response = await routeHandler(getMockContext(), request, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect( + routeParamsMock.analyticsService.reportPermissionsPolicyViolation + ).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security/server/routes/analytics/record_violations.ts b/x-pack/plugins/security/server/routes/analytics/record_violations.ts new file mode 100644 index 0000000000000..8e59fc52831d2 --- /dev/null +++ b/x-pack/plugins/security/server/routes/analytics/record_violations.ts @@ -0,0 +1,198 @@ +/* + * 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 type { TypeOf } from '@kbn/config-schema'; +import { schema } from '@kbn/config-schema'; + +import type { RouteDefinitionParams } from '..'; + +/** + * # Tracking CSP violations + * + * Add the following settings to your `kibana.dev.yml`: + * + * ```yml + * server.customResponseHeaders.Reporting-Endpoints: violations-endpoint="https://localhost:5601/xyz/internal/security/analytics/_record_violations" + * csp.report_to: [violations-endpoint] + * ``` + * + * Note: The endpoint has to be on HTTPS and has to be a fully qualified URL including protocol and + * hostname, otherwise browsers might not send any reports. When using `server.publicBaseUrl` setting + * you should use the same origin for the reporting endpoint since Kibana does not support + * cross-origin content negotiation so browsers would not be able to send any report. + * + * # Debugging CSP violations + * + * CSP violations are tracked (batched) using event based telemetry. + * + * To print telemetry events to the terminal add the following config to your `kibana.dev.yml`: + * + * ```yml + * logging.loggers: + * - name: analytics + * level: all + * appenders: [console] + * ``` + */ + +/** + * Schema that validates CSP violation reports according to W3C spec. + * + * https://www.w3.org/TR/CSP3/#reporting + */ +const cspViolationReportSchema = schema.object( + { + type: schema.literal('csp-violation'), + age: schema.maybe(schema.number()), + url: schema.string(), + user_agent: schema.maybe(schema.string()), + body: schema.object( + { + documentURL: schema.string(), + referrer: schema.maybe(schema.string()), + blockedURL: schema.maybe(schema.string()), + effectiveDirective: schema.string(), + originalPolicy: schema.string(), + sourceFile: schema.maybe(schema.string()), + sample: schema.maybe(schema.string()), + disposition: schema.oneOf([schema.literal('enforce'), schema.literal('report')]), + statusCode: schema.number(), + lineNumber: schema.maybe(schema.number()), + columnNumber: schema.maybe(schema.number()), + }, + { unknowns: 'ignore' } + ), + }, + { unknowns: 'ignore' } +); + +/** + * Interface that represents a CSP violation report according to W3C spec. + */ +export type CSPViolationReport = TypeOf; + +/** + * Schema that validates permissions policy violation reports according to W3C spec. + * + * https://w3c.github.io/webappsec-permissions-policy/#reporting + */ +export const permissionsPolicyViolationReportSchema = schema.object( + { + type: schema.literal('permissions-policy-violation'), + age: schema.maybe(schema.number()), + url: schema.string(), + user_agent: schema.maybe(schema.string()), + body: schema.object( + { + /** + * The string identifying the policy-controlled feature whose policy has been violated. This string can be used for grouping and counting related reports. + */ + featureId: schema.string(), + /** + * If known, the file where the violation occured, or null otherwise. + */ + sourceFile: schema.maybe(schema.string()), + /** + * If known, the line number in sourceFile where the violation occured, or null otherwise. + */ + lineNumber: schema.maybe(schema.number()), + /** + * If known, the column number in sourceFile where the violation occured, or null otherwise. + */ + columnNumber: schema.maybe(schema.number()), + /** + * A string indicating whether the violated permissions policy was enforced in this case. disposition will be set to "enforce" if the policy was enforced, or "report" if the violation resulted only in this report being generated (with no further action taken by the user agent in response to the violation). + */ + disposition: schema.oneOf([schema.literal('enforce'), schema.literal('report')]), + }, + { unknowns: 'ignore' } + ), + }, + { unknowns: 'ignore' } +); + +/** + * Interface that represents a permissions policy violation report according to W3C spec. + */ +export type PermissionsPolicyViolationReport = TypeOf< + typeof permissionsPolicyViolationReportSchema +>; + +/** + * This endpoint receives reports from the user's browser via the Reporting API when one of our + * `Content-Security-Policy` or `Permissions-Policy` directives have been violated. + */ +export function defineRecordViolations({ router, analyticsService }: RouteDefinitionParams) { + router.post( + { + path: '/internal/security/analytics/_record_violations', + validate: { + /** + * Chrome supports CSP3 spec and sends an array of reports. Safari only sends a single + * report so catering for both here. + */ + body: schema.oneOf([ + schema.arrayOf( + schema.oneOf([cspViolationReportSchema, permissionsPolicyViolationReportSchema]) + ), + cspViolationReportSchema, + ]), + }, + options: { + /** + * Browsers will stop sending reports for the duration of the browser session and without + * further retries once this endpoint has returned a single 403. This would effectively + * prevent us from capture any reports. To work around this behaviour we optionally + * authenticate users but silently ignore any reports that have been received from + * unauthenticated users. + */ + authRequired: 'optional', + /** + * This endpoint is called by the browser in the background so `kbn-xsrf` header is not sent. + */ + xsrfRequired: false, + access: 'internal', + body: { + /** + * Both `application/reports+json` (CSP3 spec) and `application/csp-report` (Safari) are + * valid values but Hapi does not parse the request body when `application/csp-report` is + * specified so enforcing JSON mime-type for this endpoint. + */ + override: 'application/json', + }, + }, + }, + async (context, request, response) => { + if (request.auth.isAuthenticated) { + const reports = Array.isArray(request.body) ? request.body : [request.body]; + const now = Date.now(); + reports.forEach((report) => { + if (report.type === 'csp-violation') { + analyticsService.reportCSPViolation({ + created: `${now + (report.age ?? 0)}`, + url: report.url, + user_agent: report.user_agent ?? getLastHeader(request.headers['user-agent']), + ...report.body, + }); + } else if (report.type === 'permissions-policy-violation') { + analyticsService.reportPermissionsPolicyViolation({ + created: `${now + (report.age ?? 0)}`, + url: report.url, + user_agent: report.user_agent ?? getLastHeader(request.headers['user-agent']), + ...report.body, + }); + } + }); + } + return response.ok(); + } + ); +} + +function getLastHeader(header: string | string[] | undefined) { + return Array.isArray(header) ? header[header.length - 1] : header; +} diff --git a/x-pack/plugins/security/tsconfig.json b/x-pack/plugins/security/tsconfig.json index f8cc40c2dff42..2a055a61bbad5 100644 --- a/x-pack/plugins/security/tsconfig.json +++ b/x-pack/plugins/security/tsconfig.json @@ -62,6 +62,7 @@ "@kbn/core-http-server-mocks", "@kbn/core-user-settings-server", "@kbn/remote-clusters-plugin", + "@kbn/analytics-client", ], "exclude": [ "target/**/*", diff --git a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/response_headers.ts b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/response_headers.ts index 56a52914cf9a0..2a4d7296b8183 100644 --- a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/response_headers.ts +++ b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/response_headers.ts @@ -13,7 +13,7 @@ export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); describe('security/response_headers', function () { - const defaultCSP = `script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self'`; + const defaultCSP = `script-src 'report-sample' 'self'; worker-src 'report-sample' 'self' blob:; style-src 'report-sample' 'self' 'unsafe-inline'; frame-ancestors 'self'`; const defaultCOOP = 'same-origin'; const defaultPermissionsPolicy = 'camera=(), display-capture=(), fullscreen=(self), geolocation=(), microphone=(), web-share=()'; From 36776af50f884d00168605bfae51877d7430b21e Mon Sep 17 00:00:00 2001 From: Nicolas Chaulet Date: Thu, 19 Oct 2023 16:06:17 -0400 Subject: [PATCH 3/8] [Fleet] Fix fetching package detail info after package policy creation (#169275) --- .../epm/screens/detail/index.test.tsx | 22 ++++++++++++++- .../sections/epm/screens/detail/index.tsx | 28 +++++++++++++------ .../fleet/public/hooks/use_request/epm.ts | 3 +- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.test.tsx b/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.test.tsx index f6c850b759d56..2191ed37e959e 100644 --- a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.test.tsx +++ b/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.test.tsx @@ -102,6 +102,9 @@ describe('when on integration detail', () => { describe('and the package is not installed and prerelease enabled', () => { beforeEach(async () => { + mockedApi.responseProvider.getSettings.mockReturnValue({ + item: { prerelease_integrations_enabled: true, id: '', fleet_server_hosts: [] }, + }); mockGAAndPrereleaseVersions('1.0.0-beta'); await render(); await act(() => mockedApi.waitForApi()); @@ -165,6 +168,9 @@ describe('when on integration detail', () => { describe('and a custom UI extension is NOT registered', () => { beforeEach(async () => { + mockedApi.responseProvider.getSettings.mockReturnValue({ + item: { prerelease_integrations_enabled: false, id: '', fleet_server_hosts: [] }, + }); await render(); await act(() => mockedApi.waitForApi()); // All those waitForApi call are needed to avoid flakyness because details conditionnaly refetch multiple time @@ -201,6 +207,9 @@ describe('when on integration detail', () => { beforeEach(async () => { let setWasRendered: () => void; + mockedApi.responseProvider.getSettings.mockReturnValue({ + item: { prerelease_integrations_enabled: false, id: '', fleet_server_hosts: [] }, + }); lazyComponentWasRendered = new Promise((resolve) => { setWasRendered = resolve; }); @@ -268,6 +277,12 @@ describe('when on integration detail', () => { }); await render(); + + await act(() => mockedApi.waitForApi()); + // All those waitForApi call are needed to avoid flakyness because details conditionnaly refetch multiple time + await act(() => mockedApi.waitForApi()); + await act(() => mockedApi.waitForApi()); + await act(() => mockedApi.waitForApi()); }); afterEach(() => { @@ -275,7 +290,7 @@ describe('when on integration detail', () => { lazyComponentWasRendered = undefined; }); - it('should display "assets" tab in navigation', () => { + it('should display "assets" tab in navigation', async () => { expect(renderResult.getByTestId('tab-assets')); }); @@ -293,6 +308,11 @@ describe('when on integration detail', () => { describe('and the Add integration button is clicked', () => { beforeEach(async () => { await render(); + await act(() => mockedApi.waitForApi()); + // All those waitForApi call are needed to avoid flakyness because details conditionnaly refetch multiple time + await act(() => mockedApi.waitForApi()); + await act(() => mockedApi.waitForApi()); + await act(() => mockedApi.waitForApi()); }); it('should link to the create page', () => { diff --git a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.tsx b/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.tsx index 463dd453f8f86..b2a2ad23ba514 100644 --- a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.tsx +++ b/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.tsx @@ -168,7 +168,6 @@ export function Detail() { } return getPackageInstallStatus(packageInfo?.name)?.status; }, [packageInfo, getPackageInstallStatus]); - const isInstalled = useMemo( () => packageInstallStatus === InstallStatus.installed || @@ -186,7 +185,7 @@ export function Detail() { boolean | undefined >(); - const { data: settings } = useGetSettingsQuery(); + const { data: settings, isInitialLoading: isSettingsInitialLoading } = useGetSettingsQuery(); useEffect(() => { const isEnabled = Boolean(settings?.item.prerelease_integrations_enabled) || prerelease; @@ -199,10 +198,19 @@ export function Detail() { data: packageInfoData, error: packageInfoError, isLoading: packageInfoLoading, + isFetchedAfterMount: packageInfoIsFetchedAfterMount, refetch: refetchPackageInfo, - } = useGetPackageInfoByKeyQuery(pkgName, pkgVersion, { - prerelease: prereleaseIntegrationsEnabled, - }); + } = useGetPackageInfoByKeyQuery( + pkgName, + pkgVersion, + { + prerelease: prereleaseIntegrationsEnabled, + }, + { + enabled: !isSettingsInitialLoading, // Load only after settings are loaded + refetchOnMount: 'always', + } + ); const [latestGAVersion, setLatestGAVersion] = useState(); const [latestPrereleaseVersion, setLatestPrereleaseVersion] = useState(); @@ -246,14 +254,18 @@ export function Detail() { } }, [packageInstallStatus, oldPackageInstallStatus, refetchPackageInfo]); - const isLoading = packageInfoLoading || isPermissionCheckLoading || firstTimeUserLoading; + const isLoading = + packageInfoLoading || + isPermissionCheckLoading || + firstTimeUserLoading || + !packageInfoIsFetchedAfterMount; const showCustomTab = useUIExtension(packageInfoData?.item?.name ?? '', 'package-detail-custom') !== undefined; // Track install status state useEffect(() => { - if (packageInfoData?.item) { + if (packageInfoIsFetchedAfterMount && packageInfoData?.item) { const packageInfoResponse = packageInfoData.item; setPackageInfo(packageInfoResponse); @@ -267,7 +279,7 @@ export function Detail() { setPackageInstallStatus({ name, status, version: installedVersion || null }); } } - }, [packageInfoData, setPackageInstallStatus, setPackageInfo]); + }, [packageInfoData, packageInfoIsFetchedAfterMount, setPackageInstallStatus, setPackageInfo]); const integrationInfo = useMemo( () => diff --git a/x-pack/plugins/fleet/public/hooks/use_request/epm.ts b/x-pack/plugins/fleet/public/hooks/use_request/epm.ts index 03b2b5973c98a..a09e975d57e16 100644 --- a/x-pack/plugins/fleet/public/hooks/use_request/epm.ts +++ b/x-pack/plugins/fleet/public/hooks/use_request/epm.ts @@ -122,6 +122,7 @@ export const useGetPackageInfoByKeyQuery = ( queryOptions: { // If enabled is false, the query will not be fetched enabled?: boolean; + refetchOnMount?: boolean | 'always'; } = { enabled: true, } @@ -143,7 +144,7 @@ export const useGetPackageInfoByKeyQuery = ( ...(ignoreUnverifiedQueryParam && { ignoreUnverified: ignoreUnverifiedQueryParam }), }, }), - { enabled: queryOptions.enabled } + { enabled: queryOptions.enabled, refetchOnMount: queryOptions.refetchOnMount } ); const confirm = async () => { From dce8eedf561409c2209bae58ecc330d8f245fa91 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Thu, 19 Oct 2023 14:09:18 -0600 Subject: [PATCH 4/8] [SLO] Account for the built-in delay for burn rate alerting (#169011) ## Summary This PR introduces a delay based on: - The interval of the date histogram which defaults to `1m` - The sync delay of the transform which defaults to `1m` - The frequency of the transform which defaults to `1m` On average the SLO data is about `180s` behind for occurances. If the user uses `5m` time slices then the delay is around `420s`. This PR attempts to mitigate this delay by subtracting the `interval + syncDelay + frequency` from `now` on any calculation for the burn rate so that the last 5 minutes is aligned with the data. Below is a visualization that shows how much delay we are seeing in an optimal environment. image Using `interval + syncDelay + frequency` accounts for the "best case scenario". Due to the nature of the transform system, the delays varies from best case of `180s` for occurrences to worst case of around `240s` which happens right before the next scheduled query; the transform query runs every `60s` which accounts for the variation between the worst and best case delay. Since the rules run on a seperate schedule, it's hard to know where we are in the `60s` cycle so the best we can do is account for the "best case". This PR also fixes #168747 ### Note to the reviewer The changes made to `evaluate.ts` and `build_query.ts` look more extensive than they really are. I felt like #168735 made some unnecessary refactors when they simply could have done a minimal change and left the rest of the code alone; it would have been less risky. This also cause several issues during the merge which is why I ultimately decided to revert the changes from #168735. --- .../slo/error_rate_chart/error_rate_chart.tsx | 9 +- .../error_rate_chart/use_lens_definition.ts | 8 +- .../slo/get_delay_in_seconds_from_slo.ts | 29 ++++ .../services/get_delay_in_seconds_from_slo.ts | 18 +++ .../services/get_lookback_date_range.ts | 23 +++ .../lib/rules/slo_burn_rate/executor.ts | 35 +---- .../__snapshots__/build_query.test.ts.snap | 124 ++++++++-------- .../slo_burn_rate/lib/build_query.test.ts | 26 ++-- .../rules/slo_burn_rate/lib/build_query.ts | 62 ++++---- .../lib/rules/slo_burn_rate/lib/evaluate.ts | 19 +-- .../server/services/slo/sli_client.test.ts | 75 ++++++---- .../server/services/slo/sli_client.ts | 136 +++++++++++------- 12 files changed, 337 insertions(+), 227 deletions(-) create mode 100644 x-pack/plugins/observability/public/utils/slo/get_delay_in_seconds_from_slo.ts create mode 100644 x-pack/plugins/observability/server/domain/services/get_delay_in_seconds_from_slo.ts create mode 100644 x-pack/plugins/observability/server/domain/services/get_lookback_date_range.ts diff --git a/x-pack/plugins/observability/public/components/slo/error_rate_chart/error_rate_chart.tsx b/x-pack/plugins/observability/public/components/slo/error_rate_chart/error_rate_chart.tsx index 0c804afb8bea3..7b84d8c05675f 100644 --- a/x-pack/plugins/observability/public/components/slo/error_rate_chart/error_rate_chart.tsx +++ b/x-pack/plugins/observability/public/components/slo/error_rate_chart/error_rate_chart.tsx @@ -10,6 +10,7 @@ import { SLOResponse } from '@kbn/slo-schema'; import moment from 'moment'; import React from 'react'; import { useKibana } from '../../../utils/kibana_react'; +import { getDelayInSecondsFromSLO } from '../../../utils/slo/get_delay_in_seconds_from_slo'; import { useLensDefinition } from './use_lens_definition'; interface Props { @@ -22,14 +23,18 @@ export function ErrorRateChart({ slo, fromRange }: Props) { lens: { EmbeddableComponent }, } = useKibana().services; const lensDef = useLensDefinition(slo); + const delayInSeconds = getDelayInSecondsFromSLO(slo); + + const from = moment(fromRange).subtract(delayInSeconds, 'seconds').toISOString(); + const to = moment().subtract(delayInSeconds, 'seconds').toISOString(); return ( { - return winDef.longDuration.isShorterThan(acc.longDuration) ? acc : winDef; - }, burnRateWindows[0]); - const { dateStart, dateEnd } = getTimeRange( - `${longestLookbackWindow.longDuration.value}${longestLookbackWindow.longDuration.unit}` - ); - - const results = await evaluate( - esClient.asCurrentUser, - slo, - params, - dateStart, - dateEnd, - burnRateWindows - ); + // We only need the end timestamp to base all of queries on. The length of the time range + // doesn't matter for our use case since we allow the user to customize the window sizes, + const { dateEnd } = getTimeRange('1m'); + const results = await evaluate(esClient.asCurrentUser, slo, params, new Date(dateEnd)); if (results.length > 0) { for (const result of results) { @@ -212,19 +200,6 @@ export const getRuleExecutor = ({ return { state: {} }; }; -export function getBurnRateWindows(windows: WindowSchema[]) { - return windows.map((winDef) => { - return { - ...winDef, - longDuration: new Duration(winDef.longWindow.value, toDurationUnit(winDef.longWindow.unit)), - shortDuration: new Duration( - winDef.shortWindow.value, - toDurationUnit(winDef.shortWindow.unit) - ), - }; - }); -} - function getActionGroupName(id: string) { switch (id) { case HIGH_PRIORITY_ACTION.id: diff --git a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/__snapshots__/build_query.test.ts.snap b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/__snapshots__/build_query.test.ts.snap index 8699b28ab5a5e..77597b4fea26a 100644 --- a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/__snapshots__/build_query.test.ts.snap +++ b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/__snapshots__/build_query.test.ts.snap @@ -21,8 +21,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T22:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -70,8 +70,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:55:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:52:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -119,8 +119,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -168,8 +168,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:30:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:27:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -217,8 +217,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-30T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -266,8 +266,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T22:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T21:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -315,8 +315,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -364,8 +364,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -445,8 +445,8 @@ Object { Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -478,8 +478,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T22:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -505,7 +505,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -527,8 +527,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:55:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:51:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -554,7 +554,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -576,8 +576,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -603,7 +603,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -625,8 +625,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:30:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:26:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -652,7 +652,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -674,8 +674,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-30T23:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -701,7 +701,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -723,8 +723,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T22:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T21:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -750,7 +750,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -772,8 +772,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -799,7 +799,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -821,8 +821,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -848,7 +848,7 @@ Object { }, "script": Object { "params": Object { - "target": 0.999, + "target": 0.98, }, "source": "params.total != null && params.total > 0 ? (1 - (params.good / params.total)) / (1 - params.target) : 0", }, @@ -902,8 +902,8 @@ Object { Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:56:00.000Z", + "lt": "2022-12-31T23:56:00.000Z", }, }, }, @@ -935,8 +935,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T22:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -984,8 +984,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:55:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:52:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1033,8 +1033,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1082,8 +1082,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T23:30:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T23:27:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1131,8 +1131,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-30T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1180,8 +1180,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T22:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T21:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1229,8 +1229,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1278,8 +1278,8 @@ Object { "filter": Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-31T18:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-31T17:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, @@ -1362,8 +1362,8 @@ Object { Object { "range": Object { "@timestamp": Object { - "gte": "2022-12-29T00:00:00.000Z", - "lt": "2023-01-01T00:00:00.000Z", + "gte": "2022-12-28T23:57:00.000Z", + "lt": "2022-12-31T23:57:00.000Z", }, }, }, diff --git a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.test.ts b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.test.ts index 6a10733b13690..a77db347edacd 100644 --- a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.test.ts +++ b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.test.ts @@ -7,11 +7,13 @@ import { createBurnRateRule } from '../fixtures/rule'; import { buildQuery } from './build_query'; -import { createKQLCustomIndicator, createSLO } from '../../../../services/slo/fixtures/slo'; -import { getBurnRateWindows } from '../executor'; +import { + createKQLCustomIndicator, + createSLO, + createSLOWithTimeslicesBudgetingMethod, +} from '../../../../services/slo/fixtures/slo'; -const DATE_START = '2022-12-29T00:00:00.000Z'; -const DATE_END = '2023-01-01T00:00:00.000Z'; +const STARTED_AT = new Date('2023-01-01T00:00:00.000Z'); describe('buildQuery()', () => { it('should return a valid query for occurrences', () => { @@ -20,8 +22,7 @@ describe('buildQuery()', () => { indicator: createKQLCustomIndicator(), }); const rule = createBurnRateRule(slo); - const burnRateWindows = getBurnRateWindows(rule.windows); - expect(buildQuery(slo, DATE_START, DATE_END, burnRateWindows)).toMatchSnapshot(); + expect(buildQuery(STARTED_AT, slo, rule)).toMatchSnapshot(); }); it('should return a valid query with afterKey', () => { const slo = createSLO({ @@ -29,21 +30,14 @@ describe('buildQuery()', () => { indicator: createKQLCustomIndicator(), }); const rule = createBurnRateRule(slo); - const burnRateWindows = getBurnRateWindows(rule.windows); - expect( - buildQuery(slo, DATE_START, DATE_END, burnRateWindows, { - instanceId: 'example', - }) - ).toMatchSnapshot(); + expect(buildQuery(STARTED_AT, slo, rule, { instanceId: 'example' })).toMatchSnapshot(); }); it('should return a valid query for timeslices', () => { - const slo = createSLO({ + const slo = createSLOWithTimeslicesBudgetingMethod({ id: 'test-slo', indicator: createKQLCustomIndicator(), - budgetingMethod: 'timeslices', }); const rule = createBurnRateRule(slo); - const burnRateWindows = getBurnRateWindows(rule.windows); - expect(buildQuery(slo, DATE_START, DATE_END, burnRateWindows)).toMatchSnapshot(); + expect(buildQuery(STARTED_AT, slo, rule)).toMatchSnapshot(); }); }); diff --git a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.ts b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.ts index fad8c1ef5d1d7..6b9842abfab48 100644 --- a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.ts +++ b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/build_query.ts @@ -5,12 +5,13 @@ * 2.0. */ -import moment from 'moment'; import { timeslicesBudgetingMethodSchema } from '@kbn/slo-schema'; -import { Duration, SLO, toMomentUnitOfTime } from '../../../../domain/models'; -import { WindowSchema } from '../types'; +import { Duration, SLO, toDurationUnit } from '../../../../domain/models'; +import { BurnRateRuleParams, WindowSchema } from '../types'; +import { getDelayInSecondsFromSLO } from '../../../../domain/services/get_delay_in_seconds_from_slo'; +import { getLookbackDateRange } from '../../../../domain/services/get_lookback_date_range'; -export type BurnRateWindowWithDuration = WindowSchema & { +type BurnRateWindowWithDuration = WindowSchema & { longDuration: Duration; shortDuration: Duration; }; @@ -100,13 +101,14 @@ function buildWindowAgg( } function buildWindowAggs( - startedAt: string, + startedAt: Date, slo: SLO, - burnRateWindows: BurnRateWindowWithDuration[] + burnRateWindows: BurnRateWindowWithDuration[], + delayInSeconds = 0 ) { return burnRateWindows.reduce((acc, winDef, index) => { - const shortDateRange = getLookbackDateRange(startedAt, winDef.shortDuration); - const longDateRange = getLookbackDateRange(startedAt, winDef.longDuration); + const shortDateRange = getLookbackDateRange(startedAt, winDef.shortDuration, delayInSeconds); + const longDateRange = getLookbackDateRange(startedAt, winDef.longDuration, delayInSeconds); const windowId = generateWindowId(index); return { ...acc, @@ -154,12 +156,32 @@ function buildEvaluation(burnRateWindows: BurnRateWindowWithDuration[]) { } export function buildQuery( + startedAt: Date, slo: SLO, - dateStart: string, - dateEnd: string, - burnRateWindows: BurnRateWindowWithDuration[], + params: BurnRateRuleParams, afterKey?: EvaluationAfterKey ) { + const delayInSeconds = getDelayInSecondsFromSLO(slo); + const burnRateWindows = params.windows.map((winDef) => { + return { + ...winDef, + longDuration: new Duration(winDef.longWindow.value, toDurationUnit(winDef.longWindow.unit)), + shortDuration: new Duration( + winDef.shortWindow.value, + toDurationUnit(winDef.shortWindow.unit) + ), + }; + }); + + const longestLookbackWindow = burnRateWindows.reduce((acc, winDef) => { + return winDef.longDuration.isShorterThan(acc.longDuration) ? acc : winDef; + }, burnRateWindows[0]); + const longestDateRange = getLookbackDateRange( + startedAt, + longestLookbackWindow.longDuration, + delayInSeconds + ); + return { size: 0, query: { @@ -170,8 +192,8 @@ export function buildQuery( { range: { '@timestamp': { - gte: dateStart, - lt: dateEnd, + gte: longestDateRange.from.toISOString(), + lt: longestDateRange.to.toISOString(), }, }, }, @@ -186,22 +208,10 @@ export function buildQuery( sources: [{ instanceId: { terms: { field: 'slo.instanceId' } } }], }, aggs: { - ...buildWindowAggs(dateEnd, slo, burnRateWindows), + ...buildWindowAggs(startedAt, slo, burnRateWindows, delayInSeconds), ...buildEvaluation(burnRateWindows), }, }, }, }; } - -function getLookbackDateRange(startedAt: string, duration: Duration): { from: Date; to: Date } { - const unit = toMomentUnitOfTime(duration.unit); - const now = moment(startedAt); - const from = now.clone().subtract(duration.value, unit); - const to = now.clone(); - - return { - from: from.toDate(), - to: to.toDate(), - }; -} diff --git a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/evaluate.ts b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/evaluate.ts index cdd408297f137..8461382fc1564 100644 --- a/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/evaluate.ts +++ b/x-pack/plugins/observability/server/lib/rules/slo_burn_rate/lib/evaluate.ts @@ -12,7 +12,6 @@ import { BurnRateRuleParams } from '../types'; import { SLO_DESTINATION_INDEX_PATTERN } from '../../../../assets/constants'; import { buildQuery, - BurnRateWindowWithDuration, EvaluationAfterKey, generateAboveThresholdKey, generateBurnRateKey, @@ -66,13 +65,12 @@ export interface EvalutionAggResults { async function queryAllResults( esClient: ElasticsearchClient, slo: SLO, - dateStart: string, - dateEnd: string, - burnRateWindows: BurnRateWindowWithDuration[], + params: BurnRateRuleParams, + startedAt: Date, buckets: EvaluationBucket[] = [], lastAfterKey?: { instanceId: string } ): Promise { - const queryAndAggs = buildQuery(slo, dateStart, dateEnd, burnRateWindows, lastAfterKey); + const queryAndAggs = buildQuery(startedAt, slo, params, lastAfterKey); const results = await esClient.search({ index: SLO_DESTINATION_INDEX_PATTERN, ...queryAndAggs, @@ -86,9 +84,8 @@ async function queryAllResults( return queryAllResults( esClient, slo, - dateStart, - dateEnd, - burnRateWindows, + params, + startedAt, [...buckets, ...results.aggregations.instances.buckets], results.aggregations.instances.after_key ); @@ -98,11 +95,9 @@ export async function evaluate( esClient: ElasticsearchClient, slo: SLO, params: BurnRateRuleParams, - dateStart: string, - dateEnd: string, - burnRateWindows: BurnRateWindowWithDuration[] + startedAt: Date ) { - const buckets = await queryAllResults(esClient, slo, dateStart, dateEnd, burnRateWindows); + const buckets = await queryAllResults(esClient, slo, params, startedAt); return transformBucketToResults(buckets, params); } diff --git a/x-pack/plugins/observability/server/services/slo/sli_client.test.ts b/x-pack/plugins/observability/server/services/slo/sli_client.test.ts index 112a3162ba53d..1160448854c95 100644 --- a/x-pack/plugins/observability/server/services/slo/sli_client.test.ts +++ b/x-pack/plugins/observability/server/services/slo/sli_client.test.ts @@ -27,11 +27,18 @@ const commonEsResponse = { }, }; +const TEST_DATE = new Date('2023-01-01T00:00:00.000Z'); + describe('SummaryClient', () => { let esClientMock: ElasticsearchClientMock; beforeEach(() => { esClientMock = elasticsearchServiceMock.createElasticsearchClient(); + jest.useFakeTimers().setSystemTime(TEST_DATE); + }); + + afterAll(() => { + jest.useRealTimers(); }); describe('fetchSLIDataFrom', () => { @@ -51,11 +58,11 @@ describe('SummaryClient', () => { [LONG_WINDOW]: { buckets: [ { - key: '2022-11-08T13:53:00.000Z-2022-11-08T14:53:00.000Z', - from: 1667915580000, - from_as_string: '2022-11-08T13:53:00.000Z', - to: 1667919180000, - to_as_string: '2022-11-08T14:53:00.000Z', + key: '2022-12-31T22:54:00.000Z-2022-12-31T23:54:00.000Z', + from: 1672527240000, + from_as_string: '2022-12-31T22:54:00.000Z', + to: 1672530840000, + to_as_string: '2022-12-31T23:54:00.000Z', doc_count: 60, total: { value: 32169, @@ -69,11 +76,11 @@ describe('SummaryClient', () => { [SHORT_WINDOW]: { buckets: [ { - key: '2022-11-08T14:48:00.000Z-2022-11-08T14:53:00.000Z', - from: 1667918880000, - from_as_string: '2022-11-08T14:48:00.000Z', - to: 1667919180000, - to_as_string: '2022-11-08T14:53:00.000Z', + key: '2022-12-31T23:49:00.000Z-2022-12-31T23:54:00.000Z', + from: 1672530540000, + from_as_string: '2022-12-31T23:49:00.000Z', + to: 1672530840000, + to_as_string: '2022-12-31T23:54:00.000Z', doc_count: 5, total: { value: 2211, @@ -95,7 +102,12 @@ describe('SummaryClient', () => { [LONG_WINDOW]: { date_range: { field: '@timestamp', - ranges: [{ from: 'now-1h/m', to: 'now/m' }], + ranges: [ + { + from: '2022-12-31T22:54:00.000Z', + to: '2022-12-31T23:54:00.000Z', + }, + ], }, aggs: { good: { sum: { field: 'slo.numerator' } }, @@ -105,7 +117,12 @@ describe('SummaryClient', () => { [SHORT_WINDOW]: { date_range: { field: '@timestamp', - ranges: [{ from: 'now-5m/m', to: 'now/m' }], + ranges: [ + { + from: '2022-12-31T23:49:00.000Z', + to: '2022-12-31T23:54:00.000Z', + }, + ], }, aggs: { good: { sum: { field: 'slo.numerator' } }, @@ -141,11 +158,11 @@ describe('SummaryClient', () => { [LONG_WINDOW]: { buckets: [ { - key: '2022-11-08T13:53:00.000Z-2022-11-08T14:53:00.000Z', - from: 1667915580000, - from_as_string: '2022-11-08T13:53:00.000Z', - to: 1667919180000, - to_as_string: '2022-11-08T14:53:00.000Z', + key: '2022-12-31T22:36:00.000Z-2022-12-31T23:36:00.000Z', + from: 1672526160000, + from_as_string: '2022-12-31T22:36:00.000Z', + to: 1672529760000, + to_as_string: '2022-12-31T23:36:00.000Z', doc_count: 60, total: { value: 32169, @@ -159,11 +176,11 @@ describe('SummaryClient', () => { [SHORT_WINDOW]: { buckets: [ { - key: '2022-11-08T14:48:00.000Z-2022-11-08T14:53:00.000Z', - from: 1667918880000, - from_as_string: '2022-11-08T14:48:00.000Z', - to: 1667919180000, - to_as_string: '2022-11-08T14:53:00.000Z', + key: '2022-12-31T23:31:00.000Z-2022-12-31T23:36:00.000Z', + from: 1672529460000, + from_as_string: '2022-12-31T23:31:00.000Z', + to: 1672529760000, + to_as_string: '2022-12-31T23:36:00.000Z', doc_count: 5, total: { value: 2211, @@ -185,7 +202,12 @@ describe('SummaryClient', () => { [LONG_WINDOW]: { date_range: { field: '@timestamp', - ranges: [{ from: 'now-1h/m', to: 'now/m' }], + ranges: [ + { + from: '2022-12-31T22:36:00.000Z', + to: '2022-12-31T23:36:00.000Z', + }, + ], }, aggs: { good: { @@ -203,7 +225,12 @@ describe('SummaryClient', () => { [SHORT_WINDOW]: { date_range: { field: '@timestamp', - ranges: [{ from: 'now-5m/m', to: 'now/m' }], + ranges: [ + { + from: '2022-12-31T23:31:00.000Z', + to: '2022-12-31T23:36:00.000Z', + }, + ], }, aggs: { good: { diff --git a/x-pack/plugins/observability/server/services/slo/sli_client.ts b/x-pack/plugins/observability/server/services/slo/sli_client.ts index c3b3fbc3dd880..5614345d3aefd 100644 --- a/x-pack/plugins/observability/server/services/slo/sli_client.ts +++ b/x-pack/plugins/observability/server/services/slo/sli_client.ts @@ -18,13 +18,13 @@ import { ALL_VALUE, occurrencesBudgetingMethodSchema, timeslicesBudgetingMethodSchema, - toMomentUnitOfTime, } from '@kbn/slo-schema'; import { assertNever } from '@kbn/std'; -import moment from 'moment'; import { SLO_DESTINATION_INDEX_PATTERN } from '../../assets/constants'; import { DateRange, Duration, IndicatorData, SLO } from '../../domain/models'; import { InternalQueryError } from '../../errors'; +import { getDelayInSecondsFromSLO } from '../../domain/services/get_delay_in_seconds_from_slo'; +import { getLookbackDateRange } from '../../domain/services/get_lookback_date_range'; export interface SLIClient { fetchSLIDataFrom( @@ -55,13 +55,22 @@ export class DefaultSLIClient implements SLIClient { a.duration.isShorterThan(b.duration) ? 1 : -1 ); const longestLookbackWindow = sortedLookbackWindows[0]; - const longestDateRange = getLookbackDateRange(longestLookbackWindow.duration); + const delayInSeconds = getDelayInSecondsFromSLO(slo); + const longestDateRange = getLookbackDateRange( + new Date(), + longestLookbackWindow.duration, + delayInSeconds + ); if (occurrencesBudgetingMethodSchema.is(slo.budgetingMethod)) { const result = await this.esClient.search({ ...commonQuery(slo, instanceId, longestDateRange), index: SLO_DESTINATION_INDEX_PATTERN, - aggs: toLookbackWindowsAggregationsQuery(sortedLookbackWindows), + aggs: toLookbackWindowsAggregationsQuery( + longestDateRange.to, + sortedLookbackWindows, + delayInSeconds + ), }); return handleWindowedResult(result.aggregations, lookbackWindows); @@ -71,7 +80,11 @@ export class DefaultSLIClient implements SLIClient { const result = await this.esClient.search({ ...commonQuery(slo, instanceId, longestDateRange), index: SLO_DESTINATION_INDEX_PATTERN, - aggs: toLookbackWindowsSlicedAggregationsQuery(slo, sortedLookbackWindows), + aggs: toLookbackWindowsSlicedAggregationsQuery( + longestDateRange.to, + sortedLookbackWindows, + delayInSeconds + ), }); return handleWindowedResult(result.aggregations, lookbackWindows); @@ -110,53 +123,82 @@ function commonQuery( }; } -function toLookbackWindowsAggregationsQuery(sortedLookbackWindow: LookbackWindow[]) { +function toLookbackWindowsAggregationsQuery( + startedAt: Date, + sortedLookbackWindow: LookbackWindow[], + delayInSeconds = 0 +) { return sortedLookbackWindow.reduce>( - (acc, lookbackWindow) => ({ - ...acc, - [lookbackWindow.name]: { - date_range: { - field: '@timestamp', - ranges: [{ from: `now-${lookbackWindow.duration.format()}/m`, to: 'now/m' }], - }, - aggs: { - good: { sum: { field: 'slo.numerator' } }, - total: { sum: { field: 'slo.denominator' } }, + (acc, lookbackWindow) => { + const lookbackDateRange = getLookbackDateRange( + startedAt, + lookbackWindow.duration, + delayInSeconds + ); + + return { + ...acc, + [lookbackWindow.name]: { + date_range: { + field: '@timestamp', + ranges: [ + { + from: lookbackDateRange.from.toISOString(), + to: lookbackDateRange.to.toISOString(), + }, + ], + }, + aggs: { + good: { sum: { field: 'slo.numerator' } }, + total: { sum: { field: 'slo.denominator' } }, + }, }, - }, - }), + }; + }, {} ); } -function toLookbackWindowsSlicedAggregationsQuery(slo: SLO, lookbackWindows: LookbackWindow[]) { +function toLookbackWindowsSlicedAggregationsQuery( + startedAt: Date, + lookbackWindows: LookbackWindow[], + delayInSeconds = 0 +) { return lookbackWindows.reduce>( - (acc, lookbackWindow) => ({ - ...acc, - [lookbackWindow.name]: { - date_range: { - field: '@timestamp', - ranges: [ - { - from: `now-${lookbackWindow.duration.format()}/m`, - to: 'now/m', - }, - ], - }, - aggs: { - good: { - sum: { - field: 'slo.isGoodSlice', - }, + (acc, lookbackWindow) => { + const lookbackDateRange = getLookbackDateRange( + startedAt, + lookbackWindow.duration, + delayInSeconds + ); + + return { + ...acc, + [lookbackWindow.name]: { + date_range: { + field: '@timestamp', + ranges: [ + { + from: lookbackDateRange.from.toISOString(), + to: lookbackDateRange.to.toISOString(), + }, + ], }, - total: { - value_count: { - field: 'slo.isGoodSlice', + aggs: { + good: { + sum: { + field: 'slo.isGoodSlice', + }, + }, + total: { + value_count: { + field: 'slo.isGoodSlice', + }, }, }, }, - }, - }), + }; + }, {} ); } @@ -191,15 +233,3 @@ function handleWindowedResult( return indicatorDataPerLookbackWindow; } - -function getLookbackDateRange(duration: Duration): { from: Date; to: Date } { - const unit = toMomentUnitOfTime(duration.unit); - const now = moment.utc().startOf('minute'); - const from = now.clone().subtract(duration.value, unit); - const to = now.clone(); - - return { - from: from.toDate(), - to: to.toDate(), - }; -} From b8ecaa22d1e2f4306c3049681e8b8e40ee1c78f6 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Thu, 19 Oct 2023 14:26:11 -0600 Subject: [PATCH 5/8] [Dashboard] Fix flaky snapshot test (#169415) Closes https://github.com/elastic/kibana/issues/167942 ## Summary This PR fixes the attached flaky test by ensuring that everything on the page has loaded before taking the snapshot. Because these snapshot tests are on an empty dashboard, the previous call to `waitForRenderComplete` (which ensures each **panel** has been rendered/loaded) didn't actually do anything; so, I replaced it with individual `waitFor` calls that ensure the dashboard is in (approximately) the correct state before the screenshot is taken. ### [Flaky Test Runner](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3632) ![image](https://github.com/elastic/kibana/assets/8698078/e4a406ec-336b-4321-b933-c3948bb90956) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --- .../apps/dashboard/group6/dashboard_snapshots.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/functional/apps/dashboard/group6/dashboard_snapshots.ts b/test/functional/apps/dashboard/group6/dashboard_snapshots.ts index 401c1fa649006..3c743e9021a21 100644 --- a/test/functional/apps/dashboard/group6/dashboard_snapshots.ts +++ b/test/functional/apps/dashboard/group6/dashboard_snapshots.ts @@ -29,6 +29,8 @@ export default function ({ const kibanaServer = getService('kibanaServer'); const dashboardPanelActions = getService('dashboardPanelActions'); const dashboardAddPanel = getService('dashboardAddPanel'); + const testSubjects = getService('testSubjects'); + const retry = getService('retry'); describe('dashboard snapshots', function describeIndexTests() { before(async function () { @@ -96,6 +98,15 @@ export default function ({ }); describe('compare controls snapshot', async () => { + const waitForPageReady = async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await retry.waitFor('page ready for screenshot', async () => { + const queryBarVisible = await testSubjects.exists('globalQueryBar'); + const controlGroupVisible = await testSubjects.exists('controls-group-wrapper'); + return queryBarVisible && controlGroupVisible; + }); + }; + before(async () => { await PageObjects.dashboard.gotoDashboardLandingPage(); await PageObjects.dashboard.clickNewDashboard(); @@ -115,6 +126,7 @@ export default function ({ }); it('in light mode', async () => { + await waitForPageReady(); const percentDifference = await screenshot.compareAgainstBaseline( 'dashboard_controls_light', updateBaselines @@ -127,8 +139,7 @@ export default function ({ 'theme:darkMode': true, }); await browser.refresh(); - await PageObjects.dashboard.waitForRenderComplete(); - + await waitForPageReady(); const percentDifference = await screenshot.compareAgainstBaseline( 'dashboard_controls_dark', updateBaselines From 44f3910763996da35cb9a22ce8042da309319d91 Mon Sep 17 00:00:00 2001 From: Jordan <51442161+JordanSh@users.noreply.github.com> Date: Thu, 19 Oct 2023 23:30:48 +0300 Subject: [PATCH 6/8] [Cloud Security] Enable credential fields in integration edit (#169365) --- .../aws_credentials_form/aws_credentials_form.tsx | 9 --------- .../components/fleet_extensions/gcp_credential_form.tsx | 3 --- .../components/fleet_extensions/policy_template_form.tsx | 4 ++-- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/aws_credentials_form/aws_credentials_form.tsx b/x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/aws_credentials_form/aws_credentials_form.tsx index 5c12396942d77..4be4ac082f60f 100644 --- a/x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/aws_credentials_form/aws_credentials_form.tsx +++ b/x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/aws_credentials_form/aws_credentials_form.tsx @@ -255,7 +255,6 @@ export const AwsCredentialsForm = ({ {setupFormat === 'manual' && ( <> { updatePolicy( @@ -271,7 +270,6 @@ export const AwsCredentialsForm = ({ { updatePolicy(getPosturePolicy(newPolicy, input.type, { [key]: { value } })); @@ -286,11 +284,9 @@ export const AwsCredentialsForm = ({ const AwsCredentialTypeSelector = ({ type, onChange, - disabled, }: { onChange(type: AwsCredentialsType): void; type: AwsCredentialsType; - disabled: boolean; }) => ( ; onChange: (key: string, value: string) => void; - disabled: boolean; }) => (
{fields.map((field) => ( @@ -325,7 +318,6 @@ const AwsInputVarFields = ({ <> {field.type === 'password' && (

From 60bb1f8da1f50e0cca42d4c73fc9c730e5c76a0a Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Thu, 19 Oct 2023 14:34:39 -0600 Subject: [PATCH 7/8] [Security Solution] [Elastic AI Assistant] Throw error if Knowledge Base is enabled but ELSER is unavailable (#169330) ## Summary This fixes the Knowledge Base UX a bit by throwing an error if somehow ELSER has been disabled in the background, and instructs the user on how to resolve or to disable the Knowledge Base to continue. Additionally, if ELSER is not available, we prevent the enabling of the Knowledge Base as to not provide a degraded experience when ELSER and the ES|QL documentation is not available.

> [!NOTE] > `isModelInstalled` logic has been updated to not just check the model `definition_status`, but to actually ensure that it's deployed by checking to see that it is `started` and `fully_allocated`. This better guards ELSER availability as the previous check would return true if the model was just downloaded and not actually deployed. Also resolves: https://github.com/elastic/kibana/issues/169403 ## Test Instructions After enabling the KB, disable the ELSER deployment in the `Trained Models` ML UI and then try using the assistant. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../impl/assistant/api.tsx | 2 +- .../impl/assistant/context_pills/index.tsx | 39 +++++++---- .../knowledge_base_settings.tsx | 37 ++++++---- .../impl/knowledge_base/translations.ts | 7 ++ .../elasticsearch_store.test.ts | 68 ++++++++++++++++--- .../elasticsearch_store.ts | 15 ++-- .../execute_custom_llm_chain/index.test.ts | 28 ++++++++ .../execute_custom_llm_chain/index.ts | 8 +++ .../routes/post_actions_connector_execute.ts | 4 ++ 9 files changed, 165 insertions(+), 43 deletions(-) diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx index 8ccb2e72cfee9..d6b61ee70b1ae 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx @@ -95,7 +95,7 @@ export const fetchConnectorExecuteAction = async ({ }; } catch (error) { return { - response: API_ERROR, + response: `${API_ERROR}\n\n${error?.body?.message ?? error?.message}`, isError: true, }; } diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/context_pills/index.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/context_pills/index.tsx index be5374e37bd67..89aef125b5d0e 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/context_pills/index.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/context_pills/index.tsx @@ -66,21 +66,30 @@ const ContextPillsComponent: React.FC = ({ return ( - {sortedPromptContexts.map(({ description, id, getPromptContext, tooltip }) => ( - - - selectPromptContext(id)} - > - {description} - - - - ))} + {sortedPromptContexts.map(({ description, id, getPromptContext, tooltip }) => { + // Workaround for known issue where tooltip won't dismiss after button state is changed once clicked + // See: https://github.com/elastic/eui/issues/6488#issuecomment-1379656704 + const button = ( + selectPromptContext(id)} + > + {description} + + ); + return ( + + {selectedPromptContexts[id] != null ? ( + button + ) : ( + {button} + )} + + ); + })} ); }; diff --git a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings.tsx b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings.tsx index bed527b6434e3..e49602a52411a 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings.tsx @@ -20,6 +20,7 @@ import { EuiFlexItem, EuiHealth, EuiButtonEmpty, + EuiToolTip, EuiSwitch, } from '@elastic/eui'; @@ -56,8 +57,8 @@ export const KnowledgeBaseSettings: React.FC = React.memo( const { mutate: deleteKB, isLoading: isDeletingUpKB } = useDeleteKnowledgeBase({ http }); // Resource enabled state - const isKnowledgeBaseEnabled = - (kbStatus?.index_exists && kbStatus?.pipeline_exists && kbStatus?.elser_exists) ?? false; + const isElserEnabled = kbStatus?.elser_exists ?? false; + const isKnowledgeBaseEnabled = (kbStatus?.index_exists && kbStatus?.pipeline_exists) ?? false; const isESQLEnabled = kbStatus?.esql_exists ?? false; // Resource availability state @@ -65,9 +66,11 @@ export const KnowledgeBaseSettings: React.FC = React.memo( const isKnowledgeBaseAvailable = knowledgeBase.assistantLangChain && kbStatus?.elser_exists; const isESQLAvailable = knowledgeBase.assistantLangChain && isKnowledgeBaseAvailable && isKnowledgeBaseEnabled; + // Prevent enabling if elser doesn't exist, but always allow to disable + const isSwitchDisabled = !kbStatus?.elser_exists && !knowledgeBase.assistantLangChain; // Calculated health state for EuiHealth component - const elserHealth = kbStatus?.elser_exists ? 'success' : 'subdued'; + const elserHealth = isElserEnabled ? 'success' : 'subdued'; const knowledgeBaseHealth = isKnowledgeBaseEnabled ? 'success' : 'subdued'; const esqlHealth = isESQLEnabled ? 'success' : 'subdued'; @@ -93,16 +96,24 @@ export const KnowledgeBaseSettings: React.FC = React.memo( return isLoadingKb ? ( ) : ( - + + + ); - }, [isLoadingKb, knowledgeBase.assistantLangChain, onEnableAssistantLangChainChange]); + }, [ + isLoadingKb, + isSwitchDisabled, + knowledgeBase.assistantLangChain, + onEnableAssistantLangChainChange, + ]); ////////////////////////////////////////////////////////////////////////////////////////// // Knowledge Base Resource @@ -205,7 +216,7 @@ export const KnowledgeBaseSettings: React.FC = React.memo( display="columnCompressedSwitch" label={i18n.KNOWLEDGE_BASE_LABEL} css={css` - div { + .euiFormRow__labelWrapper { min-width: 95px !important; } `} diff --git a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/translations.ts b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/translations.ts index 95417ddf6a889..1aa295e311e68 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/translations.ts +++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/translations.ts @@ -36,6 +36,13 @@ export const KNOWLEDGE_BASE_LABEL = i18n.translate( } ); +export const KNOWLEDGE_BASE_TOOLTIP = i18n.translate( + 'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettings.knowledgeBaseTooltip', + { + defaultMessage: 'ELSER must be configured to enable the Knowledge Base', + } +); + export const KNOWLEDGE_BASE_DESCRIPTION = i18n.translate( 'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettings.knowledgeBaseDescription', { diff --git a/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.test.ts b/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.test.ts index 1de907c3ddc9c..0dc1cd10499cc 100644 --- a/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.test.ts +++ b/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.test.ts @@ -9,7 +9,7 @@ import { elasticsearchServiceMock } from '@kbn/core-elasticsearch-server-mocks'; import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { IndicesCreateResponse, - MlGetTrainedModelsResponse, + MlGetTrainedModelsStatsResponse, } from '@elastic/elasticsearch/lib/api/types'; import { Document } from 'langchain/document'; @@ -142,17 +142,69 @@ describe('ElasticsearchStore', () => { }); }); - describe('Model Management', () => { - it('Checks if a model is installed', async () => { - mockEsClient.ml.getTrainedModels.mockResolvedValue({ - trained_model_configs: [{ fully_defined: true }], - } as MlGetTrainedModelsResponse); + describe('isModelInstalled', () => { + it('returns true if model is started and fully allocated', async () => { + mockEsClient.ml.getTrainedModelsStats.mockResolvedValue({ + trained_model_stats: [ + { + deployment_stats: { + state: 'started', + allocation_status: { + state: 'fully_allocated', + }, + }, + }, + ], + } as MlGetTrainedModelsStatsResponse); const isInstalled = await esStore.isModelInstalled('.elser_model_2'); expect(isInstalled).toBe(true); - expect(mockEsClient.ml.getTrainedModels).toHaveBeenCalledWith({ - include: 'definition_status', + expect(mockEsClient.ml.getTrainedModelsStats).toHaveBeenCalledWith({ + model_id: '.elser_model_2', + }); + }); + + it('returns false if model is not started', async () => { + mockEsClient.ml.getTrainedModelsStats.mockResolvedValue({ + trained_model_stats: [ + { + deployment_stats: { + state: 'starting', + allocation_status: { + state: 'fully_allocated', + }, + }, + }, + ], + } as MlGetTrainedModelsStatsResponse); + + const isInstalled = await esStore.isModelInstalled('.elser_model_2'); + + expect(isInstalled).toBe(false); + expect(mockEsClient.ml.getTrainedModelsStats).toHaveBeenCalledWith({ + model_id: '.elser_model_2', + }); + }); + + it('returns false if model is not fully allocated', async () => { + mockEsClient.ml.getTrainedModelsStats.mockResolvedValue({ + trained_model_stats: [ + { + deployment_stats: { + state: 'started', + allocation_status: { + state: 'starting', + }, + }, + }, + ], + } as MlGetTrainedModelsStatsResponse); + + const isInstalled = await esStore.isModelInstalled('.elser_model_2'); + + expect(isInstalled).toBe(false); + expect(mockEsClient.ml.getTrainedModelsStats).toHaveBeenCalledWith({ model_id: '.elser_model_2', }); }); diff --git a/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.ts b/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.ts index 52f3fe87275db..b6c8f37384862 100644 --- a/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.ts +++ b/x-pack/plugins/elastic_assistant/server/lib/langchain/elasticsearch_store/elasticsearch_store.ts @@ -37,7 +37,7 @@ interface CreateIndexParams { } /** - * A fallback for the the query `size` that determines how many documents to + * A fallback for the query `size` that determines how many documents to * return from Elasticsearch when performing a similarity search. * * The size is typically determined by the implementation of LangChain's @@ -360,14 +360,17 @@ export class ElasticsearchStore extends VectorStore { * @param modelId ID of the model to check * @returns Promise indicating whether the model is installed */ - async isModelInstalled(modelId: string): Promise { + async isModelInstalled(modelId?: string): Promise { try { - const getResponse = await this.esClient.ml.getTrainedModels({ - model_id: modelId, - include: 'definition_status', + const getResponse = await this.esClient.ml.getTrainedModelsStats({ + model_id: modelId ?? this.model, }); - return Boolean(getResponse.trained_model_configs[0]?.fully_defined); + return getResponse.trained_model_stats.some( + (stats) => + stats.deployment_stats?.state === 'started' && + stats.deployment_stats?.allocation_status.state === 'fully_allocated' + ); } catch (e) { // Returns 404 if it doesn't exist return false; diff --git a/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.test.ts b/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.test.ts index e63da9257aa36..7f06b4b456194 100644 --- a/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.test.ts +++ b/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.test.ts @@ -16,6 +16,7 @@ import { langChainMessages } from '../../../__mocks__/lang_chain_messages'; import { ESQL_RESOURCE } from '../../../routes/knowledge_base/constants'; import { ResponseBody } from '../types'; import { callAgentExecutor } from '.'; +import { ElasticsearchStore } from '../elasticsearch_store/elasticsearch_store'; jest.mock('../llm/actions_client_llm'); @@ -36,6 +37,13 @@ jest.mock('langchain/agents', () => ({ })), })); +jest.mock('../elasticsearch_store/elasticsearch_store', () => ({ + ElasticsearchStore: jest.fn().mockImplementation(() => ({ + asRetriever: jest.fn(), + isModelInstalled: jest.fn().mockResolvedValue(true), + })), +})); + const mockConnectorId = 'mock-connector-id'; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -129,4 +137,24 @@ describe('callAgentExecutor', () => { status: 'ok', }); }); + + it('throws an error if ELSER model is not installed', async () => { + (ElasticsearchStore as unknown as jest.Mock).mockImplementationOnce(() => ({ + isModelInstalled: jest.fn().mockResolvedValue(false), + })); + + await expect( + callAgentExecutor({ + actions: mockActions, + connectorId: mockConnectorId, + esClient: esClientMock, + langChainMessages, + logger: mockLogger, + request: mockRequest, + kbResource: ESQL_RESOURCE, + }) + ).rejects.toThrow( + 'Please ensure ELSER is configured to use the Knowledge Base, otherwise disable the Knowledge Base in Advanced Settings to continue.' + ); + }); }); diff --git a/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.ts b/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.ts index 694bd44bfd471..713a330b46c48 100644 --- a/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.ts +++ b/x-pack/plugins/elastic_assistant/server/lib/langchain/execute_custom_llm_chain/index.ts @@ -47,6 +47,14 @@ export const callAgentExecutor = async ({ elserId, kbResource ); + + const modelExists = await esStore.isModelInstalled(); + if (!modelExists) { + throw new Error( + 'Please ensure ELSER is configured to use the Knowledge Base, otherwise disable the Knowledge Base in Advanced Settings to continue.' + ); + } + const chain = RetrievalQAChain.fromLLM(llm, esStore.asRetriever()); const tools: Tool[] = [ diff --git a/x-pack/plugins/elastic_assistant/server/routes/post_actions_connector_execute.ts b/x-pack/plugins/elastic_assistant/server/routes/post_actions_connector_execute.ts index 8da820288ae1b..299d8ade24a3f 100644 --- a/x-pack/plugins/elastic_assistant/server/routes/post_actions_connector_execute.ts +++ b/x-pack/plugins/elastic_assistant/server/routes/post_actions_connector_execute.ts @@ -44,12 +44,16 @@ export const postActionsConnectorExecuteRoute = ( // if not langchain, call execute action directly and return the response: if (!request.body.assistantLangChain) { + logger.debug('Executing via actions framework directly, assistantLangChain: false'); const result = await executeAction({ actions, request, connectorId }); return response.ok({ body: result, }); } + // TODO: Add `traceId` to actions request when calling via langchain + logger.debug('Executing via langchain, assistantLangChain: true'); + // get a scoped esClient for assistant memory const esClient = (await context.core).elasticsearch.client.asCurrentUser; From d6f237a3e63baf0b4ddc6b734f93b687aa4b4307 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 19 Oct 2023 16:02:42 -0500 Subject: [PATCH 8/8] [ci/on-merge] Remove parallelism from profiling and apm tests (#169405) These tests are not sharded across agents, and historical data is not captured for flaky tests. I'm leaving the pull request parallelism flags as is in case the goal is to prevent flakiness, but in both cases operations strongly prefers integration with the flaky test runner instead. --- .buildkite/pipelines/on_merge_unsupported_ftrs.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.buildkite/pipelines/on_merge_unsupported_ftrs.yml b/.buildkite/pipelines/on_merge_unsupported_ftrs.yml index 6dee27db71659..8221e27ea7502 100644 --- a/.buildkite/pipelines/on_merge_unsupported_ftrs.yml +++ b/.buildkite/pipelines/on_merge_unsupported_ftrs.yml @@ -27,7 +27,6 @@ steps: queue: n2-4-spot depends_on: build timeout_in_minutes: 120 - parallelism: 4 retry: automatic: - exit_status: '-1' @@ -41,7 +40,6 @@ steps: queue: n2-4-spot depends_on: build timeout_in_minutes: 120 - parallelism: 4 retry: automatic: - exit_status: '-1'