-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Changes from 12 commits
2000b5e
e06bd7e
63bd38d
8af0458
25a2b08
c7ccaf8
a77677d
619268a
8666077
23d1d5b
e3fabe5
7191672
5b9159b
e84f295
2c930af
91af172
ff1c0b6
4f46742
ab8dcaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -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}`; | ||
|
||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in this commit e84f295 |
||
sourceFieldsParams: sourceFields, | ||
generateSourceFieldsFromHits: true, | ||
}), | ||
index: null, | ||
|
@@ -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 |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know why these values changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEFAULT_VALUES.SOURCE_FIELDS
seems to be[]
, and this seems to be the only reference to it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh true, I can remove it.
There was a problem hiding this comment.
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