Skip to content
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

Sessions tab improvements #131583

Merged
merged 6 commits into from
May 6, 2022

Conversation

mitodrummer
Copy link
Contributor

Summary

The PR makes changes to the sessions tab query strategy, and updates the default table columns (including the addition of display names for each to improve readability).

The main change to the query involves removing the process.entry_leader.same_as_process contraint, which caused the table to only load entry session leader events. Infering a list of sessions from only session leaders leads to a couple problems.

a) if a session did not record or had filtered out a session leader, we would not see a row for it (even if there were process events for the session). Many non interactive service sessions would not show because of this.
b) when doing a top level search for events which may have happened within a session (e.g searching for all sessions where an 'ls' command was typed), we always return 0 results (because we were only searching entry session leader events). This opens up a much more compelling use case for the sessions tab, and makes finding relevant sessions possible.

Also, in revamping the session table default columns, process.pid (not important) and process.end (difficult to infer) were dropped. Which should hopefully address the concerns in this ticket #130360

Preview of the default columns:
image

Checklist

Delete any items that are not applicable to this PR.

mitodrummer added 3 commits May 4, 2022 14:00
…solves a few problems wrt to query ability. default columns modified and display names provided for each
@mitodrummer mitodrummer added v8.2.1 Team: AWP: Visualization AWP team that does most fullstack work in kibana labels May 4, 2022
@mitodrummer mitodrummer requested review from animehart, zizhouW, opauloh and a team May 4, 2022 22:56
@mitodrummer mitodrummer requested a review from a team as a code owner May 4, 2022 22:56
@mitodrummer mitodrummer added auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement labels May 4, 2022
@mitodrummer
Copy link
Contributor Author

One question I do have is wrt cache busting localstorage to erase the default columns stored for the sessions table. I noticed I had to call localStorage.clear() anytime i made changes to these defaults. Is there a mechanism to force all users to wipe these defaults from local storage? cc @kqualters-elastic @YulNaumenko @michaelolo24

@mitodrummer
Copy link
Contributor Author

One question I do have is wrt cache busting localstorage to erase the default columns stored for the sessions table. I noticed I had to call localStorage.clear() anytime i made changes to these defaults. Is there a mechanism to force all users to wipe these defaults from local storage? cc @kqualters-elastic @YulNaumenko @michaelolo24

Would changing the timeline id do just that, or would that cause problems elsewhere?
image

@opauloh
Copy link
Contributor

opauloh commented May 5, 2022

Would changing the timeline id do just that, or would that cause problems elsewhere?

I also didn't see another local storage clearing mechanism, so it seems to be the only way to do so unless local storage gets erased when the kibana version is upgraded.

But it's safe to change the timeline id value since it has nothing to do with search strategy, however, you will need to change the across all files that reference hosts-page-sessions, (3 type files and 2 tests files).

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I loaded some data using elf and didn't detected any degradation on performance, loaded 300k+ sessions, and sessions with 100k+ events

image

image

image

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔥

},
{
columnHeaderType: defaultColumnHeaderType,
id: MAPPED_PROCESS_END_COLUMN,
display: 'process.end',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glad we got rid of that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I loaded some data using elf and didn't detected any degradation on performance, loaded 300k+ sessions, and sessions with 100k+ events

image

image

image

awesome, thanks for doing that @opauloh !

@kqualters-elastic
Copy link
Contributor

@mitodrummer changing the timeline id will work as a cache busting mechanism, alternatively, if you just want to filter out any no longer existing columns or something along those lines, code to "migrate" local storage settings exists here x-pack/plugins/security_solution/public/timelines/containers/local_storage/index.tsx

@mitodrummer mitodrummer added v8.3.0 and removed v8.2.1 labels May 5, 2022
Copy link
Contributor

@zizhouW zizhouW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mitodrummer mitodrummer added backport:skip This commit does not require backporting and removed auto-backport Deprecated - use backport:version if exact versions are needed labels May 5, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2800 2799 -1

Async chunks

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

id before after diff
securitySolution 4.8MB 4.8MB +561.0B

Page load bundle

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

id before after diff
securitySolution 249.4KB 249.4KB +3.0B
timelines 275.9KB 275.9KB +6.0B
total +9.0B

History

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

@mitodrummer mitodrummer merged commit 743cce0 into elastic:main May 6, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 9, 2022
…hromium-to-print-pdf-part-1

* 'main' of github.com:elastic/kibana: (59 commits)
  [Cloud Posture] Enabled findings group by feature (elastic#131780)
  [EBT] Fix `userId` generation (elastic#131701)
  [RAM] Add shareable rule tag filter (elastic#130710)
  Optimize package installation performance, phase 2 (elastic#131627)
  [Screenshotting] instrument for benchmark tests using new EventLogger class (elastic#130356)
  [Connector] Adding internal route for requesting ad-hoc ServiceNow access token (elastic#131171)
  [ci] bump kibana-buildkite-library (elastic#131754)
  [Synthetics] UI clean up (elastic#131598)
  [RsponseOps] Fix flaky rules list test (elastic#131567)
  [Cases] Add severity field to create case (elastic#131626)
  [Discover] Monospace font in Document Explorer (elastic#131513)
  Sessions tab improvements (elastic#131583)
  Add cloud icon "ess-icon" at the end of the config keys in "alerting" documentation (elastic#131735)
  [DOCS] Updates deprecation text for legacy APIs (elastic#131741)
  [ci] break out skip patterns so they can change without triggering CI (elastic#131726)
  Adjust search session management page font size (elastic#131291)
  [Unified search] Fix uptime css problem (elastic#131730)
  [Actionable Observability] Link to filtered rules page (elastic#131629)
  Add openAPI specifications for cases endpoint (elastic#131275)
  Display rule API key owner to users who can manage API keys (elastic#131662)
  ...

# Conflicts:
#	x-pack/plugins/screenshotting/server/formats/pdf/index.ts
#	x-pack/plugins/screenshotting/server/screenshots/observable.ts
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* session tab query modified query all events, not just entry leaders. solves a few problems wrt to query ability. default columns modified and display names provided for each

* snapshot updated

* readded test

* Default sort set to process.entry_leader.start desc

* sessions tab timeline id changed to cache bust localstorage for table column configs

* missed a couple spots for session tab timeline id update

Co-authored-by: mitodrummer <[email protected]>
@ghost
Copy link

ghost commented Jun 9, 2022

Hi @mitodrummer,

We have validated above issue on 8.3.0 BC3 and it's working fine ✅.

Build info:

Version: 8.3.0 BC3
Build: 53272
Commit: 7a0df2bca36ced2a898420cbb193a9dba0782a7a
  • Default columns are shown correctly on the Hosts > Session View are as follows:
  1. Actions Pass
  2. Started Pass
  3. Executable Pass
  4. User Pass
  5. Interactive Pass
  6. Hostname Pass
  7. Type Pass
  8. Source IP Pass

Screenshots:

image

Hence, marking this as QA Validated.

Thanks!

@ghost ghost added the QA:Validated Issue has been validated by QA label Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting QA:Validated Issue has been validated by QA release_note:enhancement Team: AWP: Visualization AWP team that does most fullstack work in kibana v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants