[Endpoint] resolver v1 events by kqualters-elastic · Pull Request #59233 · elastic/kibana

kqualters-elastic · 2020-03-03T22:18:31Z

Summary

Changes resolver to work with both v0 and v1 events. Also adds some placeholder logic for retrieving the ancestors of a process.

…o task/resolver-v1-events

oatkiller

Other than the comments in here, let's talk about refactoring the middleware before merge

oatkiller · 2020-03-03T22:19:07Z

x-pack/plugins/endpoint/common/types.ts


 export type ResolverEvent = EndpointEvent | LegacyEndpointEvent;

+export function isLegacyEvent(


Would you plz move this out of types

❔ If its sole purpose seems to be as a type discriminant, where should it go? A selector?

i think 'model' would be good. I'd say 'no' to selector as it doesn't take global state as its only param (doesn't take it at all).

oatkiller · 2020-03-03T22:19:26Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+          content: (
+            <>
+              <EuiSpacer />
+              <AlertDetailResolver selectedEvent={(alertDetailsData as unknown) as ResolverEvent} />


Why is this unsafe cast needed? can we work out a way to remove it?

imo we should get rid of the AlertData type, as it's really just a variation of an EndpointEvent

does removing the cast change anything then?

@oatkiller @kqualters-elastic @marshallmain ResolverEvent is defined as:

ResolverEvent = EndpointEvent | LegacyEndpointEvent;

AlertData, which alertDetailsData is a type of, is defined as:

export type AlertData = AlertEvent & AlertMetadata;

We have malware_event, network, process, and registry ECS schemas defined here: https://github.com/elastic/endpoint-app-team/tree/master/schemas/v1

Does it make sense to model our types after these schemas? If so, I think our AlertEvent type should follow the malware_event ECS and our EndpointEvent type should probably be a union of a new ProcessEvent type and our existing AlertEvent.

yes agreed, wouldn't hurt to support LegacyEndpointEvent as well

I think that adds up. That being said, if malware_event, network, process, and registry are in the same index, I don't see why they shouldn't be the same schema. In fact, I don't see why ECS shouldn't just be a single schema. Its not Elastic Common Schemas is it?

oatkiller · 2020-03-03T22:19:47Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/resolver.tsx

  width: 100%;
  display: flex;
  flex-grow: 1;
+  // ugly demo hack until can play nice with eui


If this is still here by the time we merge, let's remove the comment and add a github issue instead

oatkiller · 2020-03-03T22:20:40Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/process_event.ts

-  const {
-    endgame: { event_type_full: type, event_subtype_full: subType },
-  } = passedEvent;
+export function eventType(passedEvent: ResolverEvent) {


could you add a return type here?

'processCreated' | 'processRan' | ... etc

oatkiller · 2020-03-03T22:20:57Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/process_event.ts

 */
-export function uniquePidForProcess(event: LegacyEndpointEvent) {
-  return event.endgame.unique_pid;
+export function uniquePidForProcess(event: ResolverEvent) {


Could you add an explicit return type here

oatkiller · 2020-03-03T22:21:04Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/process_event.ts

 */
-export function uniqueParentPidForProcess(event: LegacyEndpointEvent) {
-  return event.endgame.unique_ppid;
+export function uniqueParentPidForProcess(event: ResolverEvent) {


Could you add an explicit return type here

oatkiller · 2020-03-03T22:26:13Z

x-pack/plugins/endpoint/public/embeddables/resolver/view/panel.tsx

        let dateTime;
-        if (processEvent.endgame.timestamp_utc) {
-          const date = new Date(processEvent.endgame.timestamp_utc);
+        const eventTimestamp = isLegacyEvent(processEvent)


we should centralize this type of logic into an event model

import { timestamp } from 'models/event' timestamp(processEvent)

bkimmel · 2020-03-05T15:06:00Z

x-pack/plugins/endpoint/common/types.ts

    category: string;
    type: string;
    id: string;
+    kind: string;


❔ At a glance, we have three things (category, type, and now kind) that are basically perfect synonyms. Can we do something to make this type more readable or useful? Either changing from string to the things that can actually be there, or documenting the differences?

@bkimmel I can link you to the docs that describe the differences. We could use enums I suppose if we wanted but we'd have to update them as ecs changes. I think it'd probably go better the document the differences route.

@bkimmel long term we need a way to keep these types in sync w/ ECS. Part of that should be getting descriptions of each field. In the meantime, getting comments on each field here is on my radar.

bkimmel · 2020-03-05T15:13:18Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+          <EuiText>Alert Status: Open</EuiText>
+          <EuiSpacer />
+        </section>
+        <EuiTabbedContent tabs={tabs} initialSelectedTab={tabs[0]} className={className} />


🆗 Nice use of EuiTabbedContent

bkimmel · 2020-03-05T15:28:07Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+      <>
+        <section className="details-overview-summary">
+          <EuiTitle size="s">
+            <h3>


❔ I think the h3 might be ( ❔ ) redundant but maybe you have to use the size="l" variant of EUITitle

bkimmel · 2020-03-05T15:32:03Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/indexed_process_tree.ts

-  const idToChildren = new Map<number | undefined, LegacyEndpointEvent[]>();
-  const idToValue = new Map<number, LegacyEndpointEvent>();
+export function factory(processes: ResolverEvent[]): IndexedProcessTree {
+  const idToChildren = new Map<string | undefined, ResolverEvent[]>();


🆗 Cool, I didn't know you could use Type directions in constructors that way.

bkimmel · 2020-03-05T15:53:12Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/middleware.ts

+            {
+              children: [{ lifecycle: childEvents }],
+            },
+            { events: relatedEvents },


❔ Maybe also receive ancestors here instead of setting it above in the let and pushing to it

Adding string or array for category and type

…events

…s-elastic/kibana into task/resolver-v1-events

…events

elasticmachine · 2020-03-10T13:33:25Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

elasticmachine · 2020-03-10T13:33:38Z

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

bkimmel · 2020-03-12T14:02:56Z

x-pack/plugins/endpoint/common/models.ts

+export function isLegacyEvent(
+  event: EndpointEvent | LegacyEndpointEvent
+): event is LegacyEndpointEvent {
+  return (event as LegacyEndpointEvent).endgame !== undefined;


❔ I think you could skip the cast with return 'endgame' in event;

bkimmel · 2020-03-12T14:06:11Z

x-pack/plugins/endpoint/common/types.ts

  event: {
-    category: string;
-    type: string;
+    category: string | string[];


❔ Should this stuff get typed as readonly

my suggestion is to leave the types in types.ts as plain as possible. this way, you could use them when constructing a EndpointEvent. If you need a readonly (or partial, or deepreadonly, or deeppartial, etc) type, you can use a generic type for it. e.g. function takesReadonlyEvent(event: DeepReadonly<EndpointEvent>) {

bkimmel

👍 with a couple ❔

peluja1012 · 2020-03-13T10:15:17Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+    const alertDetailsData = useAlertListSelector(selectors.selectedAlertDetailsData);
+    if (alertDetailsData === undefined) {
+      return null;
+    }


Could you please remove the selectedAlertIsLegacyEndpointEvent selector if it's no longer used?

marshallmain · 2020-03-17T22:08:13Z

x-pack/plugins/endpoint/common/types.ts

    name: string;
    parent?: {
      entity_id: string;
+      parent?: {


why do we need process.parent.parent?

we don't, not sure how this was added. bad merge or something. removing.

oatkiller · 2020-03-18T14:52:44Z

x-pack/plugins/endpoint/common/types.ts

  event: {
-    category: string;
-    type: string;
+    category: string | string[];


my suggestion is to leave the types in types.ts as plain as possible. this way, you could use them when constructing a EndpointEvent. If you need a readonly (or partial, or deepreadonly, or deeppartial, etc) type, you can use a generic type for it. e.g. function takesReadonlyEvent(event: DeepReadonly<EndpointEvent>) {

oatkiller · 2020-03-18T14:55:28Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+          content: (
+            <>
+              <EuiSpacer />
+              <AlertDetailResolver selectedEvent={(alertDetailsData as unknown) as ResolverEvent} />


I think that adds up. That being said, if malware_event, network, process, and registry are in the same index, I don't see why they shouldn't be the same schema. In fact, I don't see why ECS shouldn't just be a single schema. Its not Elastic Common Schemas is it?

oatkiller · 2020-03-18T14:56:21Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+          </EuiText>
+          <EuiSpacer />
+          <EuiText>
+            Endpoint Status: <EuiHealth color="success">Online</EuiHealth>


Would you please i18n this?

oatkiller · 2020-03-18T14:56:25Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/details/overview/index.tsx

+          <EuiText>
+            Endpoint Status: <EuiHealth color="success">Online</EuiHealth>
+          </EuiText>
+          <EuiText>Alert Status: Open</EuiText>


Would you please i18n this?

oatkiller · 2020-03-18T14:57:48Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/actions.ts

     * Optional because they could have unselected the event.
     */
-    selectedEvent?: LegacyEndpointEvent;
+    selectedEvent?: ResolverEvent;


could you please mark this as readonly for consisency

oatkiller · 2020-03-18T15:00:44Z

x-pack/plugins/endpoint/public/embeddables/resolver/types.ts

  mock: jest.Mocked<Omit<SideEffectors, 'ResizeObserver'>> & Pick<SideEffectors, 'ResizeObserver'>;
 }

+export type ResolverProcessType =


would you please add a doc comment here

oatkiller · 2020-03-18T15:02:00Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/middleware.ts

    next(action);
    if (action.type === 'userChangedSelectedEvent') {
-      if (context?.services.http) {
+      if (context?.services.http && action.payload.selectedEvent) {


would you please add a comment generally explaining this? Maybe:

/** * concurrently fetches a process's details, its ancestors, and its related events. */

marshallmain · 2020-03-18T15:13:47Z

x-pack/plugins/endpoint/common/types.ts

-    category: string;
-    type: string;
+    category: string | string[];
+    type: string | string[];


when would category and type be string arrays?

I'm not sure exactly when this is the case, I believe some events can have more than one of each in the case where it was hard to bucket into exactly one existing ECS value. @jonathan-buttner could probably tell you in more detail, this comes from kqualters-elastic#1

kibanamachine · 2020-03-18T17:00:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: bcf1035

History

💚 Build #34086 succeeded ef7b568
💚 Build #34016 succeeded f38635c
💔 Build #33632 failed 7b13c14
💚 Build #32058 succeeded e5c0a1d
💚 Build #32051 succeeded 971866a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…nless * 'app/painless' of github.com:elastic/kibana: (64 commits) Fix filter scope in bool query (#60488) change index pattern id to be the same as index pattern title (#60436) [Endpoint] resolver v1 events (#59233) Branding fixes for dashboard, loader and space selector (#60073) skip flaky suite (#60535) [SIEM][Detection Engine] Fixes bug with timeline templates not working Fixed errors which are happening if switch between alert types (#60453) [EPM] Add mapping field types to index template generation v2 (#60266) [NP] Cutover ensureDefaultIndexPattern to kibana_utils (#59895) Closes #60265. Adds Beta badge to service map (#60482) [Visualize] Duplicated query filters in es request (#60106) [ML] Disable functional transform tests Fixes to service map single node banner (#60072) [Uptime] replace fetch with kibana http (#59881) Upgrade @types/node to match Node.js runtime (#60368) [License Management] NP migration (#60250) Fix create alert button from not showing in alerts list (#60444) [SIEM][Case] Update connector through flyout (#60307) add data-test-subj where possible on SO management table (#60226) Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (#60406) ...

* upstream/app/painless: (66 commits) Another i18n issue Fix i18n Fix filter scope in bool query (elastic#60488) change index pattern id to be the same as index pattern title (elastic#60436) [Endpoint] resolver v1 events (elastic#59233) Branding fixes for dashboard, loader and space selector (elastic#60073) skip flaky suite (elastic#60535) [SIEM][Detection Engine] Fixes bug with timeline templates not working Fixed errors which are happening if switch between alert types (elastic#60453) [EPM] Add mapping field types to index template generation v2 (elastic#60266) [NP] Cutover ensureDefaultIndexPattern to kibana_utils (elastic#59895) Closes elastic#60265. Adds Beta badge to service map (elastic#60482) [Visualize] Duplicated query filters in es request (elastic#60106) [ML] Disable functional transform tests Fixes to service map single node banner (elastic#60072) [Uptime] replace fetch with kibana http (elastic#59881) Upgrade @types/node to match Node.js runtime (elastic#60368) [License Management] NP migration (elastic#60250) Fix create alert button from not showing in alerts list (elastic#60444) [SIEM][Case] Update connector through flyout (elastic#60307) ...

kibanamachine · 2020-03-19T17:18:44Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 59233 or prevent reminders by adding the backport:skip label.

* Unifying the test index name for resolver and alerts * Endpoint isn't sending the agent field so check for it * Update resolver to use either legacy or ecs events * Use correct format for child events api * Adding string or array for category and type * Add return types to process event models * Create a common/models.ts for common event logic * Decrease resolver min height * Update types to match cli tool * Add a smoke test for resolver rendering nodes, remove unused selector * Add common/models/event * Internationalize some strings, address pr comments Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co> Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co>

jonathan-buttner and others added 4 commits March 2, 2020 14:38

Unifying the test index name for resolver and alerts

a9fcd64

Endpoint isn't sending the agent field so check for it

4223a42

Merge remote-tracking branch 'buttner/feature/single-event-index' int…

8562b20

…o task/resolver-v1-events

Update resolver to use either legacy or ecs events

940959e

oatkiller reviewed Mar 3, 2020

View reviewed changes

Use correct format for child events api

692a945

bkimmel reviewed Mar 5, 2020

View reviewed changes

kqualters-elastic and others added 8 commits March 6, 2020 16:07

Poorly handle empty child events response

fa4c139

Adding string or array for category and type

479f813

Merge pull request #1 from jonathan-buttner/add-array

d9db350

Adding string or array for category and type

Add return types to process event models

ca85aa4

Merge remote-tracking branch 'upstream/master' into task/resolver-v1-…

c160677

…events

Merge branch 'task/resolver-v1-events' of https://github.com/kqualter…

084ac79

…s-elastic/kibana into task/resolver-v1-events

Create a common/models.ts for common event logic

5e06e8d

Merge remote-tracking branch 'upstream/master' into task/resolver-v1-…

971866a

…events

kqualters-elastic marked this pull request as ready for review March 10, 2020 13:13

kqualters-elastic requested a review from a team as a code owner March 10, 2020 13:13

Decrease resolver min height

e5c0a1d

kqualters-elastic changed the title ~~Task/resolver v1 events~~ [Endpoint] resolver v1 events Mar 10, 2020

kqualters-elastic added the Feature:Endpoint Elastic Endpoint feature label Mar 10, 2020

kqualters-elastic added the Team:Endpoint Data Visibility Team managing the endpoint resolver label Mar 10, 2020

kqualters-elastic added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0 labels Mar 10, 2020

bkimmel reviewed Mar 12, 2020

View reviewed changes

bkimmel approved these changes Mar 12, 2020

View reviewed changes

Merge branch 'master' into task/resolver-v1-events

aa3912f

peluja1012 reviewed Mar 13, 2020

View reviewed changes

kqualters-elastic added 5 commits March 13, 2020 15:29

Merge branch 'master' into task/resolver-v1-events

3eea057

Merge branch 'master' into task/resolver-v1-events

074bba0

Update types to match cli tool

7b13c14

Merge branch 'master' into task/resolver-v1-events

7b13e0e

Add a smoke test for resolver rendering nodes, remove unused selector

9e4a829

dplumlee approved these changes Mar 17, 2020

View reviewed changes

Make parent process name optional

840911c

marshallmain reviewed Mar 17, 2020

View reviewed changes

kqualters-elastic added 2 commits March 17, 2020 18:14

Remove parent.parent from endpoint event

f38635c

Add common/models/event

ef7b568

peluja1012 approved these changes Mar 18, 2020

View reviewed changes

oatkiller suggested changes Mar 18, 2020

View reviewed changes

marshallmain reviewed Mar 18, 2020

View reviewed changes

Internationalize some strings, address pr comments

bcf1035

oatkiller approved these changes Mar 18, 2020

View reviewed changes

kqualters-elastic merged commit 64af780 into elastic:master Mar 18, 2020

kqualters-elastic deleted the task/resolver-v1-events branch March 18, 2020 17:18

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 19, 2020

kqualters-elastic mentioned this pull request Mar 19, 2020

[7.x] [Endpoint] resolver v1 events (#59233) #60685

Merged

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 19, 2020


		export type ResolverEvent = EndpointEvent \| LegacyEndpointEvent;

		export function isLegacyEvent(

Comments

Conversation

kqualters-elastic commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

oatkiller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peluja1012 Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 10, 2020

Uh oh!

elasticmachine commented Mar 10, 2020

Uh oh!

bkimmel Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkimmel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kqualters-elastic commented Mar 3, 2020 •

edited

Loading

peluja1012 Mar 13, 2020 •

edited

Loading

bkimmel Mar 12, 2020 •

edited

Loading