[Security Solution] Discover Timeline Integration URL Sync + Fixes

[logeekal]

This PR looks at adding below functionalities to Discover Timeline Integration:

Discover state URL Sync.

It changes the url sync strategy ( based on whether the discover is running in standalone or embedded mode ) of discover so that is easy for security solution to read/change the global/local state in the url.

Before	After
http://localhost:5601/app/discover?#/?_g=(filters:!(),refreshInterval:(pause:!t,value:60000),time:(from:now-15m,to:now))&_a=(columns:!(),filters:!(),index:security-solution-default,interval:auto,query:(language:kuery,query:''),sort:!(!('@timestamp',desc)))	http://localhost:5601/app/security/alerts?_g=(filters:!(),refreshInterval:(pause:!t,value:60000),time:(from:now-15m,to:now))&_a=(columns:!(),filters:!(),index:security-solution-default,interval:auto,query:(language:kuery,query:%27%27),sort:!(!(%27@timestamp%27,desc)))

@davismcphee
As an alternative, I wanted to use customization fwk for this but we are in catch-22 situation where we need stateContainer to define a customization but we want to do customization while defining the stateContainer. Let me know if you any ideas/thoughts regarding this.

Lens visualizations UI actions customization

Lens visualization has triggers for certain ui actions. Below are those triggers:

1. Brush
2. Single Value Filter
3. Multiple Value Filter
4. ⚠️ tableRowContextMenuClick - Not sure what is this.

kibana/src/plugins/visualizations/public/embeddable/events.ts

Lines 24 to 31 in 48b7acf

	export const VIS_EVENT_TO_TRIGGER: VisEventToTrigger = {
	applyFilter: APPLY_FILTER_TRIGGER,
	brush: SELECT_RANGE_TRIGGER,
	filter: VALUE_CLICK_TRIGGER,
	multiFilter: MULTI_VALUE_CLICK_TRIGGER,
	tableRowContextMenuClick: ROW_CLICK_TRIGGER,
	};

This triggers are bound to the global data services, which causes issue when triggered in security solution. Lens provided a way to override these triggers via component props. This PR use customization fwk to override and pass those props. Below videos shows the demo.

Screen.Recording.2023-08-10.at.15.36.50.mov

Discover state persistant in Security Solution

• This PR adds discover's app state it in Security solution Redux.
• TODO - Syncing with local storage remaining. Will commit soon in this PR.

Unit / Cypress Tests

• Added unit tests + cypress tests
• TODO - Some are still in progress.
• ⚠️ TODO - Copy these cypress tests to serverless environment.

[davismcphee]

Code-only review so far. The Discover changes are looking good to me overall! Just have a few initial questions and comments.

As an alternative, I wanted to use customization fwk for this but we are in catch-22 situation where we need stateContainer to define a customization but we want to do customization while defining the stateContainer. Let me know if you any ideas/thoughts regarding this.

Yeah I can see how this would be a challenge, and it would be difficult to change due to how Discover's state is all derived from the URL. I think this is a valid use of mode though, so I'm fine with this approach. If we find ourselves needing mode much more though, we may want to consider a simple abstraction using something like context over prop drilling.

[michaelolo24]

Just one more minor note. I'm not sure how we want to handle this, but currently creating a new timeline doesn't reset the discover state. We can probably handle that easily in a follow up pr

[michaelolo24]

Really nice work here. Very well tested and very thought out. Really liked your code organization 👏🏾 👏🏾 . Refreshing, navigating to and from the discover tab, and closing and reopening timeline all retain the discover state (query, filters, histogram, table columns, rows per page, and unified field list). Had a few minor nits, and the clearing discover state on a new timeline...If we decide we want to do that. Really nice work @logeekal 🚀 LGTM!

[davismcphee]

Apologies for the delay on this review, I got the chance today to doing another round of code review and some local testing. It seems to being working great! Everything I tested worked as expected including browser history, persisting Discover state when Timeline is minimized, and the custom histogram actions 👍

On a side note, the custom histogram actions are a great use of the customization framework. One idea I'd suggest as a future improvement (but out of scope of this PR) would be to leverage the customization framework to use the same cell actions in Timeline Discover that are used in Security Dashboards saved search panels, now that the Discover grid supports cell actions.

This is just about ready to go on my end. I left a couple of very minor suggestions, but mainly just a concern about some of the state syncing logic in the Discover tab.

[davismcphee]

I don't think I understand this suggestion. Are you referring to redirecting requests to Discover to another app based on an appId prop?

[logeekal]

Just one more minor note. I'm not sure how we want to handle this, but currently creating a new timeline doesn't reset the discover state. We can probably handle that easily in a follow up pr

Yep, follow up PR will have this functionality. I think it makes sense to have this functionality in conjunction with saving.

[davismcphee]

Thanks for the latest changes! Pulled and tested locally again and everything is still working as expected + no more duplicate state subscriptions. Great work on this, LGTM 👍

[davismcphee]

yes, for determining path, if discover is being used at a different path in both security and observability serverless modes.

Observability currently doesn't use Discover at a different path or embedded in another app. They're using the profile functionality in the customization framework to customize Discover as a top level app, so their use case around URLs and paths is a bit different (and already supported).

guess that might not work with locators and the like, as those run during discover setup.

Yeah I agree, I don't think this would be possible through the container component.

Probably makes more sense to do something at a config level then? maybe a discover.location setting that we set only in serverless.obit.yml/serverless.security.yml?

I'd like for the Security redirecting logic not to live in the Discover codebase or be owned/maintained by the Data Discovery team, so I'd prefer not to take this approach. I'd be open to a YAML config that disables access to Discover as a top level app or similar, but owning and maintaining the redirect logic should be the responsibility of the Security Solution.

Does it make sense to add parameter in discover.locator called, appId where locator will develop the URL based on the appID given?

I'd also prefer to avoid this since it would introduce knowledge of the Security redirect logic directly into the Discover codebase.

[dej611]

I had a review and no issue so far from testing locally.
I would like also @stratoula opinion here before approving tho.

[stratoula]

@logeekal thanx for trying my suggestion, it is fine. LGTM!

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 76fd62f

Failed CI Steps

• FTR Configs #37
• Investigations - Security Solution Cypress Tests #6

Test Failures

• [job] [logs] FTR Configs #37 / Agents fleet_upgrade_agent "before each" hook for "should respond 400 if trying to upgrade an agent that is not upgradeable"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
discover	628	629	+1
securitySolution	4473	4482	+9
total			+10

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
data	2581	2583	+2
discover	52	72	+20
unifiedSearch	104	110	+6
total			+28

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
dataViewEditor	39.9KB	39.9KB	+44.0B
discover	562.7KB	563.1KB	+374.0B
kibanaReact	209.3KB	209.4KB	+36.0B
lens	1.4MB	1.4MB	+31.0B
securitySolution	16.0MB	16.0MB	+15.8KB
unifiedHistogram	46.7KB	46.8KB	+139.0B
total			+16.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
discover	15	17	+2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
data	404.6KB	404.6KB	+42.0B
securitySolution	62.6KB	62.6KB	-15.0B
unifiedSearch	34.7KB	34.8KB	+64.0B
total			+91.0B

Unknown metric groups

API count

id	before	after	diff
data	3308	3310	+2
discover	79	99	+20
unifiedSearch	142	148	+6
total			+28

History

• 💔 Build #154031 failed fc171ed
• 💔 Build #154009 failed 456be46
• 💚 Build #153094 succeeded 828f5ab
• 💔 Build #152760 failed f59b1ec
• 💔 Build #152072 failed 8b66bf1

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

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation