Skip to content

[Security Solution] Make new Add Data page more fine grained#115016

Merged
kevinlog merged 24 commits intoelastic:masterfrom
kevinlog:task/make-empty-page-finer-grained
Oct 19, 2021
Merged

[Security Solution] Make new Add Data page more fine grained#115016
kevinlog merged 24 commits intoelastic:masterfrom
kevinlog:task/make-empty-page-finer-grained

Conversation

@kevinlog
Copy link
Copy Markdown
Contributor

Summary

This PR makes the new Add Data page more fine grained in the Security app. Before, the page covered the entire app which changed some workflows. After gathering feedback from the Security team and PMs, this change only adds the new empty state to pages that already used the empty state.

In addition, there is a new hook to detect pages at the top level which use the empty state to ensure that the background color is subdued to match that of the Empty State.

Overview page:
image

Hosts page:
image

Network page:
image

Timelines page:
image

Examples of some pages that didn't have the empty state before now restored when !indicesExist:
image

image

image

Checklist

Delete any items that are not applicable to this PR.

@kevinlog kevinlog added release_note:enhancement v8.0.0 Team:Detections and Resp Security Detection Response Team Team:Threat Hunting Security Solution Threat Hunting Team Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.16.0 labels Oct 14, 2021
@kevinlog kevinlog requested a review from a team as a code owner October 14, 2021 14:09
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@kevinlog kevinlog requested a review from cchaos October 14, 2021 14:09
@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented Oct 14, 2021

@kevinlog FYI, there is some general work being done to the Add Data cards that will overlap with this work. #114911. @snide has taken that PR over for me since I'm out the rest of the week. But there might need to be some coordination there.

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

}
}, [pageName, indicesExist]);

return [showEmptyState];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we're only returning a single boolean here would it make sense to do return showEmptyState instead of having it as an array?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Code changes look good to me!

Copy link
Copy Markdown
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

I checkout and tested this PR locally, and detection pages are now not covered by the Add Data screen when users have insufficient privileges. I added a couple of nits, but everything else looks good to me 👍

This PR, however, doesn't address this issue when users see the Add Data screen on other pages when they don't have enough privileges. But this is likely out of the scope of this work and could be handled separately.

@XavierM
Copy link
Copy Markdown
Contributor

XavierM commented Oct 15, 2021

@xcrzx This PR should take care of the privileges but only in 8.0.0

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog kevinlog added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 18, 2021
@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented Oct 19, 2021

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

The code changes LGTM (I also pushed a fix from the rebase). I'll leave it up to the Security team to decide which pages benefit from the "No data" screen.

Copy link
Copy Markdown
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

App services code change lgtm

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kevinlog kevinlog enabled auto-merge (squash) October 19, 2021 15:16
@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/categorization_field_examples·ts.apis Machine Learning jobs Categorization example endpoint - partially valid, more than 75% are null

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]     │
[00:00:00]       └-: apis
[00:00:00]         └-> "before all" hook in "apis"
[00:11:58]         └-: Machine Learning
[00:11:58]           └-> "before all" hook in "Machine Learning"
[00:11:58]           └-> "before all" hook in "Machine Learning"
[00:11:58]             │ debg creating role ft_ml_source
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_source]
[00:11:58]             │ debg creating role ft_ml_source_readonly
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_source_readonly]
[00:11:58]             │ debg creating role ft_ml_dest
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_dest]
[00:11:58]             │ debg creating role ft_ml_dest_readonly
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_dest_readonly]
[00:11:58]             │ debg creating role ft_ml_ui_extras
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_ui_extras]
[00:11:58]             │ debg creating role ft_default_space_ml_all
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_all]
[00:11:58]             │ debg creating role ft_default_space1_ml_all
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space1_ml_all]
[00:11:58]             │ debg creating role ft_all_spaces_ml_all
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_all_spaces_ml_all]
[00:11:58]             │ debg creating role ft_default_space_ml_read
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_read]
[00:11:58]             │ debg creating role ft_default_space1_ml_read
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space1_ml_read]
[00:11:58]             │ debg creating role ft_all_spaces_ml_read
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_all_spaces_ml_read]
[00:11:58]             │ debg creating role ft_default_space_ml_none
[00:11:58]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_none]
[00:11:58]             │ debg creating user ft_ml_poweruser
[00:11:58]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser]
[00:11:58]             │ debg created user ft_ml_poweruser
[00:11:58]             │ debg creating user ft_ml_poweruser_spaces
[00:11:58]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_spaces]
[00:11:58]             │ debg created user ft_ml_poweruser_spaces
[00:11:58]             │ debg creating user ft_ml_poweruser_space1
[00:11:58]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_space1]
[00:11:58]             │ debg created user ft_ml_poweruser_space1
[00:11:58]             │ debg creating user ft_ml_poweruser_all_spaces
[00:11:58]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_all_spaces]
[00:11:58]             │ debg created user ft_ml_poweruser_all_spaces
[00:11:58]             │ debg creating user ft_ml_viewer
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer]
[00:11:59]             │ debg created user ft_ml_viewer
[00:11:59]             │ debg creating user ft_ml_viewer_spaces
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_spaces]
[00:11:59]             │ debg created user ft_ml_viewer_spaces
[00:11:59]             │ debg creating user ft_ml_viewer_space1
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_space1]
[00:11:59]             │ debg created user ft_ml_viewer_space1
[00:11:59]             │ debg creating user ft_ml_viewer_all_spaces
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_all_spaces]
[00:11:59]             │ debg created user ft_ml_viewer_all_spaces
[00:11:59]             │ debg creating user ft_ml_unauthorized
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_unauthorized]
[00:11:59]             │ debg created user ft_ml_unauthorized
[00:11:59]             │ debg creating user ft_ml_unauthorized_spaces
[00:11:59]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_unauthorized_spaces]
[00:11:59]             │ debg created user ft_ml_unauthorized_spaces
[00:16:23]           └-: jobs
[00:16:23]             └-> "before all" hook in "jobs"
[00:16:23]             └-: Categorization example endpoint - 
[00:16:23]               └-> "before all" hook for "valid with good number of tokens"
[00:16:23]               └-> "before all" hook for "valid with good number of tokens"
[00:16:23]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Loading "mappings.json"
[00:16:23]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Loading "data.json.gz"
[00:16:23]                 │ info [o.e.c.m.MetadataCreateIndexService] [node-01] [ft_categorization] creating index, cause [api], templates [], shards [1]/[0]
[00:16:23]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Created index "ft_categorization"
[00:16:23]                 │ debg [x-pack/test/functional/es_archives/ml/categorization] "ft_categorization" settings {"index":{"number_of_replicas":"0","number_of_shards":"1"}}
[00:16:24]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Indexed 1501 docs into "ft_categorization"
[00:16:24]                 │ debg applying update to kibana config: {"dateFormat:tz":"UTC"}
[00:16:25]               └-> valid with good number of tokens
[00:16:25]                 └-> "before each" hook: global before each for "valid with good number of tokens"
[00:16:25]                 └- ✓ pass  (213ms)
[00:16:25]               └-> invalid, too many tokens.
[00:16:25]                 └-> "before each" hook: global before each for "invalid, too many tokens."
[00:16:25]                 │ info [r.suppressed] [node-01] path: /_analyze, params: {}
[00:16:25]                 │      org.elasticsearch.transport.RemoteTransportException: [node-01][127.0.0.1:63201][indices:admin/analyze[s]]
[00:16:25]                 │      Caused by: java.lang.IllegalStateException: The number of tokens produced by calling _analyze has exceeded the allowed maximum of [10000]. This limit can be set by changing the [index.analyze.max_token_count] index level setting.
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction$TokenCounter.increment(TransportAnalyzeAction.java:397) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.simpleAnalyze(TransportAnalyzeAction.java:229) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:204) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:122) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:110) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.support.single.shard.TransportSingleShardAction.lambda$asyncShardOperation$0(TransportSingleShardAction.java:99) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.ActionRunnable.lambda$supply$0(ActionRunnable.java:47) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.ActionRunnable$2.doRun(ActionRunnable.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:737) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
[00:16:25]                 │      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
[00:16:25]                 │      	at java.lang.Thread.run(Thread.java:833) [?:?]
[00:16:25]                 │ info [r.suppressed] [node-01] path: /_analyze, params: {}
[00:16:25]                 │      org.elasticsearch.transport.RemoteTransportException: [node-01][127.0.0.1:63201][indices:admin/analyze[s]]
[00:16:25]                 │      Caused by: java.lang.IllegalStateException: The number of tokens produced by calling _analyze has exceeded the allowed maximum of [10000]. This limit can be set by changing the [index.analyze.max_token_count] index level setting.
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction$TokenCounter.increment(TransportAnalyzeAction.java:397) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.simpleAnalyze(TransportAnalyzeAction.java:229) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:204) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:122) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:110) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.support.single.shard.TransportSingleShardAction.lambda$asyncShardOperation$0(TransportSingleShardAction.java:99) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.ActionRunnable.lambda$supply$0(ActionRunnable.java:47) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.action.ActionRunnable$2.doRun(ActionRunnable.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:737) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:16:25]                 │      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
[00:16:25]                 │      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
[00:16:25]                 │      	at java.lang.Thread.run(Thread.java:833) [?:?]
[00:16:25]                 └- ✓ pass  (215ms)
[00:16:25]               └-> partially valid, more than 75% are null
[00:16:25]                 └-> "before each" hook: global before each for "partially valid, more than 75% are null"
[00:16:25]                 └- ✖ fail: apis Machine Learning jobs Categorization example endpoint -  partially valid, more than 75% are null
[00:16:25]                 │       Error: expected 249 to sort of equal 250
[00:16:25]                 │       + expected - actual
[00:16:25]                 │ 
[00:16:25]                 │       -249
[00:16:25]                 │       +250
[00:16:25]                 │       
[00:16:25]                 │       at Assertion.assert (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:16:25]                 │       at Assertion.eql (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:244:8)
[00:16:25]                 │       at Context.<anonymous> (test/api_integration/apis/ml/jobs/categorization_field_examples.ts:303:36)
[00:16:25]                 │       at runMicrotasks (<anonymous>)
[00:16:25]                 │       at processTicksAndRejections (node:internal/process/task_queues:96:5)
[00:16:25]                 │       at Object.apply (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16)
[00:16:25]                 │ 
[00:16:25]                 │ 

Stack Trace

Error: expected 249 to sort of equal 250
    at Assertion.assert (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:244:8)
    at Context.<anonymous> (test/api_integration/apis/ml/jobs/categorization_field_examples.ts:303:36)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Object.apply (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16) {
  actual: '249',
  expected: '250',
  showDiff: true
}

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2764 2765 +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.6MB 4.6MB -69.0B

Page load bundle

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

id before after diff
kibanaReact 80.9KB 80.9KB +32.0B

History

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

@kevinlog kevinlog merged commit 9bc4865 into elastic:master Oct 19, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 19, 2021
…#115016)

* [Security Solution] Make new Add Data page more fine grained
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

@kevinlog kevinlog deleted the task/make-empty-page-finer-grained branch October 19, 2021 17:13
kibanamachine added a commit that referenced this pull request Oct 19, 2021
#115606)

* [Security Solution] Make new Add Data page more fine grained

Co-authored-by: Kevin Logan <56395104+kevinlog@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Detections and Resp Security Detection Response Team Team:Threat Hunting Security Solution Threat Hunting Team v7.16.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants