Skip to content

Commit

Permalink
[Feature] Add the ability to configure record limit/count from the UI (
Browse files Browse the repository at this point in the history
…#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]>
  • Loading branch information
szkly and Swiddis committed Sep 9, 2024
1 parent 1aadb2e commit 7b4d181
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .cypress/integration/02-edit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ interface ReportDefinitionDetails {
created: string;
lastUpdated: string;
source: string;
recordLimit: number | undefined;
timePeriod: string;
fileFormat: string;
status: string | undefined;
Expand All @@ -63,6 +64,7 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a
created: '',
lastUpdated: '',
source: '',
recordLimit: 0,
timePeriod: '',
fileFormat: '',
status: '',
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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',
Expand Down
15 changes: 15 additions & 0 deletions public/components/main/report_details/report_details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ interface ReportDetails {
created: string;
lastUpdated: string;
source: string;
recordLimit: number | undefined;
time_period: string;
defaultFileFormat: string;
state: string | undefined;
Expand Down Expand Up @@ -85,6 +86,7 @@ export function ReportDetails(props: { match?: any; setBreadcrumbs?: any; httpCl
created: '',
lastUpdated: '',
source: '',
recordLimit: 0,
time_period: '',
defaultFileFormat: '',
state: '',
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
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,
Expand All @@ -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,
Expand Down Expand Up @@ -83,7 +99,7 @@ export function ReportSettings(props: ReportSettingProps) {
settingsReportSourceErrorMessage,
showTimeRangeError,
showTriggerIntervalNaNError,
showCronError
showCronError,
} = props;

const [reportName, setReportName] = useState('');
Expand All @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -782,6 +815,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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down

0 comments on commit 7b4d181

Please sign in to comment.