[Manual Backport] Allow arbitrary report record counts (#427)

* [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 <[email protected]> * fix(components/report-settings): set type for event target Signed-off-by: Károly Szakály <[email protected]> * feat(components/report-settings): load existing record limit in edit mode Signed-off-by: Károly Szakály <[email protected]> * feat(components/report-definition-details): show record limit on report definition details page Signed-off-by: Károly Szakály <[email protected]> * feat(components/report-definition-details): omit record limit for non saved search sources Signed-off-by: Károly Szakály <[email protected]> * feat(components/report-details): show record limit on report details page Signed-off-by: Károly Szakály <[email protected]> * tests(cypress): add `force: true` to name input Signed-off-by: Károly Szakály <[email protected]> * tests(jest/snapshots): update snapshots Signed-off-by: Károly Szakály <[email protected]> * feat(components/report-settings): add proper i18n label for record limit input Signed-off-by: Károly Szakály <[email protected]> * Update release notes Signed-off-by: Simeon Widdis <[email protected]> * Add potential OOM warning for large record limits Signed-off-by: Simeon Widdis <[email protected]> * Make memory warning more conservative based on original limit Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Károly Szakály <[email protected]> Signed-off-by: Simeon Widdis <[email protected]> Co-authored-by: Simeon Widdis <[email protected]> * Fix merge conflict markers Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Károly Szakály <[email protected]> Signed-off-by: Simeon Widdis <[email protected]> Co-authored-by: Károly Szakály <[email protected]>
opensearch-project · Sep 9, 2024 · 6dc0fbb · 6dc0fbb
1 parent 4c2f078
commit 6dc0fbb
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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))