From 69599e0b49ef44f174247adaa49f74bafc1c8735 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 27 Aug 2020 20:00:42 -0400 Subject: [PATCH 01/17] [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (#76145) ## Summary Fixes bug found in 7.9. **Current behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table does not update until after you refresh or toggle between `Closed` and `Open` **New behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table updates --- .../events_viewer/events_viewer.test.tsx | 42 ++++++++++++++ .../events_viewer/events_viewer.tsx | 6 +- .../common/components/events_viewer/index.tsx | 7 ++- .../components/alerts_table/index.tsx | 55 +++++++++++++------ 4 files changed, 92 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.test.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.test.tsx index 833688ae57993..6f77d15913d07 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.test.tsx @@ -50,6 +50,10 @@ const utilityBar = (refetch: inputsModel.Refetch, totalCount: number) => (
); +const exceptionsModal = (refetch: inputsModel.Refetch) => ( +
+); + const eventsViewerDefaultProps = { browserFields: {}, columns: [], @@ -460,4 +464,42 @@ describe('EventsViewer', () => { }); }); }); + + describe('exceptions modal', () => { + test('it renders exception modal if "exceptionsModal" callback exists', async () => { + const wrapper = mount( + + + + + + ); + + await waitFor(() => { + wrapper.update(); + + expect(wrapper.find(`[data-test-subj="mock-exceptions-modal"]`).exists()).toBeTruthy(); + }); + }); + + test('it does not render exception modal if "exceptionModal" callback does not exist', async () => { + const wrapper = mount( + + + + + + ); + + await waitFor(() => { + wrapper.update(); + + expect(wrapper.find(`[data-test-subj="mock-exceptions-modal"]`).exists()).toBeFalsy(); + }); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx index 436386077e725..ebda64efabf65 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx @@ -109,6 +109,7 @@ interface Props { utilityBar?: (refetch: inputsModel.Refetch, totalCount: number) => React.ReactNode; // If truthy, the graph viewer (Resolver) is showing graphEventId: string | undefined; + exceptionsModal?: (refetch: inputsModel.Refetch) => React.ReactNode; } const EventsViewerComponent: React.FC = ({ @@ -134,6 +135,7 @@ const EventsViewerComponent: React.FC = ({ toggleColumn, utilityBar, graphEventId, + exceptionsModal, }) => { const { globalFullScreen } = useFullScreen(); const columnsHeader = isEmpty(columns) ? defaultHeaders : columns; @@ -259,6 +261,7 @@ const EventsViewerComponent: React.FC = ({ )} + {exceptionsModal && exceptionsModal(refetch)} {utilityBar && !resolverIsShowing(graphEventId) && ( {utilityBar?.(refetch, totalCountMinusDeleted)} )} @@ -335,5 +338,6 @@ export const EventsViewer = React.memo( prevProps.start === nextProps.start && prevProps.sort === nextProps.sort && prevProps.utilityBar === nextProps.utilityBar && - prevProps.graphEventId === nextProps.graphEventId + prevProps.graphEventId === nextProps.graphEventId && + prevProps.exceptionsModal === nextProps.exceptionsModal ); diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx index e4520dab4626a..ec56a3a1bd8d3 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx @@ -43,6 +43,7 @@ export interface OwnProps { headerFilterGroup?: React.ReactNode; pageFilters?: Filter[]; utilityBar?: (refetch: inputsModel.Refetch, totalCount: number) => React.ReactNode; + exceptionsModal?: (refetch: inputsModel.Refetch) => React.ReactNode; } type Props = OwnProps & PropsFromRedux; @@ -74,6 +75,7 @@ const StatefulEventsViewerComponent: React.FC = ({ utilityBar, // If truthy, the graph viewer (Resolver) is showing graphEventId, + exceptionsModal, }) => { const [ { docValueFields, browserFields, indexPatterns, isLoading: isLoadingIndexPattern }, @@ -156,6 +158,7 @@ const StatefulEventsViewerComponent: React.FC = ({ toggleColumn={toggleColumn} utilityBar={utilityBar} graphEventId={graphEventId} + exceptionsModal={exceptionsModal} /> @@ -220,6 +223,7 @@ type PropsFromRedux = ConnectedProps; export const StatefulEventsViewer = connector( React.memo( StatefulEventsViewerComponent, + // eslint-disable-next-line complexity (prevProps, nextProps) => prevProps.id === nextProps.id && deepEqual(prevProps.columns, nextProps.columns) && @@ -240,6 +244,7 @@ export const StatefulEventsViewer = connector( prevProps.showCheckboxes === nextProps.showCheckboxes && prevProps.start === nextProps.start && prevProps.utilityBar === nextProps.utilityBar && - prevProps.graphEventId === nextProps.graphEventId + prevProps.graphEventId === nextProps.graphEventId && + prevProps.exceptionsModal === nextProps.exceptionsModal ) ); diff --git a/x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx b/x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx index 07e69d850f173..854565ace9b4b 100644 --- a/x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx @@ -228,7 +228,7 @@ export const AlertsTableComponent: React.FC = ({ exceptionListType, alertData, }: AddExceptionModalBaseProps) => { - if (alertData !== null && alertData !== undefined) { + if (alertData != null) { setShouldShowAddExceptionModal(true); setAddExceptionModalState({ ruleName, @@ -441,9 +441,43 @@ export const AlertsTableComponent: React.FC = ({ closeAddExceptionModal(); }, [closeAddExceptionModal]); - const onAddExceptionConfirm = useCallback(() => closeAddExceptionModal(), [ - closeAddExceptionModal, - ]); + const onAddExceptionConfirm = useCallback( + (refetch: inputsModel.Refetch) => (): void => { + refetch(); + closeAddExceptionModal(); + }, + [closeAddExceptionModal] + ); + + // Callback for creating the AddExceptionModal and allowing it + // access to the refetchQuery to update the page + const exceptionModalCallback = useCallback( + (refetchQuery: inputsModel.Refetch) => { + if (shouldShowAddExceptionModal) { + return ( + + ); + } else { + return <>; + } + }, + [ + addExceptionModalState, + filterGroup, + onAddExceptionCancel, + onAddExceptionConfirm, + shouldShowAddExceptionModal, + ] + ); if (loading || indexPatternsLoading || isEmpty(signalsIndex)) { return ( @@ -465,19 +499,8 @@ export const AlertsTableComponent: React.FC = ({ id={timelineId} start={from} utilityBar={utilityBarCallback} + exceptionsModal={exceptionModalCallback} /> - {shouldShowAddExceptionModal === true && addExceptionModalState.alertData !== null && ( - - )} ); }; From 1b75690b73c8b23de57575de0cb2bb71d899be16 Mon Sep 17 00:00:00 2001 From: Lisa Cawley Date: Thu, 27 Aug 2020 18:56:31 -0700 Subject: [PATCH 02/17] [Transforms] Avoid using "Are you sure" (#75932) --- .../action_delete/delete_action_modal.tsx | 32 +------------------ .../action_start/start_action_modal.tsx | 5 ++- .../translations/translations/ja-JP.json | 5 --- .../translations/translations/zh-CN.json | 5 --- 4 files changed, 3 insertions(+), 44 deletions(-) diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_delete/delete_action_modal.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_delete/delete_action_modal.tsx index 3c06af1eecb59..b7d4413e9455a 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_delete/delete_action_modal.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_delete/delete_action_modal.tsx @@ -15,8 +15,6 @@ import { EuiSpacer, EuiSwitch, } from '@elastic/eui'; -import { FormattedMessage } from '@kbn/i18n/react'; -import { TRANSFORM_STATE } from '../../../../../../common'; import { DeleteAction } from './use_delete_action'; @@ -42,26 +40,11 @@ export const DeleteActionModal: FC = ({ } ); const deleteModalTitle = i18n.translate('xpack.transform.transformList.deleteModalTitle', { - defaultMessage: 'Delete {transformId}', + defaultMessage: 'Delete {transformId}?', values: { transformId: items[0] && items[0].config.id }, }); const bulkDeleteModalContent = ( <> -

- {shouldForceDelete ? ( - - ) : ( - - )} -

{ @@ -100,19 +83,6 @@ export const DeleteActionModal: FC = ({ const deleteModalContent = ( <> -

- {items[0] && items[0] && items[0].stats.state === TRANSFORM_STATE.FAILED ? ( - - ) : ( - - )} -

{userCanDeleteIndex && ( diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_start/start_action_modal.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_start/start_action_modal.tsx index 9f7dbee10cef0..ebc88918de6d5 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_start/start_action_modal.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_start/start_action_modal.tsx @@ -18,7 +18,7 @@ export const StartActionModal: FC = ({ closeModal, items, startAndC values: { count: items && items.length }, }); const startModalTitle = i18n.translate('xpack.transform.transformList.startModalTitle', { - defaultMessage: 'Start {transformId}', + defaultMessage: 'Start {transformId}?', values: { transformId: items[0] && items[0].config.id }, }); @@ -40,8 +40,7 @@ export const StartActionModal: FC = ({ closeModal, items, startAndC

{i18n.translate('xpack.transform.transformList.startModalBody', { defaultMessage: - 'A transform will increase search and indexing load in your cluster. Please stop the transform if excessive load is experienced. Are you sure you want to start {count, plural, one {this} other {these}} {count} {count, plural, one {transform} other {transforms}}?', - values: { count: items.length }, + 'A transform increases search and indexing load in your cluster. If excessive load is experienced, stop the transform.', })}

diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index c99980fe6205c..dbbe6072b1fe5 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -18154,17 +18154,14 @@ "xpack.transform.transformForm.sizeNotationPlaceholder": "例: {example1}、{example2}、{example3}、{example4}", "xpack.transform.transformList.bulkDeleteDestIndexPatternSuccessMessage": "{count}個のディスティネーションインデックス{count, plural, one {パターン} other {パターン}}を正常に削除しました。", "xpack.transform.transformList.bulkDeleteDestIndexSuccessMessage": "{count}個のディスティネーション{count, plural, one {インデックス} other {インデックス}}を正常に削除しました。", - "xpack.transform.transformList.bulkDeleteModalBody": "{count, plural, one {この} other {これらの}} {count} {count, plural, one {変換} other {変換}}を削除しますか?", "xpack.transform.transformList.bulkDeleteModalTitle": "{count} 件の{count, plural, one {変換} other {変換}}を削除", "xpack.transform.transformList.bulkDeleteTransformSuccessMessage": "{count} {count, plural, one {個の変換} other {個の変換}}を正常に削除しました。", - "xpack.transform.transformList.bulkForceDeleteModalBody": "{count, plural, one {この} other {これらの}} {count} {count, plural, one {変換} other {変換}}を強制的に削除しますか?{count, plural, one {} other {}}現在の状態に関係なく、{count, plural, one {個の変換} other {個の変換}}は削除されます。", "xpack.transform.transformList.bulkStartModalTitle": "{count} 件の{count, plural, one {変換} other {変換}}を開始", "xpack.transform.transformList.completeBatchTransformBulkActionToolTip": "1 つまたは複数の変換が完了済みの一斉変換で、再度開始できません。", "xpack.transform.transformList.completeBatchTransformToolTip": "{transformId} は完了済みの一斉変換で、再度開始できません。", "xpack.transform.transformList.createTransformButton": "変換の作成", "xpack.transform.transformList.deleteActionDisabledToolTipContent": "削除するにはデータフレームジョブを停止してください。", "xpack.transform.transformList.deleteBulkActionDisabledToolTipContent": "削除するには、選択された変換のうちの 1 つまたは複数を停止する必要があります。", - "xpack.transform.transformList.deleteModalBody": "この変換を削除してよろしいですか？", "xpack.transform.transformList.deleteModalCancelButton": "キャンセル", "xpack.transform.transformList.deleteModalDeleteButton": "削除", "xpack.transform.transformList.deleteModalTitle": "{transformId} 削除", @@ -18187,14 +18184,12 @@ "xpack.transform.transformList.editTransformGenericErrorMessage": "変換を削除するためのAPIエンドポイントの呼び出し中にエラーが発生しました。", "xpack.transform.transformList.editTransformSuccessMessage": "変換{transformId}が更新されました。", "xpack.transform.transformList.errorWithCheckingIfUserCanDeleteIndexNotificationErrorMessage": "ユーザーがディスティネーションインデックスを削除できるかどうかを確認するときにエラーが発生しました。", - "xpack.transform.transformList.forceDeleteModalBody": "この変換を強制的に削除しますか?現在の状態に関係なく、変換が削除されます。", "xpack.transform.transformList.refreshButtonLabel": "更新", "xpack.transform.transformList.rowCollapse": "{transformId} の詳細を非表示", "xpack.transform.transformList.rowExpand": "{transformId} の詳細を表示", "xpack.transform.transformList.showDetailsColumn.screenReaderDescription": "このカラムには変換ごとの詳細を示すクリック可能なコントロールが含まれます", "xpack.transform.transformList.startedTransformBulkToolTip": "1 つまたは複数の変換が既に開始済みです。", "xpack.transform.transformList.startedTransformToolTip": "{transformId} は既に開始済みです。", - "xpack.transform.transformList.startModalBody": "変換は、クラスターの検索とインデックスによる負荷を増やします。過剰な負荷が生じた場合は変換を停止してください。{count, plural, one {この} other {これら}} {count} 件の{count, plural, one {変換} other {変換}}を開始してよろしいですか？", "xpack.transform.transformList.startModalCancelButton": "キャンセル", "xpack.transform.transformList.startModalStartButton": "開始", "xpack.transform.transformList.startModalTitle": "{transformId} を開始", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 9ffa81a921ba8..ed849c1050497 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -18161,17 +18161,14 @@ "xpack.transform.transformForm.sizeNotationPlaceholder": "示例：{example1}、{example2}、{example3}、{example4}", "xpack.transform.transformList.bulkDeleteDestIndexPatternSuccessMessage": "已成功删除 {count} 个目标索引{count, plural, one {模式} other {模式}}。", "xpack.transform.transformList.bulkDeleteDestIndexSuccessMessage": "已成功删除 {count} 个目标{count, plural, one {索引} other {索引}}。", - "xpack.transform.transformList.bulkDeleteModalBody": "确定要删除{count, plural, one {这} other {这}} {count} 个{count, plural, one {转换} other {转换}}？", "xpack.transform.transformList.bulkDeleteModalTitle": "删除 {count} 个 {count, plural, one {转换} other {转换}}？", "xpack.transform.transformList.bulkDeleteTransformSuccessMessage": "已成功删除 {count} 个{count, plural, one {转换} other {转换}}。", - "xpack.transform.transformList.bulkForceDeleteModalBody": "确定要强制删除{count, plural, one {这} other {这}} {count} 个{count, plural, one {转换} other {转换}}？无论{count, plural, one {转换} other {转换}}的当前状态如何，都将删除{count, plural, one {} other {}}。", "xpack.transform.transformList.bulkStartModalTitle": "启动 {count} 个 {count, plural, one {转换} other {转换}}？", "xpack.transform.transformList.completeBatchTransformBulkActionToolTip": "一个或多个转换为已完成批量转换，无法重新启动。", "xpack.transform.transformList.completeBatchTransformToolTip": "{transformId} 为已完成批量转换，无法重新启动。", "xpack.transform.transformList.createTransformButton": "创建转换", "xpack.transform.transformList.deleteActionDisabledToolTipContent": "停止数据帧作业，以便将其删除。", "xpack.transform.transformList.deleteBulkActionDisabledToolTipContent": "一个或多个选定数据帧转换必须停止，才能删除。", - "xpack.transform.transformList.deleteModalBody": "是否确定要删除此转换？", "xpack.transform.transformList.deleteModalCancelButton": "取消", "xpack.transform.transformList.deleteModalDeleteButton": "删除", "xpack.transform.transformList.deleteModalTitle": "删除 {transformId}", @@ -18194,14 +18191,12 @@ "xpack.transform.transformList.editTransformGenericErrorMessage": "调用用于更新转换的 API 终端时发生错误。", "xpack.transform.transformList.editTransformSuccessMessage": "转换 {transformId} 已更新。", "xpack.transform.transformList.errorWithCheckingIfUserCanDeleteIndexNotificationErrorMessage": "检查用户是否可以删除目标索引时发生错误", - "xpack.transform.transformList.forceDeleteModalBody": "确定要强制删除此索引？无论转换的当前状态如何，都将删除。", "xpack.transform.transformList.refreshButtonLabel": "刷新", "xpack.transform.transformList.rowCollapse": "隐藏 {transformId} 的详情", "xpack.transform.transformList.rowExpand": "显示 {transformId} 的详情", "xpack.transform.transformList.showDetailsColumn.screenReaderDescription": "此列包含可单击控件，用于显示每个转换的更多详情", "xpack.transform.transformList.startedTransformBulkToolTip": "一个或多个选定数据帧转换已启动。", "xpack.transform.transformList.startedTransformToolTip": "{transformId} 已启动。", - "xpack.transform.transformList.startModalBody": "转换将增加集群的搜索和索引负荷。如果负荷超载，请停止转换。是否确定要启动{count, plural, one {这} other {这}} {count} 个 {count, plural, one {转换} other {转换}}？", "xpack.transform.transformList.startModalCancelButton": "取消", "xpack.transform.transformList.startModalStartButton": "启动", "xpack.transform.transformList.startModalTitle": "启动 {transformId}", From b1cb8b8c7f5af8d3b2113e899244a0535163d4a8 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Thu, 27 Aug 2020 21:19:36 -0700 Subject: [PATCH 03/17] [APM] Transaction duration anomaly alerting integration (#75719) * Closes #72636. Adds alerting integration for APM transaction duration anomalies. * Code review feedback * Display alert summary with the selected anomaly severity label instead of the anomaly score. * - refactored ALL_OPTION and NOT_DEFINED_OPTION to be shared from common/environment_filter_values - utilize getEnvironmentLabel in the alerting trigger components and added support for the 'All' label * refactor get_all_environments to minimize exports and be more consistent and clean * - Reorg the alerts menu for different alert types (threshold/anomaly) - default environment alert settings to the selected filter * - Filters default transaction type to only those supported in the APM anomaly detection jobs - Removes Service name and transaction type from the set of expressions in the alerting setup * - remove bell icon from alerts menu * Adds target service back into the anomaly alert setup as a ready-only expression Co-authored-by: Elastic Machine --- x-pack/plugins/apm/common/alert_types.ts | 19 +++ .../apm/common/environment_filter_values.ts | 32 +++-- .../AlertIntegrations/index.tsx | 79 +++++----- .../components/app/ServiceDetails/index.tsx | 24 ++-- .../shared/EnvironmentFilter/index.tsx | 21 +-- .../shared/ErrorRateAlertTrigger/index.tsx | 14 +- .../PopoverExpression/index.tsx | 4 +- .../TransactionDurationAlertTrigger/index.tsx | 14 +- .../SelectAnomalySeverity.tsx | 108 ++++++++++++++ .../index.tsx | 131 +++++++++++++++++ .../apm/public/hooks/useEnvironments.tsx | 14 +- x-pack/plugins/apm/public/plugin.ts | 15 ++ .../server/lib/alerts/register_apm_alerts.ts | 8 ++ .../alerts/register_error_rate_alert_type.ts | 2 +- ...egister_transaction_duration_alert_type.ts | 2 +- ...transaction_duration_anomaly_alert_type.ts | 136 ++++++++++++++++++ .../get_anomaly_detection_jobs.ts | 2 +- .../get_ml_jobs_with_apm_group.ts | 8 +- .../lib/anomaly_detection/has_legacy_jobs.ts | 2 +- .../lib/environments/get_all_environments.ts | 2 +- .../get_environment_ui_filter_es.test.ts | 2 +- .../get_environment_ui_filter_es.ts | 2 +- .../apm/server/lib/helpers/setup_request.ts | 27 ++-- .../lib/service_map/get_service_anomalies.ts | 11 +- .../charts/get_anomaly_data/index.ts | 5 +- .../server/lib/ui_filters/get_environments.ts | 2 +- x-pack/plugins/apm/server/plugin.ts | 1 + .../providers/anomaly_detectors.ts | 8 +- .../translations/translations/ja-JP.json | 2 +- .../translations/translations/zh-CN.json | 2 +- 30 files changed, 577 insertions(+), 122 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/shared/TransactionDurationAnomalyAlertTrigger/SelectAnomalySeverity.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/TransactionDurationAnomalyAlertTrigger/index.tsx create mode 100644 x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts diff --git a/x-pack/plugins/apm/common/alert_types.ts b/x-pack/plugins/apm/common/alert_types.ts index ad826a446d823..a1161354e04f4 100644 --- a/x-pack/plugins/apm/common/alert_types.ts +++ b/x-pack/plugins/apm/common/alert_types.ts @@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n'; export enum AlertType { ErrorRate = 'apm.error_rate', TransactionDuration = 'apm.transaction_duration', + TransactionDurationAnomaly = 'apm.transaction_duration_anomaly', } export const ALERT_TYPES_CONFIG = { @@ -45,6 +46,24 @@ export const ALERT_TYPES_CONFIG = { defaultActionGroupId: 'threshold_met', producer: 'apm', }, + [AlertType.TransactionDurationAnomaly]: { + name: i18n.translate('xpack.apm.transactionDurationAnomalyAlert.name', { + defaultMessage: 'Transaction duration anomaly', + }), + actionGroups: [ + { + id: 'threshold_met', + name: i18n.translate( + 'xpack.apm.transactionDurationAlert.thresholdMet', + { + defaultMessage: 'Threshold met', + } + ), + }, + ], + defaultActionGroupId: 'threshold_met', + producer: 'apm', + }, }; export const TRANSACTION_ALERT_AGGREGATION_TYPES = { diff --git a/x-pack/plugins/apm/common/environment_filter_values.ts b/x-pack/plugins/apm/common/environment_filter_values.ts index 38b6f480ca3d3..e231f37a170ed 100644 --- a/x-pack/plugins/apm/common/environment_filter_values.ts +++ b/x-pack/plugins/apm/common/environment_filter_values.ts @@ -6,14 +6,30 @@ import { i18n } from '@kbn/i18n'; -export const ENVIRONMENT_ALL = 'ENVIRONMENT_ALL'; -export const ENVIRONMENT_NOT_DEFINED = 'ENVIRONMENT_NOT_DEFINED'; +const ENVIRONMENT_ALL_VALUE = 'ENVIRONMENT_ALL'; +const ENVIRONMENT_NOT_DEFINED_VALUE = 'ENVIRONMENT_NOT_DEFINED'; + +const environmentLabels: Record = { + [ENVIRONMENT_ALL_VALUE]: i18n.translate( + 'xpack.apm.filter.environment.allLabel', + { defaultMessage: 'All' } + ), + [ENVIRONMENT_NOT_DEFINED_VALUE]: i18n.translate( + 'xpack.apm.filter.environment.notDefinedLabel', + { defaultMessage: 'Not defined' } + ), +}; + +export const ENVIRONMENT_ALL = { + value: ENVIRONMENT_ALL_VALUE, + text: environmentLabels[ENVIRONMENT_ALL_VALUE], +}; + +export const ENVIRONMENT_NOT_DEFINED = { + value: ENVIRONMENT_NOT_DEFINED_VALUE, + text: environmentLabels[ENVIRONMENT_NOT_DEFINED_VALUE], +}; export function getEnvironmentLabel(environment: string) { - if (environment === ENVIRONMENT_NOT_DEFINED) { - return i18n.translate('xpack.apm.filter.environment.notDefinedLabel', { - defaultMessage: 'Not defined', - }); - } - return environment; + return environmentLabels[environment] || environment; } diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/AlertIntegrations/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/AlertIntegrations/index.tsx index 80d5f739bea5a..27c4a37e09c00 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/AlertIntegrations/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/AlertIntegrations/index.tsx @@ -18,27 +18,37 @@ import { useApmPluginContext } from '../../../../hooks/useApmPluginContext'; const alertLabel = i18n.translate( 'xpack.apm.serviceDetails.alertsMenu.alerts', - { - defaultMessage: 'Alerts', - } + { defaultMessage: 'Alerts' } +); +const transactionDurationLabel = i18n.translate( + 'xpack.apm.serviceDetails.alertsMenu.transactionDuration', + { defaultMessage: 'Transaction duration' } +); +const errorRateLabel = i18n.translate( + 'xpack.apm.serviceDetails.alertsMenu.errorRate', + { defaultMessage: 'Error rate' } ); - const createThresholdAlertLabel = i18n.translate( 'xpack.apm.serviceDetails.alertsMenu.createThresholdAlert', - { - defaultMessage: 'Create threshold alert', - } + { defaultMessage: 'Create threshold alert' } +); +const createAnomalyAlertAlertLabel = i18n.translate( + 'xpack.apm.serviceDetails.alertsMenu.createAnomalyAlert', + { defaultMessage: 'Create anomaly alert' } ); -const CREATE_THRESHOLD_ALERT_PANEL_ID = 'create_threshold'; +const CREATE_TRANSACTION_DURATION_ALERT_PANEL_ID = + 'create_transaction_duration'; +const CREATE_ERROR_RATE_ALERT_PANEL_ID = 'create_error_rate'; interface Props { canReadAlerts: boolean; canSaveAlerts: boolean; + canReadAnomalies: boolean; } export function AlertIntegrations(props: Props) { - const { canSaveAlerts, canReadAlerts } = props; + const { canSaveAlerts, canReadAlerts, canReadAnomalies } = props; const plugin = useApmPluginContext(); @@ -52,9 +62,7 @@ export function AlertIntegrations(props: Props) { iconSide="right" onClick={() => setPopoverOpen(true)} > - {i18n.translate('xpack.apm.serviceDetails.alertsMenu.alerts', { - defaultMessage: 'Alerts', - })} + {alertLabel} ); @@ -66,10 +74,10 @@ export function AlertIntegrations(props: Props) { ...(canSaveAlerts ? [ { - name: createThresholdAlertLabel, - panel: CREATE_THRESHOLD_ALERT_PANEL_ID, - icon: 'bell', + name: transactionDurationLabel, + panel: CREATE_TRANSACTION_DURATION_ALERT_PANEL_ID, }, + { name: errorRateLabel, panel: CREATE_ERROR_RATE_ALERT_PANEL_ID }, ] : []), ...(canReadAlerts @@ -77,9 +85,7 @@ export function AlertIntegrations(props: Props) { { name: i18n.translate( 'xpack.apm.serviceDetails.alertsMenu.viewActiveAlerts', - { - defaultMessage: 'View active alerts', - } + { defaultMessage: 'View active alerts' } ), href: plugin.core.http.basePath.prepend( '/app/management/insightsAndAlerting/triggersActions/alerts' @@ -91,29 +97,38 @@ export function AlertIntegrations(props: Props) { ], }, { - id: CREATE_THRESHOLD_ALERT_PANEL_ID, - title: createThresholdAlertLabel, + id: CREATE_TRANSACTION_DURATION_ALERT_PANEL_ID, + title: transactionDurationLabel, items: [ { - name: i18n.translate( - 'xpack.apm.serviceDetails.alertsMenu.transactionDuration', - { - defaultMessage: 'Transaction duration', - } - ), + name: createThresholdAlertLabel, onClick: () => { setAlertType(AlertType.TransactionDuration); + setPopoverOpen(false); }, }, + ...(canReadAnomalies + ? [ + { + name: createAnomalyAlertAlertLabel, + onClick: () => { + setAlertType(AlertType.TransactionDurationAnomaly); + setPopoverOpen(false); + }, + }, + ] + : []), + ], + }, + { + id: CREATE_ERROR_RATE_ALERT_PANEL_ID, + title: errorRateLabel, + items: [ { - name: i18n.translate( - 'xpack.apm.serviceDetails.alertsMenu.errorRate', - { - defaultMessage: 'Error rate', - } - ), + name: createThresholdAlertLabel, onClick: () => { setAlertType(AlertType.ErrorRate); + setPopoverOpen(false); }, }, ], diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/index.tsx index 4488a962d0ba8..b5a4ca4799afd 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/index.tsx @@ -26,19 +26,18 @@ export function ServiceDetails({ tab }: Props) { const plugin = useApmPluginContext(); const { urlParams } = useUrlParams(); const { serviceName } = urlParams; - - const canReadAlerts = !!plugin.core.application.capabilities.apm[ - 'alerting:show' - ]; - const canSaveAlerts = !!plugin.core.application.capabilities.apm[ - 'alerting:save' - ]; + const capabilities = plugin.core.application.capabilities; + const canReadAlerts = !!capabilities.apm['alerting:show']; + const canSaveAlerts = !!capabilities.apm['alerting:save']; const isAlertingPluginEnabled = 'alerts' in plugin.plugins; - const isAlertingAvailable = isAlertingPluginEnabled && (canReadAlerts || canSaveAlerts); - - const { core } = useApmPluginContext(); + const isMlPluginEnabled = 'ml' in plugin.plugins; + const canReadAnomalies = !!( + isMlPluginEnabled && + capabilities.ml.canAccessML && + capabilities.ml.canGetJobs + ); const ADD_DATA_LABEL = i18n.translate('xpack.apm.addDataButtonLabel', { defaultMessage: 'Add data', @@ -58,12 +57,15 @@ export function ServiceDetails({ tab }: Props) { )} , environment?: string ) { const nextEnvironmentQueryParam = - environment !== ENVIRONMENT_ALL ? environment : undefined; + environment !== ENVIRONMENT_ALL.value ? environment : undefined; history.push({ ...location, search: fromQuery({ @@ -32,13 +32,6 @@ function updateEnvironmentUrl( }); } -const NOT_DEFINED_OPTION = { - value: ENVIRONMENT_NOT_DEFINED, - text: i18n.translate('xpack.apm.filter.environment.notDefinedLabel', { - defaultMessage: 'Not defined', - }), -}; - const SEPARATOR_OPTION = { text: `- ${i18n.translate( 'xpack.apm.filter.environment.selectEnvironmentLabel', @@ -49,16 +42,16 @@ const SEPARATOR_OPTION = { function getOptions(environments: string[]) { const environmentOptions = environments - .filter((env) => env !== ENVIRONMENT_NOT_DEFINED) + .filter((env) => env !== ENVIRONMENT_NOT_DEFINED.value) .map((environment) => ({ value: environment, text: environment, })); return [ - ALL_OPTION, - ...(environments.includes(ENVIRONMENT_NOT_DEFINED) - ? [NOT_DEFINED_OPTION] + ENVIRONMENT_ALL, + ...(environments.includes(ENVIRONMENT_NOT_DEFINED.value) + ? [ENVIRONMENT_NOT_DEFINED] : []), ...(environmentOptions.length > 0 ? [SEPARATOR_OPTION] : []), ...environmentOptions, @@ -83,7 +76,7 @@ export function EnvironmentFilter() { defaultMessage: 'environment', })} options={getOptions(environments)} - value={environment || ENVIRONMENT_ALL} + value={environment || ENVIRONMENT_ALL.value} onChange={(event) => { updateEnvironmentUrl(location, event.target.value); }} diff --git a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.tsx b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.tsx index b457fb1092bc6..7344839795955 100644 --- a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.tsx @@ -12,8 +12,12 @@ import { ForLastExpression } from '../../../../../triggers_actions_ui/public'; import { ALERT_TYPES_CONFIG } from '../../../../common/alert_types'; import { ServiceAlertTrigger } from '../ServiceAlertTrigger'; import { PopoverExpression } from '../ServiceAlertTrigger/PopoverExpression'; -import { useEnvironments, ALL_OPTION } from '../../../hooks/useEnvironments'; +import { useEnvironments } from '../../../hooks/useEnvironments'; import { useUrlParams } from '../../../hooks/useUrlParams'; +import { + ENVIRONMENT_ALL, + getEnvironmentLabel, +} from '../../../../common/environment_filter_values'; export interface ErrorRateAlertTriggerParams { windowSize: number; @@ -39,7 +43,7 @@ export function ErrorRateAlertTrigger(props: Props) { threshold: 25, windowSize: 1, windowUnit: 'm', - environment: ALL_OPTION.value, + environment: urlParams.environment || ENVIRONMENT_ALL.value, }; const params = { @@ -51,11 +55,7 @@ export function ErrorRateAlertTrigger(props: Props) { const fields = [ + {label} + + ); +} + +const getOption = (value: SeverityScore) => { + return { + value: value.toString(10), + inputDisplay: , + dropdownDisplay: ( + <> + + + +

+ +

+ + + ), + }; +}; + +interface Props { + onChange: (value: SeverityScore) => void; + value: SeverityScore; +} + +export function SelectAnomalySeverity({ onChange, value }: Props) { + const options = ANOMALY_SCORES.map((anomalyScore) => getOption(anomalyScore)); + + return ( + { + const selectedAnomalyScore = parseInt( + selectedValue, + 10 + ) as SeverityScore; + onChange(selectedAnomalyScore); + }} + /> + ); +} diff --git a/x-pack/plugins/apm/public/components/shared/TransactionDurationAnomalyAlertTrigger/index.tsx b/x-pack/plugins/apm/public/components/shared/TransactionDurationAnomalyAlertTrigger/index.tsx new file mode 100644 index 0000000000000..911c51013a844 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/TransactionDurationAnomalyAlertTrigger/index.tsx @@ -0,0 +1,131 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { EuiExpression, EuiSelect } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import { ALERT_TYPES_CONFIG } from '../../../../common/alert_types'; +import { useEnvironments } from '../../../hooks/useEnvironments'; +import { useServiceTransactionTypes } from '../../../hooks/useServiceTransactionTypes'; +import { useUrlParams } from '../../../hooks/useUrlParams'; +import { ServiceAlertTrigger } from '../ServiceAlertTrigger'; +import { PopoverExpression } from '../ServiceAlertTrigger/PopoverExpression'; +import { + AnomalySeverity, + SelectAnomalySeverity, +} from './SelectAnomalySeverity'; +import { + ENVIRONMENT_ALL, + getEnvironmentLabel, +} from '../../../../common/environment_filter_values'; +import { + TRANSACTION_PAGE_LOAD, + TRANSACTION_REQUEST, +} from '../../../../common/transaction_types'; + +interface Params { + windowSize: number; + windowUnit: string; + serviceName: string; + transactionType: string; + environment: string; + anomalyScore: 0 | 25 | 50 | 75; +} + +interface Props { + alertParams: Params; + setAlertParams: (key: string, value: any) => void; + setAlertProperty: (key: string, value: any) => void; +} + +export function TransactionDurationAnomalyAlertTrigger(props: Props) { + const { setAlertParams, alertParams, setAlertProperty } = props; + const { urlParams } = useUrlParams(); + const transactionTypes = useServiceTransactionTypes(urlParams); + const { serviceName, start, end } = urlParams; + const { environmentOptions } = useEnvironments({ serviceName, start, end }); + const supportedTransactionTypes = transactionTypes.filter((transactionType) => + [TRANSACTION_PAGE_LOAD, TRANSACTION_REQUEST].includes(transactionType) + ); + + if (!supportedTransactionTypes.length || !serviceName) { + return null; + } + + const defaults: Params = { + windowSize: 15, + windowUnit: 'm', + transactionType: supportedTransactionTypes[0], // 'page-load' for RUM, 'request' otherwise + serviceName, + environment: urlParams.environment || ENVIRONMENT_ALL.value, + anomalyScore: 75, + }; + + const params = { + ...defaults, + ...alertParams, + }; + + const fields = [ + , + + setAlertParams('environment', e.target.value)} + compressed + /> + , + } + title={i18n.translate( + 'xpack.apm.transactionDurationAnomalyAlertTrigger.anomalySeverity', + { + defaultMessage: 'Has anomaly with severity', + } + )} + > + { + setAlertParams('anomalyScore', value); + }} + /> + , + ]; + + return ( + + ); +} + +// Default export is required for React.lazy loading +// +// eslint-disable-next-line import/no-default-export +export default TransactionDurationAnomalyAlertTrigger; diff --git a/x-pack/plugins/apm/public/hooks/useEnvironments.tsx b/x-pack/plugins/apm/public/hooks/useEnvironments.tsx index b358608701fdf..9e01dde274ff7 100644 --- a/x-pack/plugins/apm/public/hooks/useEnvironments.tsx +++ b/x-pack/plugins/apm/public/hooks/useEnvironments.tsx @@ -5,30 +5,22 @@ */ import { useMemo } from 'react'; -import { i18n } from '@kbn/i18n'; import { useFetcher } from './useFetcher'; import { - ENVIRONMENT_NOT_DEFINED, ENVIRONMENT_ALL, + ENVIRONMENT_NOT_DEFINED, } from '../../common/environment_filter_values'; import { callApmApi } from '../services/rest/createCallApmApi'; -export const ALL_OPTION = { - value: ENVIRONMENT_ALL, - text: i18n.translate('xpack.apm.environment.allLabel', { - defaultMessage: 'All', - }), -}; - function getEnvironmentOptions(environments: string[]) { const environmentOptions = environments - .filter((env) => env !== ENVIRONMENT_NOT_DEFINED) + .filter((env) => env !== ENVIRONMENT_NOT_DEFINED.value) .map((environment) => ({ value: environment, text: environment, })); - return [ALL_OPTION, ...environmentOptions]; + return [ENVIRONMENT_ALL, ...environmentOptions]; } export function useEnvironments({ diff --git a/x-pack/plugins/apm/public/plugin.ts b/x-pack/plugins/apm/public/plugin.ts index 0d25ececd5156..5d01df2a17bef 100644 --- a/x-pack/plugins/apm/public/plugin.ts +++ b/x-pack/plugins/apm/public/plugin.ts @@ -168,5 +168,20 @@ export class ApmPlugin implements Plugin { }), requiresAppContext: true, }); + + plugins.triggers_actions_ui.alertTypeRegistry.register({ + id: AlertType.TransactionDurationAnomaly, + name: i18n.translate('xpack.apm.alertTypes.transactionDurationAnomaly', { + defaultMessage: 'Transaction duration anomaly', + }), + iconClass: 'bell', + alertParamsExpression: lazy(() => + import('./components/shared/TransactionDurationAnomalyAlertTrigger') + ), + validate: () => ({ + errors: [], + }), + requiresAppContext: true, + }); } } diff --git a/x-pack/plugins/apm/server/lib/alerts/register_apm_alerts.ts b/x-pack/plugins/apm/server/lib/alerts/register_apm_alerts.ts index 4b8e9cf937a2b..44ca80143bcd9 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_apm_alerts.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_apm_alerts.ts @@ -8,12 +8,15 @@ import { Observable } from 'rxjs'; import { AlertingPlugin } from '../../../../alerts/server'; import { ActionsPlugin } from '../../../../actions/server'; import { registerTransactionDurationAlertType } from './register_transaction_duration_alert_type'; +import { registerTransactionDurationAnomalyAlertType } from './register_transaction_duration_anomaly_alert_type'; import { registerErrorRateAlertType } from './register_error_rate_alert_type'; import { APMConfig } from '../..'; +import { MlPluginSetup } from '../../../../ml/server'; interface Params { alerts: AlertingPlugin['setup']; actions: ActionsPlugin['setup']; + ml?: MlPluginSetup; config$: Observable; } @@ -22,6 +25,11 @@ export function registerApmAlerts(params: Params) { alerts: params.alerts, config$: params.config$, }); + registerTransactionDurationAnomalyAlertType({ + alerts: params.alerts, + ml: params.ml, + config$: params.config$, + }); registerErrorRateAlertType({ alerts: params.alerts, config$: params.config$, diff --git a/x-pack/plugins/apm/server/lib/alerts/register_error_rate_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_error_rate_alert_type.ts index 53843b7f7412b..61e3dfee420a5 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_error_rate_alert_type.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_error_rate_alert_type.ts @@ -75,7 +75,7 @@ export function registerErrorRateAlertType({ }); const environmentTerm = - alertParams.environment === ENVIRONMENT_ALL + alertParams.environment === ENVIRONMENT_ALL.value ? [] : [{ term: { [SERVICE_ENVIRONMENT]: alertParams.environment } }]; diff --git a/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts index a922457b14cea..ead28c325692d 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts @@ -89,7 +89,7 @@ export function registerTransactionDurationAlertType({ }); const environmentTerm = - alertParams.environment === ENVIRONMENT_ALL + alertParams.environment === ENVIRONMENT_ALL.value ? [] : [{ term: { [SERVICE_ENVIRONMENT]: alertParams.environment } }]; diff --git a/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts new file mode 100644 index 0000000000000..3abc89c470b21 --- /dev/null +++ b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts @@ -0,0 +1,136 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { schema, TypeOf } from '@kbn/config-schema'; +import { Observable } from 'rxjs'; +import { i18n } from '@kbn/i18n'; +import { KibanaRequest } from '../../../../../../src/core/server'; +import { AlertType, ALERT_TYPES_CONFIG } from '../../../common/alert_types'; +import { AlertingPlugin } from '../../../../alerts/server'; +import { APMConfig } from '../..'; +import { MlPluginSetup } from '../../../../ml/server'; +import { getMLJobIds } from '../service_map/get_service_anomalies'; + +interface RegisterAlertParams { + alerts: AlertingPlugin['setup']; + ml?: MlPluginSetup; + config$: Observable; +} + +const paramsSchema = schema.object({ + serviceName: schema.string(), + transactionType: schema.string(), + windowSize: schema.number(), + windowUnit: schema.string(), + environment: schema.string(), + anomalyScore: schema.number(), +}); + +const alertTypeConfig = + ALERT_TYPES_CONFIG[AlertType.TransactionDurationAnomaly]; + +export function registerTransactionDurationAnomalyAlertType({ + alerts, + ml, + config$, +}: RegisterAlertParams) { + alerts.registerType({ + id: AlertType.TransactionDurationAnomaly, + name: alertTypeConfig.name, + actionGroups: alertTypeConfig.actionGroups, + defaultActionGroupId: alertTypeConfig.defaultActionGroupId, + validate: { + params: paramsSchema, + }, + actionVariables: { + context: [ + { + description: i18n.translate( + 'xpack.apm.registerTransactionDurationAnomalyAlertType.variables.serviceName', + { + defaultMessage: 'Service name', + } + ), + name: 'serviceName', + }, + { + description: i18n.translate( + 'xpack.apm.registerTransactionDurationAnomalyAlertType.variables.transactionType', + { + defaultMessage: 'Transaction type', + } + ), + name: 'transactionType', + }, + ], + }, + producer: 'apm', + executor: async ({ services, params, state }) => { + if (!ml) { + return; + } + const alertParams = params as TypeOf; + const mlClient = services.getLegacyScopedClusterClient(ml.mlClient); + const request = { params: 'DummyKibanaRequest' } as KibanaRequest; + const { mlAnomalySearch } = ml.mlSystemProvider(mlClient, request); + const anomalyDetectors = ml.anomalyDetectorsProvider(mlClient, request); + + const mlJobIds = await getMLJobIds( + anomalyDetectors, + alertParams.environment + ); + const anomalySearchParams = { + body: { + size: 0, + query: { + bool: { + filter: [ + { term: { result_type: 'record' } }, + { terms: { job_id: mlJobIds } }, + { + range: { + timestamp: { + gte: `now-${alertParams.windowSize}${alertParams.windowUnit}`, + format: 'epoch_millis', + }, + }, + }, + { + term: { + partition_field_value: alertParams.serviceName, + }, + }, + { + range: { + record_score: { + gte: alertParams.anomalyScore, + }, + }, + }, + ], + }, + }, + }, + }; + + const response = ((await mlAnomalySearch( + anomalySearchParams + )) as unknown) as { hits: { total: { value: number } } }; + const hitCount = response.hits.total.value; + + if (hitCount > 0) { + const alertInstance = services.alertInstanceFactory( + AlertType.TransactionDurationAnomaly + ); + alertInstance.scheduleActions(alertTypeConfig.defaultActionGroupId, { + serviceName: alertParams.serviceName, + }); + } + + return {}; + }, + }); +} diff --git a/x-pack/plugins/apm/server/lib/anomaly_detection/get_anomaly_detection_jobs.ts b/x-pack/plugins/apm/server/lib/anomaly_detection/get_anomaly_detection_jobs.ts index 05f41cdfdffd4..ead0c79a02836 100644 --- a/x-pack/plugins/apm/server/lib/anomaly_detection/get_anomaly_detection_jobs.ts +++ b/x-pack/plugins/apm/server/lib/anomaly_detection/get_anomaly_detection_jobs.ts @@ -22,7 +22,7 @@ export async function getAnomalyDetectionJobs(setup: Setup, logger: Logger) { throw Boom.forbidden(ML_ERRORS.ML_NOT_AVAILABLE_IN_SPACE); } - const response = await getMlJobsWithAPMGroup(ml); + const response = await getMlJobsWithAPMGroup(ml.anomalyDetectors); return response.jobs .filter((job) => (job.custom_settings?.job_tags?.apm_ml_version ?? 0) >= 2) .map((job) => { diff --git a/x-pack/plugins/apm/server/lib/anomaly_detection/get_ml_jobs_with_apm_group.ts b/x-pack/plugins/apm/server/lib/anomaly_detection/get_ml_jobs_with_apm_group.ts index 5c0a3d17648aa..1c39892c3fd96 100644 --- a/x-pack/plugins/apm/server/lib/anomaly_detection/get_ml_jobs_with_apm_group.ts +++ b/x-pack/plugins/apm/server/lib/anomaly_detection/get_ml_jobs_with_apm_group.ts @@ -4,14 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Setup } from '../helpers/setup_request'; +import { MlPluginSetup } from '../../../../ml/server'; import { APM_ML_JOB_GROUP } from './constants'; // returns ml jobs containing "apm" group // workaround: the ML api returns 404 when no jobs are found. This is handled so instead of throwing an empty response is returned -export async function getMlJobsWithAPMGroup(ml: NonNullable) { +export async function getMlJobsWithAPMGroup( + anomalyDetectors: ReturnType +) { try { - return await ml.anomalyDetectors.jobs(APM_ML_JOB_GROUP); + return await anomalyDetectors.jobs(APM_ML_JOB_GROUP); } catch (e) { if (e.statusCode === 404) { return { count: 0, jobs: [] }; diff --git a/x-pack/plugins/apm/server/lib/anomaly_detection/has_legacy_jobs.ts b/x-pack/plugins/apm/server/lib/anomaly_detection/has_legacy_jobs.ts index ed66236726b9f..c1f346aa30e1f 100644 --- a/x-pack/plugins/apm/server/lib/anomaly_detection/has_legacy_jobs.ts +++ b/x-pack/plugins/apm/server/lib/anomaly_detection/has_legacy_jobs.ts @@ -23,7 +23,7 @@ export async function hasLegacyJobs(setup: Setup) { throw Boom.forbidden(ML_ERRORS.ML_NOT_AVAILABLE_IN_SPACE); } - const response = await getMlJobsWithAPMGroup(ml); + const response = await getMlJobsWithAPMGroup(ml.anomalyDetectors); return response.jobs.some( (job) => job.job_id.endsWith('high_mean_response_time') && diff --git a/x-pack/plugins/apm/server/lib/environments/get_all_environments.ts b/x-pack/plugins/apm/server/lib/environments/get_all_environments.ts index 423b87cb78c3c..29aaa98169fa5 100644 --- a/x-pack/plugins/apm/server/lib/environments/get_all_environments.ts +++ b/x-pack/plugins/apm/server/lib/environments/get_all_environments.ts @@ -48,7 +48,7 @@ export async function getAllEnvironments({ terms: { field: SERVICE_ENVIRONMENT, size: 100, - missing: includeMissing ? ENVIRONMENT_NOT_DEFINED : undefined, + missing: includeMissing ? ENVIRONMENT_NOT_DEFINED.value : undefined, }, }, }, diff --git a/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/__test__/get_environment_ui_filter_es.test.ts b/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/__test__/get_environment_ui_filter_es.test.ts index 800f809727eb6..a319bba1eabe1 100644 --- a/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/__test__/get_environment_ui_filter_es.test.ts +++ b/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/__test__/get_environment_ui_filter_es.test.ts @@ -21,7 +21,7 @@ describe('getEnvironmentUiFilterES', () => { }); it('should create a filter for missing service environments', () => { - const uiFilterES = getEnvironmentUiFilterES(ENVIRONMENT_NOT_DEFINED); + const uiFilterES = getEnvironmentUiFilterES(ENVIRONMENT_NOT_DEFINED.value); expect(uiFilterES).toHaveLength(1); expect(uiFilterES[0]).toHaveProperty( ['bool', 'must_not', 'exists', 'field'], diff --git a/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/get_environment_ui_filter_es.ts b/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/get_environment_ui_filter_es.ts index 87bc8dc968373..6ff98a9be75f9 100644 --- a/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/get_environment_ui_filter_es.ts +++ b/x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/get_environment_ui_filter_es.ts @@ -12,7 +12,7 @@ export function getEnvironmentUiFilterES(environment?: string): ESFilter[] { if (!environment) { return []; } - if (environment === ENVIRONMENT_NOT_DEFINED) { + if (environment === ENVIRONMENT_NOT_DEFINED.value) { return [{ bool: { must_not: { exists: { field: SERVICE_ENVIRONMENT } } } }]; } return [{ term: { [SERVICE_ENVIRONMENT]: environment } }]; diff --git a/x-pack/plugins/apm/server/lib/helpers/setup_request.ts b/x-pack/plugins/apm/server/lib/helpers/setup_request.ts index ddad2eb2d22dc..a242a0adb6d4c 100644 --- a/x-pack/plugins/apm/server/lib/helpers/setup_request.ts +++ b/x-pack/plugins/apm/server/lib/helpers/setup_request.ts @@ -98,7 +98,11 @@ export async function setupRequest( context, request, }), - ml: getMlSetup(context, request), + ml: getMlSetup( + context.plugins.ml, + context.core.savedObjects.client, + request + ), config, }; @@ -110,20 +114,21 @@ export async function setupRequest( } as InferSetup; } -function getMlSetup(context: APMRequestHandlerContext, request: KibanaRequest) { - if (!context.plugins.ml) { +function getMlSetup( + ml: APMRequestHandlerContext['plugins']['ml'], + savedObjectsClient: APMRequestHandlerContext['core']['savedObjects']['client'], + request: KibanaRequest +) { + if (!ml) { return; } - const ml = context.plugins.ml; const mlClient = ml.mlClient.asScoped(request); + const mlSystem = ml.mlSystemProvider(mlClient, request); return { - mlSystem: ml.mlSystemProvider(mlClient, request), - anomalyDetectors: ml.anomalyDetectorsProvider(mlClient, request), - modules: ml.modulesProvider( - mlClient, - request, - context.core.savedObjects.client - ), mlClient, + mlSystem, + modules: ml.modulesProvider(mlClient, request, savedObjectsClient), + anomalyDetectors: ml.anomalyDetectorsProvider(mlClient, request), + mlAnomalySearch: mlSystem.mlAnomalySearch, }; } diff --git a/x-pack/plugins/apm/server/lib/service_map/get_service_anomalies.ts b/x-pack/plugins/apm/server/lib/service_map/get_service_anomalies.ts index 03716382af859..ec274d20b6005 100644 --- a/x-pack/plugins/apm/server/lib/service_map/get_service_anomalies.ts +++ b/x-pack/plugins/apm/server/lib/service_map/get_service_anomalies.ts @@ -16,6 +16,8 @@ import { ML_ERRORS, } from '../../../common/anomaly_detection'; import { getMlJobsWithAPMGroup } from '../anomaly_detection/get_ml_jobs_with_apm_group'; +import { ENVIRONMENT_ALL } from '../../../common/environment_filter_values'; +import { MlPluginSetup } from '../../../../ml/server'; export const DEFAULT_ANOMALIES = { mlJobIds: [], serviceAnomalies: {} }; @@ -43,7 +45,7 @@ export async function getServiceAnomalies({ throw Boom.forbidden(ML_ERRORS.ML_NOT_AVAILABLE_IN_SPACE); } - const mlJobIds = await getMLJobIds(ml, environment); + const mlJobIds = await getMLJobIds(ml.anomalyDetectors, environment); const params = { body: { size: 0, @@ -136,16 +138,17 @@ function transformResponseToServiceAnomalies( } export async function getMLJobIds( - ml: Required['ml'], + anomalyDetectors: ReturnType, environment?: string ) { - const response = await getMlJobsWithAPMGroup(ml); + const response = await getMlJobsWithAPMGroup(anomalyDetectors); + // to filter out legacy jobs we are filtering by the existence of `apm_ml_version` in `custom_settings` // and checking that it is compatable. const mlJobs = response.jobs.filter( (job) => (job.custom_settings?.job_tags?.apm_ml_version ?? 0) >= 2 ); - if (environment) { + if (environment && environment !== ENVIRONMENT_ALL.value) { const matchingMLJob = mlJobs.find( (job) => job.custom_settings?.job_tags?.environment === environment ); diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.ts b/x-pack/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.ts index 072099bc9553c..596c3137ec19f 100644 --- a/x-pack/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.ts +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.ts @@ -66,7 +66,10 @@ export async function getAnomalySeries({ let mlJobIds: string[] = []; try { - mlJobIds = await getMLJobIds(setup.ml, uiFilters.environment); + mlJobIds = await getMLJobIds( + setup.ml.anomalyDetectors, + uiFilters.environment + ); } catch (error) { logger.error(error); return; diff --git a/x-pack/plugins/apm/server/lib/ui_filters/get_environments.ts b/x-pack/plugins/apm/server/lib/ui_filters/get_environments.ts index 98f00bf8e6555..7d3af4caa2ca3 100644 --- a/x-pack/plugins/apm/server/lib/ui_filters/get_environments.ts +++ b/x-pack/plugins/apm/server/lib/ui_filters/get_environments.ts @@ -47,7 +47,7 @@ export async function getEnvironments( environments: { terms: { field: SERVICE_ENVIRONMENT, - missing: ENVIRONMENT_NOT_DEFINED, + missing: ENVIRONMENT_NOT_DEFINED.value, }, }, }, diff --git a/x-pack/plugins/apm/server/plugin.ts b/x-pack/plugins/apm/server/plugin.ts index deafda67b806d..71202c62e6f6c 100644 --- a/x-pack/plugins/apm/server/plugin.ts +++ b/x-pack/plugins/apm/server/plugin.ts @@ -83,6 +83,7 @@ export class APMPlugin implements Plugin { registerApmAlerts({ alerts: plugins.alerts, actions: plugins.actions, + ml: plugins.ml, config$: mergedConfig$, }); } diff --git a/x-pack/plugins/ml/server/shared_services/providers/anomaly_detectors.ts b/x-pack/plugins/ml/server/shared_services/providers/anomaly_detectors.ts index 1140af0b76404..603b4fba17adb 100644 --- a/x-pack/plugins/ml/server/shared_services/providers/anomaly_detectors.ts +++ b/x-pack/plugins/ml/server/shared_services/providers/anomaly_detectors.ts @@ -23,7 +23,13 @@ export function getAnomalyDetectorsProvider({ }: SharedServicesChecks): AnomalyDetectorsProvider { return { anomalyDetectorsProvider(mlClusterClient: ILegacyScopedClusterClient, request: KibanaRequest) { - const hasMlCapabilities = getHasMlCapabilities(request); + // APM is using this service in anomaly alert, kibana alerting doesn't provide request object + // So we are adding a dummy request for now + // TODO: Remove this once kibana alerting provides request object + const hasMlCapabilities = + request.params !== 'DummyKibanaRequest' + ? getHasMlCapabilities(request) + : (_caps: string[]) => Promise.resolve(); return { async jobs(jobId?: string) { isFullLicense(); diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index dbbe6072b1fe5..dc07044ce8ed7 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -4742,7 +4742,6 @@ "xpack.apm.customLink.empty": "カスタムリンクが見つかりません。独自のカスタムリンク、たとえば特定のダッシュボードまたは外部リンクへのリンクをセットアップします。", "xpack.apm.emptyMessage.noDataFoundDescription": "別の時間範囲を試すか検索フィルターをリセットしてください。", "xpack.apm.emptyMessage.noDataFoundLabel": "データが見つかりません。", - "xpack.apm.environment.allLabel": "すべて", "xpack.apm.error.prompt.body": "詳細はブラウザの開発者コンソールをご確認ください。", "xpack.apm.error.prompt.title": "申し訳ございませんが、エラーが発生しました :(", "xpack.apm.errorGroupDetails.avgLabel": "平均", @@ -4780,6 +4779,7 @@ "xpack.apm.fetcher.error.title": "リソースの取得中にエラーが発生しました", "xpack.apm.fetcher.error.url": "URL", "xpack.apm.filter.environment.label": "環境", + "xpack.apm.filter.environment.allLabel": "すべて", "xpack.apm.filter.environment.notDefinedLabel": "未定義", "xpack.apm.filter.environment.selectEnvironmentLabel": "環境を選択", "xpack.apm.formatters.hoursTimeUnitLabel": "h", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index ed849c1050497..144bc1cac1852 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -4743,7 +4743,6 @@ "xpack.apm.customLink.empty": "未找到定制链接。设置自己的定制链接，如特定仪表板的链接或外部链接。", "xpack.apm.emptyMessage.noDataFoundDescription": "尝试其他时间范围或重置搜索筛选。", "xpack.apm.emptyMessage.noDataFoundLabel": "未找到任何数据", - "xpack.apm.environment.allLabel": "全部", "xpack.apm.error.prompt.body": "有关详情，请查看您的浏览器开发者控制台。", "xpack.apm.error.prompt.title": "抱歉，发生错误 :(", "xpack.apm.errorGroupDetails.avgLabel": "平均", @@ -4781,6 +4780,7 @@ "xpack.apm.fetcher.error.title": "提取资源时出错", "xpack.apm.fetcher.error.url": "URL", "xpack.apm.filter.environment.label": "环境", + "xpack.apm.filter.environment.allLabel": "全部", "xpack.apm.filter.environment.notDefinedLabel": "未定义", "xpack.apm.filter.environment.selectEnvironmentLabel": "选择环境", "xpack.apm.formatters.hoursTimeUnitLabel": "h", From f078672c545a36192b4669fa8b9f84eac2f558fb Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 27 Aug 2020 23:01:53 -0700 Subject: [PATCH 04/17] skip flaky suite (#76223) --- .../security_api_integration/tests/session_lifespan/cleanup.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts b/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts index d9cb671282124..dbdaf494fdf27 100644 --- a/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts +++ b/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts @@ -30,7 +30,8 @@ export default function ({ getService }: FtrProviderContext) { return (await es.search({ index: '.kibana_security_session*' })).hits.total.value; } - describe('Session Lifespan cleanup', () => { + // FLAKY: https://github.com/elastic/kibana/issues/76223 + describe.skip('Session Lifespan cleanup', () => { beforeEach(async () => { await es.cluster.health({ index: '.kibana_security_session*', waitForStatus: 'green' }); await es.deleteByQuery({ From 86870d22b84e67b996122057f139d63a1ac87e39 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 27 Aug 2020 23:02:57 -0700 Subject: [PATCH 05/17] fix eslint issue --- x-pack/plugins/apm/public/plugin.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/plugin.ts b/x-pack/plugins/apm/public/plugin.ts index 5d01df2a17bef..b950b493c0f19 100644 --- a/x-pack/plugins/apm/public/plugin.ts +++ b/x-pack/plugins/apm/public/plugin.ts @@ -175,8 +175,9 @@ export class ApmPlugin implements Plugin { defaultMessage: 'Transaction duration anomaly', }), iconClass: 'bell', - alertParamsExpression: lazy(() => - import('./components/shared/TransactionDurationAnomalyAlertTrigger') + alertParamsExpression: lazy( + () => + import('./components/shared/TransactionDurationAnomalyAlertTrigger') ), validate: () => ({ errors: [], From 712c4bdde79e1b49bb6aa0ad0979f425e57cae64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Fri, 28 Aug 2020 11:43:49 +0100 Subject: [PATCH 06/17] [APM] Prevent imports of public in server code (#75979) * renaming test file * removing eslint rule * restructuring observability types * refactoring fetchOptions type --- .eslintrc.js | 5 +- x-pack/plugins/apm/common/fetch_options.ts | 14 +++ x-pack/plugins/apm/public/hooks/useCallApi.ts | 3 +- .../apm/public/services/rest/callApi.ts | 10 +- .../public/services/rest/createCallApmApi.ts | 3 +- .../environments/get_all_environments.test.ts | 2 +- .../lib/errors/distribution/queries.test.ts | 2 +- .../apm/server/lib/errors/queries.test.ts | 2 +- .../apm/server/lib/metrics/queries.test.ts | 2 +- .../get_transaction_coordinates.ts | 2 +- .../apm/server/lib/rum_client/queries.test.ts | 2 +- .../server/lib/service_nodes/queries.test.ts | 2 +- .../lib/services/annotations/index.test.ts | 2 +- .../apm/server/lib/services/queries.test.ts | 2 +- .../agent_configuration/queries.test.ts | 2 +- .../create_or_update_custom_link.test.ts | 2 +- .../custom_link/get_transaction.test.ts | 2 +- .../custom_link/list_custom_links.test.ts | 2 +- .../apm/server/lib/traces/queries.test.ts | 2 +- .../lib/transaction_groups/queries.test.ts | 2 +- .../server/lib/transactions/queries.test.ts | 2 +- .../local_ui_filters/queries.test.ts | 2 +- .../apm/server/lib/ui_filters/queries.test.ts | 2 +- x-pack/plugins/apm/server/routes/typings.ts | 3 +- .../plugins/apm/server/utils/test_helpers.tsx | 117 ++++++++++++++++++ .../plugins/observability/typings/common.ts | 2 + 26 files changed, 159 insertions(+), 34 deletions(-) create mode 100644 x-pack/plugins/apm/common/fetch_options.ts create mode 100644 x-pack/plugins/apm/server/utils/test_helpers.tsx diff --git a/.eslintrc.js b/.eslintrc.js index a07d0830907b6..ff4ac180c3774 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -296,10 +296,7 @@ module.exports = { errorMessage: `Plugins may only import from src/core/server and src/core/public.`, }, { - target: [ - '(src|x-pack)/plugins/*/server/**/*', - '!x-pack/plugins/apm/**/*', // https://github.com/elastic/kibana/issues/67210 - ], + target: ['(src|x-pack)/plugins/*/server/**/*'], from: ['(src|x-pack)/plugins/*/public/**/*'], errorMessage: `Server code can not import from public, use a common directory.`, }, diff --git a/x-pack/plugins/apm/common/fetch_options.ts b/x-pack/plugins/apm/common/fetch_options.ts new file mode 100644 index 0000000000000..f75d7d7d2480f --- /dev/null +++ b/x-pack/plugins/apm/common/fetch_options.ts @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { HttpFetchOptions } from 'kibana/public'; + +export type FetchOptions = Omit & { + pathname: string; + isCachable?: boolean; + method?: string; + body?: any; +}; diff --git a/x-pack/plugins/apm/public/hooks/useCallApi.ts b/x-pack/plugins/apm/public/hooks/useCallApi.ts index 415e6172ae81e..3fec36e7fb24b 100644 --- a/x-pack/plugins/apm/public/hooks/useCallApi.ts +++ b/x-pack/plugins/apm/public/hooks/useCallApi.ts @@ -5,8 +5,9 @@ */ import { useMemo } from 'react'; -import { callApi, FetchOptions } from '../services/rest/callApi'; +import { callApi } from '../services/rest/callApi'; import { useApmPluginContext } from './useApmPluginContext'; +import { FetchOptions } from '../../common/fetch_options'; export function useCallApi() { const { http } = useApmPluginContext().core; diff --git a/x-pack/plugins/apm/public/services/rest/callApi.ts b/x-pack/plugins/apm/public/services/rest/callApi.ts index e1ecd6ee1185d..4ee12908b7c79 100644 --- a/x-pack/plugins/apm/public/services/rest/callApi.ts +++ b/x-pack/plugins/apm/public/services/rest/callApi.ts @@ -4,17 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ +import { HttpSetup } from 'kibana/public'; import { isString, startsWith } from 'lodash'; import LRU from 'lru-cache'; import hash from 'object-hash'; -import { HttpSetup, HttpFetchOptions } from 'kibana/public'; - -export type FetchOptions = Omit & { - pathname: string; - isCachable?: boolean; - method?: string; - body?: any; -}; +import { FetchOptions } from '../../../common/fetch_options'; function fetchOptionsWithDebug(fetchOptions: FetchOptions) { const debugEnabled = diff --git a/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts b/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts index 8babc72ef129c..08588bd03008d 100644 --- a/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts +++ b/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts @@ -4,7 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ import { HttpSetup } from 'kibana/public'; -import { callApi, FetchOptions } from './callApi'; +import { FetchOptions } from '../../../common/fetch_options'; +import { callApi } from './callApi'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { APMAPI } from '../../../server/routes/create_apm_api'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths diff --git a/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts b/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts index 25fc177694744..dfac607eb7232 100644 --- a/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts +++ b/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts @@ -8,7 +8,7 @@ import { getAllEnvironments } from './get_all_environments'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('getAllEnvironments', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts b/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts index 1c4db9173688a..0c247d0ef56da 100644 --- a/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts @@ -8,7 +8,7 @@ import { getErrorDistribution } from './get_distribution'; import { SearchParamsMock, inspectSearchParams, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; describe('error distribution queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/errors/queries.test.ts b/x-pack/plugins/apm/server/lib/errors/queries.test.ts index 158f4db58e15b..fec59393726bf 100644 --- a/x-pack/plugins/apm/server/lib/errors/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/errors/queries.test.ts @@ -9,7 +9,7 @@ import { getErrorGroups } from './get_error_groups'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('error queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/metrics/queries.test.ts b/x-pack/plugins/apm/server/lib/metrics/queries.test.ts index 3cd8ab46bc65d..fc24377ebf390 100644 --- a/x-pack/plugins/apm/server/lib/metrics/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/metrics/queries.test.ts @@ -12,7 +12,7 @@ import { getThreadCountChart } from './by_agent/java/thread_count'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; import { SERVICE_NODE_NAME_MISSING } from '../../../common/service_nodes'; describe('metrics queries', () => { diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts index 116b37a395299..4eb5ff05b45e3 100644 --- a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts +++ b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts @@ -9,7 +9,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { rangeFilter } from '../../../common/utils/range_filter'; -import { Coordinates } from '../../../../observability/public'; +import { Coordinates } from '../../../../observability/typings/common'; import { Setup, SetupTimeRange } from '../helpers/setup_request'; import { ProcessorEvent } from '../../../common/processor_event'; diff --git a/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts b/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts index 37432672c5d89..986e99f79904a 100644 --- a/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts @@ -7,7 +7,7 @@ import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; import { getClientMetrics } from './get_client_metrics'; import { getPageViewTrends } from './get_page_view_trends'; import { getPageLoadDistribution } from './get_page_load_distribution'; diff --git a/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts b/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts index 6ba9516390569..81262f90c4262 100644 --- a/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts @@ -13,7 +13,7 @@ import { getServiceNodes } from './'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; import { getServiceNodeMetadata } from '../services/get_service_node_metadata'; import { SERVICE_NODE_NAME_MISSING } from '../../../common/service_nodes'; diff --git a/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts b/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts index 4a9aa9e9f4d32..04e6ef322e9ec 100644 --- a/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts +++ b/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts @@ -7,7 +7,7 @@ import { getDerivedServiceAnnotations } from './get_derived_service_annotations' import { SearchParamsMock, inspectSearchParams, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; import noVersions from './__fixtures__/no_versions.json'; import oneVersion from './__fixtures__/one_version.json'; import multipleVersions from './__fixtures__/multiple_versions.json'; diff --git a/x-pack/plugins/apm/server/lib/services/queries.test.ts b/x-pack/plugins/apm/server/lib/services/queries.test.ts index b2fe7efeaf959..99c58a17d396a 100644 --- a/x-pack/plugins/apm/server/lib/services/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/services/queries.test.ts @@ -12,7 +12,7 @@ import { hasHistoricalAgentData } from './get_services/has_historical_agent_data import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('services queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts b/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts index 5fe9d19ffc860..f035aa937c364 100644 --- a/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts @@ -11,7 +11,7 @@ import { searchConfigurations } from './search_configurations'; import { SearchParamsMock, inspectSearchParams, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; import { findExactConfiguration } from './find_exact_configuration'; describe('agent configuration queries', () => { diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts index 0c3922b77901f..9f07c59609425 100644 --- a/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts +++ b/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts @@ -5,7 +5,7 @@ */ import { Setup } from '../../helpers/setup_request'; -import { mockNow } from '../../../../public/utils/testHelpers'; +import { mockNow } from '../../../utils/test_helpers'; import { CustomLink } from '../../../../common/custom_link/custom_link_types'; import { createOrUpdateCustomLink } from './create_or_update_custom_link'; diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts index 0a15c59ac62ae..174fc118eb205 100644 --- a/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts +++ b/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts @@ -6,7 +6,7 @@ import { inspectSearchParams, SearchParamsMock, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; import { getTransaction } from './get_transaction'; import { Setup } from '../../helpers/setup_request'; import { diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts index b09a270018711..ca468f2ed9614 100644 --- a/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts +++ b/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts @@ -8,7 +8,7 @@ import { listCustomLinks } from './list_custom_links'; import { inspectSearchParams, SearchParamsMock, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; import { Setup } from '../../helpers/setup_request'; import { SERVICE_NAME, diff --git a/x-pack/plugins/apm/server/lib/traces/queries.test.ts b/x-pack/plugins/apm/server/lib/traces/queries.test.ts index e85b45f55070b..481fc42c9a656 100644 --- a/x-pack/plugins/apm/server/lib/traces/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/traces/queries.test.ts @@ -8,7 +8,7 @@ import { getTraceItems } from './get_trace_items'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('trace queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts b/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts index 0b2ff3a72975b..5f36189224534 100644 --- a/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts @@ -8,7 +8,7 @@ import { transactionGroupsFetcher } from './fetcher'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('transaction group queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/lib/transactions/queries.test.ts b/x-pack/plugins/apm/server/lib/transactions/queries.test.ts index 586fa1798b7bc..8c8dbe1a3460a 100644 --- a/x-pack/plugins/apm/server/lib/transactions/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/transactions/queries.test.ts @@ -11,7 +11,7 @@ import { getTransaction } from './get_transaction'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { loggerMock } from '../../../../../../src/core/server/logging/logger.mock'; diff --git a/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts b/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts index 92ee67de49314..4cbb9efe012e6 100644 --- a/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts @@ -8,7 +8,7 @@ import { getLocalUIFilters } from './'; import { SearchParamsMock, inspectSearchParams, -} from '../../../../public/utils/testHelpers'; +} from '../../../utils/test_helpers'; import { getServicesProjection } from '../../../projections/services'; describe('local ui filter queries', () => { diff --git a/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts b/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts index 175319565b8a1..24e1c1a7f654c 100644 --- a/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts +++ b/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts @@ -8,7 +8,7 @@ import { getEnvironments } from './get_environments'; import { SearchParamsMock, inspectSearchParams, -} from '../../../public/utils/testHelpers'; +} from '../../utils/test_helpers'; describe('ui filter queries', () => { let mock: SearchParamsMock; diff --git a/x-pack/plugins/apm/server/routes/typings.ts b/x-pack/plugins/apm/server/routes/typings.ts index c95719da881ea..97013273c9bcf 100644 --- a/x-pack/plugins/apm/server/routes/typings.ts +++ b/x-pack/plugins/apm/server/routes/typings.ts @@ -15,10 +15,9 @@ import { PickByValue, Optional } from 'utility-types'; import { Observable } from 'rxjs'; import { Server } from 'hapi'; import { ObservabilityPluginSetup } from '../../../observability/server'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { FetchOptions } from '../../public/services/rest/callApi'; import { SecurityPluginSetup } from '../../../security/server'; import { MlPluginSetup } from '../../../ml/server'; +import { FetchOptions } from '../../common/fetch_options'; import { APMConfig } from '..'; export interface Params { diff --git a/x-pack/plugins/apm/server/utils/test_helpers.tsx b/x-pack/plugins/apm/server/utils/test_helpers.tsx new file mode 100644 index 0000000000000..98c1436b2b9b8 --- /dev/null +++ b/x-pack/plugins/apm/server/utils/test_helpers.tsx @@ -0,0 +1,117 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { + ESFilter, + ESSearchResponse, + ESSearchRequest, +} from '../../typings/elasticsearch'; +import { PromiseReturnType } from '../../typings/common'; +import { APMConfig } from '..'; + +interface Options { + mockResponse?: ( + request: ESSearchRequest + ) => ESSearchResponse; +} + +interface MockSetup { + start: number; + end: number; + apmEventClient: any; + internalClient: any; + config: APMConfig; + uiFiltersES: ESFilter[]; + indices: { + /* eslint-disable @typescript-eslint/naming-convention */ + 'apm_oss.sourcemapIndices': string; + 'apm_oss.errorIndices': string; + 'apm_oss.onboardingIndices': string; + 'apm_oss.spanIndices': string; + 'apm_oss.transactionIndices': string; + 'apm_oss.metricsIndices': string; + /* eslint-enable @typescript-eslint/naming-convention */ + apmAgentConfigurationIndex: string; + apmCustomLinkIndex: string; + }; +} + +export async function inspectSearchParams( + fn: (mockSetup: MockSetup) => Promise, + options: Options = {} +) { + const spy = jest.fn().mockImplementation(async (request) => { + return options.mockResponse + ? options.mockResponse(request) + : { + hits: { + hits: { + total: { + value: 0, + }, + }, + }, + }; + }); + + let response; + let error; + + const mockSetup = { + start: 1528113600000, + end: 1528977600000, + apmEventClient: { search: spy } as any, + internalClient: { search: spy } as any, + config: new Proxy( + {}, + { + get: (_, key) => { + switch (key) { + default: + return 'myIndex'; + + case 'xpack.apm.metricsInterval': + return 30; + } + }, + } + ) as APMConfig, + uiFiltersES: [{ term: { 'my.custom.ui.filter': 'foo-bar' } }], + indices: { + /* eslint-disable @typescript-eslint/naming-convention */ + 'apm_oss.sourcemapIndices': 'myIndex', + 'apm_oss.errorIndices': 'myIndex', + 'apm_oss.onboardingIndices': 'myIndex', + 'apm_oss.spanIndices': 'myIndex', + 'apm_oss.transactionIndices': 'myIndex', + 'apm_oss.metricsIndices': 'myIndex', + /* eslint-enable @typescript-eslint/naming-convention */ + apmAgentConfigurationIndex: 'myIndex', + apmCustomLinkIndex: 'myIndex', + }, + dynamicIndexPattern: null as any, + }; + try { + response = await fn(mockSetup); + } catch (err) { + error = err; + // we're only extracting the search params + } + + return { + params: spy.mock.calls[0][0], + response, + error, + spy, + teardown: () => spy.mockClear(), + }; +} + +export type SearchParamsMock = PromiseReturnType; + +export function mockNow(date: string | number | Date) { + const fakeNow = new Date(date).getTime(); + return jest.spyOn(Date, 'now').mockReturnValue(fakeNow); +} diff --git a/x-pack/plugins/observability/typings/common.ts b/x-pack/plugins/observability/typings/common.ts index 579c4fe3bbb90..19afac0c0d2b8 100644 --- a/x-pack/plugins/observability/typings/common.ts +++ b/x-pack/plugins/observability/typings/common.ts @@ -9,3 +9,5 @@ export type ObservabilityApp = 'infra_metrics' | 'infra_logs' | 'apm' | 'uptime' export type PromiseReturnType = Func extends (...args: any[]) => Promise ? Value : Func; + +export { Coordinates } from '../public/typings/fetch_overview_data/'; From 3fc3f58c62068ac8d1caf254d344661708502d14 Mon Sep 17 00:00:00 2001 From: Robert Austin Date: Fri, 28 Aug 2020 08:53:04 -0400 Subject: [PATCH 07/17] [Resolver] model `location.search` in redux (#76140) Read location.search from the redux store instead of a hook so that the entire view has a single (synchronized) source of truth. Also, no longer pass `pushToQueryParams` function to various components. --- .../public/resolver/models/location_search.ts | 39 ++++++ .../public/resolver/store/actions.ts | 28 ++++ .../public/resolver/store/data/action.ts | 20 --- .../resolver/store/data/selectors.test.ts | 24 +++- .../public/resolver/store/reducer.ts | 7 + .../public/resolver/store/selectors.ts | 9 ++ .../public/resolver/store/ui/selectors.ts | 80 +++++++----- .../public/resolver/types.ts | 20 ++- .../view/panels/event_counts_for_process.tsx | 5 +- .../public/resolver/view/panels/index.tsx | 21 +-- .../view/panels/panel_content_error.tsx | 123 +++++++++--------- .../resolver/view/panels/process_details.tsx | 7 +- .../view/panels/process_event_list.tsx | 6 +- .../view/panels/process_list_with_counts.tsx | 14 +- .../view/panels/related_event_detail.tsx | 13 +- .../resolver/view/process_event_dot.tsx | 4 +- ...s => use_replace_breadcrumb_parameters.ts} | 24 ++-- .../view/use_state_syncing_actions.ts | 6 +- 18 files changed, 271 insertions(+), 179 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/resolver/models/location_search.ts rename x-pack/plugins/security_solution/public/resolver/view/{use_resolver_query_params.ts => use_replace_breadcrumb_parameters.ts} (73%) diff --git a/x-pack/plugins/security_solution/public/resolver/models/location_search.ts b/x-pack/plugins/security_solution/public/resolver/models/location_search.ts new file mode 100644 index 0000000000000..8c21043c268d5 --- /dev/null +++ b/x-pack/plugins/security_solution/public/resolver/models/location_search.ts @@ -0,0 +1,39 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +/** + * The legacy `crumbEvent` and `crumbId` parameters. + * @deprecated + */ +export function breadcrumbParameters( + locationSearch: string, + resolverComponentInstanceID: string +): { crumbEvent: string; crumbId: string } { + const urlSearchParams = new URLSearchParams(locationSearch); + const { eventKey, idKey } = parameterNames(resolverComponentInstanceID); + return { + // Use `''` for backwards compatibility with deprecated code. + crumbEvent: urlSearchParams.get(eventKey) ?? '', + crumbId: urlSearchParams.get(idKey) ?? '', + }; +} + +/** + * Parameter names based on the `resolverComponentInstanceID`. + */ +function parameterNames( + resolverComponentInstanceID: string +): { + idKey: string; + eventKey: string; +} { + const idKey: string = `resolver-${resolverComponentInstanceID}-id`; + const eventKey: string = `resolver-${resolverComponentInstanceID}-event`; + return { + idKey, + eventKey, + }; +} diff --git a/x-pack/plugins/security_solution/public/resolver/store/actions.ts b/x-pack/plugins/security_solution/public/resolver/store/actions.ts index 29c03215e9ff4..e03f24d78e2a2 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/actions.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/actions.ts @@ -101,9 +101,37 @@ interface UserSelectedRelatedEventCategory { }; } +/** + * Used by `useStateSyncingActions` hook. + * This is dispatched when external sources provide new parameters for Resolver. + * When the component receives a new 'databaseDocumentID' prop, this is fired. + */ +interface AppReceivedNewExternalProperties { + type: 'appReceivedNewExternalProperties'; + /** + * Defines the externally provided properties that Resolver acknowledges. + */ + payload: { + /** + * the `_id` of an ES document. This defines the origin of the Resolver graph. + */ + databaseDocumentID?: string; + /** + * An ID that uniquely identifies this Resolver instance from other concurrent Resolvers. + */ + resolverComponentInstanceID: string; + + /** + * The `search` part of the URL of this page. + */ + locationSearch: string; + }; +} + export type ResolverAction = | CameraAction | DataAction + | AppReceivedNewExternalProperties | UserBroughtProcessIntoView | UserFocusedOnResolverNode | UserSelectedResolverNode diff --git a/x-pack/plugins/security_solution/public/resolver/store/data/action.ts b/x-pack/plugins/security_solution/public/resolver/store/data/action.ts index b6edf68aa7dc2..466c37d4ad5f1 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/data/action.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/data/action.ts @@ -60,30 +60,10 @@ interface ServerReturnedRelatedEventData { readonly payload: ResolverRelatedEvents; } -/** - * Used by `useStateSyncingActions` hook. - * This is dispatched when external sources provide new parameters for Resolver. - * When the component receives a new 'databaseDocumentID' prop, this is fired. - */ -interface AppReceivedNewExternalProperties { - type: 'appReceivedNewExternalProperties'; - /** - * Defines the externally provided properties that Resolver acknowledges. - */ - payload: { - /** - * the `_id` of an ES document. This defines the origin of the Resolver graph. - */ - databaseDocumentID?: string; - resolverComponentInstanceID: string; - }; -} - export type DataAction = | ServerReturnedResolverData | ServerFailedToReturnResolverData | ServerFailedToReturnRelatedEventData | ServerReturnedRelatedEventData - | AppReceivedNewExternalProperties | AppRequestedResolverData | AppAbortedResolverDataRequest; diff --git a/x-pack/plugins/security_solution/public/resolver/store/data/selectors.test.ts b/x-pack/plugins/security_solution/public/resolver/store/data/selectors.test.ts index 15a981d460730..dc478ede72790 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/data/selectors.test.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/data/selectors.test.ts @@ -6,8 +6,8 @@ import * as selectors from './selectors'; import { DataState } from '../../types'; +import { ResolverAction } from '../actions'; import { dataReducer } from './reducer'; -import { DataAction } from './action'; import { createStore } from 'redux'; import { mockTreeWithNoAncestorsAnd2Children, @@ -20,7 +20,7 @@ import { uniquePidForProcess } from '../../models/process_event'; import { EndpointEvent } from '../../../../common/endpoint/types'; describe('data state', () => { - let actions: DataAction[] = []; + let actions: ResolverAction[] = []; /** * Get state, given an ordered collection of actions. @@ -68,7 +68,13 @@ describe('data state', () => { actions = [ { type: 'appReceivedNewExternalProperties', - payload: { databaseDocumentID, resolverComponentInstanceID }, + payload: { + databaseDocumentID, + resolverComponentInstanceID, + + // `locationSearch` doesn't matter for this test + locationSearch: '', + }, }, ]; }); @@ -120,7 +126,13 @@ describe('data state', () => { actions = [ { type: 'appReceivedNewExternalProperties', - payload: { databaseDocumentID, resolverComponentInstanceID }, + payload: { + databaseDocumentID, + resolverComponentInstanceID, + + // `locationSearch` doesn't matter for this test + locationSearch: '', + }, }, { type: 'appRequestedResolverData', @@ -182,6 +194,8 @@ describe('data state', () => { payload: { databaseDocumentID: firstDatabaseDocumentID, resolverComponentInstanceID: resolverComponentInstanceID1, + // `locationSearch` doesn't matter for this test + locationSearch: '', }, }, // this happens when the middleware starts the request @@ -195,6 +209,8 @@ describe('data state', () => { payload: { databaseDocumentID: secondDatabaseDocumentID, resolverComponentInstanceID: resolverComponentInstanceID2, + // `locationSearch` doesn't matter for this test + locationSearch: '', }, }, ]; diff --git a/x-pack/plugins/security_solution/public/resolver/store/reducer.ts b/x-pack/plugins/security_solution/public/resolver/store/reducer.ts index d0f9701fe944e..bf62fd0e60df8 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/reducer.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/reducer.ts @@ -48,6 +48,13 @@ const uiReducer: Reducer = ( selectedNode: nodeID, }; return next; + } else if (action.type === 'appReceivedNewExternalProperties') { + const next: ResolverUIState = { + ...state, + locationSearch: action.payload.locationSearch, + resolverComponentInstanceID: action.payload.resolverComponentInstanceID, + }; + return next; } else { return state; } diff --git a/x-pack/plugins/security_solution/public/resolver/store/selectors.ts b/x-pack/plugins/security_solution/public/resolver/store/selectors.ts index f50aeed3f4d48..909a907626f30 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/selectors.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/selectors.ts @@ -301,6 +301,15 @@ export const ariaFlowtoNodeID: ( } ); +/** + * The legacy `crumbEvent` and `crumbId` parameters. + * @deprecated + */ +export const breadcrumbParameters = composeSelectors( + uiStateSelector, + uiSelectors.breadcrumbParameters +); + /** * Calls the `secondSelector` with the result of the `selector`. Use this when re-exporting a * concern-specific selector. `selector` should return the concern-specific state. diff --git a/x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts b/x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts index 91a2cbecbc04c..5315ffb3c5fdb 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts @@ -1,30 +1,50 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { createSelector } from 'reselect'; -import { ResolverUIState } from '../../types'; - -/** - * id of the "current" tree node (fake-focused) - */ -export const ariaActiveDescendant = createSelector( - (uiState: ResolverUIState) => uiState, - /* eslint-disable no-shadow */ - ({ ariaActiveDescendant }) => { - return ariaActiveDescendant; - } -); - -/** - * id of the currently "selected" tree node - */ -export const selectedNode = createSelector( - (uiState: ResolverUIState) => uiState, - /* eslint-disable no-shadow */ - ({ selectedNode }: ResolverUIState) => { - return selectedNode; - } -); +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { createSelector } from 'reselect'; +import { ResolverUIState } from '../../types'; +import * as locationSearchModel from '../../models/location_search'; + +/** + * id of the "current" tree node (fake-focused) + */ +export const ariaActiveDescendant = createSelector( + (uiState: ResolverUIState) => uiState, + /* eslint-disable no-shadow */ + ({ ariaActiveDescendant }) => { + return ariaActiveDescendant; + } +); + +/** + * id of the currently "selected" tree node + */ +export const selectedNode = createSelector( + (uiState: ResolverUIState) => uiState, + /* eslint-disable no-shadow */ + ({ selectedNode }: ResolverUIState) => { + return selectedNode; + } +); + +/** + * The legacy `crumbEvent` and `crumbId` parameters. + * @deprecated + */ +export const breadcrumbParameters = createSelector( + (state: ResolverUIState) => state.locationSearch, + (state: ResolverUIState) => state.resolverComponentInstanceID, + (locationSearch, resolverComponentInstanceID) => { + if (locationSearch === undefined || resolverComponentInstanceID === undefined) { + // Equivalent to `null` + return { + crumbId: '', + crumbEvent: '', + }; + } + return locationSearchModel.breadcrumbParameters(locationSearch, resolverComponentInstanceID); + } +); diff --git a/x-pack/plugins/security_solution/public/resolver/types.ts b/x-pack/plugins/security_solution/public/resolver/types.ts index 9ebe3fa14e842..e8304bf838e2d 100644 --- a/x-pack/plugins/security_solution/public/resolver/types.ts +++ b/x-pack/plugins/security_solution/public/resolver/types.ts @@ -50,6 +50,16 @@ export interface ResolverUIState { * `nodeID` of the selected node */ readonly selectedNode: string | null; + + /** + * The `search` part of the URL. + */ + readonly locationSearch?: string; + + /** + * An ID that is used to differentiate this Resolver instance from others concurrently running on the same page. + */ + readonly resolverComponentInstanceID?: string; } /** @@ -198,7 +208,12 @@ export interface DataState { * The id used for the pending request, if there is one. */ readonly pendingRequestDatabaseDocumentID?: string; - readonly resolverComponentInstanceID: string | undefined; + + /** + * An ID that is used to differentiate this Resolver instance from others concurrently running on the same page. + * Used to prevent collisions in things like query parameters. + */ + readonly resolverComponentInstanceID?: string; /** * The parameters and response from the last successful request. @@ -510,8 +525,9 @@ export interface ResolverProps { * Used as the origin of the Resolver graph. */ databaseDocumentID?: string; + /** - * A string literal describing where in the application resolver is located. + * An ID that is used to differentiate this Resolver instance from others concurrently running on the same page. * Used to prevent collisions in things like query parameters. */ resolverComponentInstanceID: string; diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/event_counts_for_process.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/event_counts_for_process.tsx index c528ba547e6ae..f81dc174d8128 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/event_counts_for_process.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/event_counts_for_process.tsx @@ -12,7 +12,7 @@ import { StyledBreadcrumbs } from './panel_content_utilities'; import * as event from '../../../../common/endpoint/models/event'; import { ResolverEvent, ResolverNodeStats } from '../../../../common/endpoint/types'; -import { CrumbInfo } from '../../types'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; /** * This view gives counts for all the related events of a process grouped by related event type. @@ -27,11 +27,9 @@ import { CrumbInfo } from '../../types'; */ export const EventCountsForProcess = memo(function EventCountsForProcess({ processEvent, - pushToQueryParams, relatedStats, }: { processEvent: ResolverEvent; - pushToQueryParams: (queryStringKeyValuePair: CrumbInfo) => unknown; relatedStats: ResolverNodeStats; }) { interface EventCountsTableView { @@ -62,6 +60,7 @@ export const EventCountsForProcess = memo(function EventCountsForProcess({ defaultMessage: 'Events', } ); + const pushToQueryParams = useReplaceBreadcrumbParameters(); const crumbs = useMemo(() => { return [ { diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx index b3c4eefe5fae7..98b737de8fa59 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx @@ -17,7 +17,6 @@ import { EventCountsForProcess } from './event_counts_for_process'; import { ProcessDetails } from './process_details'; import { ProcessListWithCounts } from './process_list_with_counts'; import { RelatedEventDetail } from './related_event_detail'; -import { useResolverQueryParams } from '../use_resolver_query_params'; /** * The team decided to use this table to determine which breadcrumbs/view to display: @@ -39,7 +38,7 @@ const PanelContent = memo(function PanelContent() { const { timestamp } = useContext(SideEffectContext); - const { pushToQueryParams, queryParams } = useResolverQueryParams(); + const queryParams = useSelector(selectors.breadcrumbParameters); const graphableProcesses = useSelector(selectors.graphableProcesses); const graphableProcessEntityIds = useMemo(() => { @@ -164,16 +163,13 @@ const PanelContent = memo(function PanelContent() { const panelInstance = useMemo(() => { if (panelToShow === 'processDetails') { - return ( - - ); + return ; } if (panelToShow === 'eventCountsForProcess') { return ( ); @@ -183,7 +179,6 @@ const PanelContent = memo(function PanelContent() { return ( @@ -198,21 +193,13 @@ const PanelContent = memo(function PanelContent() { ); } // The default 'Event List' / 'List of all processes' view - return ; - }, [ - uiSelectedEvent, - crumbEvent, - crumbId, - pushToQueryParams, - relatedStatsForIdFromParams, - panelToShow, - ]); + return ; + }, [uiSelectedEvent, crumbEvent, crumbId, relatedStatsForIdFromParams, panelToShow]); return <>{panelInstance}; }); diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_error.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_error.tsx index b93ef6146f1cf..4162412861f57 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_error.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_error.tsx @@ -1,62 +1,61 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { i18n } from '@kbn/i18n'; -import { EuiSpacer, EuiText, EuiButtonEmpty } from '@elastic/eui'; -import React, { memo, useMemo } from 'react'; -import { StyledBreadcrumbs } from './panel_content_utilities'; -import { CrumbInfo } from '../../types'; - -/** - * Display an error in the panel when something goes wrong and give the user a way to "retreat" back to a default state. - * - * @param {function} pushToQueryparams A function to update the hash value in the URL to control panel state - * @param {string} translatedErrorMessage The message to display in the panel when something goes wrong - */ -export const PanelContentError = memo(function ({ - translatedErrorMessage, - pushToQueryParams, -}: { - translatedErrorMessage: string; - pushToQueryParams: (arg0: CrumbInfo) => unknown; -}) { - const crumbs = useMemo(() => { - return [ - { - text: i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.events', { - defaultMessage: 'Events', - }), - onClick: () => { - pushToQueryParams({ crumbId: '', crumbEvent: '' }); - }, - }, - { - text: i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.error', { - defaultMessage: 'Error', - }), - onClick: () => {}, - }, - ]; - }, [pushToQueryParams]); - return ( - <> - - - {translatedErrorMessage} - - { - pushToQueryParams({ crumbId: '', crumbEvent: '' }); - }} - > - {i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.goBack', { - defaultMessage: 'Click this link to return to the list of all processes.', - })} - - - ); -}); -PanelContentError.displayName = 'TableServiceError'; +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { i18n } from '@kbn/i18n'; +import { EuiSpacer, EuiText, EuiButtonEmpty } from '@elastic/eui'; +import React, { memo, useMemo } from 'react'; +import { StyledBreadcrumbs } from './panel_content_utilities'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; + +/** + * Display an error in the panel when something goes wrong and give the user a way to "retreat" back to a default state. + * + * @param {function} pushToQueryparams A function to update the hash value in the URL to control panel state + * @param {string} translatedErrorMessage The message to display in the panel when something goes wrong + */ +export const PanelContentError = memo(function ({ + translatedErrorMessage, +}: { + translatedErrorMessage: string; +}) { + const pushToQueryParams = useReplaceBreadcrumbParameters(); + const crumbs = useMemo(() => { + return [ + { + text: i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.events', { + defaultMessage: 'Events', + }), + onClick: () => { + pushToQueryParams({ crumbId: '', crumbEvent: '' }); + }, + }, + { + text: i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.error', { + defaultMessage: 'Error', + }), + onClick: () => {}, + }, + ]; + }, [pushToQueryParams]); + return ( + <> + + + {translatedErrorMessage} + + { + pushToQueryParams({ crumbId: '', crumbEvent: '' }); + }} + > + {i18n.translate('xpack.securitySolution.endpoint.resolver.panel.error.goBack', { + defaultMessage: 'Click this link to return to the list of all processes.', + })} + + + ); +}); +PanelContentError.displayName = 'TableServiceError'; diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/process_details.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/process_details.tsx index 1ec56b8aa169a..01fa912caa866 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/process_details.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/process_details.tsx @@ -31,7 +31,8 @@ import { import { CubeForProcess } from './cube_for_process'; import { ResolverEvent } from '../../../../common/endpoint/types'; import { useResolverTheme } from '../assets'; -import { CrumbInfo, ResolverState } from '../../types'; +import { ResolverState } from '../../types'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; const StyledDescriptionList = styled(EuiDescriptionList)` &.euiDescriptionList.euiDescriptionList--column dt.euiDescriptionList__title.desc-title { @@ -49,10 +50,8 @@ const StyledTitle = styled('h4')` */ export const ProcessDetails = memo(function ProcessDetails({ processEvent, - pushToQueryParams, }: { processEvent: ResolverEvent; - pushToQueryParams: (queryStringKeyValuePair: CrumbInfo) => unknown; }) { const processName = event.eventName(processEvent); const entityId = event.entityId(processEvent); @@ -127,6 +126,8 @@ export const ProcessDetails = memo(function ProcessDetails({ return processDescriptionListData; }, [processEvent]); + const pushToQueryParams = useReplaceBreadcrumbParameters(); + const crumbs = useMemo(() => { return [ { diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/process_event_list.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/process_event_list.tsx index a710d3ad846b3..5fe33530f05dc 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/process_event_list.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/process_event_list.tsx @@ -16,7 +16,7 @@ import { ResolverEvent, ResolverNodeStats } from '../../../../common/endpoint/ty import * as selectors from '../../store/selectors'; import { useResolverDispatch } from '../use_resolver_dispatch'; import { RelatedEventLimitWarning } from '../limit_warnings'; -import { CrumbInfo } from '../../types'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; /** * This view presents a list of related events of a given type for a given process. @@ -129,10 +129,8 @@ export const ProcessEventList = memo(function ProcessEventList({ processEvent, eventType, relatedStats, - pushToQueryParams, }: { processEvent: ResolverEvent; - pushToQueryParams: (arg0: CrumbInfo) => unknown; eventType: string; relatedStats: ResolverNodeStats; }) { @@ -169,6 +167,8 @@ export const ProcessEventList = memo(function ProcessEventList({ } }, [relatedsReady, dispatch, processEntityId]); + const pushToQueryParams = useReplaceBreadcrumbParameters(); + const waitCrumbs = useMemo(() => { return [ { diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/process_list_with_counts.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/process_list_with_counts.tsx index e42140feb928b..6035255824b1c 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/process_list_with_counts.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/process_list_with_counts.tsx @@ -3,6 +3,9 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + +/* eslint-disable react/display-name */ + import React, { memo, useContext, useCallback, useMemo } from 'react'; import { EuiBasicTableColumn, @@ -22,7 +25,7 @@ import { SideEffectContext } from '../side_effect_context'; import { CubeForProcess } from './cube_for_process'; import { SafeResolverEvent } from '../../../../common/endpoint/types'; import { LimitWarning } from '../limit_warnings'; -import { CrumbInfo } from '../../types'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; const StyledLimitWarning = styled(LimitWarning)` flex-flow: row wrap; @@ -46,14 +49,8 @@ const StyledLimitWarning = styled(LimitWarning)` /** * The "default" view for the panel: A list of all the processes currently in the graph. - * - * @param {function} pushToQueryparams A function to update the hash value in the URL to control panel state */ -export const ProcessListWithCounts = memo(function ProcessListWithCounts({ - pushToQueryParams, -}: { - pushToQueryParams: (queryStringKeyValuePair: CrumbInfo) => unknown; -}) { +export const ProcessListWithCounts = memo(() => { interface ProcessTableView { name?: string; timestamp?: Date; @@ -63,6 +60,7 @@ export const ProcessListWithCounts = memo(function ProcessListWithCounts({ const dispatch = useResolverDispatch(); const { timestamp } = useContext(SideEffectContext); const isProcessTerminated = useSelector(selectors.isProcessTerminated); + const pushToQueryParams = useReplaceBreadcrumbParameters(); const handleBringIntoViewClick = useCallback( (processTableViewItem) => { dispatch({ diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx index dfafbae9c9a16..4762c615ba793 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx @@ -16,7 +16,8 @@ import { ResolverEvent } from '../../../../common/endpoint/types'; import * as selectors from '../../store/selectors'; import { useResolverDispatch } from '../use_resolver_dispatch'; import { PanelContentError } from './panel_content_error'; -import { CrumbInfo, ResolverState } from '../../types'; +import { ResolverState } from '../../types'; +import { useReplaceBreadcrumbParameters } from '../use_replace_breadcrumb_parameters'; // Adding some styles to prevent horizontal scrollbars, per request from UX review const StyledDescriptionList = memo(styled(EuiDescriptionList)` @@ -76,15 +77,13 @@ function entriesForDisplay(entries: Array<{ title: string; description: string } * This view presents a detailed view of all the available data for a related event, split and titled by the "section" * it appears in the underlying ResolverEvent */ -export const RelatedEventDetail = memo(function RelatedEventDetail({ +export const RelatedEventDetail = memo(function ({ relatedEventId, parentEvent, - pushToQueryParams, countForParent, }: { relatedEventId: string; parentEvent: ResolverEvent; - pushToQueryParams: (queryStringKeyValuePair: CrumbInfo) => unknown; countForParent: number | undefined; }) { const processName = (parentEvent && event.eventName(parentEvent)) || '*'; @@ -130,6 +129,8 @@ export const RelatedEventDetail = memo(function RelatedEventDetail({ selectors.relatedEventDisplayInfoByEntityAndSelfId(state)(processEntityId, relatedEventId) ); + const pushToQueryParams = useReplaceBreadcrumbParameters(); + const waitCrumbs = useMemo(() => { return [ { @@ -247,9 +248,7 @@ export const RelatedEventDetail = memo(function RelatedEventDetail({ defaultMessage: 'Related event not found.', } ); - return ( - - ); + return ; } return ( diff --git a/x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx b/x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx index baa8ce1fcdd86..2aacc5f9176c4 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx @@ -18,7 +18,7 @@ import { ResolverEvent, SafeResolverEvent } from '../../../common/endpoint/types import { useResolverDispatch } from './use_resolver_dispatch'; import * as eventModel from '../../../common/endpoint/models/event'; import * as selectors from '../store/selectors'; -import { useResolverQueryParams } from './use_resolver_query_params'; +import { useReplaceBreadcrumbParameters } from './use_replace_breadcrumb_parameters'; interface StyledActionsContainer { readonly color: string; @@ -242,7 +242,7 @@ const UnstyledProcessEventDot = React.memo( }); }, [dispatch, nodeID]); - const { pushToQueryParams } = useResolverQueryParams(); + const pushToQueryParams = useReplaceBreadcrumbParameters(); const handleClick = useCallback(() => { if (animationTarget.current?.beginElement) { diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts b/x-pack/plugins/security_solution/public/resolver/view/use_replace_breadcrumb_parameters.ts similarity index 73% rename from x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts rename to x-pack/plugins/security_solution/public/resolver/view/use_replace_breadcrumb_parameters.ts index b6c229181e9f7..6d819337e447d 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_replace_breadcrumb_parameters.ts @@ -4,12 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useCallback, useMemo } from 'react'; +import { useCallback } from 'react'; import { useHistory, useLocation } from 'react-router-dom'; import { useQueryStringKeys } from './use_query_string_keys'; import { CrumbInfo } from '../types'; -export function useResolverQueryParams() { +/** + * @deprecated + * Update the browser's `search` with data from `queryStringState`. The URL search parameter names + * will include Resolver's `resolverComponentInstanceID`. + */ +export function useReplaceBreadcrumbParameters(): (queryStringState: CrumbInfo) => void { /** * This updates the breadcrumb nav and the panel view. It's supplied to each * panel content view to allow them to dispatch transitions to each other. @@ -17,7 +22,7 @@ export function useResolverQueryParams() { const history = useHistory(); const urlSearch = useLocation().search; const { idKey, eventKey } = useQueryStringKeys(); - const pushToQueryParams = useCallback( + return useCallback( (queryStringState: CrumbInfo) => { const urlSearchParams = new URLSearchParams(urlSearch); @@ -39,17 +44,4 @@ export function useResolverQueryParams() { }, [history, urlSearch, idKey, eventKey] ); - const queryParams: CrumbInfo = useMemo(() => { - const urlSearchParams = new URLSearchParams(urlSearch); - return { - // Use `''` for backwards compatibility with deprecated code. - crumbEvent: urlSearchParams.get(eventKey) ?? '', - crumbId: urlSearchParams.get(idKey) ?? '', - }; - }, [urlSearch, idKey, eventKey]); - - return { - pushToQueryParams, - queryParams, - }; } diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_state_syncing_actions.ts b/x-pack/plugins/security_solution/public/resolver/view/use_state_syncing_actions.ts index 642a054e8c519..eaba4438bb1fe 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_state_syncing_actions.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_state_syncing_actions.ts @@ -5,6 +5,7 @@ */ import { useLayoutEffect } from 'react'; +import { useLocation } from 'react-router-dom'; import { useResolverDispatch } from './use_resolver_dispatch'; /** @@ -22,10 +23,11 @@ export function useStateSyncingActions({ resolverComponentInstanceID: string; }) { const dispatch = useResolverDispatch(); + const locationSearch = useLocation().search; useLayoutEffect(() => { dispatch({ type: 'appReceivedNewExternalProperties', - payload: { databaseDocumentID, resolverComponentInstanceID }, + payload: { databaseDocumentID, resolverComponentInstanceID, locationSearch }, }); - }, [dispatch, databaseDocumentID, resolverComponentInstanceID]); + }, [dispatch, databaseDocumentID, resolverComponentInstanceID, locationSearch]); } From 641a5ad77f5ab727cf61b4fdd97a275f4645e8ac Mon Sep 17 00:00:00 2001 From: Constance Date: Fri, 28 Aug 2020 08:38:23 -0700 Subject: [PATCH 08/17] [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (#76106) * Refactor enterprise_search_request_handler to a class that stores config/log - So that routes don't have to continuously pass it in - just params that actually change (paths, queries) + misc refactors/lint fixes from code review * Update mocks/dependencies so other tests pass type checks * Update /api/workplace_search/overview endpoint to use new request handler * Update /api/app_search/engines route to use new handler - This required updating the shared handler to accept custom params set by the route, since the engines route transmutes some param names - createRequest mock also had to be updated to correctly reflect a curried function in order to get tests passing * DRY out hasValidData to a reusable/cleaner call * Update shared handler to output specific message for auth errors - Check for /login URL (indicates auth issue) - Refactor final catch return to allow being passed messages from previous throw - Change hasValidData fallback to only log (potentially sensitive?) JSON data to server, not to client - Minor test cleanup - move error tests to single describe block, minor URL/newline cleanup * Update handler to pass method & body requests + pass back response code - This will support future POST requests as well as support (e.g.) 404 responses Minor test refactors: - Set up new request defaults (method, empty body) + expected output & convert KibanaAuthHeader to string since they're set so close to one another - group request pass tests into a describe block, remove response body portion of their tests (since we don't really care about that for those tests) - group response tests into describe block - clarify how passed handler params arg overrides request.params * PR feedback: Update custom params arg to take an object instead of a query string --- .../server/__mocks__/index.ts | 7 +- .../__mocks__/routerDependencies.mock.ts | 8 + .../enterprise_search_request_handler.test.ts | 153 +++++++++++++----- .../lib/enterprise_search_request_handler.ts | 103 ++++++++---- .../enterprise_search/server/plugin.ts | 12 +- .../routes/app_search/credentials.test.ts | 27 +--- .../server/routes/app_search/credentials.ts | 10 +- .../server/routes/app_search/engines.test.ts | 121 ++++---------- .../server/routes/app_search/engines.ts | 47 ++---- .../enterprise_search/telemetry.test.ts | 4 +- .../routes/workplace_search/overview.test.ts | 120 +++----------- .../routes/workplace_search/overview.ts | 39 +---- 12 files changed, 295 insertions(+), 356 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/index.ts b/x-pack/plugins/enterprise_search/server/__mocks__/index.ts index 3cca5e21ce9c3..69f37856b6f2d 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/index.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/index.ts @@ -5,4 +5,9 @@ */ export { MockRouter } from './router.mock'; -export { mockConfig, mockLogger, mockDependencies } from './routerDependencies.mock'; +export { + mockConfig, + mockLogger, + mockRequestHandler, + mockDependencies, +} from './routerDependencies.mock'; diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts index 7a244be96cfc4..b08a14b540e55 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts @@ -9,6 +9,13 @@ import { ConfigType } from '../'; export const mockLogger = loggingSystemMock.createLogger().get(); +export const mockRequestHandler = { + createRequest: jest.fn(() => () => {}), + hasValidData(data: any) { + return (this.createRequest as jest.Mock).mock.calls[0][0].hasValidData(data); + }, +}; + export const mockConfig = { enabled: true, host: 'http://localhost:3002', @@ -24,4 +31,5 @@ export const mockDependencies = { // Mock router should be handled on a per-test basis config: mockConfig, log: mockLogger, + enterpriseSearchRequestHandler: mockRequestHandler as any, }; diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts index f0c003936996e..3f3f182433144 100644 --- a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts +++ b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts @@ -6,7 +6,7 @@ import { mockConfig, mockLogger } from '../__mocks__'; -import { createEnterpriseSearchRequestHandler } from './enterprise_search_request_handler'; +import { EnterpriseSearchRequestHandler } from './enterprise_search_request_handler'; jest.mock('node-fetch'); // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -14,12 +14,16 @@ const fetchMock = require('node-fetch') as jest.Mock; const { Response } = jest.requireActual('node-fetch'); const responseMock = { - ok: jest.fn(), + custom: jest.fn(), customError: jest.fn(), }; -const KibanaAuthHeader = 'Basic 123'; -describe('createEnterpriseSearchRequestHandler', () => { +describe('EnterpriseSearchRequestHandler', () => { + const enterpriseSearchRequestHandler = new EnterpriseSearchRequestHandler({ + config: mockConfig, + log: mockLogger, + }) as any; + beforeEach(() => { jest.clearAllMocks(); fetchMock.mockReset(); @@ -33,9 +37,7 @@ describe('createEnterpriseSearchRequestHandler', () => { EnterpriseSearchAPI.mockReturn(responseBody); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/as/credentials/collection', }); @@ -47,82 +49,151 @@ describe('createEnterpriseSearchRequestHandler', () => { }); EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection?type=indexed&pageIndex=1' + 'http://localhost:3002/as/credentials/collection?type=indexed&pageIndex=1', + { method: 'GET' } ); - expect(responseMock.ok).toHaveBeenCalledWith({ + expect(responseMock.custom).toHaveBeenCalledWith({ body: responseBody, + statusCode: 200, }); }); - describe('when an API request fails', () => { - it('should return 502 with a message', async () => { - EnterpriseSearchAPI.mockReturnError(); + describe('request passing', () => { + it('passes route method', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, - path: '/as/credentials/collection', + await makeAPICall(requestHandler, { route: { method: 'POST' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'POST', }); - await makeAPICall(requestHandler); + await makeAPICall(requestHandler, { route: { method: 'DELETE' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'DELETE', + }); + }); + + it('passes request body', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); + await makeAPICall(requestHandler, { body: { bodacious: true } }); + + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + body: '{"bodacious":true}', + }); + }); + + it('passes custom params set by the handler, which override request params', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/example', + params: { someQuery: true }, + }); + await makeAPICall(requestHandler, { query: { someQuery: false } }); EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection' + 'http://localhost:3002/api/example?someQuery=true' ); + }); + }); - expect(responseMock.customError).toHaveBeenCalledWith({ - body: 'Error connecting or fetching data from Enterprise Search', - statusCode: 502, - }); + describe('response passing', () => { + it('returns the response status code from Enterprise Search', async () => { + EnterpriseSearchAPI.mockReturn({}, { status: 404 }); + + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); + await makeAPICall(requestHandler); + + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example'); + expect(responseMock.custom).toHaveBeenCalledWith({ body: {}, statusCode: 404 }); }); + + // TODO: It's possible we may also pass back headers at some point + // from Enterprise Search, e.g. the x-read-only mode header }); - describe('when `hasValidData` fails', () => { - it('should return 502 with a message', async () => { - const responseBody = { - foo: 'bar', - }; + describe('error handling', () => { + afterEach(() => { + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('Error connecting to Enterprise Search') + ); + }); + + it('returns an error when an API request fails', async () => { + EnterpriseSearchAPI.mockReturnError(); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/failed' }); + + await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/failed'); - EnterpriseSearchAPI.mockReturn(responseBody); + expect(responseMock.customError).toHaveBeenCalledWith({ + body: 'Error connecting to Enterprise Search: Failed', + statusCode: 502, + }); + }); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, - path: '/as/credentials/collection', - hasValidData: (body?: any) => - Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number', + it('returns an error when `hasValidData` fails', async () => { + EnterpriseSearchAPI.mockReturn({ results: false }); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/invalid', + hasValidData: (body?: any) => Array.isArray(body?.results), }); await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/invalid'); - EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection' + expect(responseMock.customError).toHaveBeenCalledWith({ + body: 'Error connecting to Enterprise Search: Invalid data received', + statusCode: 502, + }); + expect(mockLogger.debug).toHaveBeenCalledWith( + 'Invalid data received from : {"results":false}' ); + }); + + it('returns an error when user authentication to Enterprise Search fails', async () => { + EnterpriseSearchAPI.mockReturn({}, { url: 'http://localhost:3002/login' }); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/unauthenticated', + }); + + await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/unauthenticated'); expect(responseMock.customError).toHaveBeenCalledWith({ - body: 'Error connecting or fetching data from Enterprise Search', + body: 'Error connecting to Enterprise Search: Cannot authenticate Enterprise Search user', statusCode: 502, }); }); }); + + it('has a helper for checking empty objects', async () => { + expect(enterpriseSearchRequestHandler.isEmptyObj({})).toEqual(true); + expect(enterpriseSearchRequestHandler.isEmptyObj({ empty: false })).toEqual(false); + }); }); const makeAPICall = (handler: Function, params = {}) => { - const request = { headers: { authorization: KibanaAuthHeader }, ...params }; + const request = { + headers: { authorization: 'Basic 123' }, + route: { method: 'GET' }, + body: {}, + ...params, + }; return handler(null, request, responseMock); }; const EnterpriseSearchAPI = { shouldHaveBeenCalledWith(expectedUrl: string, expectedParams = {}) { expect(fetchMock).toHaveBeenCalledWith(expectedUrl, { - headers: { Authorization: KibanaAuthHeader }, + headers: { Authorization: 'Basic 123' }, + method: 'GET', + body: undefined, ...expectedParams, }); }, - mockReturn(response: object) { + mockReturn(response: object, options?: object) { fetchMock.mockImplementation(() => { - return Promise.resolve(new Response(JSON.stringify(response))); + return Promise.resolve(new Response(JSON.stringify(response), options)); }); }, mockReturnError() { diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts index 11152aa651743..8f31bd9063d4a 100644 --- a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts +++ b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts @@ -7,6 +7,7 @@ import fetch from 'node-fetch'; import querystring from 'querystring'; import { + RequestHandler, RequestHandlerContext, KibanaRequest, KibanaResponseFactory, @@ -14,56 +15,90 @@ import { } from 'src/core/server'; import { ConfigType } from '../index'; -interface IEnterpriseSearchRequestParams { +interface IConstructorDependencies { config: ConfigType; log: Logger; +} +interface IRequestParams { path: string; + params?: object; hasValidData?: (body?: ResponseBody) => boolean; } +export interface IEnterpriseSearchRequestHandler { + createRequest(requestParams?: object): RequestHandler, unknown>; +} /** - * This helper function creates a single standard DRY way of handling + * This helper lib creates a single standard DRY way of handling * Enterprise Search API requests. * * This handler assumes that it will essentially just proxy the * Enterprise Search API request, so the request body and request * parameters are simply passed through. */ -export function createEnterpriseSearchRequestHandler({ - config, - log, - path, - hasValidData = () => true, -}: IEnterpriseSearchRequestParams) { - return async ( - _context: RequestHandlerContext, - request: KibanaRequest, unknown>, - response: KibanaResponseFactory - ) => { - try { - const enterpriseSearchUrl = config.host as string; - const params = request.query ? `?${querystring.stringify(request.query)}` : ''; - const url = `${encodeURI(enterpriseSearchUrl)}${path}${params}`; +export class EnterpriseSearchRequestHandler { + private enterpriseSearchUrl: string; + private log: Logger; + + constructor({ config, log }: IConstructorDependencies) { + this.log = log; + this.enterpriseSearchUrl = config.host as string; + } + + createRequest({ + path, + params = {}, + hasValidData = () => true, + }: IRequestParams) { + return async ( + _context: RequestHandlerContext, + request: KibanaRequest, unknown>, + response: KibanaResponseFactory + ) => { + try { + // Set up API URL + const queryParams = { ...request.query, ...params }; + const queryString = !this.isEmptyObj(queryParams) + ? `?${querystring.stringify(queryParams)}` + : ''; + const url = encodeURI(this.enterpriseSearchUrl + path + queryString); + + // Set up API options + const { method } = request.route; + const headers = { Authorization: request.headers.authorization as string }; + const body = !this.isEmptyObj(request.body as object) + ? JSON.stringify(request.body) + : undefined; + + // Call the Enterprise Search API and pass back response to the front-end + const apiResponse = await fetch(url, { method, headers, body }); + + if (apiResponse.url.endsWith('/login')) { + throw new Error('Cannot authenticate Enterprise Search user'); + } + + const { status } = apiResponse; + const json = await apiResponse.json(); - const apiResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); + if (hasValidData(json)) { + return response.custom({ statusCode: status, body: json }); + } else { + this.log.debug(`Invalid data received from <${url}>: ${JSON.stringify(json)}`); + throw new Error('Invalid data received'); + } + } catch (e) { + const errorMessage = `Error connecting to Enterprise Search: ${e?.message || e.toString()}`; - const body = await apiResponse.json(); + this.log.error(errorMessage); + if (e instanceof Error) this.log.debug(e.stack as string); - if (hasValidData(body)) { - return response.ok({ body }); - } else { - throw new Error(`Invalid data received: ${JSON.stringify(body)}`); + return response.customError({ statusCode: 502, body: errorMessage }); } - } catch (e) { - log.error(`Cannot connect to Enterprise Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); + }; + } - return response.customError({ - statusCode: 502, - body: 'Error connecting or fetching data from Enterprise Search', - }); - } - }; + // Small helper + isEmptyObj(obj: object) { + return Object.keys(obj).length === 0; + } } diff --git a/x-pack/plugins/enterprise_search/server/plugin.ts b/x-pack/plugins/enterprise_search/server/plugin.ts index ef8c72f0cbca5..617210a544262 100644 --- a/x-pack/plugins/enterprise_search/server/plugin.ts +++ b/x-pack/plugins/enterprise_search/server/plugin.ts @@ -26,6 +26,11 @@ import { } from '../common/constants'; import { ConfigType } from './'; import { checkAccess } from './lib/check_access'; +import { + EnterpriseSearchRequestHandler, + IEnterpriseSearchRequestHandler, +} from './lib/enterprise_search_request_handler'; + import { registerConfigDataRoute } from './routes/enterprise_search/config_data'; import { registerTelemetryRoute } from './routes/enterprise_search/telemetry'; @@ -48,6 +53,7 @@ export interface IRouteDependencies { router: IRouter; config: ConfigType; log: Logger; + enterpriseSearchRequestHandler: IEnterpriseSearchRequestHandler; getSavedObjectsService?(): SavedObjectsServiceStart; } @@ -65,6 +71,7 @@ export class EnterpriseSearchPlugin implements Plugin { { usageCollection, security, features }: PluginsSetup ) { const config = await this.config.pipe(first()).toPromise(); + const log = this.logger; /** * Register space/feature control @@ -84,7 +91,7 @@ export class EnterpriseSearchPlugin implements Plugin { * Register user access to the Enterprise Search plugins */ capabilities.registerSwitcher(async (request: KibanaRequest) => { - const dependencies = { config, security, request, log: this.logger }; + const dependencies = { config, security, request, log }; const { hasAppSearchAccess, hasWorkplaceSearchAccess } = await checkAccess(dependencies); @@ -105,7 +112,8 @@ export class EnterpriseSearchPlugin implements Plugin { * Register routes */ const router = http.createRouter(); - const dependencies = { router, config, log: this.logger }; + const enterpriseSearchRequestHandler = new EnterpriseSearchRequestHandler({ config, log }); + const dependencies = { router, config, log, enterpriseSearchRequestHandler }; registerConfigDataRoute(dependencies); registerEnginesRoute(dependencies); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts index 682c17aea6d52..000e6d63b5999 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts @@ -4,15 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerCredentialsRoutes } from './credentials'; -jest.mock('../../lib/enterprise_search_request_handler', () => ({ - createEnterpriseSearchRequestHandler: jest.fn(), -})); -import { createEnterpriseSearchRequestHandler } from '../../lib/enterprise_search_request_handler'; - describe('credentials routes', () => { describe('GET /api/app_search/credentials', () => { let mockRouter: MockRouter; @@ -22,16 +17,13 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerCredentialsRoutes({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - it('creates a handler with createEnterpriseSearchRequestHandler', () => { - expect(createEnterpriseSearchRequestHandler).toHaveBeenCalledWith({ - config: mockConfig, - log: mockLogger, + it('creates a request handler', () => { + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ path: '/as/credentials/collection', hasValidData: expect.any(Function), }); @@ -59,11 +51,7 @@ describe('credentials routes', () => { ], }; - const { - hasValidData, - } = (createEnterpriseSearchRequestHandler as jest.Mock).mock.calls[0][0]; - - expect(hasValidData(response)).toBe(true); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); it('should correctly validate that a response does not have data', () => { @@ -71,10 +59,7 @@ describe('credentials routes', () => { foo: 'bar', }; - const hasValidData = (createEnterpriseSearchRequestHandler as jest.Mock).mock.calls[0][0] - .hasValidData; - - expect(hasValidData(response)).toBe(false); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts index d9539692069f0..432f54c8e5b1c 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts @@ -7,7 +7,6 @@ import { schema } from '@kbn/config-schema'; import { IRouteDependencies } from '../../plugin'; -import { createEnterpriseSearchRequestHandler } from '../../lib/enterprise_search_request_handler'; interface ICredential { id: string; @@ -28,7 +27,10 @@ interface ICredentialsResponse { }; } -export function registerCredentialsRoutes({ router, config, log }: IRouteDependencies) { +export function registerCredentialsRoutes({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/app_search/credentials', @@ -38,9 +40,7 @@ export function registerCredentialsRoutes({ router, config, log }: IRouteDepende }), }, }, - createEnterpriseSearchRequestHandler({ - config, - log, + enterpriseSearchRequestHandler.createRequest({ path: '/as/credentials/collection', hasValidData: (body?: ICredentialsResponse) => { return Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number'; diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts index 03edab89d1b99..cd22ff98b01ce 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts @@ -4,16 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerEnginesRoute } from './engines'; -jest.mock('node-fetch'); -const fetch = jest.requireActual('node-fetch'); -const { Response } = fetch; -// eslint-disable-next-line @typescript-eslint/no-var-requires -const fetchMock = require('node-fetch') as jest.Mocked; - describe('engine routes', () => { describe('GET /api/app_search/engines', () => { const AUTH_HEADER = 'Basic 123'; @@ -34,71 +28,51 @@ describe('engine routes', () => { mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerEnginesRoute({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - describe('when the underlying App Search API returns a 200', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturn({ - results: [{ name: 'engine1' }], - meta: { page: { total_results: 1 } }, - }); - }); - - it('should return 200 with a list of engines from the App Search API', async () => { - await mockRouter.callRoute(mockRequest); + it('creates a request handler', () => { + mockRouter.callRoute(mockRequest); - expect(mockRouter.response.ok).toHaveBeenCalledWith({ - body: { results: [{ name: 'engine1' }], meta: { page: { total_results: 1 } } }, - }); + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/collection', + params: { type: 'indexed', 'page[current]': 1, 'page[size]': 10 }, + hasValidData: expect.any(Function), }); }); - describe('when the App Search URL is invalid', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturnError(); - }); - - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); + it('passes custom parameters to enterpriseSearchRequestHandler', () => { + mockRouter.callRoute({ query: { type: 'meta', pageIndex: 99 } }); - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith('Cannot connect to App Search: Failed'); - expect(mockLogger.debug).not.toHaveBeenCalled(); + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/collection', + params: { type: 'meta', 'page[current]': 99, 'page[size]': 10 }, + hasValidData: expect.any(Function), }); }); - describe('when the App Search API returns invalid data', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturnInvalidData(); + describe('hasValidData', () => { + it('should correctly validate that the response has data', () => { + const response = { + meta: { + page: { + total_results: 1, + }, + }, + results: [], + }; + + mockRouter.callRoute(mockRequest); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith( - 'Cannot connect to App Search: Error: Invalid data received from App Search: {"foo":"bar"}' - ); - expect(mockLogger.debug).toHaveBeenCalled(); + it('should correctly validate that a response does not have data', () => { + const response = {}; + + mockRouter.callRoute(mockRequest); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); @@ -128,36 +102,5 @@ describe('engine routes', () => { mockRouter.shouldThrow(request); }); }); - - const AppSearchAPI = { - shouldBeCalledWith(expectedUrl: string, expectedParams: object) { - return { - andReturn(response: object) { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify(response))); - }); - }, - andReturnInvalidData() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify({ foo: 'bar' }))); - }); - }, - andReturnError() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.reject('Failed'); - }); - }, - }; - }, - }; }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts index 7190772fb92bb..49fc5b100e0d1 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts @@ -4,14 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import fetch from 'node-fetch'; -import querystring from 'querystring'; import { schema } from '@kbn/config-schema'; import { IRouteDependencies } from '../../plugin'; import { ENGINES_PAGE_SIZE } from '../../../common/constants'; -export function registerEnginesRoute({ router, config, log }: IRouteDependencies) { +interface IEnginesResponse { + results: object[]; + meta: { page: { total_results: number } }; +} + +export function registerEnginesRoute({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/app_search/engines', @@ -23,37 +29,18 @@ export function registerEnginesRoute({ router, config, log }: IRouteDependencies }, }, async (context, request, response) => { - try { - const enterpriseSearchUrl = config.host as string; - const { type, pageIndex } = request.query; + const { type, pageIndex } = request.query; - const params = querystring.stringify({ + return enterpriseSearchRequestHandler.createRequest({ + path: '/as/engines/collection', + params: { type, 'page[current]': pageIndex, 'page[size]': ENGINES_PAGE_SIZE, - }); - const url = `${encodeURI(enterpriseSearchUrl)}/as/engines/collection?${params}`; - - const enginesResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); - - const engines = await enginesResponse.json(); - const hasValidData = - Array.isArray(engines?.results) && typeof engines?.meta?.page?.total_results === 'number'; - - if (hasValidData) { - return response.ok({ body: engines }); - } else { - // Either a completely incorrect Enterprise Search host URL was configured, or App Search is returning bad data - throw new Error(`Invalid data received from App Search: ${JSON.stringify(engines)}`); - } - } catch (e) { - log.error(`Cannot connect to App Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); - - return response.customError({ statusCode: 502, body: 'cannot-connect' }); - } + }, + hasValidData: (body?: IEnginesResponse) => + Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number', + })(context, request, response); } ); } diff --git a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts index daf0a1e895a61..acddd3539965a 100644 --- a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts @@ -5,7 +5,7 @@ */ import { loggingSystemMock, savedObjectsServiceMock } from 'src/core/server/mocks'; -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockLogger, mockDependencies } from '../../__mocks__'; jest.mock('../../collectors/lib/telemetry', () => ({ incrementUICounter: jest.fn(), @@ -28,10 +28,10 @@ describe('Enterprise Search Telemetry API', () => { mockRouter = new MockRouter({ method: 'put', payload: 'body' }); registerTelemetryRoute({ + ...mockDependencies, router: mockRouter.router, getSavedObjectsService: () => savedObjectsServiceMock.createStartContract(), log: mockLogger, - config: mockConfig, }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts index 69e8354e8b2f7..a9bd4020e74b7 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts @@ -4,127 +4,47 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerWSOverviewRoute } from './overview'; -jest.mock('node-fetch'); -const fetch = jest.requireActual('node-fetch'); -const { Response } = fetch; -// eslint-disable-next-line @typescript-eslint/no-var-requires -const fetchMock = require('node-fetch') as jest.Mocked; - -const ORG_ROUTE = 'http://localhost:3002/ws/org'; - -describe('engine routes', () => { +describe('Overview route', () => { describe('GET /api/workplace_search/overview', () => { - const AUTH_HEADER = 'Basic 123'; - const mockRequest = { - headers: { - authorization: AUTH_HEADER, - }, - query: {}, - }; - - const mockRouter = new MockRouter({ method: 'get', payload: 'query' }); + let mockRouter: MockRouter; beforeEach(() => { jest.clearAllMocks(); - mockRouter.createRouter(); + mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerWSOverviewRoute({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - describe('when the underlying Workplace Search API returns a 200', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturn({ accountsCount: 1 }); - }); - - it('should return 200 with a list of overview from the Workplace Search API', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.ok).toHaveBeenCalledWith({ - body: { accountsCount: 1 }, - headers: { 'content-type': 'application/json' }, - }); + it('creates a request handler', () => { + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/ws/org', + hasValidData: expect.any(Function), }); }); - describe('when the Workplace Search URL is invalid', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturnError(); - }); - - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith('Cannot connect to Workplace Search: Failed'); - expect(mockLogger.debug).not.toHaveBeenCalled(); - }); - }); + describe('hasValidData', () => { + it('should correctly validate that the response has data', () => { + const response = { + accountsCount: 1, + }; - describe('when the Workplace Search API returns invalid data', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturnInvalidData(); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); + it('should correctly validate that a response does not have data', () => { + const response = { + foo: 'bar', + }; - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith( - 'Cannot connect to Workplace Search: Error: Invalid data received from Workplace Search: {"foo":"bar"}' - ); - expect(mockLogger.debug).toHaveBeenCalled(); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); - - const WorkplaceSearchAPI = { - shouldBeCalledWith(expectedUrl: string, expectedParams: object) { - return { - andReturn(response: object) { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify(response))); - }); - }, - andReturnInvalidData() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify({ foo: 'bar' }))); - }); - }, - andReturnError() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.reject('Failed'); - }); - }, - }; - }, - }; }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts index 9e5d94ac1b4fe..8cfd65725a23a 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts @@ -4,43 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import fetch from 'node-fetch'; - import { IRouteDependencies } from '../../plugin'; -export function registerWSOverviewRoute({ router, config, log }: IRouteDependencies) { +export function registerWSOverviewRoute({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/workplace_search/overview', validate: false, }, - async (context, request, response) => { - try { - const entSearchUrl = config.host as string; - const url = `${encodeURI(entSearchUrl)}/ws/org`; - - const overviewResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); - - const body = await overviewResponse.json(); - const hasValidData = typeof body?.accountsCount === 'number'; - - if (hasValidData) { - return response.ok({ - body, - headers: { 'content-type': 'application/json' }, - }); - } else { - // Either a completely incorrect Enterprise Search host URL was configured, or Workplace Search is returning bad data - throw new Error(`Invalid data received from Workplace Search: ${JSON.stringify(body)}`); - } - } catch (e) { - log.error(`Cannot connect to Workplace Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); - - return response.customError({ statusCode: 502, body: 'cannot-connect' }); - } - } + enterpriseSearchRequestHandler.createRequest({ + path: '/ws/org', + hasValidData: (body: { accountsCount: number }) => typeof body?.accountsCount === 'number', + }) ); } From 1e20cd4cf44fe1395cc99277a6ac940adb236f49 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Fri, 28 Aug 2020 11:04:24 -0600 Subject: [PATCH 09/17] [Maps] add drilldown support map embeddable (#75598) * [Maps] add drilldown support * filter actions view * use i18n getter * remove unused changed * tslint fixes * more tslint changes * clean-up and API doc changes * update snapshots * do not show first drilldown in tooltip * add light grey line to seperate feature property rows * Improving tooltip row styles (#36) * Improving tooltip actions row styles * Removind unecessary comment * update snapshot and add functional test * switch UiActionsActionDefinition to Action and remove unneeded checks Co-authored-by: Elastic Machine Co-authored-by: Elizabet Oliveira --- ...-data-public.action_global_apply_filter.md | 11 ++ .../kibana-plugin-plugins-data-public.md | 1 + src/plugins/data/public/index.ts | 2 +- src/plugins/data/public/public.api.md | 5 + .../common/descriptor_types/map_descriptor.ts | 1 + .../geometry_filter_form.test.js.snap | 16 ++ .../public/components/_action_select.scss | 3 + .../maps/public/components/_index.scss | 1 + .../maps/public/components/action_select.tsx | 87 +++++++++++ .../components/distance_filter_form.tsx | 23 ++- .../public/components/geometry_filter_form.js | 17 ++ .../feature_properties.test.js.snap | 96 +++++++++++- .../map/features_tooltip/_index.scss | 12 +- .../feature_geometry_filter_form.js | 38 +---- .../features_tooltip/feature_properties.js | 145 +++++++++++++++--- .../feature_properties.test.js | 30 +++- .../map/features_tooltip/features_tooltip.js | 73 +++++++-- .../map/mb/draw_control/draw_control.js | 25 ++- .../map/mb/tooltip_control/tooltip_control.js | 2 + .../map/mb/tooltip_control/tooltip_popover.js | 2 + .../connected_components/map/mb/view.js | 2 + .../map_container/map_container.tsx | 16 +- .../toolbar_overlay/toolbar_overlay.js | 8 +- .../tools_control/tools_control.js | 6 + .../maps/public/embeddable/map_embeddable.tsx | 42 ++++- .../maps/embeddable/tooltip_filter_actions.js | 57 +++++-- .../es_archives/maps/kibana/data.json | 6 +- 27 files changed, 608 insertions(+), 119 deletions(-) create mode 100644 docs/development/plugins/data/public/kibana-plugin-plugins-data-public.action_global_apply_filter.md create mode 100644 x-pack/plugins/maps/public/components/_action_select.scss create mode 100644 x-pack/plugins/maps/public/components/action_select.tsx diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.action_global_apply_filter.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.action_global_apply_filter.md new file mode 100644 index 0000000000000..14075ba1beba0 --- /dev/null +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.action_global_apply_filter.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) > [ACTION\_GLOBAL\_APPLY\_FILTER](./kibana-plugin-plugins-data-public.action_global_apply_filter.md) + +## ACTION\_GLOBAL\_APPLY\_FILTER variable + +Signature: + +```typescript +ACTION_GLOBAL_APPLY_FILTER = "ACTION_GLOBAL_APPLY_FILTER" +``` diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.md index a5453c7c51d5b..09702df4fdb54 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.md @@ -89,6 +89,7 @@ | Variable | Description | | --- | --- | +| [ACTION\_GLOBAL\_APPLY\_FILTER](./kibana-plugin-plugins-data-public.action_global_apply_filter.md) | | | [AggGroupLabels](./kibana-plugin-plugins-data-public.agggrouplabels.md) | | | [AggGroupNames](./kibana-plugin-plugins-data-public.agggroupnames.md) | | | [baseFormattersPublic](./kibana-plugin-plugins-data-public.baseformatterspublic.md) | | diff --git a/src/plugins/data/public/index.ts b/src/plugins/data/public/index.ts index eb5703f1c63c1..27b16c57ffecf 100644 --- a/src/plugins/data/public/index.ts +++ b/src/plugins/data/public/index.ts @@ -440,7 +440,7 @@ export { export { isTimeRange, isQuery, isFilter, isFilters } from '../common'; -export { ApplyGlobalFilterActionContext } from './actions'; +export { ACTION_GLOBAL_APPLY_FILTER, ApplyGlobalFilterActionContext } from './actions'; export * from '../common/field_mapping'; diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 9a2a82e8ed206..de4ec58dfdab3 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -78,6 +78,11 @@ import { UnregisterCallback } from 'history'; import { UnwrapPromiseOrReturn } from '@kbn/utility-types'; import { UserProvidedValues } from 'src/core/server/types'; +// Warning: (ae-missing-release-tag) "ACTION_GLOBAL_APPLY_FILTER" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export const ACTION_GLOBAL_APPLY_FILTER = "ACTION_GLOBAL_APPLY_FILTER"; + // Warning: (ae-forgotten-export) The symbol "AggConfigSerialized" needs to be exported by the entry point index.d.ts // Warning: (ae-missing-release-tag) "AggConfigOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/x-pack/plugins/maps/common/descriptor_types/map_descriptor.ts b/x-pack/plugins/maps/common/descriptor_types/map_descriptor.ts index ed0656a2fc265..d064dfb1c4a37 100644 --- a/x-pack/plugins/maps/common/descriptor_types/map_descriptor.ts +++ b/x-pack/plugins/maps/common/descriptor_types/map_descriptor.ts @@ -53,6 +53,7 @@ export type TooltipState = { }; export type DrawState = { + actionId: string; drawType: DRAW_TYPE; filterLabel?: string; // point radius filter alias geoFieldName?: string; diff --git a/x-pack/plugins/maps/public/components/__snapshots__/geometry_filter_form.test.js.snap b/x-pack/plugins/maps/public/components/__snapshots__/geometry_filter_form.test.js.snap index 85a073c8d9ace..2d39a52dfe974 100644 --- a/x-pack/plugins/maps/public/components/__snapshots__/geometry_filter_form.test.js.snap +++ b/x-pack/plugins/maps/public/components/__snapshots__/geometry_filter_form.test.js.snap @@ -38,6 +38,10 @@ exports[`should not render relation select when geo field is geo_point 1`] = ` } } /> + @@ -121,6 +125,10 @@ exports[`should not show "within" relation when filter geometry is not closed 1` value="INTERSECTS" /> + @@ -177,6 +185,10 @@ exports[`should render error message 1`] = ` } } /> + @@ -267,6 +279,10 @@ exports[`should render relation select when geo field is geo_shape 1`] = ` value="INTERSECTS" /> + diff --git a/x-pack/plugins/maps/public/components/_action_select.scss b/x-pack/plugins/maps/public/components/_action_select.scss new file mode 100644 index 0000000000000..be903ad7d6962 --- /dev/null +++ b/x-pack/plugins/maps/public/components/_action_select.scss @@ -0,0 +1,3 @@ +.mapActionSelectIcon { + margin-right: $euiSizeS; +} diff --git a/x-pack/plugins/maps/public/components/_index.scss b/x-pack/plugins/maps/public/components/_index.scss index 76e27338bdcd4..76ce9f1bc79e3 100644 --- a/x-pack/plugins/maps/public/components/_index.scss +++ b/x-pack/plugins/maps/public/components/_index.scss @@ -1,3 +1,4 @@ +@import 'action_select'; @import 'metric_editors'; @import './geometry_filter'; @import 'tooltip_selector/tooltip_selector'; diff --git a/x-pack/plugins/maps/public/components/action_select.tsx b/x-pack/plugins/maps/public/components/action_select.tsx new file mode 100644 index 0000000000000..ad61a6a129974 --- /dev/null +++ b/x-pack/plugins/maps/public/components/action_select.tsx @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { Component } from 'react'; +import { EuiFormRow, EuiSuperSelect, EuiIcon } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { ActionExecutionContext, Action } from 'src/plugins/ui_actions/public'; + +interface Props { + value?: string; + onChange: (value: string) => void; + getFilterActions?: () => Promise; + getActionContext?: () => ActionExecutionContext; +} + +interface State { + actions: Action[]; +} + +export class ActionSelect extends Component { + private _isMounted = false; + state: State = { + actions: [], + }; + + componentDidMount() { + this._isMounted = true; + this._loadActions(); + } + + componentWillUnmount() { + this._isMounted = false; + } + + async _loadActions() { + if (!this.props.getFilterActions || !this.props.getActionContext) { + return; + } + const actions = await this.props.getFilterActions(); + if (this._isMounted) { + this.setState({ actions }); + } + } + + render() { + if (this.state.actions.length === 0 || !this.props.getActionContext) { + return null; + } + + if (this.state.actions.length === 1 && this.props.value === this.state.actions[0].id) { + return null; + } + + const actionContext = this.props.getActionContext(); + const options = this.state.actions.map((action) => { + const iconType = action.getIconType(actionContext); + return { + value: action.id, + inputDisplay: ( +
+ {iconType ? : null} + {action.getDisplayName(actionContext)} +
+ ), + }; + }); + + return ( + + + + ); + } +} diff --git a/x-pack/plugins/maps/public/components/distance_filter_form.tsx b/x-pack/plugins/maps/public/components/distance_filter_form.tsx index 768be1558bd69..24d9aec5b77b4 100644 --- a/x-pack/plugins/maps/public/components/distance_filter_form.tsx +++ b/x-pack/plugins/maps/public/components/distance_filter_form.tsx @@ -14,18 +14,25 @@ import { EuiTextAlign, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { ActionExecutionContext, Action } from 'src/plugins/ui_actions/public'; import { MultiIndexGeoFieldSelect } from './multi_index_geo_field_select'; import { GeoFieldWithIndex } from './geo_field_with_index'; +import { ActionSelect } from './action_select'; +import { ACTION_GLOBAL_APPLY_FILTER } from '../../../../../src/plugins/data/public'; interface Props { className?: string; buttonLabel: string; geoFields: GeoFieldWithIndex[]; + getFilterActions?: () => Promise; + getActionContext?: () => ActionExecutionContext; onSubmit: ({ + actionId, filterLabel, indexPatternId, geoFieldName, }: { + actionId: string; filterLabel: string; indexPatternId: string; geoFieldName: string; @@ -33,12 +40,14 @@ interface Props { } interface State { + actionId: string; selectedField: GeoFieldWithIndex | undefined; filterLabel: string; } export class DistanceFilterForm extends Component { - state = { + state: State = { + actionId: ACTION_GLOBAL_APPLY_FILTER, selectedField: this.props.geoFields.length ? this.props.geoFields[0] : undefined, filterLabel: '', }; @@ -53,11 +62,16 @@ export class DistanceFilterForm extends Component { }); }; + _onActionIdChange = (value: string) => { + this.setState({ actionId: value }); + }; + _onSubmit = () => { if (!this.state.selectedField) { return; } this.props.onSubmit({ + actionId: this.state.actionId, filterLabel: this.state.filterLabel, indexPatternId: this.state.selectedField.indexPatternId, geoFieldName: this.state.selectedField.geoFieldName, @@ -86,6 +100,13 @@ export class DistanceFilterForm extends Component { onChange={this._onGeoFieldChange} /> + + diff --git a/x-pack/plugins/maps/public/components/geometry_filter_form.js b/x-pack/plugins/maps/public/components/geometry_filter_form.js index d5cdda3c1c324..fde07e8c16bc5 100644 --- a/x-pack/plugins/maps/public/components/geometry_filter_form.js +++ b/x-pack/plugins/maps/public/components/geometry_filter_form.js @@ -20,11 +20,15 @@ import { i18n } from '@kbn/i18n'; import { ES_GEO_FIELD_TYPE, ES_SPATIAL_RELATIONS } from '../../common/constants'; import { getEsSpatialRelationLabel } from '../../common/i18n_getters'; import { MultiIndexGeoFieldSelect } from './multi_index_geo_field_select'; +import { ActionSelect } from './action_select'; +import { ACTION_GLOBAL_APPLY_FILTER } from '../../../../../src/plugins/data/public'; export class GeometryFilterForm extends Component { static propTypes = { buttonLabel: PropTypes.string.isRequired, geoFields: PropTypes.array.isRequired, + getFilterActions: PropTypes.func, + getActionContext: PropTypes.func, intitialGeometryLabel: PropTypes.string.isRequired, onSubmit: PropTypes.func.isRequired, isFilterGeometryClosed: PropTypes.bool, @@ -36,6 +40,7 @@ export class GeometryFilterForm extends Component { }; state = { + actionId: ACTION_GLOBAL_APPLY_FILTER, selectedField: this.props.geoFields.length ? this.props.geoFields[0] : undefined, geometryLabel: this.props.intitialGeometryLabel, relation: ES_SPATIAL_RELATIONS.INTERSECTS, @@ -57,8 +62,13 @@ export class GeometryFilterForm extends Component { }); }; + _onActionIdChange = (value) => { + this.setState({ actionId: value }); + }; + _onSubmit = () => { this.props.onSubmit({ + actionId: this.state.actionId, geometryLabel: this.state.geometryLabel, indexPatternId: this.state.selectedField.indexPatternId, geoFieldName: this.state.selectedField.geoFieldName, @@ -134,6 +144,13 @@ export class GeometryFilterForm extends Component { {this._renderRelationInput()} + + {error} diff --git a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/__snapshots__/feature_properties.test.js.snap b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/__snapshots__/feature_properties.test.js.snap index 3b3d82c92fbb7..29df06a64a3f2 100644 --- a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/__snapshots__/feature_properties.test.js.snap +++ b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/__snapshots__/feature_properties.test.js.snap @@ -1,11 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`FeatureProperties should not show filter button 1`] = ` +exports[`FeatureProperties should render 1`] = `
+
+
@@ -56,12 +60,13 @@ exports[`FeatureProperties should show error message if unable to load tooltip c `; -exports[`FeatureProperties should show only filter button for filterable properties 1`] = ` +exports[`FeatureProperties should show filter button for filterable properties 1`] = ` + +
- + > + +
+ +
+`; + +exports[`FeatureProperties should show view actions button when there are available actions 1`] = ` + + + + + + + + +
+ prop1 + + + + + + + + + + +
+ prop2 + +
diff --git a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/_index.scss b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/_index.scss index fb75cc1e2db69..abd747c8fa47a 100644 --- a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/_index.scss +++ b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/_index.scss @@ -1,6 +1,5 @@ .mapFeatureTooltip_table { width: 100%; - display: block; max-height: calc(49vh - #{$euiSizeXL * 2}); td { @@ -8,6 +7,10 @@ } } +.mapFeatureTooltip_row { + border-bottom: 1px solid $euiColorLightestShade; +} + .mapFeatureTooltip_actionLinks { padding: $euiSizeXS; } @@ -20,3 +23,10 @@ max-width: $euiSizeXL * 4; font-weight: $euiFontWeightSemiBold; } + +.mapFeatureTooltip_actionsRow { + > span { + display: flex; + justify-content: flex-end; + } +} diff --git a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js index b0ce52b4db7ab..98267965fd30f 100644 --- a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js +++ b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js @@ -4,9 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Component, Fragment } from 'react'; -import { EuiIcon } from '@elastic/eui'; -import { FormattedMessage } from '@kbn/i18n/react'; +import React, { Component } from 'react'; import { i18n } from '@kbn/i18n'; import { URL_MAX_LENGTH } from '../../../../../../../src/core/public'; @@ -95,28 +93,7 @@ export class FeatureGeometryFilterForm extends Component { this.props.onClose(); }; - _renderHeader() { - return ( - - ); - } - - _renderForm() { + render() { return ( ); } - - render() { - return ( - - {this._renderHeader()} - {this._renderForm()} - - ); - } } diff --git a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_properties.js b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_properties.js index 5e2a153b2ccbf..edd501f266690 100644 --- a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_properties.js +++ b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/feature_properties.js @@ -5,12 +5,21 @@ */ import React from 'react'; -import { EuiCallOut, EuiLoadingSpinner, EuiTextAlign, EuiButtonIcon } from '@elastic/eui'; +import { + EuiCallOut, + EuiLoadingSpinner, + EuiTextAlign, + EuiButtonEmpty, + EuiIcon, + EuiContextMenu, +} from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { ACTION_GLOBAL_APPLY_FILTER } from '../../../../../../../src/plugins/data/public'; export class FeatureProperties extends React.Component { state = { properties: null, + actions: [], loadPropertiesErrorMsg: null, prevWidth: null, prevHeight: null, @@ -21,6 +30,7 @@ export class FeatureProperties extends React.Component { this.prevLayerId = undefined; this.prevFeatureId = undefined; this._loadProperties(); + this._loadActions(); } componentDidUpdate() { @@ -31,6 +41,16 @@ export class FeatureProperties extends React.Component { this._isMounted = false; } + async _loadActions() { + if (!this.props.getFilterActions) { + return; + } + const actions = await this.props.getFilterActions(); + if (this._isMounted) { + this.setState({ actions }); + } + } + _loadProperties = async () => { this._fetchProperties({ nextFeatureId: this.props.featureId, @@ -39,6 +59,10 @@ export class FeatureProperties extends React.Component { }); }; + _showFilterActions = (tooltipProperty) => { + this.props.showFilterActions(this._renderFilterActions(tooltipProperty)); + }; + _fetchProperties = async ({ nextLayerId, nextFeatureId, mbProperties }) => { if (this.prevLayerId === nextLayerId && this.prevFeatureId === nextFeatureId) { // do not reload same feature properties @@ -83,35 +107,108 @@ export class FeatureProperties extends React.Component { } if (this._isMounted) { - this.setState({ - properties, - }); + this.setState({ properties }); } }; + _renderFilterActions(tooltipProperty) { + const panel = { + id: 0, + items: this.state.actions.map((action) => { + const actionContext = this.props.getActionContext(); + const iconType = action.getIconType(actionContext); + const name = action.getDisplayName(actionContext); + return { + name, + icon: iconType ? : null, + onClick: async () => { + this.props.onCloseTooltip(); + const filters = await tooltipProperty.getESFilters(); + this.props.addFilters(filters, action.id); + }, + ['data-test-subj']: `mapFilterActionButton__${name}`, + }; + }), + }; + + return ( +
+ (this._node = node)} + > + + + + + +
+ {tooltipProperty.getPropertyName()} + +
+ +
+ ); + } + _renderFilterCell(tooltipProperty) { if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { - return null; + return ; } - return ( - - { - this.props.onCloseTooltip(); - const filters = await tooltipProperty.getESFilters(); - this.props.addFilters(filters); - }} - aria-label={i18n.translate('xpack.maps.tooltip.filterOnPropertyAriaLabel', { - defaultMessage: 'Filter on property', - })} - data-test-subj="mapTooltipCreateFilterButton" - /> + const applyFilterButton = ( + { + this.props.onCloseTooltip(); + const filters = await tooltipProperty.getESFilters(); + this.props.addFilters(filters); + }} + aria-label={i18n.translate('xpack.maps.tooltip.filterOnPropertyAriaLabel', { + defaultMessage: 'Filter on property', + })} + data-test-subj="mapTooltipCreateFilterButton" + > + + + ); + + return this.state.actions.length === 0 || + (this.state.actions.length === 1 && + this.state.actions[0].id === ACTION_GLOBAL_APPLY_FILTER) ? ( + {applyFilterButton} + ) : ( + + + {applyFilterButton} + { + this._showFilterActions(tooltipProperty); + }} + aria-label={i18n.translate('xpack.maps.tooltip.viewActionsTitle', { + defaultMessage: 'View filter actions', + })} + data-test-subj="mapTooltipMoreActionsButton" + > + + + ); } @@ -154,7 +251,7 @@ export class FeatureProperties extends React.Component { const rows = this.state.properties.map((tooltipProperty) => { const label = tooltipProperty.getPropertyName(); return ( - + {label} {}, showFilterButtons: false, + getFilterActions: () => { + return [{ id: ACTION_GLOBAL_APPLY_FILTER }]; + }, }; const mockTooltipProperties = [ @@ -44,10 +48,29 @@ const mockTooltipProperties = [ ]; describe('FeatureProperties', () => { - test('should not show filter button', async () => { + test('should render', async () => { + const component = shallow( + { + return mockTooltipProperties; + }} + /> + ); + + // Ensure all promises resolve + await new Promise((resolve) => process.nextTick(resolve)); + // Ensure the state changes are reflected + component.update(); + + expect(component).toMatchSnapshot(); + }); + + test('should show filter button for filterable properties', async () => { const component = shallow( { return mockTooltipProperties; }} @@ -62,7 +85,7 @@ describe('FeatureProperties', () => { expect(component).toMatchSnapshot(); }); - test('should show only filter button for filterable properties', async () => { + test('should show view actions button when there are available actions', async () => { const component = shallow( { loadFeatureProperties={() => { return mockTooltipProperties; }} + getFilterActions={() => { + return [{ id: 'drilldown1' }]; + }} /> ); diff --git a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/features_tooltip.js b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/features_tooltip.js index d91bc8e803ab9..8547219b42e30 100644 --- a/x-pack/plugins/maps/public/connected_components/map/features_tooltip/features_tooltip.js +++ b/x-pack/plugins/maps/public/connected_components/map/features_tooltip/features_tooltip.js @@ -4,20 +4,22 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment } from 'react'; -import { EuiLink } from '@elastic/eui'; +import React, { Component, Fragment } from 'react'; +import { EuiIcon, EuiLink } from '@elastic/eui'; import { FeatureProperties } from './feature_properties'; -import { FormattedMessage } from '@kbn/i18n/react'; import { GEO_JSON_TYPE, ES_GEO_FIELD_TYPE } from '../../../../common/constants'; import { FeatureGeometryFilterForm } from './feature_geometry_filter_form'; import { TooltipHeader } from './tooltip_header'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; const VIEWS = { PROPERTIES_VIEW: 'PROPERTIES_VIEW', GEOMETRY_FILTER_VIEW: 'GEOMETRY_FILTER_VIEW', + FILTER_ACTIONS_VIEW: 'FILTER_ACTIONS_VIEW', }; -export class FeaturesTooltip extends React.Component { +export class FeaturesTooltip extends Component { state = {}; static getDerivedStateFromProps(nextProps, prevState) { @@ -41,7 +43,11 @@ export class FeaturesTooltip extends React.Component { }; _showPropertiesView = () => { - this.setState({ view: VIEWS.PROPERTIES_VIEW }); + this.setState({ view: VIEWS.PROPERTIES_VIEW, filterView: null }); + }; + + _showFilterActionsView = (filterView) => { + this.setState({ view: VIEWS.FILTER_ACTIONS_VIEW, filterView }); }; _renderActions(geoFields) { @@ -96,6 +102,22 @@ export class FeaturesTooltip extends React.Component { }); }; + _renderBackButton(label) { + return ( + + ); + } + render() { if (!this.state.currentFeature) { return null; @@ -109,14 +131,36 @@ export class FeaturesTooltip extends React.Component { if (this.state.view === VIEWS.GEOMETRY_FILTER_VIEW && currentFeatureGeometry) { return ( - + + {this._renderBackButton( + i18n.translate('xpack.maps.tooltip.showGeometryFilterViewLinkLabel', { + defaultMessage: 'Filter by geometry', + }) + )} + + + ); + } + + if (this.state.view === VIEWS.FILTER_ACTIONS_VIEW) { + return ( + + {this._renderBackButton( + i18n.translate('xpack.maps.tooltip.showAddFilterActionsViewLabel', { + defaultMessage: 'Filter actions', + }) + )} + {this.state.filterView} + ); } @@ -137,6 +181,9 @@ export class FeaturesTooltip extends React.Component { showFilterButtons={!!this.props.addFilters && this.props.isLocked} onCloseTooltip={this.props.closeTooltip} addFilters={this.props.addFilters} + getFilterActions={this.props.getFilterActions} + getActionContext={this.props.getActionContext} + showFilterActions={this._showFilterActionsView} /> {this._renderActions(geoFields)} diff --git a/x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js b/x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js index 6de936fa4a8f1..49675ac6a3924 100644 --- a/x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js +++ b/x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js @@ -62,11 +62,12 @@ export class DrawControl extends React.Component { } }, 0); - _onDraw = (e) => { + _onDraw = async (e) => { if (!e.features.length) { return; } + let filter; if (this.props.drawState.drawType === DRAW_TYPE.DISTANCE) { const circle = e.features[0]; const distanceKm = _.round( @@ -82,7 +83,7 @@ export class DrawControl extends React.Component { } else if (distanceKm <= 100) { precision = 3; } - const filter = createDistanceFilterWithMeta({ + filter = createDistanceFilterWithMeta({ alias: this.props.drawState.filterLabel, distanceKm, geoFieldName: this.props.drawState.geoFieldName, @@ -92,17 +93,12 @@ export class DrawControl extends React.Component { _.round(circle.properties.center[1], precision), ], }); - this.props.addFilters([filter]); - this.props.disableDrawState(); - return; - } - - const geometry = e.features[0].geometry; - // MapboxDraw returns coordinates with 12 decimals. Round to a more reasonable number - roundCoordinates(geometry.coordinates); + } else { + const geometry = e.features[0].geometry; + // MapboxDraw returns coordinates with 12 decimals. Round to a more reasonable number + roundCoordinates(geometry.coordinates); - try { - const filter = createSpatialFilterWithGeometry({ + filter = createSpatialFilterWithGeometry({ geometry: this.props.drawState.drawType === DRAW_TYPE.BOUNDS ? getBoundingBoxGeometry(geometry) @@ -113,7 +109,10 @@ export class DrawControl extends React.Component { geometryLabel: this.props.drawState.geometryLabel, relation: this.props.drawState.relation, }); - this.props.addFilters([filter]); + } + + try { + await this.props.addFilters([filter], this.props.drawState.actionId); } catch (error) { // TODO notify user why filter was not created console.error(error); diff --git a/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index 84a29db852539..87d6f8e1d8e71 100644 --- a/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -195,6 +195,8 @@ export class TooltipControl extends React.Component { mbMap={this.props.mbMap} layerList={this.props.layerList} addFilters={this.props.addFilters} + getFilterActions={this.props.getFilterActions} + getActionContext={this.props.getActionContext} renderTooltipContent={this.props.renderTooltipContent} geoFields={this.props.geoFields} features={features} diff --git a/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js b/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js index 6c42057680408..4cfddf0034039 100644 --- a/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js +++ b/x-pack/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js @@ -117,6 +117,8 @@ export class TooltipPopover extends Component { _renderTooltipContent = () => { const publicProps = { addFilters: this.props.addFilters, + getFilterActions: this.props.getFilterActions, + getActionContext: this.props.getActionContext, closeTooltip: this.props.closeTooltip, features: this.props.features, isLocked: this.props.isLocked, diff --git a/x-pack/plugins/maps/public/connected_components/map/mb/view.js b/x-pack/plugins/maps/public/connected_components/map/mb/view.js index 5a38f6039ae4b..22c374aceedd5 100644 --- a/x-pack/plugins/maps/public/connected_components/map/mb/view.js +++ b/x-pack/plugins/maps/public/connected_components/map/mb/view.js @@ -309,6 +309,8 @@ export class MBMap extends React.Component { diff --git a/x-pack/plugins/maps/public/connected_components/map_container/map_container.tsx b/x-pack/plugins/maps/public/connected_components/map_container/map_container.tsx index beb1eb0947c50..bf75c86ac249d 100644 --- a/x-pack/plugins/maps/public/connected_components/map_container/map_container.tsx +++ b/x-pack/plugins/maps/public/connected_components/map_container/map_container.tsx @@ -11,6 +11,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiCallOut } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import uuid from 'uuid/v4'; import { Filter } from 'src/plugins/data/public'; +import { ActionExecutionContext, Action } from 'src/plugins/ui_actions/public'; // @ts-expect-error import { MBMap } from '../map/mb'; // @ts-expect-error @@ -35,7 +36,9 @@ import 'mapbox-gl/dist/mapbox-gl.css'; const RENDER_COMPLETE_EVENT = 'renderComplete'; interface Props { - addFilters: ((filters: Filter[]) => void) | null; + addFilters: ((filters: Filter[]) => Promise) | null; + getFilterActions?: () => Promise; + getActionContext?: () => ActionExecutionContext; areLayersLoaded: boolean; cancelAllInFlightRequests: () => void; exitFullScreen: () => void; @@ -183,6 +186,8 @@ export class MapContainer extends Component { render() { const { addFilters, + getFilterActions, + getActionContext, flyoutDisplay, isFullScreen, exitFullScreen, @@ -230,11 +235,18 @@ export class MapContainer extends Component { {!this.props.hideToolbarOverlay && ( - + )} diff --git a/x-pack/plugins/maps/public/connected_components/toolbar_overlay/toolbar_overlay.js b/x-pack/plugins/maps/public/connected_components/toolbar_overlay/toolbar_overlay.js index a4f85163512f7..a9dc3f822060c 100644 --- a/x-pack/plugins/maps/public/connected_components/toolbar_overlay/toolbar_overlay.js +++ b/x-pack/plugins/maps/public/connected_components/toolbar_overlay/toolbar_overlay.js @@ -12,14 +12,18 @@ import { FitToData } from './fit_to_data'; export class ToolbarOverlay extends React.Component { _renderToolsControl() { - const { addFilters, geoFields } = this.props; + const { addFilters, geoFields, getFilterActions, getActionContext } = this.props; if (!addFilters || !geoFields.length) { return null; } return ( - + ); } diff --git a/x-pack/plugins/maps/public/connected_components/toolbar_overlay/tools_control/tools_control.js b/x-pack/plugins/maps/public/connected_components/toolbar_overlay/tools_control/tools_control.js index a06def086b861..017f0369e0b73 100644 --- a/x-pack/plugins/maps/public/connected_components/toolbar_overlay/tools_control/tools_control.js +++ b/x-pack/plugins/maps/public/connected_components/toolbar_overlay/tools_control/tools_control.js @@ -123,6 +123,8 @@ export class ToolsControl extends Component { className="mapDrawControl__geometryFilterForm" buttonLabel={DRAW_SHAPE_LABEL_SHORT} geoFields={this.props.geoFields} + getFilterActions={this.props.getFilterActions} + getActionContext={this.props.getActionContext} intitialGeometryLabel={i18n.translate( 'xpack.maps.toolbarOverlay.drawShape.initialGeometryLabel', { @@ -141,6 +143,8 @@ export class ToolsControl extends Component { className="mapDrawControl__geometryFilterForm" buttonLabel={DRAW_BOUNDS_LABEL_SHORT} geoFields={this.props.geoFields} + getFilterActions={this.props.getFilterActions} + getActionContext={this.props.getActionContext} intitialGeometryLabel={i18n.translate( 'xpack.maps.toolbarOverlay.drawBounds.initialGeometryLabel', { @@ -161,6 +165,8 @@ export class ToolsControl extends Component { geoFields={this.props.geoFields.filter(({ geoFieldType }) => { return geoFieldType === ES_GEO_FIELD_TYPE.GEO_POINT; })} + getFilterActions={this.props.getFilterActions} + getActionContext={this.props.getActionContext} onSubmit={this._initiateDistanceDraw} /> ), diff --git a/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx b/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx index 43ff274b1353f..1cb393bede956 100644 --- a/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx +++ b/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx @@ -11,7 +11,12 @@ import { render, unmountComponentAtNode } from 'react-dom'; import { Subscription } from 'rxjs'; import { Unsubscribe } from 'redux'; import { Embeddable, IContainer } from '../../../../../src/plugins/embeddable/public'; -import { APPLY_FILTER_TRIGGER } from '../../../../../src/plugins/ui_actions/public'; +import { ACTION_GLOBAL_APPLY_FILTER } from '../../../../../src/plugins/data/public'; +import { + APPLY_FILTER_TRIGGER, + ActionExecutionContext, + TriggerContextMapping, +} from '../../../../../src/plugins/ui_actions/public'; import { esFilters, TimeRange, @@ -99,6 +104,10 @@ export class MapEmbeddable extends Embeddable this.onContainerStateChanged(input)); } + supportedTriggers(): Array { + return [APPLY_FILTER_TRIGGER]; + } + setRenderTooltipContent = (renderTooltipContent: RenderToolTipContent) => { this._renderTooltipContent = renderTooltipContent; }; @@ -226,6 +235,8 @@ export class MapEmbeddable extends Embeddable @@ -243,13 +254,36 @@ export class MapEmbeddable extends Embeddable(replaceLayerList(this._layerList)); } - addFilters = (filters: Filter[]) => { - getUiActions().executeTriggerActions(APPLY_FILTER_TRIGGER, { - embeddable: this, + addFilters = async (filters: Filter[], actionId: string = ACTION_GLOBAL_APPLY_FILTER) => { + const executeContext = { + ...this.getActionContext(), filters, + }; + const action = getUiActions().getAction(actionId); + if (!action) { + throw new Error('Unable to apply filter, could not locate action'); + } + action.execute(executeContext); + }; + + getFilterActions = async () => { + return await getUiActions().getTriggerCompatibleActions(APPLY_FILTER_TRIGGER, { + embeddable: this, + filters: [], }); }; + getActionContext = () => { + const trigger = getUiActions().getTrigger(APPLY_FILTER_TRIGGER); + if (!trigger) { + throw new Error('Unable to get context, could not locate trigger'); + } + return { + embeddable: this, + trigger, + } as ActionExecutionContext; + }; + destroy() { super.destroy(); if (this._unsubscribeFromStore) { diff --git a/x-pack/test/functional/apps/maps/embeddable/tooltip_filter_actions.js b/x-pack/test/functional/apps/maps/embeddable/tooltip_filter_actions.js index a996910d4787a..10754d20118e9 100644 --- a/x-pack/test/functional/apps/maps/embeddable/tooltip_filter_actions.js +++ b/x-pack/test/functional/apps/maps/embeddable/tooltip_filter_actions.js @@ -13,31 +13,62 @@ export default function ({ getPageObjects, getService }) { const filterBar = getService('filterBar'); describe('tooltip filter actions', () => { - before(async () => { + async function loadDashboardAndOpenTooltip() { await kibanaServer.uiSettings.replace({ defaultIndex: 'c698b940-e149-11e8-a35a-370a8516603a', }); await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); await PageObjects.dashboard.loadSavedDashboard('dash for tooltip filter action test'); await PageObjects.maps.lockTooltipAtPosition(200, -200); - }); + } + + describe('apply filter to current view', () => { + before(async () => { + await loadDashboardAndOpenTooltip(); + }); + + it('should display create filter button when tooltip is locked', async () => { + const exists = await testSubjects.exists('mapTooltipCreateFilterButton'); + expect(exists).to.be(true); + }); + + it('should create filters when create filter button is clicked', async () => { + await testSubjects.click('mapTooltipCreateFilterButton'); + await testSubjects.click('applyFiltersPopoverButton'); + + // TODO: Fix me #64861 + // const hasSourceFilter = await filterBar.hasFilter('name', 'charlie'); + // expect(hasSourceFilter).to.be(true); - it('should display create filter button when tooltip is locked', async () => { - const exists = await testSubjects.exists('mapTooltipCreateFilterButton'); - expect(exists).to.be(true); + const hasJoinFilter = await filterBar.hasFilter('shape_name', 'charlie'); + expect(hasJoinFilter).to.be(true); + }); }); - it('should create filters when create filter button is clicked', async () => { - await testSubjects.click('mapTooltipCreateFilterButton'); - await testSubjects.click('applyFiltersPopoverButton'); + describe('panel actions', () => { + before(async () => { + await loadDashboardAndOpenTooltip(); + }); + + it('should display more actions button when tooltip is locked', async () => { + const exists = await testSubjects.exists('mapTooltipMoreActionsButton'); + expect(exists).to.be(true); + }); + + it('should trigger drilldown action when clicked', async () => { + await testSubjects.click('mapTooltipMoreActionsButton'); + await testSubjects.click('mapFilterActionButton__drilldown1'); - // TODO: Fix me #64861 - // const hasSourceFilter = await filterBar.hasFilter('name', 'charlie'); - // expect(hasSourceFilter).to.be(true); + // Assert on new dashboard with filter from action + await PageObjects.dashboard.waitForRenderComplete(); + const panelCount = await PageObjects.dashboard.getPanelCount(); + expect(panelCount).to.equal(2); - const hasJoinFilter = await filterBar.hasFilter('shape_name', 'charlie'); - expect(hasJoinFilter).to.be(true); + const hasJoinFilter = await filterBar.hasFilter('shape_name', 'charlie'); + expect(hasJoinFilter).to.be(true); + }); }); }); } diff --git a/x-pack/test/functional/es_archives/maps/kibana/data.json b/x-pack/test/functional/es_archives/maps/kibana/data.json index 198174bccb286..0f1fd3c09d706 100644 --- a/x-pack/test/functional/es_archives/maps/kibana/data.json +++ b/x-pack/test/functional/es_archives/maps/kibana/data.json @@ -1048,7 +1048,7 @@ "title" : "dash for tooltip filter action test", "hits" : 0, "description" : "Zoomed in so entire screen is covered by filter so click to open tooltip can not miss.", - "panelsJSON" : "[{\"gridData\":{\"x\":0,\"y\":0,\"w\":48,\"h\":26,\"i\":\"1\"},\"version\":\"8.0.0\",\"panelIndex\":\"1\",\"embeddableConfig\":{\"mapCenter\":{\"lat\":-1.31919,\"lon\":59.53306,\"zoom\":9.67},\"isLayerTOCOpen\":false,\"openTOCDetails\":[\"n1t6f\"]},\"panelRefName\":\"panel_0\"}]", + "panelsJSON" : "[{\"version\":\"8.0.0\",\"gridData\":{\"x\":0,\"y\":0,\"w\":48,\"h\":26,\"i\":\"1\"},\"panelIndex\":\"1\",\"embeddableConfig\":{\"mapCenter\":{\"lat\":-1.31919,\"lon\":59.53306,\"zoom\":9.67},\"isLayerTOCOpen\":false,\"openTOCDetails\":[\"n1t6f\"],\"hiddenLayers\":[],\"enhancements\":{\"dynamicActions\":{\"events\":[{\"eventId\":\"669a3521-1215-4228-9ced-77e2edf5ad17\",\"triggers\":[\"FILTER_TRIGGER\"],\"action\":{\"name\":\"drilldown1\",\"config\":{\"dashboardId\":\"19906970-2e40-11e9-85cb-6965aae20f13\",\"useCurrentFilters\":true,\"useCurrentDateRange\":true},\"factoryId\":\"DASHBOARD_TO_DASHBOARD_DRILLDOWN\"}}]}}},\"panelRefName\":\"panel_0\"}]", "optionsJSON" : "{\"useMargins\":true,\"hidePanelTitles\":false}", "version" : 1, "timeRestore" : true, @@ -1071,9 +1071,9 @@ } ], "migrationVersion" : { - "dashboard" : "7.0.0" + "dashboard" : "7.3.0" }, - "updated_at" : "2019-06-14T14:09:25.039Z" + "updated_at" : "2020-08-26T14:32:27.854Z" } } } From 5f781dcfecc9bcd96d9fa5ecfe5cd3e1cdedf094 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 31 Aug 2020 08:44:27 +0200 Subject: [PATCH 10/17] add client-side feature usage API (#75486) * add client-side feature_usage API * use route context for notify feature usage route --- x-pack/plugins/licensing/public/mocks.ts | 3 + x-pack/plugins/licensing/public/plugin.ts | 5 +- .../services/feature_usage_service.mock.ts | 45 ++++++++++ .../services/feature_usage_service.test.ts | 69 ++++++++++++++++ .../public/services/feature_usage_service.ts | 68 +++++++++++++++ .../licensing/public/services/index.ts | 11 +++ x-pack/plugins/licensing/public/types.ts | 9 ++ x-pack/plugins/licensing/server/plugin.ts | 6 +- .../plugins/licensing/server/routes/index.ts | 5 ++ .../licensing/server/routes/internal/index.ts | 8 ++ .../routes/internal/notify_feature_usage.ts | 32 ++++++++ .../routes/internal/register_feature.ts | 43 ++++++++++ .../licensing_plugin/public/feature_usage.ts | 82 +++++++++++++++++++ x-pack/test/licensing_plugin/public/index.ts | 1 + 14 files changed, 384 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugins/licensing/public/services/feature_usage_service.mock.ts create mode 100644 x-pack/plugins/licensing/public/services/feature_usage_service.test.ts create mode 100644 x-pack/plugins/licensing/public/services/feature_usage_service.ts create mode 100644 x-pack/plugins/licensing/public/services/index.ts create mode 100644 x-pack/plugins/licensing/server/routes/internal/index.ts create mode 100644 x-pack/plugins/licensing/server/routes/internal/notify_feature_usage.ts create mode 100644 x-pack/plugins/licensing/server/routes/internal/register_feature.ts create mode 100644 x-pack/test/licensing_plugin/public/feature_usage.ts diff --git a/x-pack/plugins/licensing/public/mocks.ts b/x-pack/plugins/licensing/public/mocks.ts index 8421a343d91ca..1ddde892de0d9 100644 --- a/x-pack/plugins/licensing/public/mocks.ts +++ b/x-pack/plugins/licensing/public/mocks.ts @@ -6,12 +6,14 @@ import { BehaviorSubject } from 'rxjs'; import { LicensingPluginSetup, LicensingPluginStart } from './types'; import { licenseMock } from '../common/licensing.mock'; +import { featureUsageMock } from './services/feature_usage_service.mock'; const createSetupMock = () => { const license = licenseMock.createLicense(); const mock: jest.Mocked = { license$: new BehaviorSubject(license), refresh: jest.fn(), + featureUsage: featureUsageMock.createSetup(), }; mock.refresh.mockResolvedValue(license); @@ -23,6 +25,7 @@ const createStartMock = () => { const mock: jest.Mocked = { license$: new BehaviorSubject(license), refresh: jest.fn(), + featureUsage: featureUsageMock.createStart(), }; mock.refresh.mockResolvedValue(license); diff --git a/x-pack/plugins/licensing/public/plugin.ts b/x-pack/plugins/licensing/public/plugin.ts index ec42a73f610c0..aa0c25364f2c7 100644 --- a/x-pack/plugins/licensing/public/plugin.ts +++ b/x-pack/plugins/licensing/public/plugin.ts @@ -6,12 +6,12 @@ import { Observable, Subject, Subscription } from 'rxjs'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/public'; - import { ILicense } from '../common/types'; import { LicensingPluginSetup, LicensingPluginStart } from './types'; import { createLicenseUpdate } from '../common/license_update'; import { License } from '../common/license'; import { mountExpiredBanner } from './expired_banner'; +import { FeatureUsageService } from './services'; export const licensingSessionStorageKey = 'xpack.licensing'; @@ -39,6 +39,7 @@ export class LicensingPlugin implements Plugin Promise; private license$?: Observable; + private featureUsage = new FeatureUsageService(); constructor( context: PluginInitializerContext, @@ -116,6 +117,7 @@ export class LicensingPlugin implements Plugin => { + const mock = { + register: jest.fn(), + }; + + return mock; +}; + +const createStartMock = (): jest.Mocked => { + const mock = { + notifyUsage: jest.fn(), + }; + + return mock; +}; + +const createServiceMock = (): jest.Mocked> => { + const mock = { + setup: jest.fn(), + start: jest.fn(), + }; + + mock.setup.mockImplementation(() => createSetupMock()); + mock.start.mockImplementation(() => createStartMock()); + + return mock; +}; + +export const featureUsageMock = { + create: createServiceMock, + createSetup: createSetupMock, + createStart: createStartMock, +}; diff --git a/x-pack/plugins/licensing/public/services/feature_usage_service.test.ts b/x-pack/plugins/licensing/public/services/feature_usage_service.test.ts new file mode 100644 index 0000000000000..eba2d1e67b509 --- /dev/null +++ b/x-pack/plugins/licensing/public/services/feature_usage_service.test.ts @@ -0,0 +1,69 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { httpServiceMock } from '../../../../../src/core/public/mocks'; +import { FeatureUsageService } from './feature_usage_service'; + +describe('FeatureUsageService', () => { + let http: ReturnType; + let service: FeatureUsageService; + + beforeEach(() => { + http = httpServiceMock.createSetupContract(); + service = new FeatureUsageService(); + }); + + describe('#setup', () => { + describe('#register', () => { + it('calls the endpoint with the correct parameters', async () => { + const setup = service.setup({ http }); + await setup.register('my-feature', 'platinum'); + expect(http.post).toHaveBeenCalledTimes(1); + expect(http.post).toHaveBeenCalledWith('/internal/licensing/feature_usage/register', { + body: JSON.stringify({ + featureName: 'my-feature', + licenseType: 'platinum', + }), + }); + }); + }); + }); + + describe('#start', () => { + describe('#notifyUsage', () => { + it('calls the endpoint with the correct parameters', async () => { + service.setup({ http }); + const start = service.start({ http }); + await start.notifyUsage('my-feature', 42); + + expect(http.post).toHaveBeenCalledTimes(1); + expect(http.post).toHaveBeenCalledWith('/internal/licensing/feature_usage/notify', { + body: JSON.stringify({ + featureName: 'my-feature', + lastUsed: 42, + }), + }); + }); + + it('correctly convert dates', async () => { + service.setup({ http }); + const start = service.start({ http }); + + const now = new Date(); + + await start.notifyUsage('my-feature', now); + + expect(http.post).toHaveBeenCalledTimes(1); + expect(http.post).toHaveBeenCalledWith('/internal/licensing/feature_usage/notify', { + body: JSON.stringify({ + featureName: 'my-feature', + lastUsed: now.getTime(), + }), + }); + }); + }); + }); +}); diff --git a/x-pack/plugins/licensing/public/services/feature_usage_service.ts b/x-pack/plugins/licensing/public/services/feature_usage_service.ts new file mode 100644 index 0000000000000..588d22eeb818d --- /dev/null +++ b/x-pack/plugins/licensing/public/services/feature_usage_service.ts @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import isDate from 'lodash/isDate'; +import type { HttpSetup, HttpStart } from 'src/core/public'; +import { LicenseType } from '../../common/types'; + +/** @public */ +export interface FeatureUsageServiceSetup { + /** + * Register a feature to be able to notify of it's usages using the {@link FeatureUsageServiceStart | service start contract}. + */ + register(featureName: string, licenseType: LicenseType): Promise; +} + +/** @public */ +export interface FeatureUsageServiceStart { + /** + * Notify of a registered feature usage at given time. + * + * @param featureName - the name of the feature to notify usage of + * @param usedAt - Either a `Date` or an unix timestamp with ms. If not specified, it will be set to the current time. + */ + notifyUsage(featureName: string, usedAt?: Date | number): Promise; +} + +interface SetupDeps { + http: HttpSetup; +} + +interface StartDeps { + http: HttpStart; +} + +/** + * @internal + */ +export class FeatureUsageService { + public setup({ http }: SetupDeps): FeatureUsageServiceSetup { + return { + register: async (featureName, licenseType) => { + await http.post('/internal/licensing/feature_usage/register', { + body: JSON.stringify({ + featureName, + licenseType, + }), + }); + }, + }; + } + + public start({ http }: StartDeps): FeatureUsageServiceStart { + return { + notifyUsage: async (featureName, usedAt = Date.now()) => { + const lastUsed = isDate(usedAt) ? usedAt.getTime() : usedAt; + await http.post('/internal/licensing/feature_usage/notify', { + body: JSON.stringify({ + featureName, + lastUsed, + }), + }); + }, + }; + } +} diff --git a/x-pack/plugins/licensing/public/services/index.ts b/x-pack/plugins/licensing/public/services/index.ts new file mode 100644 index 0000000000000..fc890dd3c927d --- /dev/null +++ b/x-pack/plugins/licensing/public/services/index.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { + FeatureUsageService, + FeatureUsageServiceSetup, + FeatureUsageServiceStart, +} from './feature_usage_service'; diff --git a/x-pack/plugins/licensing/public/types.ts b/x-pack/plugins/licensing/public/types.ts index 71a4a452d163d..43b146c51d9a8 100644 --- a/x-pack/plugins/licensing/public/types.ts +++ b/x-pack/plugins/licensing/public/types.ts @@ -6,6 +6,7 @@ import { Observable } from 'rxjs'; import { ILicense } from '../common/types'; +import { FeatureUsageServiceSetup, FeatureUsageServiceStart } from './services'; /** @public */ export interface LicensingPluginSetup { @@ -19,6 +20,10 @@ export interface LicensingPluginSetup { * @deprecated in favour of the counterpart provided from start contract */ refresh(): Promise; + /** + * APIs to register licensed feature usage. + */ + featureUsage: FeatureUsageServiceSetup; } /** @public */ @@ -31,4 +36,8 @@ export interface LicensingPluginStart { * Triggers licensing information re-fetch. */ refresh(): Promise; + /** + * APIs to manage licensed feature usage. + */ + featureUsage: FeatureUsageServiceStart; } diff --git a/x-pack/plugins/licensing/server/plugin.ts b/x-pack/plugins/licensing/server/plugin.ts index 6cdba0ac46644..2ee8d26419571 100644 --- a/x-pack/plugins/licensing/server/plugin.ts +++ b/x-pack/plugins/licensing/server/plugin.ts @@ -133,7 +133,9 @@ export class LicensingPlugin implements Plugin ) { registerInfoRoute(router); registerFeatureUsageRoute(router, getStartServices); + registerRegisterFeatureRoute(router, featureUsageSetup); + registerNotifyFeatureUsageRoute(router); } diff --git a/x-pack/plugins/licensing/server/routes/internal/index.ts b/x-pack/plugins/licensing/server/routes/internal/index.ts new file mode 100644 index 0000000000000..a3b06c223fc12 --- /dev/null +++ b/x-pack/plugins/licensing/server/routes/internal/index.ts @@ -0,0 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { registerNotifyFeatureUsageRoute } from './notify_feature_usage'; +export { registerRegisterFeatureRoute } from './register_feature'; diff --git a/x-pack/plugins/licensing/server/routes/internal/notify_feature_usage.ts b/x-pack/plugins/licensing/server/routes/internal/notify_feature_usage.ts new file mode 100644 index 0000000000000..ec70472574be3 --- /dev/null +++ b/x-pack/plugins/licensing/server/routes/internal/notify_feature_usage.ts @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { schema } from '@kbn/config-schema'; +import { IRouter } from 'src/core/server'; + +export function registerNotifyFeatureUsageRoute(router: IRouter) { + router.post( + { + path: '/internal/licensing/feature_usage/notify', + validate: { + body: schema.object({ + featureName: schema.string(), + lastUsed: schema.number(), + }), + }, + }, + async (context, request, response) => { + const { featureName, lastUsed } = request.body; + + context.licensing.featureUsage.notifyUsage(featureName, lastUsed); + + return response.ok({ + body: { + success: true, + }, + }); + } + ); +} diff --git a/x-pack/plugins/licensing/server/routes/internal/register_feature.ts b/x-pack/plugins/licensing/server/routes/internal/register_feature.ts new file mode 100644 index 0000000000000..14f7952f86f5a --- /dev/null +++ b/x-pack/plugins/licensing/server/routes/internal/register_feature.ts @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { schema } from '@kbn/config-schema'; +import { IRouter } from 'src/core/server'; +import { LicenseType, LICENSE_TYPE } from '../../../common/types'; +import { FeatureUsageServiceSetup } from '../../services'; + +export function registerRegisterFeatureRoute( + router: IRouter, + featureUsageSetup: FeatureUsageServiceSetup +) { + router.post( + { + path: '/internal/licensing/feature_usage/register', + validate: { + body: schema.object({ + featureName: schema.string(), + licenseType: schema.string({ + validate: (value) => { + if (!(value in LICENSE_TYPE)) { + return `Invalid license type: ${value}`; + } + }, + }), + }), + }, + }, + async (context, request, response) => { + const { featureName, licenseType } = request.body; + + featureUsageSetup.register(featureName, licenseType as LicenseType); + + return response.ok({ + body: { + success: true, + }, + }); + } + ); +} diff --git a/x-pack/test/licensing_plugin/public/feature_usage.ts b/x-pack/test/licensing_plugin/public/feature_usage.ts new file mode 100644 index 0000000000000..15d302d71bfab --- /dev/null +++ b/x-pack/test/licensing_plugin/public/feature_usage.ts @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../services'; +import { + LicensingPluginSetup, + LicensingPluginStart, + LicenseType, +} from '../../../plugins/licensing/public'; +import '../../../../test/plugin_functional/plugins/core_provider_plugin/types'; + +interface FeatureUsage { + last_used?: number; + license_level: LicenseType; + name: string; +} + +// eslint-disable-next-line import/no-default-export +export default function (ftrContext: FtrProviderContext) { + const { getService, getPageObjects } = ftrContext; + const supertest = getService('supertest'); + const browser = getService('browser'); + const PageObjects = getPageObjects(['common', 'security']); + + const registerFeature = async (featureName: string, licenseType: LicenseType) => { + await browser.executeAsync( + async (feature, type, cb) => { + const { setup } = window._coreProvider; + const licensing: LicensingPluginSetup = setup.plugins.licensing; + await licensing.featureUsage.register(feature, type); + cb(); + }, + featureName, + licenseType + ); + }; + + const notifyFeatureUsage = async (featureName: string, lastUsed: number) => { + await browser.executeAsync( + async (feature, time, cb) => { + const { start } = window._coreProvider; + const licensing: LicensingPluginStart = start.plugins.licensing; + await licensing.featureUsage.notifyUsage(feature, time); + cb(); + }, + featureName, + lastUsed + ); + }; + + describe('feature_usage API', () => { + before(async () => { + await PageObjects.security.login(); + }); + + it('allows to register features to the server', async () => { + await registerFeature('test-client-A', 'gold'); + await registerFeature('test-client-B', 'enterprise'); + + const response = await supertest.get('/api/licensing/feature_usage').expect(200); + const features = response.body.features.map(({ name }: FeatureUsage) => name); + + expect(features).to.contain('test-client-A'); + expect(features).to.contain('test-client-B'); + }); + + it('allows to notify feature usage', async () => { + const now = new Date(); + + await notifyFeatureUsage('test-client-A', now.getTime()); + + const response = await supertest.get('/api/licensing/feature_usage').expect(200); + const features = response.body.features as FeatureUsage[]; + + expect(features.find((f) => f.name === 'test-client-A')?.last_used).to.be(now.toISOString()); + expect(features.find((f) => f.name === 'test-client-B')?.last_used).to.be(null); + }); + }); +} diff --git a/x-pack/test/licensing_plugin/public/index.ts b/x-pack/test/licensing_plugin/public/index.ts index 86a3c21cfdb39..268a74c56bd72 100644 --- a/x-pack/test/licensing_plugin/public/index.ts +++ b/x-pack/test/licensing_plugin/public/index.ts @@ -10,6 +10,7 @@ import { FtrProviderContext } from '../services'; export default function ({ loadTestFile }: FtrProviderContext) { describe('Licensing plugin public client', function () { this.tags('ciGroup2'); + loadTestFile(require.resolve('./feature_usage')); // MUST BE LAST! CHANGES LICENSE TYPE! loadTestFile(require.resolve('./updates')); }); From 9ddd49a9f0d080dda5161eab0dd4f48d7baa9ebd Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 31 Aug 2020 12:40:44 +0300 Subject: [PATCH 11/17] Hides advanced json for count metric (#74636) * remove advanced json for count agg * Remove only advanced json from count agg * use Constant from data plugin * add the logic to data plugin * remove json arg from function definition * remove unecessary translations Co-authored-by: Elastic Machine --- .../data/common/search/aggs/agg_type.test.ts | 11 +++++++++++ src/plugins/data/common/search/aggs/agg_type.ts | 17 +++++++++++------ .../data/common/search/aggs/metrics/count.ts | 1 + .../common/search/aggs/metrics/count_fn.test.ts | 14 -------------- .../data/common/search/aggs/metrics/count_fn.ts | 12 +----------- .../public/components/agg_params_helper.ts | 2 +- .../apps/visualize/_point_series_options.js | 5 +++++ .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 9 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_type.test.ts b/src/plugins/data/common/search/aggs/agg_type.test.ts index 2fcc6b97b1cc6..bf1136159dfe8 100644 --- a/src/plugins/data/common/search/aggs/agg_type.test.ts +++ b/src/plugins/data/common/search/aggs/agg_type.test.ts @@ -99,6 +99,17 @@ describe('AggType Class', () => { expect(aggType.params[1].name).toBe('customLabel'); }); + test('disables json param', () => { + const aggType = new AggType({ + name: 'name', + title: 'title', + json: false, + }); + + expect(aggType.params.length).toBe(1); + expect(aggType.params[0].name).toBe('customLabel'); + }); + test('can disable customLabel', () => { const aggType = new AggType({ name: 'smart agg', diff --git a/src/plugins/data/common/search/aggs/agg_type.ts b/src/plugins/data/common/search/aggs/agg_type.ts index 0ba2bb66e7758..2ee604c1bf25d 100644 --- a/src/plugins/data/common/search/aggs/agg_type.ts +++ b/src/plugins/data/common/search/aggs/agg_type.ts @@ -47,6 +47,7 @@ export interface AggTypeConfig< getRequestAggs?: ((aggConfig: TAggConfig) => TAggConfig[]) | (() => TAggConfig[] | void); getResponseAggs?: ((aggConfig: TAggConfig) => TAggConfig[]) | (() => TAggConfig[] | void); customLabels?: boolean; + json?: boolean; decorateAggConfig?: () => any; postFlightRequest?: ( resp: any, @@ -235,13 +236,17 @@ export class AggType< if (config.params && config.params.length && config.params[0] instanceof BaseParamType) { this.params = config.params as TParam[]; } else { - // always append the raw JSON param + // always append the raw JSON param unless it is configured to false const params: any[] = config.params ? [...config.params] : []; - params.push({ - name: 'json', - type: 'json', - advanced: true, - }); + + if (config.json !== false) { + params.push({ + name: 'json', + type: 'json', + advanced: true, + }); + } + // always append custom label if (config.customLabels !== false) { diff --git a/src/plugins/data/common/search/aggs/metrics/count.ts b/src/plugins/data/common/search/aggs/metrics/count.ts index d990599586e81..9c9f36651f4d2 100644 --- a/src/plugins/data/common/search/aggs/metrics/count.ts +++ b/src/plugins/data/common/search/aggs/metrics/count.ts @@ -28,6 +28,7 @@ export const getCountMetricAgg = () => defaultMessage: 'Count', }), hasNoDsl: true, + json: false, makeLabel() { return i18n.translate('data.search.aggs.metrics.countLabel', { defaultMessage: 'Count', diff --git a/src/plugins/data/common/search/aggs/metrics/count_fn.test.ts b/src/plugins/data/common/search/aggs/metrics/count_fn.test.ts index 846feb9296fca..32189f07581e6 100644 --- a/src/plugins/data/common/search/aggs/metrics/count_fn.test.ts +++ b/src/plugins/data/common/search/aggs/metrics/count_fn.test.ts @@ -34,7 +34,6 @@ describe('agg_expression_functions', () => { "id": undefined, "params": Object { "customLabel": undefined, - "json": undefined, }, "schema": undefined, "type": "count", @@ -42,18 +41,5 @@ describe('agg_expression_functions', () => { } `); }); - - test('correctly parses json string argument', () => { - const actual = fn({ - json: '{ "foo": true }', - }); - - expect(actual.value.params.json).toEqual({ foo: true }); - expect(() => { - fn({ - json: '/// intentionally malformed json ///', - }); - }).toThrowErrorMatchingInlineSnapshot(`"Unable to parse json argument string"`); - }); }); }); diff --git a/src/plugins/data/common/search/aggs/metrics/count_fn.ts b/src/plugins/data/common/search/aggs/metrics/count_fn.ts index 338ca18209299..7d4616ffdc619 100644 --- a/src/plugins/data/common/search/aggs/metrics/count_fn.ts +++ b/src/plugins/data/common/search/aggs/metrics/count_fn.ts @@ -20,7 +20,6 @@ import { i18n } from '@kbn/i18n'; import { ExpressionFunctionDefinition } from 'src/plugins/expressions/common'; import { AggExpressionType, AggExpressionFunctionArgs, METRIC_TYPES } from '../'; -import { getParsedValue } from '../utils/get_parsed_value'; const fnName = 'aggCount'; @@ -55,12 +54,6 @@ export const aggCount = (): FunctionDefinition => ({ defaultMessage: 'Schema to use for this aggregation', }), }, - json: { - types: ['string'], - help: i18n.translate('data.search.aggs.metrics.count.json.help', { - defaultMessage: 'Advanced json to include when the agg is sent to Elasticsearch', - }), - }, customLabel: { types: ['string'], help: i18n.translate('data.search.aggs.metrics.count.customLabel.help', { @@ -78,10 +71,7 @@ export const aggCount = (): FunctionDefinition => ({ enabled, schema, type: METRIC_TYPES.COUNT, - params: { - ...rest, - json: getParsedValue(args, 'json'), - }, + params: rest, }, }; }, diff --git a/src/plugins/vis_default_editor/public/components/agg_params_helper.ts b/src/plugins/vis_default_editor/public/components/agg_params_helper.ts index ef2f937c8547c..b13ca32601aa9 100644 --- a/src/plugins/vis_default_editor/public/components/agg_params_helper.ts +++ b/src/plugins/vis_default_editor/public/components/agg_params_helper.ts @@ -26,7 +26,7 @@ import { IAggType, IndexPattern, IndexPatternField, -} from 'src/plugins/data/public'; +} from '../../../data/public'; import { filterAggTypes, filterAggTypeFields } from '../agg_filters'; import { groupAndSortBy, ComboBoxGroupedOptions } from '../utils'; import { AggTypeState, AggParamsState } from './agg_params_state'; diff --git a/test/functional/apps/visualize/_point_series_options.js b/test/functional/apps/visualize/_point_series_options.js index d08bfe3b90913..c88670ee8b741 100644 --- a/test/functional/apps/visualize/_point_series_options.js +++ b/test/functional/apps/visualize/_point_series_options.js @@ -24,6 +24,7 @@ export default function ({ getService, getPageObjects }) { const retry = getService('retry'); const kibanaServer = getService('kibanaServer'); const browser = getService('browser'); + const testSubjects = getService('testSubjects'); const PageObjects = getPageObjects([ 'visualize', 'header', @@ -148,6 +149,10 @@ export default function ({ getService, getPageObjects }) { }); }); + it('should not show advanced json for count agg', async function () { + await testSubjects.missingOrFail('advancedParams-1'); + }); + it('should put secondary axis on the right', async function () { const length = await PageObjects.visChart.getRightValueAxes(); expect(length).to.be(1); diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index dc07044ce8ed7..07b646df74b9f 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -1115,7 +1115,6 @@ "data.search.aggs.metrics.count.customLabel.help": "このアグリゲーションのカスタムラベルを表します", "data.search.aggs.metrics.count.enabled.help": "このアグリゲーションが有効かどうかを指定します", "data.search.aggs.metrics.count.id.help": "このアグリゲーションのID", - "data.search.aggs.metrics.count.json.help": "アグリゲーションがElasticsearchに送信されるときに含める高度なJSON", "data.search.aggs.metrics.count.schema.help": "このアグリゲーションで使用するスキーマ", "data.search.aggs.metrics.countLabel": "カウント", "data.search.aggs.metrics.countTitle": "カウント", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 144bc1cac1852..ffd7d0cfb0f87 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -1116,7 +1116,6 @@ "data.search.aggs.metrics.count.customLabel.help": "表示此聚合的定制标签", "data.search.aggs.metrics.count.enabled.help": "指定是否启用此聚合", "data.search.aggs.metrics.count.id.help": "此聚合的 ID", - "data.search.aggs.metrics.count.json.help": "聚合发送至 Elasticsearch 时要包括的高级 json", "data.search.aggs.metrics.count.schema.help": "要用于此聚合的方案", "data.search.aggs.metrics.countLabel": "计数", "data.search.aggs.metrics.countTitle": "计数", From aac57fb1a8494d386b37bdfe21ada3cd9c8d7494 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 31 Aug 2020 11:45:57 +0200 Subject: [PATCH 12/17] [ILM] Add support for frozen phase in UI (#75968) * ILM public side changes to support frozen phase This is a very faithful duplication of the cold phase. We are also excluding the snapshot action as well as the unfollow action as these are features that require higher than basic license privilege. * added "frozen" to the server side schema * add frozen phases component * fix i18n and update jest tests * Slight restructuring to phase types and fix copy paste issues. - also fixed TS error introduced after TS v4 upgrade (delete) * fix hot phase type and remove type constraint from error messages * update validation logic Co-authored-by: Elastic Machine --- .../extend_index_management.test.js.snap | 4 + .../public/application/constants/policy.ts | 11 + .../edit_policy/components/min_age_input.tsx | 11 +- .../components/node_allocation.tsx | 6 +- .../components/set_priority_input.tsx | 6 +- .../sections/edit_policy/edit_policy.tsx | 16 +- .../edit_policy/phases/cold_phase.tsx | 7 +- .../edit_policy/phases/delete_phase.tsx | 7 +- .../edit_policy/phases/frozen_phase.tsx | 210 ++++++++++++++++++ .../sections/edit_policy/phases/hot_phase.tsx | 7 +- .../sections/edit_policy/phases/index.ts | 1 + .../edit_policy/phases/warm_phase.tsx | 7 +- .../services/policies/cold_phase.ts | 4 +- .../services/policies/frozen_phase.ts | 161 ++++++++++++++ .../services/policies/policy_serialization.ts | 11 + .../services/policies/policy_validation.ts | 18 +- .../application/services/policies/types.ts | 66 ++++-- .../public/extend_index_management/index.js | 6 + .../api/policies/register_create_route.ts | 18 ++ 19 files changed, 527 insertions(+), 50 deletions(-) create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/frozen_phase.tsx create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/services/policies/frozen_phase.ts diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/__snapshots__/extend_index_management.test.js.snap b/x-pack/plugins/index_lifecycle_management/__jest__/__snapshots__/extend_index_management.test.js.snap index 38dd49a286b58..39eb54b941ac4 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/__snapshots__/extend_index_management.test.js.snap +++ b/x-pack/plugins/index_lifecycle_management/__jest__/__snapshots__/extend_index_management.test.js.snap @@ -76,6 +76,10 @@ Array [ "value": "warm", "view": "Warm", }, + Object { + "value": "frozen", + "view": "Frozen", + }, Object { "value": "cold", "view": "Cold", diff --git a/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts b/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts index 3a19f03547b5b..fb626e7d7fe76 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts @@ -10,6 +10,7 @@ import { DeletePhase, HotPhase, WarmPhase, + FrozenPhase, } from '../services/policies/types'; export const defaultNewHotPhase: HotPhase = { @@ -47,6 +48,16 @@ export const defaultNewColdPhase: ColdPhase = { phaseIndexPriority: '0', }; +export const defaultNewFrozenPhase: FrozenPhase = { + phaseEnabled: false, + selectedMinimumAge: '0', + selectedMinimumAgeUnits: 'd', + selectedNodeAttrs: '', + selectedReplicaCount: '', + freezeEnabled: false, + phaseIndexPriority: '0', +}; + export const defaultNewDeletePhase: DeletePhase = { phaseEnabled: false, selectedMinimumAge: '0', diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/min_age_input.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/min_age_input.tsx index 11b743ecc4bb6..5128ba1c881a0 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/min_age_input.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/min_age_input.tsx @@ -12,7 +12,7 @@ import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiSelect } from import { LearnMoreLink } from './learn_more_link'; import { ErrableFormRow } from './form_errors'; import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; -import { ColdPhase, DeletePhase, Phase, Phases, WarmPhase } from '../../../services/policies/types'; +import { PhaseWithMinAge, Phases } from '../../../services/policies/types'; function getTimingLabelForPhase(phase: keyof Phases) { // NOTE: Hot phase isn't necessary, because indices begin in the hot phase. @@ -27,6 +27,11 @@ function getTimingLabelForPhase(phase: keyof Phases) { defaultMessage: 'Timing for cold phase', }); + case 'frozen': + return i18n.translate('xpack.indexLifecycleMgmt.editPolicy.phaseFrozen.minimumAgeLabel', { + defaultMessage: 'Timing for frozen phase', + }); + case 'delete': return i18n.translate('xpack.indexLifecycleMgmt.editPolicy.phaseDelete.minimumAgeLabel', { defaultMessage: 'Timing for delete phase', @@ -63,7 +68,7 @@ function getUnitsAriaLabelForPhase(phase: keyof Phases) { } } -interface Props { +interface Props { rolloverEnabled: boolean; errors?: PhaseValidationErrors; phase: keyof Phases & string; @@ -72,7 +77,7 @@ interface Props { isShowingErrors: boolean; } -export const MinAgeInput = ({ +export const MinAgeInput = ({ rolloverEnabled, errors, phaseData, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx index 0ce2c0d7ea566..b4ff62bfb03dc 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx @@ -20,7 +20,7 @@ import { LearnMoreLink } from './learn_more_link'; import { ErrableFormRow } from './form_errors'; import { useLoadNodes } from '../../../services/api'; import { NodeAttrsDetails } from './node_attrs_details'; -import { ColdPhase, Phase, Phases, WarmPhase } from '../../../services/policies/types'; +import { PhaseWithAllocationAction, Phases } from '../../../services/policies/types'; import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; const learnMoreLink = ( @@ -38,14 +38,14 @@ const learnMoreLink = ( ); -interface Props { +interface Props { phase: keyof Phases & string; errors?: PhaseValidationErrors; phaseData: T; setPhaseData: (dataKey: keyof T & string, value: string) => void; isShowingErrors: boolean; } -export const NodeAllocation = ({ +export const NodeAllocation = ({ phase, setPhaseData, errors, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/set_priority_input.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/set_priority_input.tsx index 1da7508049f24..1505532a2b16e 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/set_priority_input.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/set_priority_input.tsx @@ -10,17 +10,17 @@ import { EuiFieldNumber, EuiTextColor, EuiDescribedFormGroup } from '@elastic/eu import { LearnMoreLink } from './'; import { OptionalLabel } from './'; import { ErrableFormRow } from './'; -import { ColdPhase, HotPhase, Phase, Phases, WarmPhase } from '../../../services/policies/types'; +import { PhaseWithIndexPriority, Phases } from '../../../services/policies/types'; import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; -interface Props { +interface Props { errors?: PhaseValidationErrors; phase: keyof Phases & string; phaseData: T; setPhaseData: (dataKey: keyof T & string, value: any) => void; isShowingErrors: boolean; } -export const SetPriorityInput = ({ +export const SetPriorityInput = ({ errors, phaseData, phase, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx index c99d01b546679..db58c64a8ae8c 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx @@ -28,7 +28,7 @@ import { import { toasts } from '../../services/notification'; -import { Policy, PolicyFromES } from '../../services/policies/types'; +import { Phases, Policy, PolicyFromES } from '../../services/policies/types'; import { validatePolicy, ValidationErrors, @@ -42,7 +42,7 @@ import { } from '../../services/policies/policy_serialization'; import { ErrableFormRow, LearnMoreLink, PolicyJsonFlyout } from './components'; -import { ColdPhase, DeletePhase, HotPhase, WarmPhase } from './phases'; +import { ColdPhase, DeletePhase, FrozenPhase, HotPhase, WarmPhase } from './phases'; interface Props { policies: PolicyFromES[]; @@ -118,7 +118,7 @@ export const EditPolicy: React.FunctionComponent = ({ setIsShowingPolicyJsonFlyout(!isShowingPolicyJsonFlyout); }; - const setPhaseData = (phase: 'hot' | 'warm' | 'cold' | 'delete', key: string, value: any) => { + const setPhaseData = (phase: keyof Phases, key: string, value: any) => { setPolicy({ ...policy, phases: { @@ -303,6 +303,16 @@ export const EditPolicy: React.FunctionComponent = ({ + 0} + setPhaseData={(key, value) => setPhaseData('frozen', key, value)} + phaseData={policy.phases.frozen} + hotPhaseRolloverEnabled={policy.phases.hot.rolloverEnabled} + /> + + + 0} diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/cold_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/cold_phase.tsx index fb32752fe24ea..9df6da7a88b2f 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/cold_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/cold_phase.tsx @@ -19,7 +19,7 @@ import { } from '@elastic/eui'; import { ColdPhase as ColdPhaseInterface, Phases } from '../../../services/policies/types'; -import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; import { LearnMoreLink, @@ -36,9 +36,8 @@ const freezeLabel = i18n.translate('xpack.indexLifecycleMgmt.coldPhase.freezeInd defaultMessage: 'Freeze index', }); -const coldProperty = propertyof('cold'); -const phaseProperty = (propertyName: keyof ColdPhaseInterface) => - propertyof(propertyName); +const coldProperty: keyof Phases = 'cold'; +const phaseProperty = (propertyName: keyof ColdPhaseInterface) => propertyName; interface Props { setPhaseData: (key: keyof ColdPhaseInterface & string, value: string | boolean) => void; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/delete_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/delete_phase.tsx index d3c73090f25f2..eab93777a72bd 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/delete_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/delete_phase.tsx @@ -9,7 +9,7 @@ import { FormattedMessage } from '@kbn/i18n/react'; import { EuiDescribedFormGroup, EuiSwitch, EuiTextColor, EuiFormRow } from '@elastic/eui'; import { DeletePhase as DeletePhaseInterface, Phases } from '../../../services/policies/types'; -import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; import { ActiveBadge, @@ -20,9 +20,8 @@ import { SnapshotPolicies, } from '../components'; -const deleteProperty = propertyof('delete'); -const phaseProperty = (propertyName: keyof DeletePhaseInterface) => - propertyof(propertyName); +const deleteProperty: keyof Phases = 'delete'; +const phaseProperty = (propertyName: keyof DeletePhaseInterface) => propertyName; interface Props { setPhaseData: (key: keyof DeletePhaseInterface & string, value: string | boolean) => void; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/frozen_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/frozen_phase.tsx new file mode 100644 index 0000000000000..782906a56a9ba --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/frozen_phase.tsx @@ -0,0 +1,210 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { PureComponent, Fragment } from 'react'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { i18n } from '@kbn/i18n'; + +import { + EuiFlexGroup, + EuiFlexItem, + EuiSpacer, + EuiFieldNumber, + EuiDescribedFormGroup, + EuiSwitch, + EuiTextColor, +} from '@elastic/eui'; + +import { FrozenPhase as FrozenPhaseInterface, Phases } from '../../../services/policies/types'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; + +import { + LearnMoreLink, + ActiveBadge, + PhaseErrorMessage, + OptionalLabel, + ErrableFormRow, + MinAgeInput, + NodeAllocation, + SetPriorityInput, +} from '../components'; + +const freezeLabel = i18n.translate('xpack.indexLifecycleMgmt.frozenPhase.freezeIndexLabel', { + defaultMessage: 'Freeze index', +}); + +const frozenProperty: keyof Phases = 'frozen'; +const phaseProperty = (propertyName: keyof FrozenPhaseInterface) => propertyName; + +interface Props { + setPhaseData: (key: keyof FrozenPhaseInterface & string, value: string | boolean) => void; + phaseData: FrozenPhaseInterface; + isShowingErrors: boolean; + errors?: PhaseValidationErrors; + hotPhaseRolloverEnabled: boolean; +} +export class FrozenPhase extends PureComponent { + render() { + const { + setPhaseData, + phaseData, + errors, + isShowingErrors, + hotPhaseRolloverEnabled, + } = this.props; + + return ( +
+ +

+ +

{' '} + {phaseData.phaseEnabled && !isShowingErrors ? : null} + +
+ } + titleSize="s" + description={ + +

+ +

+ + } + id={`${frozenProperty}-${phaseProperty('phaseEnabled')}`} + checked={phaseData.phaseEnabled} + onChange={(e) => { + setPhaseData(phaseProperty('phaseEnabled'), e.target.checked); + }} + aria-controls="frozenPhaseContent" + /> + + } + fullWidth + > + + {phaseData.phaseEnabled ? ( + + + errors={errors} + phaseData={phaseData} + phase={frozenProperty} + isShowingErrors={isShowingErrors} + setPhaseData={setPhaseData} + rolloverEnabled={hotPhaseRolloverEnabled} + /> + + + + phase={frozenProperty} + setPhaseData={setPhaseData} + errors={errors} + phaseData={phaseData} + isShowingErrors={isShowingErrors} + /> + + + + + + + + } + isShowingErrors={isShowingErrors} + errors={errors?.freezeEnabled} + helpText={i18n.translate( + 'xpack.indexLifecycleMgmt.frozenPhase.replicaCountHelpText', + { + defaultMessage: 'By default, the number of replicas remains the same.', + } + )} + > + { + setPhaseData(phaseProperty('selectedReplicaCount'), e.target.value); + }} + min={0} + /> + + + + + ) : ( +
+ )} + + + {phaseData.phaseEnabled ? ( + + + + + } + description={ + + {' '} + + + } + fullWidth + titleSize="xs" + > + { + setPhaseData(phaseProperty('freezeEnabled'), e.target.checked); + }} + label={freezeLabel} + aria-label={freezeLabel} + /> + + + errors={errors} + phaseData={phaseData} + phase={frozenProperty} + isShowingErrors={isShowingErrors} + setPhaseData={setPhaseData} + /> + + ) : null} +
+ ); + } +} diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx index 22f0114d16afe..106e3b9139a9b 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx @@ -19,7 +19,7 @@ import { } from '@elastic/eui'; import { HotPhase as HotPhaseInterface, Phases } from '../../../services/policies/types'; -import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; import { LearnMoreLink, @@ -112,9 +112,8 @@ const maxAgeUnits = [ }), }, ]; -const hotProperty = propertyof('hot'); -const phaseProperty = (propertyName: keyof HotPhaseInterface) => - propertyof(propertyName); +const hotProperty: keyof Phases = 'hot'; +const phaseProperty = (propertyName: keyof HotPhaseInterface) => propertyName; interface Props { errors?: PhaseValidationErrors; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/index.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/index.ts index 8d1ace5950497..d59f2ff6413fd 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/index.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/index.ts @@ -7,4 +7,5 @@ export { HotPhase } from './hot_phase'; export { WarmPhase } from './warm_phase'; export { ColdPhase } from './cold_phase'; +export { FrozenPhase } from './frozen_phase'; export { DeletePhase } from './delete_phase'; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx index f7b8c60a5c71f..2733d01ac222d 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx @@ -30,7 +30,7 @@ import { } from '../components'; import { Phases, WarmPhase as WarmPhaseInterface } from '../../../services/policies/types'; -import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; const shrinkLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.shrinkIndexLabel', { defaultMessage: 'Shrink index', @@ -47,9 +47,8 @@ const forcemergeLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.force defaultMessage: 'Force merge data', }); -const warmProperty = propertyof('warm'); -const phaseProperty = (propertyName: keyof WarmPhaseInterface) => - propertyof(propertyName); +const warmProperty: keyof Phases = 'warm'; +const phaseProperty = (propertyName: keyof WarmPhaseInterface) => propertyName; interface Props { setPhaseData: (key: keyof WarmPhaseInterface & string, value: boolean | string) => void; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/cold_phase.ts b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/cold_phase.ts index 6cc43042ed4ff..7fa82a004b872 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/cold_phase.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/cold_phase.ts @@ -152,9 +152,9 @@ export const validateColdPhase = (phase: ColdPhase): PhaseValidationErrors { + const phase = { ...frozenPhaseInitialization }; + if (phaseSerialized === undefined || phaseSerialized === null) { + return phase; + } + + phase.phaseEnabled = true; + + if (phaseSerialized.min_age) { + const { size: minAge, units: minAgeUnits } = splitSizeAndUnits(phaseSerialized.min_age); + phase.selectedMinimumAge = minAge; + phase.selectedMinimumAgeUnits = minAgeUnits; + } + + if (phaseSerialized.actions) { + const actions = phaseSerialized.actions; + if (actions.allocate) { + const allocate = actions.allocate; + if (allocate.require) { + Object.entries(allocate.require).forEach((entry) => { + phase.selectedNodeAttrs = entry.join(':'); + }); + if (allocate.number_of_replicas) { + phase.selectedReplicaCount = allocate.number_of_replicas.toString(); + } + } + } + + if (actions.freeze) { + phase.freezeEnabled = true; + } + + if (actions.set_priority) { + phase.phaseIndexPriority = actions.set_priority.priority + ? actions.set_priority.priority.toString() + : ''; + } + } + + return phase; +}; + +export const frozenPhaseToES = ( + phase: FrozenPhase, + originalPhase?: SerializedFrozenPhase +): SerializedFrozenPhase => { + if (!originalPhase) { + originalPhase = { ...serializedPhaseInitialization }; + } + + const esPhase = { ...originalPhase }; + + if (isNumber(phase.selectedMinimumAge)) { + esPhase.min_age = `${phase.selectedMinimumAge}${phase.selectedMinimumAgeUnits}`; + } + + esPhase.actions = esPhase.actions ? { ...esPhase.actions } : {}; + + if (phase.selectedNodeAttrs) { + const [name, value] = phase.selectedNodeAttrs.split(':'); + esPhase.actions.allocate = esPhase.actions.allocate || ({} as AllocateAction); + esPhase.actions.allocate.require = { + [name]: value, + }; + } else { + if (esPhase.actions.allocate) { + // @ts-expect-error + delete esPhase.actions.allocate.require; + } + } + + if (isNumber(phase.selectedReplicaCount)) { + esPhase.actions.allocate = esPhase.actions.allocate || ({} as AllocateAction); + esPhase.actions.allocate.number_of_replicas = parseInt(phase.selectedReplicaCount, 10); + } else { + if (esPhase.actions.allocate) { + // @ts-expect-error + delete esPhase.actions.allocate.number_of_replicas; + } + } + + if ( + esPhase.actions.allocate && + !esPhase.actions.allocate.require && + !isNumber(esPhase.actions.allocate.number_of_replicas) && + isEmpty(esPhase.actions.allocate.include) && + isEmpty(esPhase.actions.allocate.exclude) + ) { + // remove allocate action if it does not define require or number of nodes + // and both include and exclude are empty objects (ES will fail to parse if we don't) + delete esPhase.actions.allocate; + } + + if (phase.freezeEnabled) { + esPhase.actions.freeze = {}; + } else { + delete esPhase.actions.freeze; + } + + if (isNumber(phase.phaseIndexPriority)) { + esPhase.actions.set_priority = { + priority: parseInt(phase.phaseIndexPriority, 10), + }; + } else { + delete esPhase.actions.set_priority; + } + + return esPhase; +}; + +export const validateFrozenPhase = (phase: FrozenPhase): PhaseValidationErrors => { + if (!phase.phaseEnabled) { + return {}; + } + + const phaseErrors = {} as PhaseValidationErrors; + + // index priority is optional, but if it's set, it needs to be a positive number + if (phase.phaseIndexPriority) { + if (!isNumber(phase.phaseIndexPriority)) { + phaseErrors.phaseIndexPriority = [numberRequiredMessage]; + } else if (parseInt(phase.phaseIndexPriority, 10) < 0) { + phaseErrors.phaseIndexPriority = [positiveNumberRequiredMessage]; + } + } + + // min age needs to be a positive number + if (!isNumber(phase.selectedMinimumAge)) { + phaseErrors.selectedMinimumAge = [numberRequiredMessage]; + } else if (parseInt(phase.selectedMinimumAge, 10) < 0) { + phaseErrors.selectedMinimumAge = [positiveNumberRequiredMessage]; + } + + return { ...phaseErrors }; +}; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_serialization.ts b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_serialization.ts index 3953521df1817..807a6fe8ec395 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_serialization.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_serialization.ts @@ -9,6 +9,7 @@ import { defaultNewDeletePhase, defaultNewHotPhase, defaultNewWarmPhase, + defaultNewFrozenPhase, serializedPhaseInitialization, } from '../../constants'; @@ -17,6 +18,7 @@ import { Policy, PolicyFromES, SerializedPolicy } from './types'; import { hotPhaseFromES, hotPhaseToES } from './hot_phase'; import { warmPhaseFromES, warmPhaseToES } from './warm_phase'; import { coldPhaseFromES, coldPhaseToES } from './cold_phase'; +import { frozenPhaseFromES, frozenPhaseToES } from './frozen_phase'; import { deletePhaseFromES, deletePhaseToES } from './delete_phase'; export const splitSizeAndUnits = (field: string): { size: string; units: string } => { @@ -53,6 +55,7 @@ export const initializeNewPolicy = (newPolicyName: string = ''): Policy => { hot: { ...defaultNewHotPhase }, warm: { ...defaultNewWarmPhase }, cold: { ...defaultNewColdPhase }, + frozen: { ...defaultNewFrozenPhase }, delete: { ...defaultNewDeletePhase }, }, }; @@ -70,6 +73,7 @@ export const deserializePolicy = (policy: PolicyFromES): Policy => { hot: hotPhaseFromES(phases.hot), warm: warmPhaseFromES(phases.warm), cold: coldPhaseFromES(phases.cold), + frozen: frozenPhaseFromES(phases.frozen), delete: deletePhaseFromES(phases.delete), }, }; @@ -94,6 +98,13 @@ export const serializePolicy = ( serializedPolicy.phases.cold = coldPhaseToES(policy.phases.cold, originalEsPolicy.phases.cold); } + if (policy.phases.frozen.phaseEnabled) { + serializedPolicy.phases.frozen = frozenPhaseToES( + policy.phases.frozen, + originalEsPolicy.phases.frozen + ); + } + if (policy.phases.delete.phaseEnabled) { serializedPolicy.phases.delete = deletePhaseToES( policy.phases.delete, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_validation.ts b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_validation.ts index 545488be2cd5e..6fdbc4babd3f3 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_validation.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/policy_validation.ts @@ -9,7 +9,17 @@ import { validateHotPhase } from './hot_phase'; import { validateWarmPhase } from './warm_phase'; import { validateColdPhase } from './cold_phase'; import { validateDeletePhase } from './delete_phase'; -import { ColdPhase, DeletePhase, HotPhase, Phase, Policy, PolicyFromES, WarmPhase } from './types'; +import { validateFrozenPhase } from './frozen_phase'; + +import { + ColdPhase, + DeletePhase, + FrozenPhase, + HotPhase, + Policy, + PolicyFromES, + WarmPhase, +} from './types'; export const propertyof = (propertyName: keyof T & string) => propertyName; @@ -100,7 +110,7 @@ export const policyNameAlreadyUsedErrorMessage = i18n.translate( defaultMessage: 'That policy name is already used.', } ); -export type PhaseValidationErrors = { +export type PhaseValidationErrors = { [P in keyof Partial]: string[]; }; @@ -108,6 +118,7 @@ export interface ValidationErrors { hot: PhaseValidationErrors; warm: PhaseValidationErrors; cold: PhaseValidationErrors; + frozen: PhaseValidationErrors; delete: PhaseValidationErrors; policyName: string[]; } @@ -148,12 +159,14 @@ export const validatePolicy = ( const hotPhaseErrors = validateHotPhase(policy.phases.hot); const warmPhaseErrors = validateWarmPhase(policy.phases.warm); const coldPhaseErrors = validateColdPhase(policy.phases.cold); + const frozenPhaseErrors = validateFrozenPhase(policy.phases.frozen); const deletePhaseErrors = validateDeletePhase(policy.phases.delete); const isValid = policyNameErrors.length === 0 && Object.keys(hotPhaseErrors).length === 0 && Object.keys(warmPhaseErrors).length === 0 && Object.keys(coldPhaseErrors).length === 0 && + Object.keys(frozenPhaseErrors).length === 0 && Object.keys(deletePhaseErrors).length === 0; return [ isValid, @@ -162,6 +175,7 @@ export const validatePolicy = ( hot: hotPhaseErrors, warm: warmPhaseErrors, cold: coldPhaseErrors, + frozen: frozenPhaseErrors, delete: deletePhaseErrors, }, ]; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts index 2e2ed5b38bb87..3d4c73cf4a82c 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts @@ -13,6 +13,7 @@ export interface Phases { hot?: SerializedHotPhase; warm?: SerializedWarmPhase; cold?: SerializedColdPhase; + frozen?: SerializedFrozenPhase; delete?: SerializedDeletePhase; } @@ -68,6 +69,16 @@ export interface SerializedColdPhase extends SerializedPhase { }; } +export interface SerializedFrozenPhase extends SerializedPhase { + actions: { + freeze?: {}; + allocate?: AllocateAction; + set_priority?: { + priority: number | null; + }; + }; +} + export interface SerializedDeletePhase extends SerializedPhase { actions: { wait_for_snapshot?: { @@ -94,47 +105,66 @@ export interface Policy { hot: HotPhase; warm: WarmPhase; cold: ColdPhase; + frozen: FrozenPhase; delete: DeletePhase; }; } -export interface Phase { +export interface CommonPhaseSettings { phaseEnabled: boolean; } -export interface HotPhase extends Phase { + +export interface PhaseWithMinAge { + selectedMinimumAge: string; + selectedMinimumAgeUnits: string; +} + +export interface PhaseWithAllocationAction { + selectedNodeAttrs: string; + selectedReplicaCount: string; +} + +export interface PhaseWithIndexPriority { + phaseIndexPriority: string; +} + +export interface HotPhase extends CommonPhaseSettings, PhaseWithIndexPriority { rolloverEnabled: boolean; selectedMaxSizeStored: string; selectedMaxSizeStoredUnits: string; selectedMaxDocuments: string; selectedMaxAge: string; selectedMaxAgeUnits: string; - phaseIndexPriority: string; } -export interface WarmPhase extends Phase { +export interface WarmPhase + extends CommonPhaseSettings, + PhaseWithMinAge, + PhaseWithAllocationAction, + PhaseWithIndexPriority { warmPhaseOnRollover: boolean; - selectedMinimumAge: string; - selectedMinimumAgeUnits: string; - selectedNodeAttrs: string; - selectedReplicaCount: string; shrinkEnabled: boolean; selectedPrimaryShardCount: string; forceMergeEnabled: boolean; selectedForceMergeSegments: string; - phaseIndexPriority: string; } -export interface ColdPhase extends Phase { - selectedMinimumAge: string; - selectedMinimumAgeUnits: string; - selectedNodeAttrs: string; - selectedReplicaCount: string; +export interface ColdPhase + extends CommonPhaseSettings, + PhaseWithMinAge, + PhaseWithAllocationAction, + PhaseWithIndexPriority { freezeEnabled: boolean; - phaseIndexPriority: string; } -export interface DeletePhase extends Phase { - selectedMinimumAge: string; - selectedMinimumAgeUnits: string; +export interface FrozenPhase + extends CommonPhaseSettings, + PhaseWithMinAge, + PhaseWithAllocationAction, + PhaseWithIndexPriority { + freezeEnabled: boolean; +} + +export interface DeletePhase extends CommonPhaseSettings, PhaseWithMinAge { waitForSnapshotPolicy: string; } diff --git a/x-pack/plugins/index_lifecycle_management/public/extend_index_management/index.js b/x-pack/plugins/index_lifecycle_management/public/extend_index_management/index.js index a1eac5264bb6a..8d01f4a4c200e 100644 --- a/x-pack/plugins/index_lifecycle_management/public/extend_index_management/index.js +++ b/x-pack/plugins/index_lifecycle_management/public/extend_index_management/index.js @@ -176,6 +176,12 @@ export const ilmFilterExtension = (indices) => { defaultMessage: 'Warm', }), }, + { + value: 'frozen', + view: i18n.translate('xpack.indexLifecycleMgmt.indexMgmtFilter.frozenLabel', { + defaultMessage: 'Frozen', + }), + }, { value: 'cold', view: i18n.translate('xpack.indexLifecycleMgmt.indexMgmtFilter.coldLabel', { diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts index 2d02802119e47..9b51164fd4c28 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts @@ -104,6 +104,23 @@ const coldPhaseSchema = schema.maybe( }) ); +const frozenPhaseSchema = schema.maybe( + schema.object({ + min_age: minAgeSchema, + actions: schema.object({ + set_priority: setPrioritySchema, + unfollow: unfollowSchema, + allocate: allocateSchema, + freeze: schema.maybe(schema.object({})), // Freeze has no options + searchable_snapshot: schema.maybe( + schema.object({ + snapshot_repository: schema.string(), + }) + ), + }), + }) +); + const deletePhaseSchema = schema.maybe( schema.object({ min_age: minAgeSchema, @@ -129,6 +146,7 @@ const bodySchema = schema.object({ hot: hotPhaseSchema, warm: warmPhaseSchema, cold: coldPhaseSchema, + frozen: frozenPhaseSchema, delete: deletePhaseSchema, }), }); From 647f397c50a74cf72c268a432e01311745a5b303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Mon, 31 Aug 2020 10:46:29 +0100 Subject: [PATCH 13/17] [APM] Chart units don't update when toggling the chart legends (#74931) * changing unit when legend is disabled * changing unit when legend is disabled * show individual units in the tooltip * addressing PR comment * increasing duration threshold * change formatter based on available legends * addressing PR comment Co-authored-by: Elastic Machine --- .../shared/charts/CustomPlot/index.js | 5 + .../TransactionCharts/BrowserLineChart.tsx | 14 +- .../TransactionLineChart/index.tsx | 16 +- .../charts/TransactionCharts/helper.test.ts | 69 +++++ .../charts/TransactionCharts/helper.tsx | 35 +++ .../shared/charts/TransactionCharts/index.tsx | 277 ++++++------------ .../charts/TransactionCharts/ml_header.tsx | 96 ++++++ .../TransactionCharts/use_formatter.test.tsx | 109 +++++++ .../charts/TransactionCharts/use_formatter.ts | 30 ++ .../formatters/__test__/duration.test.ts | 7 +- .../apm/public/utils/formatters/duration.ts | 4 +- 11 files changed, 457 insertions(+), 205 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.test.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.ts diff --git a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js index 7e74961e57ea1..41925d651e361 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js @@ -79,6 +79,10 @@ export class InnerCustomPlot extends PureComponent { return i === _i ? !disabledValue : !!disabledValue; }); + if (typeof this.props.onToggleLegend === 'function') { + this.props.onToggleLegend(nextSeriesEnabledState); + } + return { seriesEnabledState: nextSeriesEnabledState, }; @@ -235,6 +239,7 @@ InnerCustomPlot.propTypes = { }) ), noHits: PropTypes.bool, + onToggleLegend: PropTypes.func, }; InnerCustomPlot.defaultProps = { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx index 0e19c63775d31..40caf35155918 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx @@ -4,17 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; -import { i18n } from '@kbn/i18n'; import { EuiTitle } from '@elastic/eui'; -import { TransactionLineChart } from './TransactionLineChart'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import { useAvgDurationByBrowser } from '../../../../hooks/useAvgDurationByBrowser'; +import { getDurationFormatter } from '../../../../utils/formatters'; import { - getMaxY, getResponseTimeTickFormatter, getResponseTimeTooltipFormatter, -} from '.'; -import { getDurationFormatter } from '../../../../utils/formatters'; -import { useAvgDurationByBrowser } from '../../../../hooks/useAvgDurationByBrowser'; + getMaxY, +} from './helper'; +import { TransactionLineChart } from './TransactionLineChart'; export function BrowserLineChart() { const { data } = useAvgDurationByBrowser(); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx index eaad883d2f9f6..07b7f01194d5c 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx @@ -5,22 +5,13 @@ */ import React, { useCallback } from 'react'; -import { - Coordinate, - RectCoordinate, -} from '../../../../../../typings/timeseries'; +import { Coordinate, TimeSeries } from '../../../../../../typings/timeseries'; import { useChartsSync } from '../../../../../hooks/useChartsSync'; // @ts-ignore import CustomPlot from '../../CustomPlot'; interface Props { - series: Array<{ - color: string; - title: React.ReactNode; - titleShort?: React.ReactNode; - data: Array; - type: string; - }>; + series: TimeSeries[]; truncateLegends?: boolean; tickFormatY: (y: number) => React.ReactNode; formatTooltipValue: (c: Coordinate) => React.ReactNode; @@ -28,6 +19,7 @@ interface Props { height?: number; stacked?: boolean; onHover?: () => void; + onToggleLegend?: (disabledSeriesState: boolean[]) => void; } function TransactionLineChart(props: Props) { @@ -40,6 +32,7 @@ function TransactionLineChart(props: Props) { truncateLegends, stacked = false, onHover, + onToggleLegend, } = props; const syncedChartsProps = useChartsSync(); @@ -66,6 +59,7 @@ function TransactionLineChart(props: Props) { height={height} truncateLegends={truncateLegends} {...(stacked ? { stackBy: 'y' } : {})} + onToggleLegend={onToggleLegend} /> ); } diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts new file mode 100644 index 0000000000000..a476892fa4a3f --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts @@ -0,0 +1,69 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { + getResponseTimeTickFormatter, + getResponseTimeTooltipFormatter, + getMaxY, +} from './helper'; +import { + getDurationFormatter, + toMicroseconds, +} from '../../../../utils/formatters'; +import { TimeSeries } from '../../../../../typings/timeseries'; + +describe('transaction chart helper', () => { + describe('getResponseTimeTickFormatter', () => { + it('formattes time tick in minutes', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'minutes')); + const timeTickFormatter = getResponseTimeTickFormatter(formatter); + expect(timeTickFormatter(toMicroseconds(60, 'seconds'))).toEqual( + '1.0 min' + ); + }); + it('formattes time tick in seconds', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'seconds')); + const timeTickFormatter = getResponseTimeTickFormatter(formatter); + expect(timeTickFormatter(toMicroseconds(6, 'seconds'))).toEqual('6.0 s'); + }); + }); + describe('getResponseTimeTooltipFormatter', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'minutes')); + const tooltipFormatter = getResponseTimeTooltipFormatter(formatter); + it("doesn't format invalid y coordinate", () => { + expect(tooltipFormatter({ x: 1, y: undefined })).toEqual('N/A'); + expect(tooltipFormatter({ x: 1, y: null })).toEqual('N/A'); + }); + it('formattes tooltip in minutes', () => { + expect( + tooltipFormatter({ x: 1, y: toMicroseconds(60, 'seconds') }) + ).toEqual('1.0 min'); + }); + }); + describe('getMaxY', () => { + it('returns zero when empty time series', () => { + expect(getMaxY([])).toEqual(0); + }); + it('returns zero for invalid y coordinate', () => { + const timeSeries = ([ + { data: [{ x: 1 }, { x: 2 }, { x: 3, y: -1 }] }, + ] as unknown) as TimeSeries[]; + expect(getMaxY(timeSeries)).toEqual(0); + }); + it('returns the max y coordinate', () => { + const timeSeries = ([ + { + data: [ + { x: 1, y: 10 }, + { x: 2, y: 5 }, + { x: 3, y: 1 }, + ], + }, + ] as unknown) as TimeSeries[]; + expect(getMaxY(timeSeries)).toEqual(10); + }); + }); +}); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx new file mode 100644 index 0000000000000..f11a33f932553 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { flatten } from 'lodash'; +import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; +import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; +import { TimeSeries, Coordinate } from '../../../../../typings/timeseries'; +import { TimeFormatter } from '../../../../utils/formatters'; + +export function getResponseTimeTickFormatter(formatter: TimeFormatter) { + return (t: number) => { + return formatter(t).formatted; + }; +} + +export function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { + return (coordinate: Coordinate) => { + return isValidCoordinateValue(coordinate.y) + ? formatter(coordinate.y).formatted + : NOT_AVAILABLE_LABEL; + }; +} + +export function getMaxY(timeSeries: TimeSeries[]) { + const coordinates = flatten( + timeSeries.map((serie: TimeSeries) => serie.data as Coordinate[]) + ); + + const numbers: number[] = coordinates.map((c: Coordinate) => (c.y ? c.y : 0)); + + return Math.max(...numbers, 0); +} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 1f80dbf5f4d95..d11925dc0303d 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -8,38 +8,34 @@ import { EuiFlexGrid, EuiFlexGroup, EuiFlexItem, - EuiIconTip, EuiPanel, - EuiText, - EuiTitle, EuiSpacer, + EuiTitle, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; -import React, { Component } from 'react'; -import { isEmpty, flatten } from 'lodash'; -import styled from 'styled-components'; +import React from 'react'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; -import { Coordinate, TimeSeries } from '../../../../../typings/timeseries'; -import { ITransactionChartData } from '../../../../selectors/chartSelectors'; -import { IUrlParams } from '../../../../context/UrlParamsContext/types'; import { - tpmUnit, - TimeFormatter, - getDurationFormatter, - asDecimal, -} from '../../../../utils/formatters'; -import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; + TRANSACTION_PAGE_LOAD, + TRANSACTION_REQUEST, + TRANSACTION_ROUTE_CHANGE, +} from '../../../../../common/transaction_types'; +import { Coordinate } from '../../../../../typings/timeseries'; import { LicenseContext } from '../../../../context/LicenseContext'; -import { TransactionLineChart } from './TransactionLineChart'; +import { IUrlParams } from '../../../../context/UrlParamsContext/types'; +import { ITransactionChartData } from '../../../../selectors/chartSelectors'; +import { asDecimal, tpmUnit } from '../../../../utils/formatters'; import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; import { BrowserLineChart } from './BrowserLineChart'; import { DurationByCountryMap } from './DurationByCountryMap'; import { - TRANSACTION_PAGE_LOAD, - TRANSACTION_ROUTE_CHANGE, - TRANSACTION_REQUEST, -} from '../../../../../common/transaction_types'; + getResponseTimeTickFormatter, + getResponseTimeTooltipFormatter, +} from './helper'; +import { MLHeader } from './ml_header'; +import { TransactionLineChart } from './TransactionLineChart'; +import { useFormatter } from './use_formatter'; interface TransactionChartProps { charts: ITransactionChartData; @@ -47,181 +43,96 @@ interface TransactionChartProps { urlParams: IUrlParams; } -const ShiftedIconWrapper = styled.span` - padding-right: 5px; - position: relative; - top: -1px; - display: inline-block; -`; - -const ShiftedEuiText = styled(EuiText)` - position: relative; - top: 5px; -`; - -export function getResponseTimeTickFormatter(formatter: TimeFormatter) { - return (t: number) => formatter(t).formatted; -} - -export function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { - return (p: Coordinate) => { - return isValidCoordinateValue(p.y) - ? formatter(p.y).formatted - : NOT_AVAILABLE_LABEL; - }; -} - -export function getMaxY(responseTimeSeries: TimeSeries[]) { - const coordinates = flatten( - responseTimeSeries.map((serie: TimeSeries) => serie.data as Coordinate[]) - ); - - const numbers: number[] = coordinates.map((c: Coordinate) => (c.y ? c.y : 0)); - - return Math.max(...numbers, 0); -} - -export class TransactionCharts extends Component { - public getTPMFormatter = (t: number) => { - const { urlParams } = this.props; +export function TransactionCharts({ + charts, + location, + urlParams, +}: TransactionChartProps) { + const getTPMFormatter = (t: number) => { const unit = tpmUnit(urlParams.transactionType); return `${asDecimal(t)} ${unit}`; }; - public getTPMTooltipFormatter = (p: Coordinate) => { + const getTPMTooltipFormatter = (p: Coordinate) => { return isValidCoordinateValue(p.y) - ? this.getTPMFormatter(p.y) + ? getTPMFormatter(p.y) : NOT_AVAILABLE_LABEL; }; - public renderMLHeader(hasValidMlLicense: boolean | undefined) { - const { mlJobId } = this.props.charts; - - if (!hasValidMlLicense || !mlJobId) { - return null; - } - - const { serviceName, kuery, transactionType } = this.props.urlParams; - if (!serviceName) { - return null; - } + const { transactionType } = urlParams; - const hasKuery = !isEmpty(kuery); - const icon = hasKuery ? ( - - ) : ( - - ); - - return ( - - - {icon} - - {i18n.translate( - 'xpack.apm.metrics.transactionChart.machineLearningLabel', - { - defaultMessage: 'Machine learning:', - } - )}{' '} - - - View Job - - - - ); - } + const { responseTimeSeries, tpmSeries } = charts; - public render() { - const { charts, urlParams } = this.props; - const { responseTimeSeries, tpmSeries } = charts; - const { transactionType } = urlParams; - const maxY = getMaxY(responseTimeSeries); - const formatter = getDurationFormatter(maxY); + const { formatter, setDisabledSeriesState } = useFormatter( + responseTimeSeries + ); - return ( - <> - - - - - - - - {responseTimeLabel(transactionType)} - - - - {(license) => - this.renderMLHeader(license?.getFeature('ml').isAvailable) - } - - - + + + + + + + + {responseTimeLabel(transactionType)} + + + + {(license) => ( + )} - /> - - - - - - - - - {tpmLabel(transactionType)} - - - - - - - {transactionType === TRANSACTION_PAGE_LOAD && ( - <> - - - - - - - - - - - - - - - )} - - ); - } + + + + + + + + + + + + {tpmLabel(transactionType)} + + + + + + + {transactionType === TRANSACTION_PAGE_LOAD && ( + <> + + + + + + + + + + + + + + + )} + + ); } function tpmLabel(type?: string) { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx new file mode 100644 index 0000000000000..f829b5841efa9 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EuiIconTip } from '@elastic/eui'; +import { isEmpty } from 'lodash'; +import React from 'react'; +import { EuiFlexItem } from '@elastic/eui'; +import styled from 'styled-components'; +import { i18n } from '@kbn/i18n'; +import { EuiText } from '@elastic/eui'; +import { useUrlParams } from '../../../../hooks/useUrlParams'; +import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; + +interface Props { + hasValidMlLicense?: boolean; + mlJobId?: string; +} + +const ShiftedIconWrapper = styled.span` + padding-right: 5px; + position: relative; + top: -1px; + display: inline-block; +`; + +const ShiftedEuiText = styled(EuiText)` + position: relative; + top: 5px; +`; + +export function MLHeader({ hasValidMlLicense, mlJobId }: Props) { + const { urlParams } = useUrlParams(); + + if (!hasValidMlLicense || !mlJobId) { + return null; + } + + const { serviceName, kuery, transactionType } = urlParams; + if (!serviceName) { + return null; + } + + const hasKuery = !isEmpty(kuery); + const icon = hasKuery ? ( +