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

[SecuritySolution] Fix topN legend actions - filter in / out in timeline #170127

Merged
merged 18 commits into from
Oct 31, 2023

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Oct 30, 2023

Summary

https://github.com/elastic/security-team/issues/7748
#168199
#169656

Screen.Recording.2023-10-30.at.14.43.48.mov

if (!field) return;

const timeline = getTimelineById(store.getState(), TimelineId.active);
services.topValuesPopover.closePopover();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to close topN after applying a filter, as it wouldn't close itself.
Screenshot 2023-10-30 at 16 07 12

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Thank you for making these changes @angorayc !

@michaelolo24
Copy link
Contributor

@elasticmachine merge upstream

@angorayc angorayc changed the title Fix topn filters [SecuritySolution] Fix topN legend actions - filter in / out in timeline Oct 31, 2023
@angorayc angorayc marked this pull request as ready for review October 31, 2023 15:51
@angorayc angorayc requested review from a team as code owners October 31, 2023 15:51
@michaelolo24 michaelolo24 self-requested a review October 31, 2023 15:51
@botelastic botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Oct 31, 2023
@angorayc angorayc added release_note:fix Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Oct 31, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

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

@angorayc angorayc added v8.11.1 Feature:Lens Charts Security Solution Lens Charts feature labels Oct 31, 2023
Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

Very well done. This is exactly as I imagined it. Code review only.

@angorayc angorayc enabled auto-merge (squash) October 31, 2023 16:06
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests on Serverless #1 / Endpoint generated alerts "before each" hook for "should create a Detection Engine alert from an endpoint alert" "before each" hook for "should create a Detection Engine alert from an endpoint alert"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4651 4656 +5

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
expressions 1749 1765 +16

Async chunks

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

id before after diff
expressionXY 131.2KB 132.1KB +903.0B
lens 1.4MB 1.4MB +98.0B
securitySolution 12.8MB 12.9MB +18.3KB
total +19.3KB

Page load bundle

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

id before after diff
expressions 98.1KB 98.3KB +175.0B
expressionXY 38.7KB 38.7KB +48.0B
total +223.0B
Unknown metric groups

API count

id before after diff
expressions 2208 2224 +16

History

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

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.11

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 1, 2023
…n timeline (#170127) (#170269)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[SecuritySolution] Fix topN legend actions - filter in / out in
timeline (#170127)](#170127)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Angela
Chuang","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-31T17:30:08Z","message":"[SecuritySolution]
Fix topN legend actions - filter in / out in timeline (#170127)\n\n##
Summary\r\n\r\nhttps://github.com//issues/168199\r\nhttps://github.com//issues/169656\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/ff5cee55-6da5-4636-85f5-a697a302f8b5\r\n\r\n---------\r\n\r\nCo-authored-by:
Michael Olorunnisola
<[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8103a445855f72dd1755a4fd4364e4573ec6a194","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:ExpressionLanguage","Team:Threat
Hunting","Team: SecuritySolution","Team:Threat
Hunting:Explore","Feature:Lens
Charts","v8.11.0","v8.12.0","v8.11.1"],"number":170127,"url":"https://github.com/elastic/kibana/pull/170127","mergeCommit":{"message":"[SecuritySolution]
Fix topN legend actions - filter in / out in timeline (#170127)\n\n##
Summary\r\n\r\nhttps://github.com//issues/168199\r\nhttps://github.com//issues/169656\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/ff5cee55-6da5-4636-85f5-a697a302f8b5\r\n\r\n---------\r\n\r\nCo-authored-by:
Michael Olorunnisola
<[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8103a445855f72dd1755a4fd4364e4573ec6a194"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/170127","number":170127,"mergeCommit":{"message":"[SecuritySolution]
Fix topN legend actions - filter in / out in timeline (#170127)\n\n##
Summary\r\n\r\nhttps://github.com//issues/168199\r\nhttps://github.com//issues/169656\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/ff5cee55-6da5-4636-85f5-a697a302f8b5\r\n\r\n---------\r\n\r\nCo-authored-by:
Michael Olorunnisola
<[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8103a445855f72dd1755a4fd4364e4573ec6a194"}}]}]
BACKPORT-->

Co-authored-by: Angela Chuang <[email protected]>
Co-authored-by: Michael Olorunnisola <[email protected]>
kibanamachine added a commit to nreese/kibana that referenced this pull request Nov 20, 2023
…lter actions in visualizations (elastic#171284)

## Summary

Bug: elastic#171167

The [previous
implementation](elastic#170127) solved a
different bug using a new `shouldShowLegendAction` property. This
approach had a limitation on the Security Dashboards page since the
Security app has no control over the properties passed to the
visualization components when they are rendered through portable
Dashboards.

This PR fixes the problem by checking if any of the registered actions
is a "filter" action `type` in the visualizations. If customized filter
actions are found, the default filter actions hardcoded in the
visualizations code are not added, preventing duplication of filter
actions.

The specific action `type` used for the check is the
`FILTER_CELL_ACTION_TYPE = 'cellAction-filter'` constant exported by the
`@kbn/cell-actions` package.

This new approach uses a property stored in the registered actions
themselves, so we don't need to pass any extra property to the
visualization components, it works transparently. So the
`shouldShowLegendAction` property has also been cleaned.

## Demos

Timeline using `showTopN` visualization:


https://github.com/elastic/kibana/assets/17747913/491c08e8-0740-4c9e-9cb7-81267b9ba040


Alerts page using `Counts` table visualization and `showTopN`
visualization


https://github.com/elastic/kibana/assets/17747913/683eec6c-382e-4ae9-9400-c1022922e488


Portable Dashboard visualizations:


https://github.com/elastic/kibana/assets/17747913/50f09a65-488d-41f2-a5b8-3882d9c80678


Security actions are "compatible" only inside the Security app, in the
Lens app the default filter actions are displayed:

<img width="1682" alt="lens-actions-screenshot"
src="https://github.com/elastic/kibana/assets/17747913/1e7ce929-129d-45a9-ba71-8d28e3726454">

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Lens Charts Security Solution Lens Charts feature release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.11.0 v8.11.1 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants