[SECURITY SOLUTION] Add our first search strategy for all host query #75439

XavierM · 2020-08-19T14:36:59Z

Summary

Bringing our own search strategy for our security solution app, to refactor most of our graphQL / apollo query call to get our data.
We created our own security solution search strategy with a factory design to avoid creating multiple search strategies for every queries that we have in our plugin. We replace our first graphQL query by our first search strategy, it just makes sense ;).

…ion-search-strategy

elasticmachine · 2020-08-19T14:37:24Z

Pinging @elastic/siem (Team:SIEM)

XavierM · 2020-08-19T14:39:29Z

src/plugins/data/public/search/search_service.ts

      return this.searchInterceptor.search(request, options);
-    };
+    }) as ISearchGeneric;


@lukasolson, not really proud of this cast, but I did not know how to stop the type bleeding on me :(.

I just played around again trying to get rid of this cast and it was fruitless, so I'm fine with this for now.

patrykkopycinski · 2020-08-19T14:52:24Z

x-pack/plugins/security_solution/public/hosts/pages/navigation/hosts_query_tab_body.tsx

+  const [
+    loading,
+    { hosts, totalCount, pageInfo, loadPage, id, inspect, isInspected, refetch },
+  ] = useAllHost({ docValueFields, endDate, filterQuery, startDate, type });


lukasolson

Took a look at this today and tried to play around to get rid of the as but wasn't successful. I wanted to chat about this with @lizozom before I gave it the LGTM but things are looking good so far.

lukasolson · 2020-08-19T22:32:05Z

src/plugins/data/public/search/search_service.ts

      return this.searchInterceptor.search(request, options);
-    };
+    }) as ISearchGeneric;


I just played around again trying to get rid of this cast and it was fruitless, so I'm fine with this for now.

src/plugins/data/server/search/types.ts

patrykkopycinski · 2020-08-20T07:05:03Z

@elasticmachine merge upstream

Co-authored-by: Lukas Olson <[email protected]>

angorayc · 2020-08-20T11:57:53Z

x-pack/plugins/security_solution/common/search_strategy/security_solution/index.ts

+export type StrategyResponseType<T extends FactoryQueryTypes> = T extends 'host_all'
+  ? HostsStrategyResponse
+  : T extends 'host_details'
+  ? HostDetailsStrategyResponse


Please consider rename HostDetailsStrategyResponse into HostOverviewRequestResponse.
I think the response of host details would be more generic and may include more info than HostOverviewRequestResponse/

Not a big fan of the Overview naming but It makes sense to keep it the same to not confuse people so I will change it back.

angorayc · 2020-08-20T12:14:43Z

x-pack/plugins/security_solution/common/search_strategy/security_solution/hosts/index.ts

+  TimerangeInput,
+} from '..';
+
+export type HostsQueries = 'host_all' | 'host_details';


Could we rename host_details into host_overview?
As in x-pack/plugins/security_solution/server/search_strategy/security_solution/factory/hosts/index.ts line 72, the query for host details is actually host overview.

Or I'm also thinking about having an enum that contains all the queries we have in security app,
for example

enum SecuritySolutionQuries = { host = 'host', hostOverview = 'hostOverview' } type HostQueries = SecuritySolutionQuries.host | SecuritySolutionQuries.hostOverview

say the same query in host page is reused in network page in the future, we could do

type NetworkQueries = SecuritySolutionQuries... | SecuritySolutionQuries.hostOverview

and in x-pack/plugins/security_solution/public/hosts/containers/hosts/index.tsx Line 69
we could put the enum we defined as well.

I think you are right we should use an enum, but I still think we should create the enum of the queries in their respective folder and then combine them back the way we are doing it with FactoryQueryTypes

…ion-search-strategy

patrykkopycinski

That's HUUGEE @XavierM 💪
I've pulled down the branch and gave it a try to implement a search strategy for the timeline and everything went smooth 🧈
Well done!

XavierM · 2020-08-20T19:21:07Z

@elasticmachine merge upstream

lukasolson

LGTM, minor nits below.

Also wanted to mention our conversation today that I'm still not super excited about the prospect of sending the entire search request in requests after the initial request. I understand the use case, but for our default ES search strategy, we only need the ID. I think we can come up with a better mechanism for this, so I've scheduled a discussion.

x-pack/plugins/security_solution/public/hosts/containers/hosts/index.tsx

lukasolson · 2020-08-21T20:48:43Z

x-pack/plugins/security_solution/public/hosts/containers/hosts/index.tsx

+                }
+                searchSubscription$.unsubscribe();
+              } else if (response.isPartial && !response.isRunning) {
+                if (!didCancel) {


I don't believe you have to check didCancel here (or above) since if the request is aborted, it should immediately hit the error case below.

the didCancel is more about if the component gets unmount and the request comeback after the component has been unmounted like if you change the route on security solution app.

XavierM · 2020-08-22T01:34:29Z

LGTM, minor nits below.

Also wanted to mention our conversation today that I'm still not super excited about the prospect of sending the entire search request in requests after the initial request. I understand the use case, but for our default ES search strategy, we only need the ID. I think we can come up with a better mechanism for this, so I've scheduled a discussion.

I do agree with you, that we can have a better mechanism and I will love to implement it in follow up PR. Thank you for being flexible with us. It is much appreciated!

…ion-search-strategy

patrykkopycinski · 2020-08-22T05:22:44Z

@elasticmachine merge upstream

kibanamachine · 2020-08-22T12:14:00Z

💚 Build Succeeded

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

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
securitySolution	1918	+2	1916

async chunks size

id	value	diff	baseline
securitySolution	7.2MB	+238.0B	7.2MB

page load bundle size

id	value	diff	baseline
data	1.4MB	-12.0B	1.4MB
dataEnhanced	178.1KB	+80.0B	178.0KB
total		+68.0B

History

💔 Build #69858 failed 9a0b7fbad7c1ed0527c4cf5ea2eba37db39c3be5
💔 Build #69850 failed 63cde69
💚 Build #69508 succeeded a2c7e4e

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

…lastic#75439) * add security solution search strategy on server side * get security solution search strategy in the public app for all host * fix types * fix Check core API changes * thank you cypress test * Remove any by the right type IESearchRequest Co-authored-by: Lukas Olson <[email protected]> * add translation and filter error when we abort the query * pr review * fix translation * review II * fix merge issue Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Lukas Olson <[email protected]>

…75439) (#75720) * add security solution search strategy on server side * get security solution search strategy in the public app for all host * fix types * fix Check core API changes * thank you cypress test * Remove any by the right type IESearchRequest Co-authored-by: Lukas Olson <[email protected]> * add translation and filter error when we abort the query * pr review * fix translation * review II * fix merge issue Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Lukas Olson <[email protected]>

elasticmachine · 2021-09-23T14:27:06Z

Pinging @elastic/security-solution (Team: SecuritySolution)

XavierM added 5 commits August 17, 2020 08:53

add security solution search strategy on server side

38c708d

Merge branch 'master' of github.com:elastic/kibana into securiy-solut…

c3d37ca

…ion-search-strategy

Merge branch 'master' of github.com:elastic/kibana into securiy-solut…

5d80f40

…ion-search-strategy

get security solution search strategy in the public app for all host

0fdc5d6

Merge branch 'master' of github.com:elastic/kibana into securiy-solut…

e6b0a35

…ion-search-strategy

XavierM requested a review from patrykkopycinski August 19, 2020 14:36

XavierM requested review from a team as code owners August 19, 2020 14:36

XavierM self-assigned this Aug 19, 2020

XavierM added release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.10.0 v8.0.0 labels Aug 19, 2020

XavierM requested review from lukasolson and angorayc August 19, 2020 14:37

XavierM commented Aug 19, 2020

View reviewed changes

patrykkopycinski reviewed Aug 19, 2020

View reviewed changes

XavierM requested a review from MadameSheema August 19, 2020 15:35

XavierM added 3 commits August 19, 2020 12:43

fix types

9756431

fix Check core API changes

819e0ca

thank you cypress test

6309135

elastic deleted a comment from kibanamachine Aug 19, 2020

lukasolson reviewed Aug 19, 2020

View reviewed changes

elasticmachine and others added 2 commits August 20, 2020 01:05

Merge branch 'master' into securiy-solution-search-strategy

aeb5779

Remove any by the right type IESearchRequest

8cc574e

Co-authored-by: Lukas Olson <[email protected]>

angorayc reviewed Aug 20, 2020

View reviewed changes

XavierM added 2 commits August 20, 2020 08:32

Merge branch 'master' of github.com:elastic/kibana into securiy-solut…

6a59240

…ion-search-strategy

add translation and filter error when we abort the query

0b66d2a

patrykkopycinski approved these changes Aug 20, 2020

View reviewed changes

pr review

9accea5

angorayc approved these changes Aug 20, 2020

View reviewed changes

fix translation

8731418

elastic deleted a comment from kibanamachine Aug 20, 2020

lukasolson approved these changes Aug 21, 2020

View reviewed changes

XavierM added 2 commits August 21, 2020 21:35

review II

1a38f1e

Merge branch 'master' of github.com:elastic/kibana into securiy-solut…

63cde69

…ion-search-strategy

XavierM force-pushed the securiy-solution-search-strategy branch from a2c7e4e to 63cde69 Compare August 22, 2020 01:46

fix merge issue

d70def0

XavierM force-pushed the securiy-solution-search-strategy branch from 9a0b7fb to d70def0 Compare August 22, 2020 10:29

XavierM merged commit 6dbc4be into elastic:master Aug 22, 2020

XavierM mentioned this pull request Aug 22, 2020

[7.x] [SECURITY SOLUTION] Add our first search strategy for all host query (#75439) #75720

Merged

Kerry350 mentioned this pull request Sep 10, 2020

[Logs UI] [R&D] Determine scope of work for migrating to async search in the Logs UI stream view #76677

Closed

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SECURITY SOLUTION] Add our first search strategy for all host query #75439

[SECURITY SOLUTION] Add our first search strategy for all host query #75439

XavierM commented Aug 19, 2020

elasticmachine commented Aug 19, 2020

XavierM Aug 19, 2020

lukasolson Aug 19, 2020

patrykkopycinski Aug 19, 2020

lukasolson left a comment

lukasolson Aug 19, 2020

patrykkopycinski commented Aug 20, 2020

angorayc Aug 20, 2020

XavierM Aug 20, 2020

angorayc Aug 20, 2020

angorayc Aug 20, 2020

XavierM Aug 20, 2020

patrykkopycinski left a comment

XavierM commented Aug 20, 2020

lukasolson left a comment

lukasolson Aug 21, 2020

XavierM Aug 22, 2020

XavierM commented Aug 22, 2020

patrykkopycinski commented Aug 22, 2020

kibanamachine commented Aug 22, 2020

elasticmachine commented Sep 23, 2021

[SECURITY SOLUTION] Add our first search strategy for all host query #75439

[SECURITY SOLUTION] Add our first search strategy for all host query #75439

Conversation

XavierM commented Aug 19, 2020

Summary

elasticmachine commented Aug 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrykkopycinski commented Aug 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrykkopycinski left a comment

Choose a reason for hiding this comment

XavierM commented Aug 20, 2020

lukasolson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XavierM commented Aug 22, 2020

patrykkopycinski commented Aug 22, 2020

kibanamachine commented Aug 22, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

elasticmachine commented Sep 23, 2021