Skip to content

Commit

Permalink
[Custom threshold] Fix using data view runtime fields during rule exe…
Browse files Browse the repository at this point in the history
…cution for the custom threshold rule (#209133)

Fixes #200772

## 🐉  Summary

This PR fixes supporting data view runtime fields during rule execution
for the custom threshold rule.

## 🧪 How to test

1. Create a runtime field as shown below:

    |Runtime field| Preview|
    |---|---|

|![Image](https://github.com/user-attachments/assets/09fc0754-8944-486e-bf85-10b3f3464d17)|![Image](https://github.com/user-attachments/assets/e93efad5-bc3b-4306-b820-8b096dbba360)|

2. Make sure alerts are generated as expected both for regular and
no-data alerts:

![image](https://github.com/user-attachments/assets/a2174e40-11a4-4d75-8500-bfce126ba7cd)

### TODO
- [x] Add an API integration test
    - [x] Test on MKI

(cherry picked from commit 8fe5738)

# Conflicts:
#	x-pack/plugins/observability_solution/observability/server/lib/rules/custom_threshold/lib/is_populated_object.test.ts
#	x-pack/plugins/observability_solution/observability/server/lib/rules/custom_threshold/lib/is_populated_object.ts
  • Loading branch information
maryam-saeidi committed Feb 3, 2025
1 parent 9a0bab9 commit 77d14e7
Show file tree
Hide file tree
Showing 12 changed files with 409 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ const mockedIndex = {
const mockedDataView = {
getIndexPattern: () => 'mockedIndexPattern',
getName: () => 'mockedDataViewName',
getRuntimeMappings: () => undefined,
...mockedIndex,
};
const mockedSearchSource = {
Expand Down Expand Up @@ -971,7 +972,7 @@ describe('The custom threshold alert type', () => {
stateResult2
);
expect(stateResult3.missingGroups).toEqual([{ key: 'b', bucketKey: { groupBy0: 'b' } }]);
expect(mockedEvaluateRule.mock.calls[2][10]).toEqual([
expect(mockedEvaluateRule.mock.calls[2][11]).toEqual([
{ bucketKey: { groupBy0: 'b' }, key: 'b' },
]);
});
Expand Down Expand Up @@ -2961,7 +2962,7 @@ describe('The custom threshold alert type', () => {
stateResult2
);
expect(stateResult3.missingGroups).toEqual([{ key: 'b', bucketKey: { groupBy0: 'b' } }]);
expect(mockedEvaluateRule.mock.calls[2][10]).toEqual([
expect(mockedEvaluateRule.mock.calls[2][11]).toEqual([
{ bucketKey: { groupBy0: 'b' }, key: 'b' },
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export const createCustomThresholdExecutor = ({
const initialSearchSource = await searchSourceClient.create(params.searchConfiguration);
const dataView = initialSearchSource.getField('index')!;
const { id: dataViewId, timeFieldName } = dataView;
const runtimeMappings = dataView.getRuntimeMappings();
const dataViewIndexPattern = dataView.getIndexPattern();
const dataViewName = dataView.getName();
if (!dataViewIndexPattern) {
Expand All @@ -147,6 +148,7 @@ export const createCustomThresholdExecutor = ({
logger,
{ end: dateEnd, start: dateStart },
esQueryConfig,
runtimeMappings,
state.lastRunTimestamp,
previousMissingGroups
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { ElasticsearchClient } from '@kbn/core/server';
import type { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/types';
import type { EsQueryConfig } from '@kbn/es-query';
Expand All @@ -16,6 +17,7 @@ import {
} from '../../../../../common/custom_threshold_rule/types';
import type { BucketKey } from './get_data';
import { calculateCurrentTimeFrame, createBoolQuery } from './metric_query';
import { isPopulatedObject } from './is_populated_object';

export interface MissingGroupsRecord {
key: string;
Expand All @@ -32,7 +34,8 @@ export const checkMissingGroups = async (
logger: Logger,
timeframe: { start: number; end: number },
esQueryConfig: EsQueryConfig,
missingGroups: MissingGroupsRecord[] = []
missingGroups: MissingGroupsRecord[] = [],
runtimeMappings?: estypes.MappingRuntimeFields
): Promise<MissingGroupsRecord[]> => {
if (missingGroups.length === 0) {
return missingGroups;
Expand Down Expand Up @@ -65,6 +68,7 @@ export const checkMissingGroups = async (
terminate_after: 1,
track_total_hits: true,
query,
...(isPopulatedObject(runtimeMappings) ? { runtime_mappings: runtimeMappings } : {}),
},
];
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import moment from 'moment';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { ElasticsearchClient } from '@kbn/core/server';
import { EsQueryConfig } from '@kbn/es-query';
import type { Logger } from '@kbn/logging';
Expand Down Expand Up @@ -45,6 +46,7 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
logger: Logger,
timeframe: { start: string; end: string },
esQueryConfig: EsQueryConfig,
runtimeMappings?: estypes.MappingRuntimeFields,
lastPeriodEnd?: number,
missingGroups: MissingGroupsRecord[] = []
): Promise<Array<Record<string, Evaluation>>> => {
Expand Down Expand Up @@ -77,6 +79,7 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
alertOnGroupDisappear,
calculatedTimerange,
logger,
runtimeMappings,
lastPeriodEnd
);

Expand All @@ -90,7 +93,8 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
logger,
calculatedTimerange,
esQueryConfig,
missingGroups
missingGroups,
runtimeMappings
);

for (const missingGroup of verifiedMissingGroups) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import type { SearchResponse, AggregationsAggregate } from '@elastic/elasticsearch/lib/api/types';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { ElasticsearchClient } from '@kbn/core/server';
import type { EsQueryConfig } from '@kbn/es-query';
import type { Logger } from '@kbn/logging';
Expand Down Expand Up @@ -110,6 +111,7 @@ export const getData = async (
alertOnGroupDisappear: boolean,
timeframe: { start: number; end: number },
logger: Logger,
runtimeMappings?: estypes.MappingRuntimeFields,
lastPeriodEnd?: number,
previousResults: GetDataResponse = {},
afterKey?: Record<string, string>
Expand Down Expand Up @@ -170,6 +172,7 @@ export const getData = async (
alertOnGroupDisappear,
timeframe,
logger,
runtimeMappings,
lastPeriodEnd,
previous,
nextAfterKey
Expand Down Expand Up @@ -209,6 +212,7 @@ export const getData = async (
alertOnGroupDisappear,
searchConfiguration,
esQueryConfig,
runtimeMappings,
lastPeriodEnd,
groupBy,
afterKey,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 { isPopulatedObject } from './is_populated_object';

describe('isPopulatedObject', () => {
it('does not allow numbers', () => {
expect(isPopulatedObject(0)).toBe(false);
});
it('does not allow strings', () => {
expect(isPopulatedObject('')).toBe(false);
});
it('does not allow null', () => {
expect(isPopulatedObject(null)).toBe(false);
});
it('does not allow an empty object', () => {
expect(isPopulatedObject({})).toBe(false);
});
it('allows an object with an attribute', () => {
expect(isPopulatedObject({ attribute: 'value' })).toBe(true);
});
it('does not allow an object with a non-existing required attribute', () => {
expect(isPopulatedObject({ attribute: 'value' }, ['otherAttribute'])).toBe(false);
});
it('allows an object with an existing required attribute', () => {
expect(isPopulatedObject({ attribute: 'value' }, ['attribute'])).toBe(true);
});
it('allows an object with two existing required attributes', () => {
expect(
isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
'attribute1',
'attribute2',
])
).toBe(true);
});
it('does not allow an object with two required attributes where one does not exist', () => {
expect(
isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
'attribute1',
'otherAttribute',
])
).toBe(false);
});
it('does not allow an object with a required attribute in the prototype ', () => {
const testObject = { attribute: 'value', __proto__: { otherAttribute: 'value' } };
expect(isPopulatedObject(testObject, ['otherAttribute'])).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.
*/

/**
* A type guard to check record like object structures.
*
* Examples:
* - `isPopulatedObject({...})`
* Limits type to Record<string, unknown>
*
* - `isPopulatedObject({...}, ['attribute'])`
* Limits type to Record<'attribute', unknown>
*
* - `isPopulatedObject<keyof MyInterface>({...})`
* Limits type to a record with keys of the given interface.
* Note that you might want to add keys from the interface to the
* array of requiredAttributes to satisfy runtime requirements.
* Otherwise you'd just satisfy TS requirements but might still
* run into runtime issues.
*/
export const isPopulatedObject = <U extends string = string, T extends unknown = unknown>(
arg: unknown,
requiredAttributes: U[] = []
): arg is Record<U, T> => {
return (
typeof arg === 'object' &&
arg !== null &&
Object.keys(arg).length > 0 &&
(requiredAttributes.length === 0 || requiredAttributes.every((d) => Object.hasOwn(arg, d)))
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => {
true,
searchConfiguration,
esQueryConfig,
undefined,
void 0,
groupBy
);
Expand Down Expand Up @@ -121,6 +122,7 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => {
true,
currentSearchConfiguration,
esQueryConfig,
undefined,
void 0,
groupBy
);
Expand Down Expand Up @@ -233,6 +235,7 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => {
true,
currentSearchConfiguration,
esQueryConfig,
undefined,
void 0,
groupBy
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import moment from 'moment';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/types';
import type { EsQueryConfig, Filter } from '@kbn/es-query';
import {
Expand All @@ -24,6 +25,7 @@ import {
} from '../utils';
import { createBucketSelector } from './create_bucket_selector';
import { wrapInCurrentPeriod } from './wrap_in_period';
import { isPopulatedObject } from './is_populated_object';

export const calculateCurrentTimeFrame = (
metricParams: CustomMetricExpressionParams,
Expand Down Expand Up @@ -76,6 +78,7 @@ export const getElasticsearchMetricQuery = (
alertOnGroupDisappear: boolean,
searchConfiguration: SearchConfigurationType,
esQueryConfig: EsQueryConfig,
runtimeMappings?: estypes.MappingRuntimeFields,
lastPeriodEnd?: number,
groupBy?: string | string[],
afterKey?: Record<string, string>,
Expand Down Expand Up @@ -211,6 +214,7 @@ export const getElasticsearchMetricQuery = (
return {
track_total_hits: true,
query,
...(isPopulatedObject(runtimeMappings) ? { runtime_mappings: runtimeMappings } : {}),
size: 0,
aggs,
};
Expand Down
Loading

0 comments on commit 77d14e7

Please sign in to comment.