-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Update onboarding interstitial to handle default Fleet assets #108193
Conversation
@joshdover I believe so, but want to double check, is this logic used only for the interstitial? And if so, is it a large effort to replace the logic (where there is some) in analytics applications? I've listed out the applications that are impacted here and they are primarily the "classic" Kibana apps. If it's too much out additional work, we can skip and revisit in another issue / PR, but I think it'd be good to have consistent logic for these apps and the interstitial. Let me know what you think |
Yes, this only affects the interstitial. I wanted to test this logic first before getting the Analytics apps involved, but I'll connect with them today to see how we can replicate this change in those flows as well. Breaking this up into separate changes decreases the chances that we have an issue with backporting at least this change to 7.14.1. |
@@ -41,7 +41,6 @@ export default function () { | |||
)}`, | |||
`--elasticsearch.username=${kibanaServerTestUser.username}`, | |||
`--elasticsearch.password=${kibanaServerTestUser.password}`, | |||
`--home.disableWelcomeScreen=true`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we can remove this configuration now. We now have some logic that sets the localStorage value in the FTR when attempting to login to Kibana to handle this case in Cloud where we can't set this config value. This makes this unnecessary now.
I'll tackle that as a follow up task.
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/fleet (Team:Fleet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for working on this!
I tested locally by mimicking Cloud conditions (i.e. Fleet is set up, Fleet Server agent is enrolled and running), and also if Endpoint happens to get installed, thereby creating an empty metrics-endpoint.metadata_*
index, by visiting the Security app. Under these conditions I confirmed that the interstitial modal is shown.
Then I enrolled a regular agent that collects system metrics, and unfortunately after data was ingested I was still seeing the interstitial modal. I did some debugging, see the rest of my review comments for where the new instance logic needs to be adjusted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I retested the scenarios from my last review and the behavior is as expected now :D
Working on fixing some flakiness with the integration test when using the Fleet setup API before I merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kibana-qa changes LGTM
I have some questions on the logic;
Is this really: If no index-patterns exist in the current space, and there are indices that I have at least view_index_metadata priv on, show the add data interstitial?
(in the current space)
I don't know what the part "the user is likely not to have privileges to add data" means. Meaning adding an index pattern?
If no other indices exist that contain documents that the user has view_index_metadata privilege on, show the interstitial ** Also, are there actually two different interstitials? One where there aren't any index patterns in this space and there is data, and another when there aren't any index patterns and also no non-system index data? I've seen one page that lets me go to index patterns anyway. |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / X-Pack Fleet Functional Tests.x-pack/test/fleet_functional/apps/home/welcome·ts.home onboarding Welcome interstitial is displayed on a fresh install with Fleet setup executedStandard Out
Stack Trace
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
…c#108193) # Conflicts: # test/functional/page_objects/common_page.ts
Yes that is the behavior and how this worked previously. One piece of logic that is not expressed above is also: "have we ever shown this to the user before". We won't ever show this more than once for a user, even if they create a new Space.
The reason we need the I agree this isn't perfect logic. My goal with this change was to make the minimal amount of changes to be able to show this interstitial quickly for the use case that is most important which is brand new users of Elastic. My reasoning here is:
There is an onboarding intestitial which this change fixes and there is also a UI that is displayed in Dashboard, Discover, and Visualize when no index patterns have been created. That will be addressed separately in #108887 |
…108193) (#108985) * Update onboarding interstitial to handle default Fleet assets (#108193) * Skip flaky test Co-authored-by: Josh Dover <[email protected]>
…108193) (#110496) * Update onboarding interstitial to handle default Fleet assets (#108193) * skip flaky suite. #108193 Co-authored-by: Jonathan Budzenski <[email protected]>
Summary
Fixes #107020
This updates the logic used to decide when to show the "add data" interstitial for new users to handle the case where Fleet server and the system package have been installed, but no user data has yet actually been ingested into the cluster.
This is intended to be a shorter-term solution with the long-term solution being solving #82851 more holistically by not installing all of the system package Kibana assets by default.
It follows the following logic:
metrics-*
orlogs-*
exist, do not show the interstitialGET /_cat/indices/metrics-*,logs-*
monitor
cluster privilege ormonitor
index privilege for any index. In either case, the user is likely not to have privileges to add data.metrics-elastic_agent*
andlogs-elastic_agent*
indices from the responseChecklist
Delete any items that are not applicable to this PR.
(https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)