Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[alerts] adds support for index threshold index param string type #88540

Merged
merged 5 commits into from
Jan 25, 2021
Merged

[alerts] adds support for index threshold index param string type #88540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2439918
[alerts] adds support for index threshold index param string type
pmuellr Jan 15, 2021
2fc19f1
Merge branch 'master' into alerts/index-threshold-non-array-index-param
kibanamachine Jan 20, 2021
9f95ce7
modify existing FT to handle case of string param
pmuellr Jan 21, 2021
080063a
Merge branch 'master' into alerts/index-threshold-non-array-index-param
kibanamachine Jan 25, 2021
0b718b5
Merge branch 'master' into alerts/index-threshold-non-array-index-param
kibanamachine Jan 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions x-pack/plugins/stack_alerts/public/alert_types/threshold/expression.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ function isString(value: unknown): value is string {
return typeof value === 'string';
}

// normalize the `index` parameter to be a string array
function indexParamToArray(index: string | string[]): string[] {
if (!index) return [];
return isString(index) ? [index] : index;
}

export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
AlertTypeParamsExpressionProps<IndexThresholdAlertParams>
> = ({ alertParams, alertInterval, setAlertParams, setAlertProperty, errors, charts, data }) => {
Expand All @@ -92,6 +98,7 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
timeWindowUnit,
} = alertParams;

const indexArray = indexParamToArray(index);
const { http } = useKibana<KibanaDeps>().services;

const [indexPopoverOpen, setIndexPopoverOpen] = useState(false);
Expand Down Expand Up @@ -131,8 +138,8 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
threshold: threshold ?? DEFAULT_VALUES.THRESHOLD,
});

if (index && index.length > 0) {
const currentEsFields = await getFields(http, index);
if (indexArray.length > 0) {
const currentEsFields = await getFields(http, indexArray);
const timeFields = getTimeFieldOptions(currentEsFields);

setEsFields(currentEsFields);
Expand Down Expand Up @@ -170,7 +177,7 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
defaultMessage="Indices to query"
/>
}
isInvalid={errors.index.length > 0 && index !== undefined}
isInvalid={errors.index.length > 0 && indexArray.length > 0}
error={errors.index}
helpText={
<FormattedMessage
Expand All @@ -183,11 +190,11 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
fullWidth
async
isLoading={isIndiciesLoading}
isInvalid={errors.index.length > 0 && index !== undefined}
isInvalid={errors.index.length > 0 && indexArray.length > 0}
noSuggestions={!indexOptions.length}
options={indexOptions}
data-test-subj="thresholdIndexesComboBox"
selectedOptions={(index || []).map((anIndex: string) => {
selectedOptions={indexArray.map((anIndex: string) => {
return {
label: anIndex,
value: anIndex,
Expand Down Expand Up @@ -306,12 +313,12 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
description={i18n.translate('xpack.stackAlerts.threshold.ui.alertParams.indexLabel', {
defaultMessage: 'index',
})}
value={index && index.length > 0 ? renderIndices(index) : firstFieldOption.text}
value={indexArray.length > 0 ? renderIndices(indexArray) : firstFieldOption.text}
isActive={indexPopoverOpen}
onClick={() => {
setIndexPopoverOpen(true);
}}
isInvalid={!(index && index.length > 0 && timeField !== '')}
isInvalid={!(indexArray.length > 0 && timeField !== '')}
/>
}
isOpen={indexPopoverOpen}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/stack_alerts/public/alert_types/threshold/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface GroupByType {
}

export interface IndexThresholdAlertParams extends AlertTypeParams {
index: string[];
index: string | string[];
timeField?: string;
aggType: string;
aggField?: string;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/stack_alerts/public/alert_types/threshold/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('expression params validation', () => {
});
test('if aggField property is invalid should return proper error message', () => {
const initialParams: IndexThresholdAlertParams = {
index: ['test'],
index: 'test',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this test so that we have a jest test that will test a string index param (instead of the usual string[] type). It would be good to add a function test, I think, would need to create an alert using the API (passing a string typed index field), and then edit the alert and make sure the single index appears as expected in the UI. Do we have similar tests like this? Thoughts on where to add such a test?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use the esArchiver to load an archived alert with a string index param and then try to edit it in your functional test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be able to create one via the API - I suspect that will be easier to figuring out esArchiver :-)

Let me see if I can add a quick one ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one - I ended up changing the test in x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts, below, from using an array of indices to a single index string.

aggType: 'avg',
threshold: [],
timeWindowSize: 1,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
timeWindowUnit: 'm',
groupBy: 'all',
threshold: [1000, 5000],
index: ['.kibana_1'],
index: '.kibana_1',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already an example of testing the array version of the index param in the alert_create_flyout tests, so it seemed like changing this other usage of the index threshold alert to use the string version would be appropriate.

timeField: 'alert',
},
actions: [
Expand Down