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

[Custom threshold] Reintroduce no data setting in the custom threshold rule #188300

Merged
merged 3 commits into from
Jul 18, 2024
Merged

[Custom threshold] Reintroduce no data setting in the custom threshold rule #188300

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cc3baf
Reintroduce no data setting in the custom threshold rule
maryam-saeidi Jul 15, 2024
163fbf2
Merge branch 'main' into 188229-no-data-setting-ct
maryam-saeidi Jul 18, 2024
debc39a
Improve no data help text
maryam-saeidi Jul 18, 2024
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
70 changes: 67 additions & 3 deletions ...bservability/public/components/custom_threshold/custom_threshold_rule_expression.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Expression', () => {

async function setup(
currentOptions?: CustomThresholdPrefillOptions,
customRuleParams?: Record<string, unknown>
customRuleParams?: Partial<RuleTypeParams & AlertParams>
) {
const ruleParams: RuleTypeParams & AlertParams = {
criteria: [],
Expand Down Expand Up @@ -164,7 +164,8 @@ describe('Expression', () => {
it('should prefill the rule using the context metadata', async () => {
const index = 'changedMockedIndex';
const currentOptions: CustomThresholdPrefillOptions = {
alertOnGroupDisappear: false,
alertOnGroupDisappear: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get rid of the alertOnGroupDisappear and just use alertOnNoData for both with and without group? Since it's now mapped to a single checkbox it seems odd to keep two separate values on our schema. Or do we need to support this for backwards compatibility?

Copy link
Member Author

@maryam-saeidi maryam-saeidi Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we need to support this for backward compatibility. I've added this item to be considered for the v9 release. (i.e. removing alertOnGroupDisappear and only using alertOnNoData)

alertOnNoData: true,
groupBy: ['host.hostname'],
searchConfiguration: {
index,
Expand All @@ -191,7 +192,8 @@ describe('Expression', () => {

const { ruleParams } = await setup(currentOptions, { searchConfiguration: undefined });

expect(ruleParams.alertOnGroupDisappear).toEqual(false);
expect(ruleParams.alertOnGroupDisappear).toEqual(true);
expect(ruleParams.alertOnNoData).toEqual(true);
expect(ruleParams.groupBy).toEqual(['host.hostname']);
expect((ruleParams.searchConfiguration.query as Query).query).toBe('foo');
expect(ruleParams.searchConfiguration.index).toBe(index);
Expand All @@ -211,6 +213,68 @@ describe('Expression', () => {
]);
});

it('should only set alertOnGroupDisappear to true if there is a group by field', async () => {
const customRuleParams: Partial<RuleTypeParams & AlertParams> = {
groupBy: ['host.hostname'],
};

const { ruleParams, wrapper } = await setup({}, customRuleParams);

act(() => {
wrapper
.find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
.at(1)
.prop('onChange')?.({
target: { checked: true },
} as React.ChangeEvent<HTMLInputElement>);
});

expect(ruleParams.alertOnGroupDisappear).toEqual(true);
expect(ruleParams.alertOnNoData).toEqual(false);

// Uncheck
act(() => {
wrapper
.find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
.at(1)
.prop('onChange')?.({
target: { checked: false },
} as React.ChangeEvent<HTMLInputElement>);
});

expect(ruleParams.alertOnGroupDisappear).toEqual(false);
expect(ruleParams.alertOnNoData).toEqual(false);
});

it('should only set alertOnNoData to true if there is no group by', async () => {
const { ruleParams, wrapper } = await setup();

act(() => {
wrapper
.find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
.at(1)
.prop('onChange')?.({
target: { checked: true },
} as React.ChangeEvent<HTMLInputElement>);
});

expect(ruleParams.alertOnGroupDisappear).toEqual(false);
expect(ruleParams.alertOnNoData).toEqual(true);

// Uncheck
act(() => {
wrapper
.find('[data-test-subj="thresholdRuleAlertOnNoDataCheckbox"]')
.at(1)
.prop('onChange')?.({
target: { checked: false },
} as React.ChangeEvent<HTMLInputElement>);
});

expect(ruleParams.alertOnGroupDisappear).toEqual(false);
expect(ruleParams.alertOnNoData).toEqual(false);
});

it('should show an error message when searchSource throws an error', async () => {
const errorMessage = 'Error in searchSource create';
const kibanaMock = kibanaStartMock.startContract();
Expand Down
83 changes: 64 additions & 19 deletions ...ion/observability/public/components/custom_threshold/custom_threshold_rule_expression.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,22 @@ export default function Expressions(props: Props) {
},
} = useKibana().services;

const hasGroupBy = useMemo<boolean>(
() => !!ruleParams.groupBy && ruleParams.groupBy.length > 0,
[ruleParams.groupBy]
);

const [timeSize, setTimeSize] = useState<number | undefined>(1);
const [timeUnit, setTimeUnit] = useState<TimeUnitChar | undefined>('m');
const [dataView, setDataView] = useState<DataView>();
const [dataViewTimeFieldError, setDataViewTimeFieldError] = useState<string>();
const [searchSource, setSearchSource] = useState<ISearchSource>();
const [paramsError, setParamsError] = useState<Error>();
const [paramsWarning, setParamsWarning] = useState<string>();
const [isNoDataChecked, setIsNoDataChecked] = useState<boolean>(
(hasGroupBy && !!ruleParams.alertOnGroupDisappear) ||
(!hasGroupBy && !!ruleParams.alertOnNoData)
);
const derivedIndexPattern = useMemo<DataViewBase>(
() => ({
fields: dataView?.fields || [],
Expand Down Expand Up @@ -177,11 +186,15 @@ export default function Expressions(props: Props) {
}

if (typeof ruleParams.alertOnNoData === 'undefined') {
setRuleParams('alertOnNoData', true);
preFillAlertOnNoData();
}
if (typeof ruleParams.alertOnGroupDisappear === 'undefined') {
preFillAlertOnGroupDisappear();
}
setIsNoDataChecked(
(hasGroupBy && !!ruleParams.alertOnGroupDisappear) ||
(!hasGroupBy && !!ruleParams.alertOnNoData)
);
}, [metadata]); // eslint-disable-line react-hooks/exhaustive-deps

const onSelectDataView = useCallback(
Expand Down Expand Up @@ -250,9 +263,12 @@ export default function Expressions(props: Props) {

const onGroupByChange = useCallback(
(group: string | null | string[]) => {
const hasGroup = !!group && group.length > 0;
setRuleParams('groupBy', group && group.length ? group : '');
setRuleParams('alertOnGroupDisappear', hasGroup && isNoDataChecked);
setRuleParams('alertOnNoData', !hasGroup && isNoDataChecked);
},
[setRuleParams]
[setRuleParams, isNoDataChecked]
);

const emptyError = useMemo(() => {
Expand Down Expand Up @@ -314,20 +330,24 @@ export default function Expressions(props: Props) {
}
}, [metadata, setRuleParams]);

const preFillAlertOnNoData = useCallback(() => {
const md = metadata;
if (md && typeof md.currentOptions?.alertOnNoData !== 'undefined') {
setRuleParams('alertOnNoData', md.currentOptions.alertOnNoData);
} else {
setRuleParams('alertOnNoData', false);
}
}, [metadata, setRuleParams]);

const preFillAlertOnGroupDisappear = useCallback(() => {
const md = metadata;
if (md && typeof md.currentOptions?.alertOnGroupDisappear !== 'undefined') {
setRuleParams('alertOnGroupDisappear', md.currentOptions.alertOnGroupDisappear);
} else {
setRuleParams('alertOnGroupDisappear', true);
setRuleParams('alertOnGroupDisappear', false);
}
}, [metadata, setRuleParams]);

const hasGroupBy = useMemo(
() => ruleParams.groupBy && ruleParams.groupBy.length > 0,
[ruleParams.groupBy]
);

if (paramsError) {
return (
<>
Expand Down Expand Up @@ -540,30 +560,55 @@ export default function Expressions(props: Props) {
<EuiSpacer size="s" />
<EuiCheckbox
id="metrics-alert-group-disappear-toggle"
data-test-subj="thresholdRuleAlertOnNoDataCheckbox"
label={
<>
{i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.alertOnGroupDisappear',
{
defaultMessage: 'Alert me if a group stops reporting data',
defaultMessage: "Alert me if there's no data",
}
)}{' '}
<EuiIconTip
type="questionInCircle"
color="subdued"
content={i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.groupDisappearHelpText',
{
defaultMessage:
'Enable this to trigger the action if a previously detected group begins to report no results. This is not recommended for dynamically scaling infrastructures that may rapidly start and stop nodes automatically.',
}
)}
content={
hasGroupBy
? i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.groupDisappearHelpText',
{
defaultMessage:
'Enable this to trigger the action if a previously detected group begins to report no results. This is not recommended for dynamically scaling infrastructures that may rapidly start and stop nodes automatically.',
}
)
: i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.noDataHelpText',
{
defaultMessage:
'Enable this to trigger the action if the metric(s) do not report any data over the expected time period, or if the alert fails to query Elasticsearch',
}
)
}
/>
</>
}
disabled={!hasGroupBy}
checked={Boolean(hasGroupBy && ruleParams.alertOnGroupDisappear)}
onChange={(e) => setRuleParams('alertOnGroupDisappear', e.target.checked)}
checked={isNoDataChecked}
onChange={(e) => {
const checked = e.target.checked;
setIsNoDataChecked(checked);
if (!checked) {
setRuleParams('alertOnGroupDisappear', false);
setRuleParams('alertOnNoData', false);
} else {
if (hasGroupBy) {
setRuleParams('alertOnGroupDisappear', true);
setRuleParams('alertOnNoData', false);
} else {
setRuleParams('alertOnGroupDisappear', false);
setRuleParams('alertOnNoData', true);
}
}
}}
/>
<EuiSpacer size="m" />
</>
Expand Down
Loading