-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[ResponseOps][Maintenance Window] Maintenance Window does not apply when using wildcard via Query DSL #256622
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
Changes from all commits
86fca63
9cc1299
72cda70
cbb8cae
3039e00
03c2717
7a50cf5
7840dcc
15991ee
23c0785
8cb110a
452cbfb
271f10a
97ac8fa
fa6960a
0c5700b
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 |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * 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 { ALERT_RULE_NAME, ALERT_STATUS, ALERT_DURATION } from '@kbn/rule-data-utils'; | ||
| import { getAlertsDataViewBase } from './get_alerts_data_view_base'; | ||
|
|
||
| describe('getAlertsDataViewBase', () => { | ||
| it('should return a DataViewBase with the alerts index pattern', () => { | ||
| const dataView = getAlertsDataViewBase(); | ||
| expect(dataView.title).toBe('.alerts-*'); | ||
| expect(dataView.fields.length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('should map keyword fields with correct esTypes', () => { | ||
| const dataView = getAlertsDataViewBase(); | ||
| const ruleNameField = dataView.fields.find((f) => f.name === ALERT_RULE_NAME); | ||
|
|
||
| expect(ruleNameField).toBeDefined(); | ||
| expect(ruleNameField!.type).toBe('string'); | ||
| expect(ruleNameField!.esTypes).toEqual(['keyword']); | ||
| }); | ||
|
|
||
| it('should map date fields correctly', () => { | ||
| const dataView = getAlertsDataViewBase(); | ||
| const statusField = dataView.fields.find((f) => f.name === ALERT_STATUS); | ||
|
|
||
| expect(statusField).toBeDefined(); | ||
| expect(statusField!.type).toBe('string'); | ||
| expect(statusField!.esTypes).toEqual(['keyword']); | ||
| }); | ||
|
|
||
| it('should map long fields as number type', () => { | ||
| const dataView = getAlertsDataViewBase(); | ||
| const durationField = dataView.fields.find((f) => f.name === ALERT_DURATION); | ||
|
|
||
| expect(durationField).toBeDefined(); | ||
| expect(durationField!.type).toBe('number'); | ||
| expect(durationField!.esTypes).toEqual(['long']); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * 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 { alertFieldMap } from '@kbn/alerts-as-data-utils'; | ||
|
Contributor
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. We probably already have some code to generate mappings from the alertFieldMap, since someone has to create the mappings :-). We should find that and reuse it - exporting it or whatever to make it available here. I'm not sure if different alert indices have different mappings, but I'm 98% sure they can, so I'm not sure this approach covers 100% of the cases. I feel like we may need to have the code that evaluates the MW get passed the mappings. So the alerting code would figure out what alerting indices were going to be queried over, and pass them into the evaluator.
Contributor
Author
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. There is mappingFromFieldMap() but looks like it generates es index mappings (nested) not a DataViewBase (flat field with esTypes). I don't think we can reuse it directly, but maybe I'm wrong. However, looks like rule types can register custom fields using IRuleTypeAlerts.mappings.fieldMap. The fix adding I'll look into adding combined maps for this edge case.
Contributor
Author
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. Hey @pmuellr , I did a some investigation and for combined maps: we could aggregate all registered
Contributor
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. Ya, even if we just handle all the standard fields, that should be a great first step. Assuming we do that, let's open an issue to later deal with the custom fields. I suspect the easiest thing to do will be to get the fields from ES based on the index pattern we use, which could be a little expensive, so will be tricky to get right :-)
Contributor
Author
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. Opened the follow up issue: #259076 |
||
| import type { DataViewBase, DataViewFieldBase } from '@kbn/es-query'; | ||
|
|
||
| const ES_TYPE_TO_KBN_TYPE: Record<string, string> = { | ||
| keyword: 'string', | ||
| text: 'string', | ||
| long: 'number', | ||
| integer: 'number', | ||
| short: 'number', | ||
| byte: 'number', | ||
| double: 'number', | ||
| float: 'number', | ||
| half_float: 'number', | ||
| scaled_float: 'number', | ||
| date: 'date', | ||
| date_range: 'date_range', | ||
| boolean: 'boolean', | ||
| flattened: 'string', | ||
| version: 'string', | ||
| unmapped: 'string', | ||
| }; | ||
|
|
||
| export function getAlertsDataViewBase(): DataViewBase { | ||
| const fields: DataViewFieldBase[] = Object.entries(alertFieldMap).map(([name, def]) => ({ | ||
| name, | ||
| type: ES_TYPE_TO_KBN_TYPE[def.type] ?? def.type, | ||
| esTypes: [def.type], | ||
| scripted: false, | ||
| })); | ||
|
|
||
| return { title: '.alerts-*', fields }; | ||
| } | ||
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.
We also need to figure out, in the KQL prompter, what they return when you don't use the KQL controls, but click the "Use Query DSL" and enter in your own DSL. And then test that here as well.
It's not clear to me why using Query DSL in the KQL picker would need an index pattern, so I was wondering if there was something different about using the Use Query DSL option, that we weren't handling correctrly.
Uh oh!
There was an error while loading. Please reload this page.
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.
When the user clicks "Edit as Query DSL", the raw DSL is stored in
filter.queryand passed through scope.alerting.filters. On the server side,buildEsQuery()returns filter.query as is viatranslateToQuery(), this means no index pattern needed. You're right that Query DSL doesn't need one.The
getAlertsDataViewBase()only affects the KQL part, when the system needs field types to generate the wildcard queries. I'll add a test confirming Query DSL wildcards pass through unchanged.