no longer pass related event stats to process node component by oatkiller · Pull Request #72435 · elastic/kibana

oatkiller · 2020-07-20T13:49:32Z

Summary

The purpose of this PR is to remove props from the process node component.

relatedEventsStats selector now returns a thunk that takes a nodeID string and returns stats
relatedEventStatsForProcess selector no longer needed as it has the same signature as relatedEventsStats now has.
process node component no longer takes relatedEventsStatsForProcess

No changes in behaviour were intended.

Still works:

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

kqualters-elastic

nice 👍

bkimmel · 2020-07-20T14:15:53Z

x-pack/plugins/security_solution/public/resolver/models/resolver_tree.ts

 * This returns a map of entity_ids to stats for the related events and alerts.
 */
-export function relatedEventsStats(tree: ResolverTree): Map<string, ResolverNodeStats> {
+export function relatedEventStats(tree: ResolverTree): Map<string, ResolverNodeStats> {


❔ Isn't more correct to say events? Using the singular implies you are providing stats about one related event, which isn't really the case, right?

I see your point. My motivation for changing this was just because I kept typing it as relatedEventStats. How do you feel about nodeStats? @bkimmel

nvm, putting back the original name

To be consistent with our naming in a few other places, should we add a ByEntityId at the end, too? Like nodeStatsByEntityId

bkimmel · 2020-07-20T14:25:17Z

x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx

    const nodeID = processEventModel.uniquePidForProcess(event);

+    /**
+     * A collection of events related to the current node and statistics (e.g. counts indexed by event type)


🔴 ResolverNodeStats does not contain a collection of events related to the current node unless I'm missing something. I'd suggest we either: 1) Change the comment here or 2) If it does have a collection of events, the selector name should be something more broad like relatedEventsAndStats

i'll remove the comment. fwiw, that was the existing comment for relatedEventStatsForProcess, which contained exactly what relatedEventStats now contains.

Maybe at one point it did have that collection with it in whatever form it was being consumed here.

bkimmel

Maybe respond to that one 🔴 but looks good otherwise.

kibanamachine · 2020-07-20T16:28:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 06c81b2

Build metrics

async chunks size

id	value	diff	baseline
securitySolution	7.3MB	-507.0B	7.3MB

History

💔 Build #62961 failed 45066ae

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

…nt (elastic#72435)

* master: (60 commits) [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335) [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337) [Resolver] no longer pass related event stats to process node component (elastic#72435) Revert "skip flaky suite (elastic#72146)" [Security Solution] Cleanup endpoint telemetry (elastic#71950) Unskip dashboard embeddable rendering tests (elastic#71824) [ENDPOINT] Added unerolling status for host. (elastic#72303) [Alerting][Connectors] Increase the size of the logos (elastic#72419) [SECURITY] [Timeline] Raw events not displayed (elastic#72387) [ML] Fixes display of regression stop stats if one is NaN (elastic#72412) [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239) Fix match phrase and not match phrase comparators (elastic#71850) [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040) [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152) [Resolver] Selector performance (elastic#72380) [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026) [Ingest Manager] Do not bumb config revision during config creation (elastic#72270) [ML] Adding missing index pattern name to new job wizards (elastic#72400) [ML] improve annotation flyout performance (elastic#72299) [APM] Testing error rate API and restructuring folders (elastic#72257) ...

* master: (26 commits) [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335) [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337) [Resolver] no longer pass related event stats to process node component (elastic#72435) Revert "skip flaky suite (elastic#72146)" [Security Solution] Cleanup endpoint telemetry (elastic#71950) Unskip dashboard embeddable rendering tests (elastic#71824) [ENDPOINT] Added unerolling status for host. (elastic#72303) [Alerting][Connectors] Increase the size of the logos (elastic#72419) [SECURITY] [Timeline] Raw events not displayed (elastic#72387) [ML] Fixes display of regression stop stats if one is NaN (elastic#72412) [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239) Fix match phrase and not match phrase comparators (elastic#71850) [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040) [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152) [Resolver] Selector performance (elastic#72380) [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026) [Ingest Manager] Do not bumb config revision during config creation (elastic#72270) [ML] Adding missing index pattern name to new job wizards (elastic#72400) [ML] improve annotation flyout performance (elastic#72299) [APM] Testing error rate API and restructuring folders (elastic#72257) ...

…nt (#72435) (#72487)

…nt (#72435) (#72488)

…feature-privileges * alerting/consumer-based-rbac: (45 commits) fixed alerts test [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335) [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337) [Resolver] no longer pass related event stats to process node component (elastic#72435) Revert "skip flaky suite (elastic#72146)" [Security Solution] Cleanup endpoint telemetry (elastic#71950) Unskip dashboard embeddable rendering tests (elastic#71824) [ENDPOINT] Added unerolling status for host. (elastic#72303) [Alerting][Connectors] Increase the size of the logos (elastic#72419) [SECURITY] [Timeline] Raw events not displayed (elastic#72387) [ML] Fixes display of regression stop stats if one is NaN (elastic#72412) [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239) Fix match phrase and not match phrase comparators (elastic#71850) [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040) [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152) allow user to disable alert even if they dont have privileges to the underlying action [Resolver] Selector performance (elastic#72380) [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026) [Ingest Manager] Do not bumb config revision during config creation (elastic#72270) [ML] Adding missing index pattern name to new job wizards (elastic#72400) ...

no longer pass related event stats to process node component

45066ae

oatkiller requested review from a team as code owners July 20, 2020 13:49

oatkiller requested a review from kqualters-elastic July 20, 2020 14:01

kqualters-elastic approved these changes Jul 20, 2020

View reviewed changes

oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 20, 2020

bkimmel reviewed Jul 20, 2020

View reviewed changes

bkimmel approved these changes Jul 20, 2020

View reviewed changes

michaelolo24 approved these changes Jul 20, 2020

View reviewed changes

oatkiller added 2 commits July 20, 2020 10:47

remove incorrect comment. restore old name to relatedeventsstats

a27b106

fix error

06c81b2

oatkiller merged commit 75e4c7a into elastic:master Jul 20, 2020

oatkiller deleted the view-refactoring-1 branch July 20, 2020 16:41

oatkiller mentioned this pull request Jul 20, 2020

[7.x] [Resolver] no longer pass related event stats to process node component (#72435) #72487

Merged

oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Jul 20, 2020

[Resolver] no longer pass related event stats to process node compone…

a48f950

…nt (elastic#72435)

oatkiller mentioned this pull request Jul 20, 2020

[7.9] [Resolver] no longer pass related event stats to process node component (#72435) #72488

Merged

oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Jul 20, 2020

[Resolver] no longer pass related event stats to process node compone…

fe1a74f

…nt (elastic#72435)

oatkiller pushed a commit that referenced this pull request Jul 20, 2020

[Resolver] no longer pass related event stats to process node compone…

4ae7159

…nt (#72435) (#72487)

oatkiller pushed a commit that referenced this pull request Jul 20, 2020

[Resolver] no longer pass related event stats to process node compone…

049ed89

…nt (#72435) (#72488)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

no longer pass related event stats to process node component#72435

no longer pass related event stats to process node component#72435
oatkiller merged 3 commits intoelastic:masterfrom
oatkiller:view-refactoring-1

oatkiller commented Jul 20, 2020 •

edited

Loading

Uh oh!

kqualters-elastic left a comment

Uh oh!

bkimmel Jul 20, 2020

Uh oh!

oatkiller Jul 20, 2020

Uh oh!

oatkiller Jul 20, 2020

Uh oh!

bkimmel Jul 20, 2020

Uh oh!

oatkiller Jul 20, 2020

Uh oh!

bkimmel Jul 20, 2020

Uh oh!

oatkiller Jul 20, 2020

Uh oh!

bkimmel Jul 20, 2020

Uh oh!

bkimmel left a comment

Uh oh!

kibanamachine commented Jul 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

oatkiller commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

kqualters-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkimmel left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jul 20, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oatkiller commented Jul 20, 2020 •

edited

Loading