From cbae36e0d24fdc0f551b7ec7b16842ac115b7194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Mon, 9 Sep 2024 21:07:56 +0200 Subject: [PATCH 1/4] [Feature] Add the ability to configure record limit/count from the UI (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(components/report-settings): add field for variable record limits Signed-off-by: Károly Szakály * fix(components/report-settings): set type for event target Signed-off-by: Károly Szakály * feat(components/report-settings): load existing record limit in edit mode Signed-off-by: Károly Szakály * feat(components/report-definition-details): show record limit on report definition details page Signed-off-by: Károly Szakály * feat(components/report-definition-details): omit record limit for non saved search sources Signed-off-by: Károly Szakály * feat(components/report-details): show record limit on report details page Signed-off-by: Károly Szakály * tests(cypress): add `force: true` to name input Signed-off-by: Károly Szakály * tests(jest/snapshots): update snapshots Signed-off-by: Károly Szakály * feat(components/report-settings): add proper i18n label for record limit input Signed-off-by: Károly Szakály * Update release notes Signed-off-by: Simeon Widdis * Add potential OOM warning for large record limits Signed-off-by: Simeon Widdis * Make memory warning more conservative based on original limit Signed-off-by: Simeon Widdis --------- Signed-off-by: Károly Szakály Signed-off-by: Simeon Widdis Co-authored-by: Simeon Widdis --- .cypress/integration/02-edit.spec.ts | 2 +- .../report_definition_details.tsx | 13 ++++ .../main/report_details/report_details.tsx | 15 +++++ .../report_settings/report_settings.tsx | 65 +++++++++++++++++-- ...boards-reporting.release-notes-2.17.0.0.md | 1 + 5 files changed, 91 insertions(+), 5 deletions(-) diff --git a/.cypress/integration/02-edit.spec.ts b/.cypress/integration/02-edit.spec.ts index a87e77d4..23a48b56 100644 --- a/.cypress/integration/02-edit.spec.ts +++ b/.cypress/integration/02-edit.spec.ts @@ -24,7 +24,7 @@ describe('Cypress', () => { cy.wait(1000); // update the report name - cy.get('#reportSettingsName').type(' update name'); + cy.get('#reportSettingsName').type(' update name', { force: true }); // update report description cy.get('#reportSettingsDescription').type(' update description'); diff --git a/public/components/main/report_definition_details/report_definition_details.tsx b/public/components/main/report_definition_details/report_definition_details.tsx index e3e41904..8f87f3e5 100644 --- a/public/components/main/report_definition_details/report_definition_details.tsx +++ b/public/components/main/report_definition_details/report_definition_details.tsx @@ -45,6 +45,7 @@ interface ReportDefinitionDetails { created: string; lastUpdated: string; source: string; + recordLimit: number | undefined; timePeriod: string; fileFormat: string; status: string | undefined; @@ -63,6 +64,7 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: '', lastUpdated: '', source: '', + recordLimit: 0, timePeriod: '', fileFormat: '', status: '', @@ -417,6 +419,10 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: displayCreatedDate, lastUpdated: displayUpdatedDate, source: reportParams.report_source, + recordLimit: + reportParams.report_source != 'Saved search' + ? `\u2014` + : reportParams.core_params.limit, baseUrl: baseUrl, // TODO: need better display timePeriod: moment.duration(timeDuration).humanize(), @@ -778,6 +784,13 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a )} reportDetailsComponentContent={sourceURL(reportDefinitionDetails)} /> + + + + >>>>>> d4f95c90 ([Feature] Add the ability to configure record limit/count from the UI (#395)) EuiFlexGroup, EuiFlexItem, EuiFormRow, @@ -17,11 +22,22 @@ import { EuiHorizontalRule, EuiText, EuiSpacer, +<<<<<<< HEAD EuiRadioGroup, EuiSelect, EuiTextArea, EuiCheckboxGroup, EuiComboBox, +======= + EuiCompressedRadioGroup, + EuiCompressedTextArea, + EuiCompressedCheckboxGroup, + EuiCompressedComboBox, + EuiFormRow, + OuiCallOut, + EuiText, + EuiIcon, +>>>>>>> d4f95c90 ([Feature] Add the ability to configure record limit/count from the UI (#395)) } from '@elastic/eui'; import { REPORT_SOURCE_RADIOS, @@ -83,7 +99,7 @@ export function ReportSettings(props: ReportSettingProps) { settingsReportSourceErrorMessage, showTimeRangeError, showTriggerIntervalNaNError, - showCronError + showCronError, } = props; const [reportName, setReportName] = useState(''); @@ -102,6 +118,7 @@ export function ReportSettings(props: ReportSettingProps) { [] as any ); const [savedSearches, setSavedSearches] = useState([] as any); + const [savedSearchRecordLimit, setSavedSearchRecordLimit] = useState(10000); const [notebooksSourceSelect, setNotebooksSourceSelect] = useState([] as any); const [notebooks, setNotebooks] = useState([] as any); @@ -170,7 +187,7 @@ export function ReportSettings(props: ReportSettingProps) { reportDefinitionRequest.report_params.core_params.saved_search_id = savedSearches[0]?.value; reportDefinitionRequest.report_params.core_params.report_format = 'csv'; - reportDefinitionRequest.report_params.core_params.limit = 10000; + reportDefinitionRequest.report_params.core_params.limit = savedSearchRecordLimit; reportDefinitionRequest.report_params.core_params.excel = true; } else if (e === 'notebooksReportSource') { reportDefinitionRequest.report_params.report_source = 'Notebook'; @@ -235,6 +252,12 @@ export function ReportSettings(props: ReportSettingProps) { } }; + const handleSavedSearchRecordLimit = (e) => { + setSavedSearchRecordLimit(e.target.value); + + reportDefinitionRequest.report_params.core_params.limit = e.target.value; + }; + const handleNotebooksSelect = (e) => { setNotebooksSourceSelect(e); let fromInContext = false; @@ -595,6 +618,13 @@ export function ReportSettings(props: ReportSettingProps) { reportDefinitionRequest.report_params.report_source = reportSource; } }); + + if (reportSource == REPORT_SOURCE_TYPES.savedSearch) { + setSavedSearchRecordLimit( + response.report_definition.report_params.core_params.limit + ); + } + setDefaultFileFormat( response.report_definition.report_params.core_params.report_format ); @@ -669,8 +699,11 @@ export function ReportSettings(props: ReportSettingProps) { await httpClientProps .get('../api/observability/notebooks/') .catch((error: any) => { - console.error('error fetching notebooks, retrying with legacy api', error) - return httpClientProps.get('../api/notebooks/') + console.error( + 'error fetching notebooks, retrying with legacy api', + error + ); + return httpClientProps.get('../api/notebooks/'); }) .then(async (response: any) => { let notebooksOptions = getNotebooksOptions(response.data); @@ -782,6 +815,30 @@ export function ReportSettings(props: ReportSettingProps) { /> + 10000 ? ( + + Generating + very large reports can cause memory issues. + + ) : ( + '' + ) + } + > + + + ) : null; diff --git a/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md b/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md index 7eeddf75..02624011 100644 --- a/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md +++ b/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md @@ -3,6 +3,7 @@ Compatible with OpenSearch and OpenSearch Dashboards Version 2.17.0 ### Enhancements +* [Feature] Add the ability to configure record limit/count from the UI ([#395](https://github.com/opensearch-project/dashboards-reporting/pull/395)) * Use smaller and compressed varients of buttons and form components ([#398](https://github.com/opensearch-project/dashboards-reporting/pull/398)) * [Enhancement] De-register reporting when MDS is enabled ([#411](https://github.com/opensearch-project/dashboards-reporting/pull/411)) From 5ccae4b149e30f5a41ce87893085d5fb73e6cfba Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 9 Sep 2024 12:23:42 -0700 Subject: [PATCH 2/4] Fix merge conflict markers Signed-off-by: Simeon Widdis --- .../report_settings/report_settings.tsx | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 2b5f5a55..4f008d07 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -6,12 +6,8 @@ import React, { useEffect, useState } from 'react'; import { i18n } from '@osd/i18n'; import { -<<<<<<< HEAD EuiFieldText, -======= EuiFieldNumber, - EuiCompressedFieldText, ->>>>>>> d4f95c90 ([Feature] Add the ability to configure record limit/count from the UI (#395)) EuiFlexGroup, EuiFlexItem, EuiFormRow, @@ -20,24 +16,13 @@ import { EuiPageContent, EuiPageContentBody, EuiHorizontalRule, - EuiText, EuiSpacer, -<<<<<<< HEAD EuiRadioGroup, - EuiSelect, EuiTextArea, EuiCheckboxGroup, EuiComboBox, -======= - EuiCompressedRadioGroup, - EuiCompressedTextArea, - EuiCompressedCheckboxGroup, - EuiCompressedComboBox, - EuiFormRow, - OuiCallOut, EuiText, EuiIcon, ->>>>>>> d4f95c90 ([Feature] Add the ability to configure record limit/count from the UI (#395)) } from '@elastic/eui'; import { REPORT_SOURCE_RADIOS, From d99bc1d2142ceac12c92464c6bfafece5a9f2141 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 10 Sep 2024 11:27:58 -0700 Subject: [PATCH 3/4] Update large record report warning (#429) Signed-off-by: Simeon Widdis --- .../report_settings/report_settings.tsx | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 4f008d07..dd07bdfa 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -10,7 +10,6 @@ import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, - EuiFormRow, EuiPageHeader, EuiTitle, EuiPageContent, @@ -21,8 +20,8 @@ import { EuiTextArea, EuiCheckboxGroup, EuiComboBox, - EuiText, - EuiIcon, + EuiFormRow, + EuiCallOut, } from '@elastic/eui'; import { REPORT_SOURCE_RADIOS, @@ -800,22 +799,26 @@ export function ReportSettings(props: ReportSettingProps) { /> + {savedSearchRecordLimit > 10000 ? ( + <> + + + + ) : null} 10000 ? ( - - Generating - very large reports can cause memory issues. - - ) : ( - '' - ) - } > Date: Tue, 10 Sep 2024 12:52:58 -0700 Subject: [PATCH 4/4] Merge 2.x Signed-off-by: Simeon Widdis --- .../report_settings/report_settings.tsx | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 25bce912..a48d2f42 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -790,7 +790,6 @@ export function ReportSettings(props: ReportSettingProps) { /> -<<<<<<< HEAD {savedSearchRecordLimit > 10000 ? ( <> ) : null} -======= ->>>>>>> origin/2.x 10000 ? ( - - Generating - very large reports can cause memory issues. - - ) : ( - '' - ) - } ->>>>>>> origin/2.x >