Skip to content

Commit ef5890a

Browse files
authored
[Rules] KQL-to-DSL conversion without data view produces incorrect queries for keyword fields for Metric threshold rule (elastic#260046)
**Release Notes** Introduced a fix for metric threshold rule with custom evaluation where wildcard filters were not rendering any results to trigger alerts. **Summary** This PR resolves an issue with metric threshold rule evaluation where a data view is not passed to rule evaluation functions, resulting in a failure to successfully create a wildcard query filter and rule execution with alerts firing as expected. Resolves elastic#257282 <img width="1246" height="641" alt="image" src="https://github.com/user-attachments/assets/9702c322-8bf3-4143-b897-e2afb1c01b59" />
1 parent 31b9aaf commit ef5890a

9 files changed

Lines changed: 313 additions & 7 deletions

File tree

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_rule.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import type { ElasticsearchClient } from '@kbn/core/server';
9+
import type { DataViewBase } from '@kbn/es-query';
910
import moment from 'moment';
1011
import type { Logger } from '@kbn/logging';
1112
import { isCustom } from './metric_expression_params';
@@ -44,6 +45,7 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
4445
compositeSize: number,
4546
alertOnGroupDisappear: boolean,
4647
logger: Logger,
48+
dataView?: DataViewBase,
4749
lastPeriodEnd?: number,
4850
timeframe?: { start?: number; end: number },
4951
missingGroups: MissingGroupsRecord[] = []
@@ -72,6 +74,7 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
7274
alertOnGroupDisappear,
7375
calculatedTimerange,
7476
logger,
77+
dataView,
7578
lastPeriodEnd
7679
);
7780

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/get_data.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import type { SearchResponse, AggregationsAggregate } from '@elastic/elasticsearch/lib/api/types';
99
import type { ElasticsearchClient } from '@kbn/core/server';
10+
import type { DataViewBase } from '@kbn/es-query';
1011
import type { Logger } from '@kbn/logging';
1112
import type { EcsFieldsResponse } from '@kbn/rule-registry-plugin/common';
1213
import { COMPARATORS } from '@kbn/alerting-comparators';
@@ -121,6 +122,7 @@ export const getData = async (
121122
alertOnGroupDisappear: boolean,
122123
timeframe: { start: number; end: number },
123124
logger: Logger,
125+
dataView?: DataViewBase,
124126
lastPeriodEnd?: number,
125127
previousResults: GetDataResponse = {},
126128
afterKey?: Record<string, string>
@@ -195,6 +197,7 @@ export const getData = async (
195197
alertOnGroupDisappear,
196198
timeframe,
197199
logger,
200+
dataView,
198201
lastPeriodEnd,
199202
previous,
200203
nextAfterKey
@@ -275,7 +278,8 @@ export const getData = async (
275278
groupBy,
276279
filterQuery,
277280
afterKey,
278-
fieldsExisted
281+
fieldsExisted,
282+
dataView
279283
),
280284
};
281285
logger.trace(() => `Request: ${JSON.stringify(request)}`);

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import moment from 'moment';
99
import { COMPARATORS } from '@kbn/alerting-comparators';
10+
import type { DataViewBase } from '@kbn/es-query';
1011
import type { MetricExpressionParams } from '../../../../../common/alerting/metrics';
1112
import { Aggregators } from '../../../../../common/alerting/metrics';
1213
import { getElasticsearchMetricQuery } from './metric_query';
@@ -49,6 +50,61 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => {
4950
});
5051
});
5152

53+
describe('when using a custom aggregation with a wildcard KQL filter on a keyword field', () => {
54+
const dataView: DataViewBase = {
55+
title: 'metrics-*',
56+
fields: [{ name: 'machine.os.keyword', type: 'string', esTypes: ['keyword'] }],
57+
};
58+
59+
const customParams: MetricExpressionParams = {
60+
aggType: Aggregators.CUSTOM,
61+
timeUnit: 'm',
62+
timeSize: 1,
63+
threshold: [0],
64+
comparator: COMPARATORS.GREATER_THAN,
65+
customMetrics: [
66+
{
67+
name: 'A',
68+
aggType: Aggregators.COUNT,
69+
filter: 'machine.os.keyword: *win 7*',
70+
},
71+
],
72+
};
73+
74+
const searchBodyWithDataView = getElasticsearchMetricQuery(
75+
customParams,
76+
timeframe,
77+
100,
78+
true,
79+
void 0,
80+
void 0,
81+
void 0,
82+
void 0,
83+
void 0,
84+
dataView
85+
);
86+
87+
const searchBodyWithoutDataView = getElasticsearchMetricQuery(
88+
customParams,
89+
timeframe,
90+
100,
91+
true
92+
);
93+
94+
test('generates a wildcard query when dataView is provided', () => {
95+
const filterAgg =
96+
searchBodyWithDataView.aggs.all.aggs.currentPeriod.aggs.aggregatedValue_A.filter;
97+
expect(JSON.stringify(filterAgg)).not.toContain('query_string');
98+
expect(JSON.stringify(filterAgg)).toContain('wildcard');
99+
});
100+
101+
test('generates a query_string when no dataView is provided', () => {
102+
const filterAgg =
103+
searchBodyWithoutDataView.aggs.all.aggs.currentPeriod.aggs.aggregatedValue_A.filter;
104+
expect(JSON.stringify(filterAgg)).toContain('query_string');
105+
});
106+
});
107+
52108
describe('when passed a filterQuery', () => {
53109
const filterQuery =
54110
// This is adapted from a real-world query that previously broke alerts

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import moment from 'moment';
9+
import type { DataViewBase } from '@kbn/es-query';
910
import { isCustom, isNotCountOrCustom } from './metric_expression_params';
1011
import type { MetricExpressionParams } from '../../../../../common/alerting/metrics';
1112
import { Aggregators } from '../../../../../common/alerting/metrics';
@@ -84,7 +85,8 @@ export const getElasticsearchMetricQuery = (
8485
groupBy?: string | string[],
8586
filterQuery?: string,
8687
afterKey?: Record<string, string>,
87-
fieldsExisted?: Record<string, boolean> | null
88+
fieldsExisted?: Record<string, boolean> | null,
89+
dataView?: DataViewBase
8890
) => {
8991
const { aggType } = metricParams;
9092
if (isNotCountOrCustom(metricParams) && !metricParams.metric) {
@@ -108,7 +110,8 @@ export const getElasticsearchMetricQuery = (
108110
? createCustomMetricsAggregations(
109111
'aggregatedValue',
110112
metricParams.customMetrics,
111-
metricParams.equation
113+
metricParams.equation,
114+
dataView
112115
)
113116
: {
114117
aggregatedValue: {

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { getThresholds } from '../common/get_values';
1111
import { set } from '@kbn/safer-lodash-set';
1212
import { COMPARATORS } from '@kbn/alerting-comparators';
1313
import type {
14+
CustomMetricExpressionParams,
1415
CountMetricExpressionParams,
1516
NonCountMetricExpressionParams,
1617
} from '../../../../common/alerting/metrics';
@@ -68,6 +69,15 @@ const mockMetricsExplorerLocator = {
6869
getRedirectUrl: jest.fn(),
6970
};
7071

72+
const mockDataView = {
73+
title: 'metrics-*,metricbeat-*',
74+
fields: [{ name: 'host.name', type: 'string', esTypes: ['keyword'] }],
75+
};
76+
77+
const mockDataViewsService = {
78+
getFieldsForWildcard: jest.fn().mockResolvedValue(mockDataView.fields),
79+
};
80+
7181
const mockOptions = {
7282
executionId: '',
7383
startedAt: mockNow,
@@ -118,6 +128,7 @@ describe('The metric threshold rule type', () => {
118128
});
119129
beforeEach(() => {
120130
jest.clearAllMocks();
131+
services.getDataViews.mockResolvedValue(mockDataViewsService);
121132

122133
mockAssetDetailsLocator.getRedirectUrl.mockImplementation(
123134
({ entityId, entityType, assetDetails }: AssetDetailsLocatorParams) =>
@@ -308,6 +319,44 @@ describe('The metric threshold rule type', () => {
308319
});
309320
});
310321

322+
describe('data view fetching', () => {
323+
test('does not fetch a data view when the rule has no custom count filters', async () => {
324+
setEvaluationResults([]);
325+
326+
await executor({
327+
...mockOptions,
328+
services,
329+
params: {
330+
criteria: [baseNonCountCriterion],
331+
},
332+
});
333+
334+
expect(services.getDataViews).not.toHaveBeenCalled();
335+
expect(jest.requireMock('./lib/evaluate_rule').evaluateRule.mock.calls[0][6]).toBeUndefined();
336+
});
337+
338+
test('fetches a data view when the rule uses a filtered custom count metric', async () => {
339+
setEvaluationResults([]);
340+
341+
await executor({
342+
...mockOptions,
343+
services,
344+
params: {
345+
criteria: [baseCustomCriterion],
346+
},
347+
});
348+
349+
expect(services.getDataViews).toHaveBeenCalledTimes(1);
350+
expect(mockDataViewsService.getFieldsForWildcard).toHaveBeenCalledWith({
351+
pattern: 'metrics-*,metricbeat-*',
352+
allowNoIndex: true,
353+
});
354+
expect(jest.requireMock('./lib/evaluate_rule').evaluateRule.mock.calls[0][6]).toEqual(
355+
mockDataView
356+
);
357+
});
358+
});
359+
311360
describe('querying with a groupBy parameter', () => {
312361
const execute = (
313362
comparator: COMPARATORS,
@@ -904,7 +953,7 @@ describe('The metric threshold rule type', () => {
904953
stateResult2
905954
);
906955
expect(stateResult3.missingGroups).toEqual([{ key: 'b', bucketKey: { groupBy0: 'b' } }]);
907-
expect(mockedEvaluateRule.mock.calls[2][8]).toEqual([
956+
expect(mockedEvaluateRule.mock.calls[2][9]).toEqual([
908957
{ bucketKey: { groupBy0: 'b' }, key: 'b' },
909958
]);
910959
});
@@ -3549,3 +3598,18 @@ const baseCountCriterion = {
35493598
threshold: [0],
35503599
comparator: COMPARATORS.GREATER_THAN,
35513600
} as CountMetricExpressionParams;
3601+
3602+
const baseCustomCriterion = {
3603+
aggType: Aggregators.CUSTOM,
3604+
timeSize: 1,
3605+
timeUnit: 'm',
3606+
threshold: [0],
3607+
comparator: COMPARATORS.GREATER_THAN,
3608+
customMetrics: [
3609+
{
3610+
name: 'A',
3611+
aggType: Aggregators.COUNT,
3612+
filter: 'host.name: *foo*',
3613+
},
3614+
],
3615+
} as CustomMetricExpressionParams;

x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,11 @@ import {
3434
type Group,
3535
getFormattedGroups,
3636
} from '@kbn/alerting-rule-utils';
37+
import type { DataViewBase } from '@kbn/es-query';
3738
import { convertToBuiltInComparators } from '@kbn/observability-plugin/common/utils/convert_legacy_outside_comparator';
3839
import { getOriginalActionGroup } from '../../../utils/get_original_action_group';
39-
import { AlertStates } from '../../../../common/alerting/metrics';
40+
import { Aggregators, AlertStates } from '../../../../common/alerting/metrics';
41+
import type { MetricExpressionParams } from '../../../../common/alerting/metrics';
4042
import { createFormatter } from '../../../../common/formatters';
4143
import type { InfraBackendLibs, InfraLocators } from '../../infra_types';
4244
import {
@@ -61,6 +63,7 @@ import type { EvaluatedRuleParams, Evaluation } from './lib/evaluate_rule';
6163
import { evaluateRule } from './lib/evaluate_rule';
6264
import type { MissingGroupsRecord } from './lib/check_missing_group';
6365
import { convertStringsToMissingGroupsRecord } from './lib/convert_strings_to_missing_groups_record';
66+
import { isCustom } from './lib/metric_expression_params';
6467

6568
export type MetricThresholdAlert = Omit<
6669
ObservabilityMetricsAlert,
@@ -264,13 +267,31 @@ export const createMetricThresholdExecutor =
264267
)
265268
: [];
266269

270+
let dataView: DataViewBase | undefined;
271+
if (shouldCreateDataView(criteria)) {
272+
const dataViewsService = await services.getDataViews();
273+
try {
274+
const fields = await dataViewsService.getFieldsForWildcard({
275+
pattern: config.metricAlias,
276+
allowNoIndex: true,
277+
});
278+
dataView = {
279+
title: config.metricAlias,
280+
fields,
281+
};
282+
} catch (e) {
283+
// ignore — dataView stays undefined and toElasticsearchQuery degrades gracefully
284+
}
285+
}
286+
267287
const alertResults = await evaluateRule(
268288
services.scopedClusterClient.asCurrentUser,
269289
params as EvaluatedRuleParams,
270290
config,
271291
compositeSize,
272292
alertOnGroupDisappear,
273293
logger,
294+
dataView,
274295
state.lastRunTimestamp,
275296
{ end: startedAt.valueOf() },
276297
convertStringsToMissingGroupsRecord(previousMissingGroups)
@@ -575,6 +596,17 @@ const mapToConditionsLookup = (
575596
return result;
576597
}, {} as Record<string, unknown>);
577598

599+
const shouldCreateDataView = (criteria: MetricExpressionParams[]) =>
600+
criteria.some((criterion) => {
601+
if (!isCustom(criterion)) {
602+
return false;
603+
}
604+
605+
return criterion.customMetrics.some(
606+
(customMetric) => customMetric.aggType === Aggregators.COUNT && customMetric.filter != null
607+
);
608+
});
609+
578610
const formatAlertResult = <AlertResult>(
579611
alertResult: {
580612
metric: string;

0 commit comments

Comments
 (0)