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.

Sign up for GitHub

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

Jump to bottom

fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab #143239

Merged
merged 10 commits into from
Oct 20, 2022
Merged

fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab #143239

merged 10 commits into from
Oct 20, 2022

Conversation

jamster10
Copy link
Contributor

@jamster10 jamster10 commented Oct 12, 2022 •
edited
Loading

Summary

Initiated by #136987 where our events table request inspection contains duplicate entries for each global filter, This PR attempts to clean up our Explore code by:

Updating variable names to be more clear

  1. filters --> globalFilters: we have additional filters on these views and filters is too general.
  2. filterNetworkExternalAlertData --> sourceOrDestinationIpExistsFilter: This filter is not related to external alerts but is used to filter for source or destination ip existence.

Update filters that can be written more cleanly and operate as before

  1. sourceOrDestinationIpExistsFilter (formally filterNetworkExternalAlertData)
  2. hostNameExistsFilter

Update useInvalidFilterQuery to NOT use eslint-disable-next-line react-hooks/exhaustive-deps

Update (Network|User|Host)DetailsTabs components to explicitly take a detailFilter instead of non-descript pageFilter

Additionally, please note that the StatefulEventsViewer grabs global filters automatically. This was the reason for bug: #136987 as global filters were being passed to its pageFilters prop 😄

Checklist

@jamster10 jamster10 added bug Fixes for quality problems that affect the customer experience release_note:fix Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Explore labels Oct 12, 2022
@jamster10 jamster10 self-assigned this Oct 12, 2022
@jamster10 jamster10 requested review from a team as code owners October 12, 2022 21:17
@elasticmachine
Copy link
Contributor

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

@jamster10 jamster10 marked this pull request as draft October 13, 2022 13:32
@jamster10 jamster10 force-pushed the 136987-remove-duplicate-entry-from-host-event-query branch from 434f158 to ce56fa6 Compare October 13, 2022 14:24
@jamster10 jamster10 force-pushed the 136987-remove-duplicate-entry-from-host-event-query branch 2 times, most recently from d7060fc to 0781ea2 Compare October 13, 2022 20:27
@jamster10 jamster10 marked this pull request as ready for review October 13, 2022 20:28
@jamster10 jamster10 linked an issue Oct 13, 2022 that may be closed by this pull request
[Security Solution] The Hosts > Events table query contains duplicate entries for each filter at the top of the page #136987
Closed
@jamster10 jamster10 changed the title fix incorrect filters being passed to events table causing bad counts and inspect requests fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab Oct 13, 2022
@jamster10 jamster10 force-pushed the 136987-remove-duplicate-entry-from-host-event-query branch from 0781ea2 to da0f8d4 Compare October 13, 2022 20:42
stephmilovic
stephmilovic approved these changes Oct 19, 2022
View reviewed changes
Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

That code was a headache. Thanks for cleaning this up, hopefully it makes it easier to work in this area without messing up a query. Code looks good, manual testing went will. LGTM

@jamster10 jamster10 enabled auto-merge (squash) October 20, 2022 21:41
@jamster10 jamster10 merged commit 883f66e into elastic:main Oct 20, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 10.0MB 10.0MB -668.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 414 413 -1

Total ESLint disabled count

id before after diff
securitySolution 491 490 -1

History

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

cc @jamster10

@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 20, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 24, 2022
@jloleysens
Merge branch 'main' into files-upload-design-feedback
da22f2f 
* main: (57 commits)
  [Files] Filepicker (elastic#143111)
  [Infrastructure UI] Replace Lens table with EUI table and own api (elastic#142871)
  [api-docs] Daily api_docs build (elastic#143829)
  [api-docs] Daily api_docs build (elastic#143825)
  [api-docs] Daily api_docs build (elastic#143823)
  [Security Solution] Restructuring folders of Detection Engine + refactoring Rule Management (elastic#142950)
  [Dev tools] Fix performance issue with autocomplete suggestions (elastic#143428)
  [Security Solution] Disable ML rule's edit button link under basic license (elastic#143260)
  [Lens]  Use the language-documentation package for formula (elastic#143649)
  [api-docs] Daily api_docs build (elastic#143811)
  [Security Solution] Fix missing title on inspect pop-up (elastic#143601)
  fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab (elastic#143239)
  [Security Solution][Endpoint] `get-file` response action kibana download file API (elastic#143708)
  Rely on refresh context to update stats independently of overview cards. (elastic#143308)
  [RAM] Rule event log - Fix incorrect results when filtering by message and outcome simultaneously (elastic#143119)
  [ML] Display link to create data view from error cases in data frame analytics results pages (elastic#143596)
  Update links in README :) (elastic#143675)
  Add more tests for ml_inference_logic (elastic#143764)
  skip failing test suite (elastic#143717)
  [DOCS] Add assignees to case APIs (elastic#143610)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephmilovic stephmilovic stephmilovic approved these changes

Assignees

@jamster10 jamster10

Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience release_note:fix Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.6.0
Projects
None yet
Milestone
No milestone
5 participants
@jamster10 @elasticmachine @kibana-ci @stephmilovic @kibanamachine