-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Add new gap fill status for rules #242595
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 29 commits
80b48fa
ceb9efb
fa4df71
14d3aa1
2563e6d
ae8957a
c2875a8
e855c2c
45be9ce
7487c87
866502b
fff6356
9bd2edd
ac40100
ba4dac0
7823b31
b298b8f
161fce4
ea6761d
7fea0ee
781f918
cc5d324
56f8f33
58a564c
5ef908e
d6acab4
db21de9
d023c10
f20f19e
4bf14ea
5d0d8d4
40fa19b
db3ca63
323160a
9010034
2d9a134
ab090dc
2b777c7
806f4c1
7ac3e51
1af566f
595db05
03800ec
e7aeac4
b9b2790
176cce6
0a3eec8
c834619
266ca48
e01520d
4d67f8b
9d8cdca
cdce191
099b792
82062ee
d6ec8ed
7747db4
15167f5
651f2b9
f091826
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 | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -14146,6 +14146,14 @@ paths: | |||||||
| required: false | ||||||||
| schema: | ||||||||
| type: string | ||||||||
| - description: Aggregated gap fill statuses | ||||||||
| in: query | ||||||||
| name: gap_fill_statuses | ||||||||
| required: false | ||||||||
| schema: | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| responses: | ||||||||
| '200': | ||||||||
| content: | ||||||||
|
|
@@ -79648,6 +79656,11 @@ components: | |||||||
| enum: | ||||||||
| - delete | ||||||||
| type: string | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
|
Member
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. I'm not sure what to suggest for this yet because I need more background on the "aggregated gap fill statuses" phrase. I'm also not clear on what the text in the parenthesis is actually communicating. Are the returned results somehow affected by when gaps start and end for the specified rules?
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.
That this params used together with gaps_range_start and gaps_range_end param. We use this range to query gaps. Maybe we want for last day, last week, etc. In UI by default we use last 90 days
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. @nkhristinin @nastasha-solomon how about something like:
Suggested change
|
||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79674,6 +79687,11 @@ components: | |||||||
| enum: | ||||||||
| - disable | ||||||||
| type: string | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79713,6 +79731,11 @@ components: | |||||||
| required: | ||||||||
| - include_exceptions | ||||||||
| - include_expired_exceptions | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79813,6 +79836,11 @@ components: | |||||||
| $ref: '#/components/schemas/Security_Detections_API_BulkActionEditPayload' | ||||||||
| minItems: 1 | ||||||||
| type: array | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79844,6 +79872,11 @@ components: | |||||||
| enum: | ||||||||
| - enable | ||||||||
| type: string | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79872,6 +79905,11 @@ components: | |||||||
| enum: | ||||||||
| - export | ||||||||
| type: string | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79915,6 +79953,11 @@ components: | |||||||
| required: | ||||||||
| - start_date | ||||||||
| - end_date | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -79942,6 +79985,11 @@ components: | |||||||
| enum: | ||||||||
| - run | ||||||||
| type: string | ||||||||
| gap_fill_statuses: | ||||||||
| description: Aggregated gap fill statuses to filter rules with gaps by status (used together with gaps_range_*). | ||||||||
| items: | ||||||||
| $ref: '#/components/schemas/Security_Detections_API_GapFillStatus' | ||||||||
| type: array | ||||||||
| gaps_range_end: | ||||||||
| description: Gaps range end, valid only when query is provided | ||||||||
| type: string | ||||||||
|
|
@@ -81110,6 +81158,12 @@ components: | |||||||
| - updated_at | ||||||||
| - updatedAt | ||||||||
| type: string | ||||||||
| Security_Detections_API_GapFillStatus: | ||||||||
| enum: | ||||||||
|
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. I would add an accompanying
Suggested change
|
||||||||
| - unfilled | ||||||||
| - in_progress | ||||||||
| - filled | ||||||||
| type: string | ||||||||
| Security_Detections_API_HistoryWindowStart: | ||||||||
| description: Start date to use when checking if a term has been seen before. Supports relative dates – for example, now-30d will search the last 30 days of data when checking if a term is new. We do not recommend using absolute dates, which can cause issues with rule performance due to querying increasing amounts of data over time. | ||||||||
| format: nonempty | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,3 +12,11 @@ export const gapStatus = { | |
| } as const; | ||
|
|
||
| export type GapStatus = (typeof gapStatus)[keyof typeof gapStatus]; | ||
|
|
||
| export const gapFillStatus = { | ||
|
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. Nit: It'd be nice to have comments describing the meaning of gapStatus and gapFillStatus with links to gap definition and extra details. |
||
| UNFILLED: 'unfilled', | ||
| IN_PROGRESS: 'in_progress', | ||
| FILLED: 'filled', | ||
| } as const; | ||
|
|
||
| export type GapFillStatus = (typeof gapFillStatus)[keyof typeof gapFillStatus]; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,30 @@ | |
| * 2.0. | ||
| */ | ||
| import { schema } from '@kbn/config-schema'; | ||
| import { gapFillStatus, gapStatus } from '../../../../../constants'; | ||
|
|
||
| export const getRuleIdsWithGapBodySchema = schema.object( | ||
| { | ||
| end: schema.string(), | ||
| start: schema.string(), | ||
| statuses: schema.maybe(schema.arrayOf(schema.string())), | ||
| statuses: schema.maybe( | ||
| schema.arrayOf( | ||
| schema.oneOf([ | ||
| schema.literal(gapStatus.UNFILLED), | ||
| schema.literal(gapStatus.PARTIALLY_FILLED), | ||
| schema.literal(gapStatus.FILLED), | ||
| ]) | ||
| ) | ||
| ), | ||
| highest_priority_gap_fill_statuses: schema.maybe( | ||
| schema.arrayOf( | ||
| schema.oneOf([ | ||
| schema.literal(gapFillStatus.UNFILLED), | ||
| schema.literal(gapFillStatus.IN_PROGRESS), | ||
| schema.literal(gapFillStatus.FILLED), | ||
| ]) | ||
| ) | ||
| ), | ||
|
Comment on lines
+16
to
+35
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. Could you please comment ** statuses** and ** highest_priority_gap_fill_statuses**? In particular it'd be nice to a justification for naming choice. Like why highest_priority_gap_fill_statuses is used while it could be just gap_fill_statuses. |
||
| has_unfilled_intervals: schema.maybe(schema.boolean()), | ||
| has_in_progress_intervals: schema.maybe(schema.boolean()), | ||
| has_filled_intervals: schema.maybe(schema.boolean()), | ||
|
|
||
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.
I need help understanding the description. Can you explain in a different way what the
gap_fill_statusesreturns?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.
Represents the status of a gap fill process for a rule.
This is a derived, per-rule aggregation that summarises how well the set of gaps for a rule has been filled. It is calculated from the gap duration sums with priority: unfilled > in_progress > filled.
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.
As per my other comment I don't think "Aggregated" is a helpful enough adjective, here: I would suggest using "An array of [description of single
gap_fill_status]"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.
I'm still having trouble understanding what the
gap_fill_statusesparam actually queries and what (if any) values users can specify. The "aggregated gap status" term is still unclear to me, along with the explanation of it in the PR description. A demo of the corresponding UI element (if there is one) would be really helpful here. A sample response for this param would be helpful too.If the
gap_fill_statusesparam is meant to retrieve a summary of the gap statuses that you specify (e.g.,Filled,In progress, andUnfilled), perhaps the following revision would work:Array containing statuses that are used to indicate the states of gaps that have been reported for a rule. The values must be:
Filled,In progress, andUnfilledThere 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.
I'm still having trouble understanding what the gap_fill_statuses param actually queries and what (if any) values users can specify.
it used to filter rules by the gap_fill_statuses
"aggregated gap status" - not used anymore, and we have gap fill status only.
On the screenshot there gap fill status filter.
it's not for retrieve summary, it's only for filter rules.