Skip to content

Commit

Permalink
[BUG][OBS-UX-MNGT] Handle leading wildecard in the Custom Threshold r…
Browse files Browse the repository at this point in the history
…ule query filer and improve error msg (elastic#175683)

## Summary
Fixes elastic#174825 
Fixes elastic#174763

By adding the query configuration to the request validation and adding
the reason to the error message.

#### After
<img width="463" alt="Screenshot 2024-01-25 at 16 58 00"
src="https://github.com/elastic/kibana/assets/6838659/4a23b06c-eb2e-4edb-adc2-5ad91552fa29">
  • Loading branch information
fkanout authored Feb 7, 2024
1 parent 9df8724 commit 0721933
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { Meta, Story } from '@storybook/react/types-6-0';
import React, { useCallback, useEffect, useState } from 'react';
import { IErrorObject } from '@kbn/triggers-actions-ui-plugin/public';
import { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
import { decorateWithGlobalStorybookThemeProviders } from '../../../../test_utils/use_global_storybook_theme';
import {
Aggregators,
Expand Down Expand Up @@ -70,6 +71,7 @@ const CustomEquationEditorTemplate: Story<CustomEquationEditorProps> = (args) =>
const validationObject = validateCustomThreshold({
criteria: [expression as CustomMetricExpressionParams],
searchConfiguration: {},
uiSettings: {} as IUiSettingsClient,
});
setErrors(validationObject.errors[0]);
}, [expression]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,25 @@
* 2.0.
*/

import { EQUATION_REGEX } from './validation';
import { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
import {
CustomMetricExpressionParams,
CustomThresholdExpressionMetric,
} from '../../../../common/custom_threshold_rule/types';
import { EQUATION_REGEX, validateCustomThreshold } from './validation';

const errorReason = 'this should appear as error reason';

jest.mock('@kbn/es-query', () => {
const actual = jest.requireActual('@kbn/es-query');
return {
...actual,
buildEsQuery: jest.fn(() => {
// eslint-disable-next-line no-throw-literal
throw { shortMessage: errorReason };
}),
};
});

describe('Metric Threshold Validation', () => {
describe('valid equations', () => {
Expand All @@ -30,4 +48,46 @@ describe('Metric Threshold Validation', () => {
});
});
});
it('should throw an error when data view is not provided', () => {
const res = validateCustomThreshold({
uiSettings: {} as IUiSettingsClient,
searchConfiguration: {},
criteria: {
metrics: [
{
name: 'Test',
aggType: 'count',
field: 'system.cpu.cores',
filter: 'none valid filter',
},
] as unknown as CustomThresholdExpressionMetric[],
} as unknown as CustomMetricExpressionParams[],
});
expect(res.errors.searchConfiguration[0]).toBe('Data view is required.');
});
it('should throw an error when filter query is not valid with reason', () => {
const res = validateCustomThreshold({
uiSettings: {
get: jest.fn(),
} as unknown as IUiSettingsClient,
searchConfiguration: {
index: 'test*',
query: {
language: `kuery`,
query: 'test:tet',
},
},
criteria: {
metrics: [
{
name: 'Test',
aggType: 'count',
field: 'system.cpu.cores',
filter: 'none valid filter',
},
] as unknown as CustomThresholdExpressionMetric[],
} as unknown as CustomMetricExpressionParams[],
});
expect(res.errors.filterQuery[0]).toBe(`Filter query is invalid. ${errorReason}`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* 2.0.
*/

import { Query, SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
import { getEsQueryConfig, Query, SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { buildEsQuery, fromKueryExpression } from '@kbn/es-query';
import { i18n } from '@kbn/i18n';
import { ValidationResult } from '@kbn/triggers-actions-ui-plugin/public';
Expand All @@ -20,9 +21,11 @@ export const EQUATION_REGEX = /[^A-Z|+|\-|\s|\d+|\.|\(|\)|\/|\*|>|<|=|\?|\:|&|\!
export function validateCustomThreshold({
criteria,
searchConfiguration,
uiSettings,
}: {
criteria: CustomMetricExpressionParams[];
searchConfiguration: SerializedSearchSourceFields;
uiSettings: IUiSettingsClient;
}): ValidationResult {
const validationResult = { errors: {} };
const errors: {
Expand Down Expand Up @@ -56,14 +59,17 @@ export function validateCustomThreshold({
buildEsQuery(
undefined,
[{ query: (searchConfiguration.query as Query).query, language: 'kuery' }],
[]
[],
getEsQueryConfig(uiSettings)
);
} catch (e) {
const errorReason = e.shortMessage || '';
errors.filterQuery = [
i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.error.invalidFilterQuery',
{
defaultMessage: 'Filter query is invalid.',
values: { errorReason },
defaultMessage: `Filter query is invalid. {errorReason}`,
}
),
];
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/observability/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,11 @@ export class Plugin

coreSetup.application.register(app);

registerObservabilityRuleTypes(config, this.observabilityRuleTypeRegistry, logsExplorerLocator);
registerObservabilityRuleTypes(
this.observabilityRuleTypeRegistry,
coreSetup.uiSettings,
logsExplorerLocator
);

const assertPlatinumLicense = async () => {
const licensing = await pluginsSetup.licensing;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { lazy } from 'react';
import { i18n } from '@kbn/i18n';
import type { SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import {
ALERT_REASON,
ALERT_RULE_PARAMETERS,
Expand All @@ -16,11 +16,14 @@ import {
} from '@kbn/rule-data-utils';
import type { DiscoverAppLocatorParams } from '@kbn/discover-plugin/common';
import type { LocatorPublic } from '@kbn/share-plugin/common';
import { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
import type { MetricExpression } from '../components/custom_threshold/types';
import type { CustomThresholdExpressionMetric } from '../../common/custom_threshold_rule/types';
import type {
CustomMetricExpressionParams,
CustomThresholdExpressionMetric,
} from '../../common/custom_threshold_rule/types';
import { getViewInAppUrl } from '../../common/custom_threshold_rule/get_view_in_app_url';
import { SLO_ID_FIELD, SLO_INSTANCE_ID_FIELD } from '../../common/field_names/slo';
import { ConfigSchema } from '../plugin';
import { ObservabilityRuleTypeRegistry } from './create_observability_rule_type_registry';
import { SLO_BURN_RATE_RULE_TYPE_ID } from '../../common/constants';
import { validateBurnRateRule } from '../components/burn_rate_rule_editor/validation';
Expand Down Expand Up @@ -86,8 +89,8 @@ const getDataViewId = (searchConfiguration?: SerializedSearchSourceFields) =>
: searchConfiguration?.index?.title;

export const registerObservabilityRuleTypes = async (
config: ConfigSchema,
observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry,
uiSettings: IUiSettingsClient,
logsExplorerLocator?: LocatorPublic<DiscoverAppLocatorParams>
) => {
observabilityRuleTypeRegistry.register({
Expand Down Expand Up @@ -117,7 +120,13 @@ export const registerObservabilityRuleTypes = async (
),
priority: 100,
});

const validateCustomThresholdWithUiSettings = ({
criteria,
searchConfiguration,
}: {
criteria: CustomMetricExpressionParams[];
searchConfiguration: SerializedSearchSourceFields;
}) => validateCustomThreshold({ criteria, searchConfiguration, uiSettings });
observabilityRuleTypeRegistry.register({
id: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
description: i18n.translate(
Expand All @@ -133,7 +142,7 @@ export const registerObservabilityRuleTypes = async (
ruleParamsExpression: lazy(
() => import('../components/custom_threshold/custom_threshold_rule_expression')
),
validate: validateCustomThreshold,
validate: validateCustomThresholdWithUiSettings,
defaultActionMessage: thresholdDefaultActionMessage,
defaultRecoveryMessage: thresholdDefaultRecoveryMessage,
requiresAppContext: false,
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -28657,7 +28657,6 @@
"xpack.observability.customThreshold.rule.alertFlyout.dataViewError.noTimestamp": "La vue de données sélectionnée ne dispose pas de champ d'horodatage. Veuillez sélectionner une autre vue de données.",
"xpack.observability.customThreshold.rule.alertFlyout.defineTextQueryPrompt": "Définir le filtre de recherche (facultatif)",
"xpack.observability.customThreshold.rule.alertFlyout.error.equation.invalidCharacters": "Le champ d'équation prend en charge uniquement les caractères suivants : A-Z, +, -, /, *, (, ), ?, !, &, :, |, >, <, =",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidFilterQuery": "La requête de filtre n'est pas valide.",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidSearchConfiguration": "La vue de données est requise.",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.aggTypeRequired": "L'agrégation est requise",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.fieldRequired": "Le champ est obligatoire",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -28658,7 +28658,6 @@
"xpack.observability.customThreshold.rule.alertFlyout.dataViewError.noTimestamp": "選択したデータビューにタイムスタンプフィールドがありません。他のデータビューを選択してください。",
"xpack.observability.customThreshold.rule.alertFlyout.defineTextQueryPrompt": "クエリフィルターを定義(任意)",
"xpack.observability.customThreshold.rule.alertFlyout.error.equation.invalidCharacters": "等式フィールドでは次の文字のみを使用できます:A-Z、+、-、/、*、(、)、?、!、&、:、|、>、<、=",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidFilterQuery": "フィルタークエリは無効です。",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidSearchConfiguration": "データビューが必要です。",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.aggTypeRequired": "集約が必要です",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.fieldRequired": "フィールドが必要です",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -28642,7 +28642,6 @@
"xpack.observability.customThreshold.rule.alertFlyout.dataViewError.noTimestamp": "选定数据视图没有时间戳字段,请选择其他数据视图。",
"xpack.observability.customThreshold.rule.alertFlyout.defineTextQueryPrompt": "定义查询筛选(可选)",
"xpack.observability.customThreshold.rule.alertFlyout.error.equation.invalidCharacters": "方程字段仅支持以下字符:A-Z、+、-、/、*、(、)、?、!、&、:、|、>、<、=",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidFilterQuery": "筛选查询无效。",
"xpack.observability.customThreshold.rule.alertFlyout.error.invalidSearchConfiguration": "需要数据视图。",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.aggTypeRequired": "“聚合”必填",
"xpack.observability.customThreshold.rule.alertFlyout.error.metrics.fieldRequired": "“字段”必填",
Expand Down

0 comments on commit 0721933

Please sign in to comment.