-
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
[Endpoint] Recursive resolver children #61914
Changes from all commits
42e9f93
228ed62
d3d82a7
4d248bc
58009e5
3b35b1a
a36ef94
63ad7eb
2b4ce9f
d686f17
0731e88
dd4ac56
51fcea6
35d0631
b4a5a6a
8899993
c4bcbbb
50d5d35
e5e1c96
1d6cd24
e50dc02
139c4f2
7fdf029
3e9fa09
63f13a7
0f5963d
307318d
eed8ca3
e5a3a48
b822069
288aadd
24abb1f
8321e99
9b9f46e
7f47791
8ebf221
a607144
b47f270
b430f18
5725da1
3ae0c85
8262af9
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,59 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { schema } from '@kbn/config-schema'; | ||
|
||
/** | ||
* Used to validate GET requests for a complete resolver tree. | ||
*/ | ||
export const validateTree = { | ||
params: schema.object({ id: schema.string() }), | ||
query: schema.object({ | ||
children: schema.number({ defaultValue: 10, min: 0, max: 100 }), | ||
generations: schema.number({ defaultValue: 3, min: 0, max: 3 }), | ||
ancestors: schema.number({ defaultValue: 3, min: 0, max: 5 }), | ||
events: schema.number({ defaultValue: 100, min: 0, max: 1000 }), | ||
afterEvent: schema.maybe(schema.string()), | ||
afterChild: schema.maybe(schema.string()), | ||
legacyEndpointID: schema.maybe(schema.string()), | ||
}), | ||
}; | ||
|
||
/** | ||
* Used to validate GET requests for non process events for a specific event. | ||
*/ | ||
export const validateEvents = { | ||
params: schema.object({ id: schema.string() }), | ||
query: schema.object({ | ||
events: schema.number({ defaultValue: 100, min: 1, max: 1000 }), | ||
afterEvent: schema.maybe(schema.string()), | ||
legacyEndpointID: schema.maybe(schema.string()), | ||
}), | ||
}; | ||
|
||
/** | ||
* Used to validate GET requests for the ancestors of a process event. | ||
*/ | ||
export const validateAncestry = { | ||
params: schema.object({ id: schema.string() }), | ||
query: schema.object({ | ||
ancestors: schema.number({ defaultValue: 0, min: 0, max: 10 }), | ||
legacyEndpointID: schema.maybe(schema.string()), | ||
}), | ||
}; | ||
|
||
/** | ||
* Used to validate GET requests for children of a specified process event. | ||
*/ | ||
export const validateChildren = { | ||
params: schema.object({ id: schema.string() }), | ||
query: schema.object({ | ||
children: schema.number({ defaultValue: 10, min: 10, max: 100 }), | ||
generations: schema.number({ defaultValue: 3, min: 0, max: 3 }), | ||
afterChild: schema.maybe(schema.string()), | ||
legacyEndpointID: schema.maybe(schema.string()), | ||
}), | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,31 @@ type ImmutableObject<T> = { readonly [K in keyof T]: Immutable<T[K]> }; | |
*/ | ||
export type AlertAPIOrdering = 'asc' | 'desc'; | ||
|
||
export interface ResolverNodeStats { | ||
totalEvents: number; | ||
totalAlerts: number; | ||
} | ||
|
||
export interface ResolverNodePagination { | ||
nextChild?: string | null; | ||
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. ❔ The 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. ℹ️ Note to self to replace/merge some of the things in adjacencyMap with this so the meaning doesn't diverge 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.
Yeah I think 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. @jonathan-buttner @kqualters-elastic if there is special meaning to these interface fields, can we document it here? Also, can you clarify what "you've reached the last page" means? Data could still be coming in right? So unless we know that there will never be any more data, we will probably need to assume that there is more. Let me know if I'm wrong or unclear. I'm writing everything on a tablet and it's tedious 😅 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. TODO ask about this 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. @oatkiller Yeah that's a fair point that we won't ever know for sure that there isn't more data because we could have been received some since they last time we searched, but it's probably still useful to indicate that we know for sure that there is more data available right? For example if we only retrieve a portion of the children for a node, by indicating that we know there are more children we could display an arrow or plus sign or something in the UI indicating to the user that there is definitely more children to retrieve. |
||
nextEvent?: string | null; | ||
nextAncestor?: string | null; | ||
nextAlert?: string | null; | ||
} | ||
|
||
/** | ||
* A node that contains pointers to other nodes, arrrays of resolver events, and any metadata associated with resolver specific data | ||
*/ | ||
export interface ResolverNode { | ||
id: string; | ||
children: ResolverNode[]; | ||
events: ResolverEvent[]; | ||
lifecycle: ResolverEvent[]; | ||
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. how's this different than 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. right, 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. 🎗️ |
||
ancestors?: ResolverNode[]; | ||
pagination: ResolverNodePagination; | ||
stats?: ResolverNodeStats; | ||
} | ||
|
||
/** | ||
* Returned by 'api/endpoint/alerts' | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,5 +33,6 @@ export const AlertDetailResolver = styled( | |
width: 100%; | ||
display: flex; | ||
flex-grow: 1; | ||
min-height: 500px; | ||
/* gross demo hack */ | ||
min-height: calc(100vh - 505px); | ||
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. ❔ Did you check this on laptop-sized monitors? Seems like it would get squishy, 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 505px is the static height of the alert detail content above it, not sure what choice we have really. 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 think this is an improvement, but the long term solution should involve flexbox or something |
||
`; |
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.
Is
totalEvents
equal to the count ofnon-alert events + alert events
?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.
Do we need this info? Isn't it expensive to get a count (esp. an exact count?)
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 think the info would useful when display totals in the UI.
I think this would be useful for displaying counts,
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.
❔ Does
totalAlerts
mean total related alerts? If so, can we add that to the interface name?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.
@jonathan-buttner if we showed the user some total, we would need to explain what the total is. What is this?
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.
Sounds like we're probably going to remove the stats stuff for now, but I think what this is trying to communicate is
totalAlerts
is the total number of alerts associated with a specific process node/entity_id aka how many alerts has this specific process generated.totalEvents
is how many related events exists for this process.