From 8395ee01e689024f90d0282f3472b25f949d06a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Wed, 24 Jul 2024 19:42:39 +0200 Subject: [PATCH 01/12] feat(components/report-settings): add field for variable record limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_settings/report_settings.tsx | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 9bde1a18..ef1979f4 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -7,6 +7,7 @@ import React, { useEffect, useState } from 'react'; import { i18n } from '@osd/i18n'; import { EuiFieldText, + EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiFormRow, @@ -99,6 +100,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); @@ -167,7 +169,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'; @@ -232,6 +234,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; @@ -779,6 +787,16 @@ export function ReportSettings(props: ReportSettingProps) { /> + + + + ) : null; From 89f2a24037f6ca25e3630b711a24171a3ef4c363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Wed, 24 Jul 2024 19:46:16 +0200 Subject: [PATCH 02/12] fix(components/report-settings): set type for event target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_definitions/report_settings/report_settings.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index ef1979f4..a0cd96bf 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -234,7 +234,9 @@ export function ReportSettings(props: ReportSettingProps) { } }; - const handleSavedSearchRecordLimit = (e) => { + const handleSavedSearchRecordLimit = (e: { + target: { value: React.SetStateAction }; + }) => { setSavedSearchRecordLimit(e.target.value) reportDefinitionRequest.report_params.core_params.limit = e.target.value From 1949fcd8cfb51fffc26b4ad062e920c265cdb615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Wed, 24 Jul 2024 20:37:08 +0200 Subject: [PATCH 03/12] feat(components/report-settings): load existing record limit in edit mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_definitions/report_settings/report_settings.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index a0cd96bf..0d15a6ea 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -602,6 +602,11 @@ 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 ); From 4b71d9486db1d2acba74fee1862514b668668465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Wed, 24 Jul 2024 21:52:23 +0200 Subject: [PATCH 04/12] feat(components/report-definition-details): show record limit on report definition details page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_definition_details.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 2d44dfd2..418c24e4 100644 --- a/public/components/main/report_definition_details/report_definition_details.tsx +++ b/public/components/main/report_definition_details/report_definition_details.tsx @@ -48,6 +48,7 @@ interface ReportDefinitionDetails { created: string; lastUpdated: string; source: string; + recordLimit: number | undefined; timePeriod: string; fileFormat: string; status: string | undefined; @@ -65,6 +66,7 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: '', lastUpdated: '', source: '', + recordLimit: 0, timePeriod: '', fileFormat: '', status: '', @@ -418,6 +420,7 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: displayCreatedDate, lastUpdated: displayUpdatedDate, source: reportParams.report_source, + recordLimit: reportParams.core_params.limit, baseUrl: baseUrl, // TODO: need better display timePeriod: moment.duration(timeDuration).humanize(), @@ -780,6 +783,13 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a )} reportDetailsComponentContent={sourceURL(reportDefinitionDetails)} /> + - From 67d8f13e4dcb1cce99fb7654c4eb74f39086de8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Thu, 25 Jul 2024 20:11:21 +0200 Subject: [PATCH 05/12] feat(components/report-definition-details): omit record limit for non saved search sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_definition_details/report_definition_details.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 418c24e4..0cc86778 100644 --- a/public/components/main/report_definition_details/report_definition_details.tsx +++ b/public/components/main/report_definition_details/report_definition_details.tsx @@ -420,7 +420,10 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: displayCreatedDate, lastUpdated: displayUpdatedDate, source: reportParams.report_source, - recordLimit: reportParams.core_params.limit, + recordLimit: + reportParams.report_source != 'Saved search' + ? `\u2014` + : reportParams.core_params.limit, baseUrl: baseUrl, // TODO: need better display timePeriod: moment.duration(timeDuration).humanize(), From 632a30539e4a163185304e299fa5f92e30e09515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Thu, 25 Jul 2024 20:25:14 +0200 Subject: [PATCH 06/12] feat(components/report-details): show record limit on report details page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../main/report_details/report_details.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/public/components/main/report_details/report_details.tsx b/public/components/main/report_details/report_details.tsx index 65fa02aa..89be07e8 100644 --- a/public/components/main/report_details/report_details.tsx +++ b/public/components/main/report_details/report_details.tsx @@ -40,6 +40,7 @@ interface ReportDetails { created: string; lastUpdated: string; source: string; + recordLimit: number | undefined; time_period: string; defaultFileFormat: string; state: string | undefined; @@ -87,6 +88,7 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl created: '', lastUpdated: '', source: '', + recordLimit: 0, time_period: '', defaultFileFormat: '', state: '', @@ -220,6 +222,10 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl created: convertTimestamp(report.time_created), lastUpdated: convertTimestamp(report.last_updated), source: reportParams.report_source, + recordLimit: + reportParams.report_source != 'Saved search' + ? `\u2014` + : reportParams.core_params.limit, // TODO: we have all data needed, time_from, time_to, time_duration, // think of a way to better display time_period: (reportParams.report_source !== 'Notebook') ? parseTimePeriod(queryUrl) : `\u2014`, @@ -424,6 +430,13 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl )} reportDetailsComponentContent={sourceURL(reportDetails)} /> + + + Date: Thu, 25 Jul 2024 21:13:19 +0200 Subject: [PATCH 07/12] tests(cypress): add `force: true` to name input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .cypress/integration/02-edit.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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'); From 350ff4ef5395c58f0025737261827a415173b6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Thu, 25 Jul 2024 21:14:15 +0200 Subject: [PATCH 08/12] tests(jest/snapshots): update snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_definition_details.test.tsx.snap | 63 ++++++++++++++++--- .../report_details.test.tsx.snap | 44 +++++++++++++ 2 files changed, 98 insertions(+), 9 deletions(-) diff --git a/public/components/main/report_definition_details/__tests__/__snapshots__/report_definition_details.test.tsx.snap b/public/components/main/report_definition_details/__tests__/__snapshots__/report_definition_details.test.tsx.snap index 7bf41540..d5333bc8 100644 --- a/public/components/main/report_definition_details/__tests__/__snapshots__/report_definition_details.test.tsx.snap +++ b/public/components/main/report_definition_details/__tests__/__snapshots__/report_definition_details.test.tsx.snap @@ -229,6 +229,24 @@ exports[` panel render 5 hours recurring definition d +
+
+
+ Record limit +
+
+ 0 +
+
+
@@ -285,9 +303,6 @@ exports[` panel render 5 hours recurring definition d
-
panel render disabled daily definition, cli
+
+
+
+ Record limit +
+
+ 0 +
+
+
@@ -698,9 +731,6 @@ exports[` panel render disabled daily definition, cli
-
panel render on demand definition details 1
+
+
+
+ Record limit +
+
+ 0 +
+
+
@@ -1112,9 +1160,6 @@ exports[` panel render on demand definition details 1
-
panel render 5 hours recurring component 1`] = `
+
+
+
+ Record limit +
+
+ — +
+
+
@@ -233,6 +251,10 @@ exports[` panel render 5 hours recurring component 1`] = `
+ +
@@ -589,6 +611,24 @@ exports[` panel render on-demand component 1`] = `
+
+
+
+ Record limit +
+
+ — +
+
+
@@ -644,6 +684,10 @@ exports[` panel render on-demand component 1`] = `
+
+
From 91cc3b04c5fd23b25972273f1fe1e8fbfdf35298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Thu, 25 Jul 2024 21:24:08 +0200 Subject: [PATCH 09/12] feat(components/report-settings): add proper i18n label for record limit input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Károly Szakály --- .../report_settings/report_settings.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 0d15a6ea..93868333 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -100,7 +100,7 @@ export function ReportSettings(props: ReportSettingProps) { [] as any ); const [savedSearches, setSavedSearches] = useState([] as any); - const [savedSearchRecordLimit, setSavedSearchRecordLimit] = useState(10000) + const [savedSearchRecordLimit, setSavedSearchRecordLimit] = useState(10000); const [notebooksSourceSelect, setNotebooksSourceSelect] = useState([] as any); const [notebooks, setNotebooks] = useState([] as any); @@ -234,13 +234,11 @@ export function ReportSettings(props: ReportSettingProps) { } }; - const handleSavedSearchRecordLimit = (e: { - target: { value: React.SetStateAction }; - }) => { - setSavedSearchRecordLimit(e.target.value) + const handleSavedSearchRecordLimit = (e) => { + setSavedSearchRecordLimit(e.target.value); - reportDefinitionRequest.report_params.core_params.limit = e.target.value - } + reportDefinitionRequest.report_params.core_params.limit = e.target.value; + }; const handleNotebooksSelect = (e) => { setNotebooksSourceSelect(e); @@ -795,7 +793,11 @@ export function ReportSettings(props: ReportSettingProps) { Date: Mon, 9 Sep 2024 10:40:37 -0700 Subject: [PATCH 10/12] Update release notes Signed-off-by: Simeon Widdis --- .../opensearch-dashboards-reporting.release-notes-2.17.0.0.md | 1 + 1 file changed, 1 insertion(+) 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 437d0a3c4217bbe4861bcb56fc38edd3baed7303 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 9 Sep 2024 11:49:30 -0700 Subject: [PATCH 11/12] Add potential OOM warning for large record limits Signed-off-by: Simeon Widdis --- .../report_settings/report_settings.tsx | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 15765ef8..79801cb4 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -22,6 +22,9 @@ import { EuiCompressedCheckboxGroup, EuiCompressedComboBox, EuiFormRow, + OuiCallOut, + EuiText, + EuiIcon, } from '@elastic/eui'; import { REPORT_SOURCE_RADIOS, @@ -82,7 +85,7 @@ export function ReportSettings(props: ReportSettingProps) { settingsReportSourceErrorMessage, showTimeRangeError, showTriggerIntervalNaNError, - showCronError + showCronError, } = props; const [reportName, setReportName] = useState(''); @@ -603,7 +606,9 @@ export function ReportSettings(props: ReportSettingProps) { }); if (reportSource == REPORT_SOURCE_TYPES.savedSearch) { - setSavedSearchRecordLimit(response.report_definition.report_params.core_params.limit) + setSavedSearchRecordLimit( + response.report_definition.report_params.core_params.limit + ); } setDefaultFileFormat( @@ -680,8 +685,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); @@ -718,7 +726,7 @@ export function ReportSettings(props: ReportSettingProps) { reportSourceId === 'dashboardReportSource' ? (
500000 ? ( + + Generating + very large reports can cause memory issues. + + ) : ( + '' + ) + } > Date: Mon, 9 Sep 2024 11:53:47 -0700 Subject: [PATCH 12/12] Make memory warning more conservative based on original limit Signed-off-by: Simeon Widdis --- .../report_definitions/report_settings/report_settings.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index 79801cb4..706b484f 100644 --- a/public/components/report_definitions/report_settings/report_settings.tsx +++ b/public/components/report_definitions/report_settings/report_settings.tsx @@ -808,7 +808,7 @@ export function ReportSettings(props: ReportSettingProps) { { defaultMessage: 'Record limit' } )} helpText={ - savedSearchRecordLimit > 500000 ? ( + savedSearchRecordLimit > 10000 ? ( Generating very large reports can cause memory issues.