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

[Security Solution] updates host risk score decimal count (UI) #119228

Merged
merged 5 commits into from
Nov 29, 2021
Merged

[Security Solution] updates host risk score decimal count (UI) #119228

merged 5 commits into from
Nov 29, 2021

Conversation

ecezalp
Copy link
Contributor

@ecezalp ecezalp commented Nov 19, 2021 •
edited
Loading

rounds the long number on presentation

before
Screen Shot 2021-11-18 at 11 46 59 AM

after
Screen Shot 2021-11-19 at 3 17 37 PM

testing

  • follow the instructions in host-risk-score.md to set up the host risk score functionality.
  • Add xpack.securitySolution.enableExperimental: ['riskyHostsEnabled'] to kibana.dev.yml.
  • create an alert with a host.name value
  • confirm that the host risk score card UI displays the score with two decimals

@ecezalp ecezalp added bug Fixes for quality problems that affect the customer experience v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. auto-backport Deprecated - use backport:version if exact versions are needed Team: CTI v7.16.1 labels Nov 19, 2021
@ecezalp ecezalp requested review from a team November 19, 2021 19:31
@ecezalp ecezalp self-assigned this Nov 19, 2021
@elasticmachine
Copy link
Contributor

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

@ecezalp ecezalp changed the title updates host risk score decimal count (UI) [Security Solution] updates host risk score decimal count (UI) Nov 19, 2021
@spong spong added the Team:Detections and Resp Security Detection Response Team label Nov 19, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@ecezalp
Copy link
Contributor Author

ecezalp commented Nov 22, 2021

@elasticmachine merge upstream

@nkhristinin nkhristinin self-requested a review November 24, 2021 14:47
nkhristinin
nkhristinin reviewed Nov 24, 2021
View reviewed changes
...ity_solution/public/overview/components/overview_risky_host_links/risky_hosts_panel_view.tsx Outdated
@@ -30,6 +30,7 @@ const columns: Array<EuiTableFieldDataColumnType<LinkPanelListItem>> = [
align: 'right',
field: 'count',
name: 'Risk Score',
render: (riskScore) => (isNaN(riskScore) ? riskScore : Number.parseFloat(riskScore).toFixed(2)),
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure what value can be in riskScore, but the isNaN will return false if riskScore will be a null or ''.
And Number.parseFloat(null) will be NaN.

Here are more examples of this isNaN behaviour
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#examples

So maybe, we want to use Number.isNaN instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for catching this! updating now

nkhristinin
nkhristinin approved these changes Nov 24, 2021
View reviewed changes
Copy link
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

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

LGTM! But we need to check comment about isNaN

@ecezalp ecezalp added CTI area and removed v8.0.0 labels Nov 25, 2021
@ecezalp
Copy link
Contributor Author

ecezalp commented Nov 29, 2021

@elasticmachine merge upstream

@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 4.6MB 4.6MB +60.0B

History

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

cc @ecezalp

@ecezalp ecezalp merged commit 7e9b1bc into elastic:main Nov 29, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 29, 2021
@ecezalp @kibanamachine
[Security Solution] updates host risk score decimal count (UI) (elast…
0c8e309 
…ic#119228)

* updates host risk score decimal count

* fix function

* changes isNaN to Number.isNaN

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Ece Ozalp <[email protected]>
@ecezalp ecezalp mentioned this pull request Dec 16, 2021
Merged
ecezalp added a commit to ecezalp/kibana that referenced this pull request Dec 16, 2021
@ecezalp @kibanamachine
[Security Solution] updates host risk score decimal count (UI) (elast…
31a0f71 
…ic#119228)

* updates host risk score decimal count

* fix function

* changes isNaN to Number.isNaN

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Ece Ozalp <[email protected]>
ecezalp added a commit that referenced this pull request Dec 16, 2021
@ecezalp @kibanamachine
[Security Solution] updates host risk score decimal count (UI) (#119228…
88fbedb 
…) (#121424)

* updates host risk score decimal count

* fix function

* changes isNaN to Number.isNaN

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Ece Ozalp <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Ece Ozalp <[email protected]>
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

3 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
@ecezalp @kibanamachine @TinLe
[Security Solution] updates host risk score decimal count (UI) (elast…
866999a 
…ic#119228)

* updates host risk score decimal count

* fix function

* changes isNaN to Number.isNaN

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Ece Ozalp <[email protected]>
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

14 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

3 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkhristinin nkhristinin nkhristinin approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience CTI area release_note:skip Skip the PR/issue when compiling release notes Team: CTI Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.16.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ecezalp @elasticmachine @kibana-ci @kibanamachine @nkhristinin @spong