From 6dc0fbb03f4ee9ec4a3e223ef7ecd981f455d0ed Mon Sep 17 00:00:00 2001 From: Simeon Widdis <sawiddis@amazon.com> Date: Mon, 9 Sep 2024 13:05:46 -0700 Subject: [PATCH] [Manual Backport] Allow arbitrary report record counts (#427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Feature] Add the ability to configure record limit/count from the UI (#395) * feat(components/report-settings): add field for variable record limits Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * fix(components/report-settings): set type for event target Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * feat(components/report-settings): load existing record limit in edit mode Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * feat(components/report-definition-details): show record limit on report definition details page Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * feat(components/report-definition-details): omit record limit for non saved search sources Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * feat(components/report-details): show record limit on report details page Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * tests(cypress): add `force: true` to name input Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * tests(jest/snapshots): update snapshots Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * feat(components/report-settings): add proper i18n label for record limit input Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> * Update release notes Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Add potential OOM warning for large record limits Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Make memory warning more conservative based on original limit Signed-off-by: Simeon Widdis <sawiddis@amazon.com> --------- Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> Signed-off-by: Simeon Widdis <sawiddis@amazon.com> Co-authored-by: Simeon Widdis <sawiddis@amazon.com> * Fix merge conflict markers Signed-off-by: Simeon Widdis <sawiddis@amazon.com> --------- Signed-off-by: Károly Szakály <karoly.szakaly2000@gmail.com> Signed-off-by: Simeon Widdis <sawiddis@amazon.com> Co-authored-by: Károly Szakály <karoly.szakaly2000@gmail.com> --- .cypress/integration/02-edit.spec.ts | 2 +- .../report_definition_details.tsx | 13 +++++ .../main/report_details/report_details.tsx | 15 ++++++ .../report_settings/report_settings.tsx | 54 ++++++++++++++++--- ...boards-reporting.release-notes-2.17.0.0.md | 1 + 5 files changed, 78 insertions(+), 7 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)} /> + <ReportDetailsComponent + reportDetailsComponentTitle={i18n.translate( + 'opensearch.reports.reportDefinitionsDetails.fields.recordLimit', + { defaultMessage: 'Record limit' } + )} + reportDetailsComponentContent={reportDefinitionDetails.recordLimit} + /> <ReportDetailsComponent reportDetailsComponentTitle={i18n.translate( 'opensearch.reports.reportDefinitionsDetails.fields.timePeriod', diff --git a/public/components/main/report_details/report_details.tsx b/public/components/main/report_details/report_details.tsx index ea4ad5dc..209c5ea6 100644 --- a/public/components/main/report_details/report_details.tsx +++ b/public/components/main/report_details/report_details.tsx @@ -38,6 +38,7 @@ interface ReportDetails { created: string; lastUpdated: string; source: string; + recordLimit: number | undefined; time_period: string; defaultFileFormat: string; state: string | undefined; @@ -85,6 +86,7 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl created: '', lastUpdated: '', source: '', + recordLimit: 0, time_period: '', defaultFileFormat: '', state: '', @@ -218,6 +220,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`, @@ -423,6 +429,13 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl )} reportDetailsComponentContent={sourceURL(reportDetails)} /> + <ReportDetailsComponent + reportDetailsComponentTitle={i18n.translate( + 'opensearch.reports.reportDefinitionsDetails.fields.recordLimit', + { defaultMessage: 'Record limit' } + )} + reportDetailsComponentContent={reportDetails.recordLimit} + /> <ReportDetailsComponent reportDetailsComponentTitle={i18n.translate( 'opensearch.reports.details.reportSettings.timePeriod', @@ -437,6 +450,8 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl )} reportDetailsComponentContent={fileFormatDownload(reportDetails)} /> + </EuiFlexGroup> + <EuiFlexGroup> <ReportDetailsComponent reportDetailsComponentTitle={i18n.translate( 'opensearch.reports.details.reportSettings.state', diff --git a/public/components/report_definitions/report_settings/report_settings.tsx b/public/components/report_definitions/report_settings/report_settings.tsx index f80b713d..4f008d07 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, @@ -15,13 +16,13 @@ import { EuiPageContent, EuiPageContentBody, EuiHorizontalRule, - EuiText, EuiSpacer, EuiRadioGroup, - EuiSelect, EuiTextArea, EuiCheckboxGroup, EuiComboBox, + EuiText, + EuiIcon, } from '@elastic/eui'; import { REPORT_SOURCE_RADIOS, @@ -83,7 +84,7 @@ export function ReportSettings(props: ReportSettingProps) { settingsReportSourceErrorMessage, showTimeRangeError, showTriggerIntervalNaNError, - showCronError + showCronError, } = props; const [reportName, setReportName] = useState(''); @@ -102,6 +103,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 +172,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 +237,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 +603,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 +684,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 +800,30 @@ export function ReportSettings(props: ReportSettingProps) { /> </EuiFormRow> <EuiSpacer /> + <EuiFormRow + id="reportSourceSavedSearchRecordLimit" + label={i18n.translate( + 'opensearch.reports.reportSettingProps.form.savedSearchRecordLimit', + { defaultMessage: 'Record limit' } + )} + helpText={ + savedSearchRecordLimit > 10000 ? ( + <EuiText color="warning" size="xs"> + <EuiIcon color="warning" type="alert" size="s" /> Generating + very large reports can cause memory issues. + </EuiText> + ) : ( + '' + ) + } + > + <EuiFieldNumber + value={savedSearchRecordLimit} + onChange={handleSavedSearchRecordLimit} + min={1} + /> + </EuiFormRow> + <EuiSpacer /> </div> ) : 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))