-
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 35 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 |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { EndpointEvent, LegacyEndpointEvent } from '../types'; | ||
import { EndpointEvent, LegacyEndpointEvent, ResolverEvent } from '../types'; | ||
|
||
export function isLegacyEvent( | ||
event: EndpointEvent | LegacyEndpointEvent | ||
|
@@ -22,10 +22,33 @@ export function eventTimestamp( | |
} | ||
} | ||
|
||
/** TODO, seems wrong */ | ||
export function eventName(event: EndpointEvent | LegacyEndpointEvent): string { | ||
if (isLegacyEvent(event)) { | ||
return event.endgame.process_name ? event.endgame.process_name : ''; | ||
} else { | ||
return event.process.name; | ||
} | ||
} | ||
|
||
export function eventId(event: 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. Would you mind moving these methods that take "ResolverEvent" to a file called "resolver_event" |
||
if (isLegacyEvent(event)) { | ||
return String(event.endgame.serial_event_id); | ||
} | ||
return event.event.id; | ||
} | ||
|
||
export function entityId(event: ResolverEvent) { | ||
if (isLegacyEvent(event)) { | ||
return String(event.endgame.unique_pid); | ||
} | ||
return event.process.entity_id; | ||
} | ||
|
||
export function parentEntityId(event: ResolverEvent) { | ||
if (isLegacyEvent(event)) { | ||
const ppid = event.endgame.unique_ppid; | ||
return String(ppid); // if unique_ppid is undefined return undefined | ||
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. ❔ This will return the string "undefined" if ppid is undefined, right? Maybe put quotes around 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. Maybe we shouldn't cast undefined to a string. Could you add an explicit return type here? |
||
} | ||
return event.process.parent?.entity_id; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
* 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'; | ||
|
||
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()), | ||
}), | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,29 @@ export class EndpointAppConstants { | |
static MAX_LONG_INT = '9223372036854775807'; // 2^63-1 | ||
} | ||
|
||
export interface ResolverNodeStats { | ||
totalEvents: number; | ||
totalAlerts: number; | ||
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 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 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. ❔ Does 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 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 commentThe 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
|
||
} | ||
|
||
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; | ||
} | ||
|
||
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[]; | ||
parent?: ResolverNode | 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. I think we can remove this field, I can clean that up later if you'd like. |
||
pagination: ResolverNodePagination; | ||
stats?: ResolverNodeStats; | ||
} | ||
|
||
export interface AlertResultList { | ||
/** | ||
* The alerts restricted by page size. | ||
|
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.
Hmm, how come you think this is wrong?
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.
This came from @oatkiller 's commit, but I think it's alluding to the fact that this is an "event name" only for process event types, and not necessarily for other event types.
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.
Yup