From cea31b342e566f0dab33f2afea200c2d1d4a465d Mon Sep 17 00:00:00 2001 From: "ana.davydova@elastic.co" Date: Thu, 2 Apr 2026 22:58:44 +0200 Subject: [PATCH] [Rules] KQL-to-DSL conversion without data view produces incorrect queries for keyword fields for Metric threshold rule (#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 #257282 image --- .../metric_threshold/lib/evaluate_rule.ts | 3 + .../alerting/metric_threshold/lib/get_data.ts | 6 +- .../metric_threshold/lib/metric_query.test.ts | 56 ++++++++ .../metric_threshold/lib/metric_query.ts | 7 +- .../metric_threshold_executor.test.ts | 69 +++++++++- .../metric_threshold_executor.ts | 34 ++++- ...create_custom_metrics_aggregations.test.ts | 121 ++++++++++++++++++ .../lib/create_custom_metrics_aggregations.ts | 6 +- .../apis/metrics_ui/metric_threshold_alert.ts | 21 +++ 9 files changed, 315 insertions(+), 8 deletions(-) create mode 100644 x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.test.ts diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_rule.ts b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_rule.ts index 782bb2a47a839..2ad64091ed74c 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_rule.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_rule.ts @@ -6,6 +6,7 @@ */ import type { ElasticsearchClient } from '@kbn/core/server'; +import type { DataViewBase } from '@kbn/es-query'; import moment from 'moment'; import type { Logger } from '@kbn/logging'; import { isCustom } from './metric_expression_params'; @@ -44,6 +45,7 @@ export const evaluateRule = async @@ -195,6 +197,7 @@ export const getData = async ( alertOnGroupDisappear, timeframe, logger, + dataView, lastPeriodEnd, previous, nextAfterKey @@ -275,7 +278,8 @@ export const getData = async ( groupBy, filterQuery, afterKey, - fieldsExisted + fieldsExisted, + dataView ), }; logger.trace(() => `Request: ${JSON.stringify(request)}`); diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts index 453a6412eec94..d953bc966170b 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts @@ -7,6 +7,7 @@ import moment from 'moment'; import { COMPARATORS } from '@kbn/alerting-comparators'; +import type { DataViewBase } from '@kbn/es-query'; import type { MetricExpressionParams } from '../../../../../common/alerting/metrics'; import { Aggregators } from '../../../../../common/alerting/metrics'; import { getElasticsearchMetricQuery } from './metric_query'; @@ -49,6 +50,61 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { }); }); + describe('when using a custom aggregation with a wildcard KQL filter on a keyword field', () => { + const dataView: DataViewBase = { + title: 'metrics-*', + fields: [{ name: 'machine.os.keyword', type: 'string', esTypes: ['keyword'] }], + }; + + const customParams: MetricExpressionParams = { + aggType: Aggregators.CUSTOM, + timeUnit: 'm', + timeSize: 1, + threshold: [0], + comparator: COMPARATORS.GREATER_THAN, + customMetrics: [ + { + name: 'A', + aggType: Aggregators.COUNT, + filter: 'machine.os.keyword: *win 7*', + }, + ], + }; + + const searchBodyWithDataView = getElasticsearchMetricQuery( + customParams, + timeframe, + 100, + true, + void 0, + void 0, + void 0, + void 0, + void 0, + dataView + ); + + const searchBodyWithoutDataView = getElasticsearchMetricQuery( + customParams, + timeframe, + 100, + true + ); + + test('generates a wildcard query when dataView is provided', () => { + const filterAgg = + searchBodyWithDataView.aggs.all.aggs.currentPeriod.aggs.aggregatedValue_A.filter; + expect(JSON.stringify(filterAgg)).not.toContain('query_string'); + expect(JSON.stringify(filterAgg)).toContain('wildcard'); + }); + + test('generates a query_string when no dataView is provided', () => { + const filterAgg = + searchBodyWithoutDataView.aggs.all.aggs.currentPeriod.aggs.aggregatedValue_A.filter; + expect(JSON.stringify(filterAgg)).toContain('query_string'); + }); + }); + describe('when passed a filterQuery', () => { const filterQuery = // This is adapted from a real-world query that previously broke alerts diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts index 858a46be8c184..03fcfb64476d8 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts @@ -6,6 +6,7 @@ */ import moment from 'moment'; +import type { DataViewBase } from '@kbn/es-query'; import { isCustom, isNotCountOrCustom } from './metric_expression_params'; import type { MetricExpressionParams } from '../../../../../common/alerting/metrics'; import { Aggregators } from '../../../../../common/alerting/metrics'; @@ -84,7 +85,8 @@ export const getElasticsearchMetricQuery = ( groupBy?: string | string[], filterQuery?: string, afterKey?: Record, - fieldsExisted?: Record | null + fieldsExisted?: Record | null, + dataView?: DataViewBase ) => { const { aggType } = metricParams; if (isNotCountOrCustom(metricParams) && !metricParams.metric) { @@ -108,7 +110,8 @@ export const getElasticsearchMetricQuery = ( ? createCustomMetricsAggregations( 'aggregatedValue', metricParams.customMetrics, - metricParams.equation + metricParams.equation, + dataView ) : { aggregatedValue: { diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts index a626ed1c7337b..2dd0e58d0a9a6 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts @@ -11,6 +11,7 @@ import { getThresholds } from '../common/get_values'; import { set } from '@kbn/safer-lodash-set'; import { COMPARATORS } from '@kbn/alerting-comparators'; import type { + CustomMetricExpressionParams, CountMetricExpressionParams, NonCountMetricExpressionParams, } from '../../../../common/alerting/metrics'; @@ -66,6 +67,15 @@ const mockMetricsExplorerLocator = { getRedirectUrl: jest.fn(), }; +const mockDataView = { + title: 'metrics-*,metricbeat-*', + fields: [{ name: 'host.name', type: 'string', esTypes: ['keyword'] }], +}; + +const mockDataViewsService = { + getFieldsForWildcard: jest.fn().mockResolvedValue(mockDataView.fields), +}; + const mockOptions = { executionId: '', startedAt: mockNow, @@ -115,7 +125,8 @@ describe('The metric threshold rule type', () => { jest.setSystemTime(); }); beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); + services.getDataViews.mockResolvedValue(mockDataViewsService); mockAssetDetailsLocator.getRedirectUrl.mockImplementation( ({ assetId, assetType, assetDetails }: AssetDetailsLocatorParams) => @@ -306,6 +317,44 @@ describe('The metric threshold rule type', () => { }); }); + describe('data view fetching', () => { + test('does not fetch a data view when the rule has no custom count filters', async () => { + setEvaluationResults([]); + + await executor({ + ...mockOptions, + services, + params: { + criteria: [baseNonCountCriterion], + }, + }); + + expect(services.getDataViews).not.toHaveBeenCalled(); + expect(jest.requireMock('./lib/evaluate_rule').evaluateRule.mock.calls[0][6]).toBeUndefined(); + }); + + test('fetches a data view when the rule uses a filtered custom count metric', async () => { + setEvaluationResults([]); + + await executor({ + ...mockOptions, + services, + params: { + criteria: [baseCustomCriterion], + }, + }); + + expect(services.getDataViews).toHaveBeenCalledTimes(1); + expect(mockDataViewsService.getFieldsForWildcard).toHaveBeenCalledWith({ + pattern: 'metrics-*,metricbeat-*', + allowNoIndex: true, + }); + expect(jest.requireMock('./lib/evaluate_rule').evaluateRule.mock.calls[0][6]).toEqual( + mockDataView + ); + }); + }); + describe('querying with a groupBy parameter', () => { const execute = ( comparator: COMPARATORS, @@ -899,7 +948,7 @@ describe('The metric threshold rule type', () => { stateResult2 ); expect(stateResult3.missingGroups).toEqual([{ key: 'b', bucketKey: { groupBy0: 'b' } }]); - expect(mockedEvaluateRule.mock.calls[2][8]).toEqual([ + expect(mockedEvaluateRule.mock.calls[2][9]).toEqual([ { bucketKey: { groupBy0: 'b' }, key: 'b' }, ]); }); @@ -3447,6 +3496,7 @@ const mockLibs: any = { return Promise.resolve({ id: sourceId, configuration: { + metricAlias: 'metrics-*,metricbeat-*', logIndices: { type: 'index_pattern', indexPatternId: 'some-id', @@ -3508,3 +3558,18 @@ const baseCountCriterion = { threshold: [0], comparator: COMPARATORS.GREATER_THAN, } as CountMetricExpressionParams; + +const baseCustomCriterion = { + aggType: Aggregators.CUSTOM, + timeSize: 1, + timeUnit: 'm', + threshold: [0], + comparator: COMPARATORS.GREATER_THAN, + customMetrics: [ + { + name: 'A', + aggType: Aggregators.COUNT, + filter: 'host.name: *foo*', + }, + ], +} as CustomMetricExpressionParams; diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index 294f489b026da..b974975d111b6 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -31,9 +31,11 @@ import { getGroupByObject, type Group, } from '@kbn/alerting-rule-utils'; +import type { DataViewBase } from '@kbn/es-query'; import { convertToBuiltInComparators } from '@kbn/observability-plugin/common/utils/convert_legacy_outside_comparator'; import { getOriginalActionGroup } from '../../../utils/get_original_action_group'; -import { AlertStates } from '../../../../common/alerting/metrics'; +import { Aggregators, AlertStates } from '../../../../common/alerting/metrics'; +import type { MetricExpressionParams } from '../../../../common/alerting/metrics'; import { createFormatter } from '../../../../common/formatters'; import type { InfraBackendLibs, InfraLocators } from '../../infra_types'; import { @@ -59,6 +61,7 @@ import type { EvaluatedRuleParams, Evaluation } from './lib/evaluate_rule'; import { evaluateRule } from './lib/evaluate_rule'; import type { MissingGroupsRecord } from './lib/check_missing_group'; import { convertStringsToMissingGroupsRecord } from './lib/convert_strings_to_missing_groups_record'; +import { isCustom } from './lib/metric_expression_params'; export type MetricThresholdAlert = Omit< ObservabilityMetricsAlert, @@ -256,6 +259,23 @@ export const createMetricThresholdExecutor = ) : []; + let dataView: DataViewBase | undefined; + if (shouldCreateDataView(criteria)) { + const dataViewsService = await services.getDataViews(); + try { + const fields = await dataViewsService.getFieldsForWildcard({ + pattern: config.metricAlias, + allowNoIndex: true, + }); + dataView = { + title: config.metricAlias, + fields, + }; + } catch (e) { + // ignore — dataView stays undefined and toElasticsearchQuery degrades gracefully + } + } + const alertResults = await evaluateRule( services.scopedClusterClient.asCurrentUser, params as EvaluatedRuleParams, @@ -263,6 +283,7 @@ export const createMetricThresholdExecutor = compositeSize, alertOnGroupDisappear, logger, + dataView, state.lastRunTimestamp, { end: startedAt.valueOf() }, convertStringsToMissingGroupsRecord(previousMissingGroups) @@ -560,6 +581,17 @@ const mapToConditionsLookup = ( return result; }, {} as Record); +const shouldCreateDataView = (criteria: MetricExpressionParams[]) => + criteria.some((criterion) => { + if (!isCustom(criterion)) { + return false; + } + + return criterion.customMetrics.some( + (customMetric) => customMetric.aggType === Aggregators.COUNT && customMetric.filter != null + ); + }); + const formatAlertResult = ( alertResult: { metric: string; diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.test.ts b/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.test.ts new file mode 100644 index 0000000000000..108f5fabeeacc --- /dev/null +++ b/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.test.ts @@ -0,0 +1,121 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { DataViewBase } from '@kbn/es-query'; +import { Aggregators } from '../../common/alerting/metrics'; +import type { MetricExpressionCustomMetric } from '../../common/alerting/metrics'; +import { createCustomMetricsAggregations } from './create_custom_metrics_aggregations'; + +const keywordDataView: DataViewBase = { + title: 'metrics-*', + fields: [{ name: 'machine.os.keyword', type: 'string', esTypes: ['keyword'] }], +}; + +describe('createCustomMetricsAggregations', () => { + describe('count aggregation with a KQL filter', () => { + const countMetric: MetricExpressionCustomMetric = { + name: 'A', + aggType: Aggregators.COUNT, + filter: 'machine.os.keyword: *win 7*', + }; + + describe('when dataView is provided with keyword field mapping', () => { + const result = createCustomMetricsAggregations( + 'aggregatedValue', + [countMetric], + undefined, + keywordDataView + ); + + test('uses a wildcard query instead of query_string', () => { + const filterAgg = (result as any).aggregatedValue_A.filter; + expect(JSON.stringify(filterAgg)).not.toContain('query_string'); + expect(JSON.stringify(filterAgg)).toContain('wildcard'); + }); + + test('includes a bucket_script for the equation', () => { + expect((result as any).aggregatedValue).toHaveProperty('bucket_script'); + }); + + test('bucket_script buckets_path references the count filter agg', () => { + const script = (result as any).aggregatedValue.bucket_script; + expect(script.buckets_path).toEqual({ A: 'aggregatedValue_A>_count' }); + }); + }); + + describe('when dataView is not provided', () => { + const result = createCustomMetricsAggregations('aggregatedValue', [countMetric]); + + test('falls back to query_string for the wildcard pattern', () => { + const filterAgg = (result as any).aggregatedValue_A.filter; + expect(JSON.stringify(filterAgg)).toContain('query_string'); + }); + }); + }); + + describe('count aggregation without a filter', () => { + const countMetricNoFilter: MetricExpressionCustomMetric = { + name: 'A', + aggType: Aggregators.COUNT, + }; + + test('uses match_all when no filter is set', () => { + const result = createCustomMetricsAggregations('aggregatedValue', [countMetricNoFilter]); + const filterAgg = (result as any).aggregatedValue_A.filter; + expect(filterAgg).toEqual({ match_all: {} }); + }); + }); + + describe('non-count aggregation (average)', () => { + const avgMetric: MetricExpressionCustomMetric = { + name: 'A', + aggType: Aggregators.AVERAGE, + field: 'system.cpu.user.pct', + }; + + test('generates an avg aggregation on the specified field', () => { + const result = createCustomMetricsAggregations('aggregatedValue', [avgMetric]); + expect((result as any).aggregatedValue_A).toEqual({ avg: { field: 'system.cpu.user.pct' } }); + }); + }); + + describe('custom equation', () => { + const metrics: MetricExpressionCustomMetric[] = [ + { name: 'A', aggType: Aggregators.COUNT, filter: 'machine.os.keyword: *win 7*' }, + { name: 'B', aggType: Aggregators.AVERAGE, field: 'system.cpu.user.pct' }, + ]; + + test('translates equation variables to painless params', () => { + const result = createCustomMetricsAggregations( + 'aggregatedValue', + metrics, + 'A / B', + keywordDataView + ); + const script = (result as any).aggregatedValue.bucket_script.script.source; + expect(script).toBe('params.A / params.B'); + }); + + test('defaults equation to sum of all metrics when not provided', () => { + const result = createCustomMetricsAggregations( + 'aggregatedValue', + metrics, + undefined, + keywordDataView + ); + const script = (result as any).aggregatedValue.bucket_script.script.source; + expect(script).toBe('params.A + params.B'); + }); + }); + + describe('when customMetrics is empty', () => { + test('returns an empty object', () => { + const result = createCustomMetricsAggregations('aggregatedValue', []); + expect(result).toEqual({}); + }); + }); +}); diff --git a/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.ts b/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.ts index 43e1f9ed9233b..e896b3d498618 100644 --- a/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.ts +++ b/x-pack/solutions/observability/plugins/infra/server/lib/create_custom_metrics_aggregations.ts @@ -6,6 +6,7 @@ */ import { fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query'; +import type { DataViewBase } from '@kbn/es-query'; import { isEmpty } from 'lodash'; import type { MetricExpressionCustomMetric } from '../../common/alerting/metrics'; import type { MetricsExplorerCustomMetric } from '../../common/http_api'; @@ -19,7 +20,8 @@ const isMetricExpressionCustomMetric = ( export const createCustomMetricsAggregations = ( id: string, customMetrics: Array, - equation?: string + equation?: string, + dataView?: DataViewBase ) => { const bucketsPath: { [id: string]: string } = {}; const metricAggregations = customMetrics.reduce((acc, metric) => { @@ -34,7 +36,7 @@ export const createCustomMetricsAggregations = ( ...acc, [key]: { filter: metric.filter - ? toElasticsearchQuery(fromKueryExpression(metric.filter)) + ? toElasticsearchQuery(fromKueryExpression(metric.filter), dataView) : { match_all: {} }, }, }; diff --git a/x-pack/solutions/observability/test/api_integration/apis/metrics_ui/metric_threshold_alert.ts b/x-pack/solutions/observability/test/api_integration/apis/metrics_ui/metric_threshold_alert.ts index 0d1875746c646..3ce7e6c2247f2 100644 --- a/x-pack/solutions/observability/test/api_integration/apis/metrics_ui/metric_threshold_alert.ts +++ b/x-pack/solutions/observability/test/api_integration/apis/metrics_ui/metric_threshold_alert.ts @@ -118,6 +118,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -179,6 +180,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -243,6 +245,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -299,6 +302,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame, [{ key: 'middleware', bucketKey: { groupBy0: 'middleware' } }] ); @@ -378,6 +382,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); @@ -461,6 +466,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); @@ -521,6 +527,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -561,6 +568,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -616,6 +624,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -670,6 +679,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame, [ { key: 'web', bucketKey: { groupBy0: 'web' } }, @@ -785,6 +795,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -836,6 +847,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -876,6 +888,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -930,6 +943,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -995,6 +1009,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -1063,6 +1078,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame, [{ key: 'dev', bucketKey: { groupBy0: 'dev' } }] ); @@ -1117,6 +1133,7 @@ export default function ({ getService }: FtrProviderContext) { 10000, true, logger, + void 0, moment(gauge.midpoint).subtract(1, 'm').valueOf(), timeFrame, [{ key: 'dev', bucketKey: { groupBy0: 'dev' } }] @@ -1137,6 +1154,7 @@ export default function ({ getService }: FtrProviderContext) { 10000, true, logger, + void 0, gauge.max, timeFrame ); @@ -1225,6 +1243,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -1278,6 +1297,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([ @@ -1335,6 +1355,7 @@ export default function ({ getService }: FtrProviderContext) { true, logger, void 0, + void 0, timeFrame ); expect(results).to.eql([