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.

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

[ResponseOps] ES|QL rule type improvements - write query results to the alert doc #184541

Merged
merged 19 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import * as rt from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { AlertSchema } from './alert_schema';
import { EcsSchema } from './ecs_schema';
const ISO_DATE_PATTERN = /^d{4}-d{2}-d{2}Td{2}:d{2}:d{2}.d{3}Z$/;
export const IsoDateString = new rt.Type<string, string, unknown>(
'IsoDateString',
Expand Down Expand Up @@ -77,6 +78,6 @@ const StackAlertOptional = rt.partial({
});

// prettier-ignore
export const StackAlertSchema = rt.intersection([StackAlertRequired, StackAlertOptional, AlertSchema]);
export const StackAlertSchema = rt.intersection([StackAlertRequired, StackAlertOptional, AlertSchema, EcsSchema]);
// prettier-ignore
export type StackAlert = rt.TypeOf<typeof StackAlertSchema>;
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const DEFAULT_VALUES = {
GROUP_BY: 'all',
EXCLUDE_PREVIOUS_HITS: false,
CAN_SELECT_MULTI_TERMS: true,
SOURCE_FIELDS: [],
};
export const SERVERLESS_DEFAULT_VALUES = {
SIZE: 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { useState, Fragment, useEffect, useCallback } from 'react';
import { debounce, get } from 'lodash';
import { get } from 'lodash';
import { FormattedMessage } from '@kbn/i18n-react';
import {
EuiFieldNumber,
Expand All @@ -16,22 +16,19 @@ import {
EuiSelect,
EuiSpacer,
} from '@elastic/eui';
import { getESQLQueryColumns } from '@kbn/esql-utils';
import { getFields, RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public';
import { TextBasedLangEditor } from '@kbn/text-based-languages/public';
import { fetchFieldsFromESQL } from '@kbn/text-based-editor';
import { getIndexPatternFromESQLQuery } from '@kbn/esql-utils';
import type { AggregateQuery } from '@kbn/es-query';
import { parseDuration } from '@kbn/alerting-plugin/common';
import {
FieldOption,
firstFieldOption,
getTimeFieldOptions,
getTimeOptions,
parseAggregationResults,
} from '@kbn/triggers-actions-ui-plugin/public/common';
import { DataView } from '@kbn/data-views-plugin/common';
import { SourceFields } from '../../components/source_fields_select';
import { EsQueryRuleParams, EsQueryRuleMetaData, SearchType } from '../types';
import { DEFAULT_VALUES, SERVERLESS_DEFAULT_VALUES } from '../constants';
import { useTriggerUiActionServices } from '../util';
Expand All @@ -42,8 +39,8 @@ import { rowToDocument, toEsQueryHits, transformDatatableToEsqlTable } from '../
export const EsqlQueryExpression: React.FC<
RuleTypeParamsExpressionProps<EsQueryRuleParams<SearchType.esqlQuery>, EsQueryRuleMetaData>
> = ({ ruleParams, setRuleParams, setRuleProperty, errors }) => {
const { expressions, http, fieldFormats, isServerless, data } = useTriggerUiActionServices();
const { esqlQuery, timeWindowSize, timeWindowUnit, timeField, sourceFields } = ruleParams;
const { expressions, http, fieldFormats, isServerless } = useTriggerUiActionServices();
const { esqlQuery, timeWindowSize, timeWindowUnit, timeField } = ruleParams;

const [currentRuleParams, setCurrentRuleParams] = useState<
EsQueryRuleParams<SearchType.esqlQuery>
Expand All @@ -61,12 +58,12 @@ export const EsqlQueryExpression: React.FC<
groupBy: DEFAULT_VALUES.GROUP_BY,
termSize: DEFAULT_VALUES.TERM_SIZE,
searchType: SearchType.esqlQuery,
sourceFields: sourceFields ?? DEFAULT_VALUES.SOURCE_FIELDS,
// The sourceFields param is ignored for the ES|QL type
sourceFields: [],
});
const [query, setQuery] = useState<AggregateQuery>({ esql: '' });
const [timeFieldOptions, setTimeFieldOptions] = useState([firstFieldOption]);
const [detectTimestamp, setDetectTimestamp] = useState<boolean>(false);
const [esFields, setEsFields] = useState<FieldOption[]>([]);
const [isLoading, setIsLoading] = useState<boolean>(false);

const setParam = useCallback(
Expand All @@ -86,7 +83,6 @@ export const EsqlQueryExpression: React.FC<
if (esqlQuery) {
if (esqlQuery.esql) {
refreshTimeFields(esqlQuery);
refreshEsFields(esqlQuery, false);
}
}
if (timeField) {
Expand Down Expand Up @@ -180,32 +176,6 @@ export const EsqlQueryExpression: React.FC<
setDetectTimestamp(hasTimestamp);
};

const refreshEsFields = async (q: AggregateQuery, resetSourceFields: boolean = true) => {
let fields: FieldOption[] = [];
try {
const columns = await getESQLQueryColumns({
esqlQuery: `${get(q, 'esql')}`,
search: data.search.search,
});
if (columns.length) {
fields = columns.map((c) => ({
name: c.id,
type: c.meta.type,
normalizedType: c.meta.type,
searchable: true,
aggregatable: true,
}));
}
} catch (error) {
/** ignore error */
}

if (resetSourceFields) {
setParam('sourceFields', undefined);
}
setEsFields(fields);
};

return (
<Fragment>
<EuiFormRow
Expand All @@ -221,12 +191,11 @@ export const EsqlQueryExpression: React.FC<
>
<TextBasedLangEditor
query={query}
onTextLangQueryChange={debounce((q: AggregateQuery) => {
onTextLangQueryChange={(q: AggregateQuery) => {
setQuery(q);
setParam('esqlQuery', q);
refreshTimeFields(q);
refreshEsFields(q);
}, 1000)}
}}
expandCodeEditor={() => true}
isCodeEditorExpanded={true}
onTextLangQuerySubmit={async () => {}}
Expand All @@ -236,14 +205,6 @@ export const EsqlQueryExpression: React.FC<
isLoading={isLoading}
/>
</EuiFormRow>
<SourceFields
onChangeSourceFields={(selectedSourceFields) =>
setParam('sourceFields', selectedSourceFields)
}
esFields={esFields}
sourceFields={sourceFields}
errors={errors.sourceFields}
/>
<EuiSpacer />
<EuiFormRow
id="timeField"
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/stack_alerts/server/rule_types/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ export const STACK_ALERTS_AAD_CONFIG: IRuleTypeAlerts<StackAlertType> = {
},
},
shouldWrite: true,
useEcs: true,
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { OnlyEsqlQueryRuleParams } from '../types';
import { Comparator } from '../../../../common/comparator_types';
import { getEsqlQuery } from './fetch_esql_query';
import { getEsqlQuery, getSourceFields } from './fetch_esql_query';

const getTimeRange = () => {
const date = Date.now();
Expand Down Expand Up @@ -98,4 +98,30 @@ describe('fetchEsqlQuery', () => {
`);
});
});

describe('getSourceFields', () => {
it('should generate the correct source fields', async () => {
const sourceFields = getSourceFields({
columns: [
{ name: '@timestamp', type: 'date' },
{ name: 'ecs.version', type: 'keyword' },
{ name: 'error.code', type: 'keyword' },
],
values: [['2023-07-12T13:32:04.174Z', '1.8.0', null]],
});

expect(sourceFields).toMatchInlineSnapshot(`
Array [
Object {
"label": "ecs.version",
"searchPath": "ecs.version",
},
Object {
"label": "error.code",
"searchPath": "error.code",
},
]
`);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
* 2.0.
*/

import { intersectionBy } from 'lodash';
import { parseAggregationResults } from '@kbn/triggers-actions-ui-plugin/common';
import { SharePluginStart } from '@kbn/share-plugin/server';
import { IScopedClusterClient, Logger } from '@kbn/core/server';
import { ecsFieldMap, alertFieldMap } from '@kbn/alerts-as-data-utils';
import { OnlyEsqlQueryRuleParams } from '../types';
import { EsqlTable, toEsQueryHits } from '../../../../common';

Expand Down Expand Up @@ -47,6 +49,8 @@ export async function fetchEsqlQuery({
path: '/_query',
body: query,
});
const hits = toEsQueryHits(response);
const sourceFields = getSourceFields(response);

const link = `${publicBaseUrl}${spacePrefix}/app/management/insightsAndAlerting/triggersActions/rule/${ruleId}`;

Expand All @@ -60,10 +64,10 @@ export async function fetchEsqlQuery({
took: 0,
timed_out: false,
_shards: { failed: 0, successful: 0, total: 0 },
hits: toEsQueryHits(response),
hits,
},
resultLimit: alertLimit,
sourceFieldsParams: params.sourceFields,
Copy link
Member

Choose a reason for hiding this comment

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

Is the sourceFields parameter now completely unused, by all the flavors of es query? Not clear too me, but seems like it.

If it is, we'll want to "remove" it, though how we do that in a ZDT/BWC safe way is not clear to me. We may have to leave it in the params forever, or something. But was wondering about the <SourceFields> element as well - presumably we can get rid of that?

I think for now, adding a comment to where sourceFields is defined as a param, indicating the parameter is now ignored, would be good. And then open up a separate issue to "clean up" any remaining references to it, as best we can - unless you know we don't have anything to clean up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is still used by the other es query types, just removed for ES|QL. I will add a comment to say the param is ignored now for ES|QL

Copy link
Contributor Author

@doakalexi doakalexi Jun 10, 2024

Choose a reason for hiding this comment

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

Resolved in this commit e84f295

sourceFieldsParams: sourceFields,
generateSourceFieldsFromHits: true,
}),
index: null,
Expand Down Expand Up @@ -98,3 +102,17 @@ export const getEsqlQuery = (
};
return query;
};

export const getSourceFields = (results: EsqlTable) => {
const resultFields = results.columns.map((c) => ({
label: c.name,
searchPath: c.name,
}));
const alertFields = Object.keys(alertFieldMap);
const ecsFields = Object.keys(ecsFieldMap)
// exclude the alert fields that we don't want to override
.filter((key) => !alertFields.includes(key))
.map((key) => ({ label: key, searchPath: key }));

return intersectionBy(resultFields, ecsFields, 'label');
};
Original file line number Diff line number Diff line change
Expand Up @@ -843,9 +843,9 @@ describe('parseAggregationResults', () => {
sampleEsqlSourceFieldsHit,
],
sourceFields: {
'host.hostname': ['host-1'],
'host.id': ['1'],
'host.name': ['host-1'],
'host.hostname': ['host-1', 'host-1', 'host-1', 'host-1'],
'host.id': ['1', '1', '1', '1'],
'host.name': ['host-1', 'host-1', 'host-1', 'host-1'],
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export const parseAggregationResults = ({

sourceFieldsParams.forEach((field) => {
if (generateSourceFieldsFromHits) {
const fieldsSet = new Set<string>();
const fieldsSet: string[] = [];
groupBucket.topHitsAgg.hits.hits.forEach((hit: SearchHit<{ [key: string]: string }>) => {
if (hit._source && hit._source[field.label]) {
fieldsSet.add(hit._source[field.label]);
fieldsSet.push(hit._source[field.label]);
}
});
sourceFields[field.label] = Array.from(fieldsSet);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
RULE_INTERVAL_MILLIS,
RULE_INTERVAL_SECONDS,
RULE_TYPE_ID,
SourceField,
} from './common';
import { createDataStream, deleteDataStream } from '../../../create_test_data';

Expand All @@ -39,12 +38,6 @@ export default function ruleTests({ getService }: FtrProviderContext) {
getAllAADDocs,
} = getRuleServices(getService);

const sourceFields = [
{ label: 'host.hostname', searchPath: 'host.hostname.keyword' },
{ label: 'host.id', searchPath: 'host.id' },
{ label: 'host.name', searchPath: 'host.name' },
];

describe('rule', async () => {
let endDate: string;
let connectorId: string;
Expand Down Expand Up @@ -81,13 +74,11 @@ export default function ruleTests({ getService }: FtrProviderContext) {
name: 'never fire',
esqlQuery:
'from .kibana-alerting-test-data | stats c = count(date) by host.hostname, host.name, host.id | where c < 0',
sourceFields,
});
await createRule({
name: 'always fire',
esqlQuery:
'from .kibana-alerting-test-data | stats c = count(date) by host.hostname, host.name, host.id | where c > -1',
sourceFields,
});

const docs = await waitForDocs(2);
Expand Down Expand Up @@ -225,13 +216,11 @@ export default function ruleTests({ getService }: FtrProviderContext) {
name: 'never fire',
esqlQuery:
'from test-data-stream | stats c = count(@timestamp) by host.hostname, host.name, host.id | where c < 0',
sourceFields,
});
await createRule({
name: 'always fire',
esqlQuery:
'from test-data-stream | stats c = count(@timestamp) by host.hostname, host.name, host.id | where c > -1',
sourceFields,
});

const messagePattern = /Document count is \d+ in the last 20s. Alert when greater than 0./;
Expand Down Expand Up @@ -397,7 +386,6 @@ export default function ruleTests({ getService }: FtrProviderContext) {
groupBy?: string;
termField?: string;
termSize?: number;
sourceFields?: SourceField[];
}

async function createRule(params: CreateRuleParams): Promise<string> {
Expand Down Expand Up @@ -469,7 +457,7 @@ export default function ruleTests({ getService }: FtrProviderContext) {
termSize: params.termSize,
timeField: params.timeField || 'date',
esqlQuery: { esql: params.esqlQuery },
sourceFields: params.sourceFields,
sourceFields: [],
},
})
.expect(200);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/api_integration/apis/maps/maps_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export default function ({ getService }: FtrProviderContext) {
return fieldStat.name === 'geo_point';
}
);
expect(geoPointFieldStats.count).to.be(39);
expect(geoPointFieldStats.index_count).to.be(10);
expect(geoPointFieldStats.count).to.be(47);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why these values changed?

Copy link
Contributor Author

@doakalexi doakalexi Jun 5, 2024

Choose a reason for hiding this comment

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

They changed bc we are now mapping the ecs fields for the stack alert aad index

expect(geoPointFieldStats.index_count).to.be(11);

const geoShapeFieldStats = apiResponse.cluster_stats.indices.mappings.field_types.find(
(fieldStat: estypes.ClusterStatsFieldTypes) => {
Expand Down