[Infrastructure UI] Hosts Technical Preview discovery

crespocarlos commented 1 year ago

:notebook: Summary

We want to facilitate the discovery of the Hosts View feature.

:heavy_check_mark: Acceptance criteria

Hosts menu item will always be displayed - regardless of whether or not the feature is enabled
If the user clicks on the menu item and the feature is off, a landing page will be shown with a CTA to enable it
- Once the feature is enabled, the user will be redirected to the Hosts view
If the user clicks on the menu and the feature is on, the Hosts view will be displayed
On the Inventory UI there should be a link to Hosts View
Telemetry should be sent to Fullstory.

Telemetry

Name	Description	Rational	Notes
Hosts View Visited	Differentiate the new landing page from the hosts view	We want to differentiate between users who visit hosts in the splash screen state vs those who have it activated	A new route can be easily tracked in Fullstory, but it's good to also call `useTrackPageview({ app: 'infra_metrics', path: 'hosts_feature_enable_landing_page' });` in the landing page. (`path` value is just a suggestion)
Hosts View Enable-Feature Clicked	When someone clicks on the hosts view enable button on the splash screen, we should track when someone clicks on this	We will be using this as a measure of demand from users and also as a method for potentially recruiting cloud customers for interviews/user testing	Set `data-test-subj: "hostsView-enable-feature-button` to the landing page button
Inventory Hosts View CTA Clicked	When someone clicks on the Try it button in the Inventory UI	Measure how many people landed in the hosts view from Inventory UI	Set `data-test-subj: "inventory-hostsView-cta` to the button

:bulb: Implementation hints

The implementation should follow the latest designs:

To enable a feature flag, we can use core.uiSettings.set
We might need to make changes to plugins/observability/public/components/shared/page_template/page_template.tsx in order to display an icon next to a menu item label
It's probably a good idea to create a new route and redirect the user to the landing page if the feature is disabled.
More about telemetry: https://docs.elastic.dev/telemetry/collection/event-based-telemetry#fullstory
It may help to have access to Fullstory staging (org_id: "1397FY") in order to test the events above: https://github.com/elastic/fullstory

Other Information

Splash screen copy

Introducing: Host Analysis

Introducing our new 'Hosts' feature, now available in technical preview! With this powerful tool, you can easily view and analyse your hosts and identify any issues so you address them quickly. Get a detailed view of metrics for your hosts, see which ones are triggering the most alerts and filter the hosts you want to analyse using any KQL filter and easy breakdowns such as cloud provider and operating system.

This is an early version of the feature and we would love your feedback as we continue to develop and improve it. To access the feature, simply enable below. Don't miss out on this powerful new addition to our platform - try it out today!

elasticmachine commented 1 year ago

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

roshan-elastic commented 1 year ago

👌

jennypavlova commented 1 year ago

Hi @crespocarlos, I have some questions regarding the Implementation hints

To enable a feature flag, we can use core.uiSettings.set

I checked with the elastic user and it's possible to change the setting that way. But I talked with Nathan today and he mentioned that the advanced settings are not available for all users (This will work for all admin users or users with access to advance settings). I tested creating a new role without access to advance settings and when I logged in with a user without those permissions I wasn't able to get or set the enable flag.

For those situations, we can maybe have a message to tell the user to contact his admin to enable the feature for him instead of the enable button - for the users with read access to the advanced settings (I think probably the UX won't be perfect). This could also lead to adding another telemetry entry for seeing the page for users without access to enable it.

Another option could be to keep the flag in the locale storage (preload the value if the user can access the settings or default to false for the users who can't) and give the user access to enable the host view without changing the flag in the settings (if the user doesn't have access). Then if the flag is not saved in the settings the user will see the landing page again when he logs in next time (Also if we don't enable the feature flag then the other users will also see the landing page to enable it. It is also not perfect as we already have the flag for it which we can't read in that case to know what is the actual flag state). It's also possible that there is another way to work with the feature flags that I am not aware of. Wdyt?

It's probably a good idea to create a new route and redirect the user to the landing page if the feature is disabled.

Why is a new route needed? Correct me if I am wrong but based on the AC the user should always see the host link(s) and when he clicks on it he will either see the landing page or the host view page - so we can always redirect the user to the host view route and then based on the feature flag to decide if we should show the landing page or the host view: Video of the flow (It's still WIP state so it's for showing the navigation):

https://user-images.githubusercontent.com/14139027/213242000-fb5ef9a8-86b6-42ae-a847-9970d6dd23fc.mov

Also the link on the inventory page should be always visible (like the nav item), right?

Another side note: I think it's a good idea to cover this feature with some functional tests covering the 2 additional cases:

user without permission to enable the host view
setup with uiSettings.overrides for the enable host view flag And some component tests (based on the decision on the user permission case)

crespocarlos commented 1 year ago

I checked with the elastic user and it's possible to change the setting that way. But I talked with Nathan today and he mentioned that the advanced settings are not available for all users (This will work for all admin users or users with access to advance settings). I tested creating a new role without access to advance settings and when I logged in with a user without those permissions I wasn't able to get or set the enable flag.

Ouch, good finding. Thanks Jenny! I didn't see any of that when I looked at the code using uiSettings.set. A user without permission to get will most likely be unable to use anything that's behind a feature flag, I ran a test here and was able to get the ff value. Maybe I did something different?

For those situations, we can maybe have a message to tell the user to contact his admin to enable the feature for him instead of the enable button - for the users with read access to the advanced settings (I think probably the UX won't be perfect). This could also lead to adding another telemetry entry for seeing the page for users without access to enable it.

I like this. We'd have to check whether the user has permission to save or read the advanced settings by checking application.capabilities.advancedSettings.save value first. This keeps consistency with how security and the rest of Kibana works. So, if only those who have privilege can change what's in advanced settings, I'd rather follow the same route.

Another option could be to keep the flag in the locale storage (preload the value if the user can access the settings or default to false for the users who can't) and give the user access to enable the host view without changing the flag in the settings (if the user doesn't have access). Then if the flag is not saved in the settings the user will see the landing page again when he logs in next time (Also if we don't enable the feature flag then the other users will also see the landing page to enable it. It is also not perfect as we already have the flag for it which we can't read in that case to know what is the actual flag state).

I like this idea, but if we could keep it as simple as prompting the user to contact who has access to the advanced settings to enable it, It might be better. This could be a good alternative, if we don't want to block users from seeing the feature.

cc @roshan-elastic , unless you have a different opinion, I'd rather prompt the user to reach out to an adm to enable the feature flag. I see this feature more as a shortcut to enable the flag. An alternative is to ditch the splash screen and the ff and leave only the icon in the menu item and the button in the Inventory UI. Meaning that the Host View page will always be accessible.

Why is a new route needed? Correct me if I am wrong but based on the AC the user should always see the host link(s) and when he clicks on it he will either see the landing page or the host view page - so we can always redirect the user to the host view route and then based on the feature flag to decide if we should show the landing page or the host view: Video of the flow (It's still WIP state so it's for showing the navigation):

It's totally fine the way you did it. I just had this idea, because would be easier to track who viewed the landing page, if we track the URL itself. If we managed to track that without needing a new route, it's totally fine. Let's discuss what's possible in terms of telemetry with your solution.

Also the link on the inventory page should be always visible (like the nav item), right?

yes

Another side note: I think it's a good idea to cover this feature with some functional tests covering the 2 additional cases:

user without permission to enable the host view

setup with uiSettings.overrides for the enable host view flag

And some component tests (based on the decision on the user permission case)

Great idea!

edit: added more info.

roshan-elastic commented 1 year ago

Hey @jennypavlova @crespocarlos, this is great work - very thorough and I like the suggestions...makes my life easy!

cc @roshan-elastic , unless you have a different opinion, I'd rather prompt the user to reach out to an adm to enable the feature flag. I see this feature more as a shortcut to enable the flag. An alternative is to ditch the splash screen and the ff and leave only the icon in the menu item and the button in the Inventory UI. Meaning that the Host View page will always be accessible.

Even though I asking people to reach out to their 'admin' is a pet hate of mine, I like this for a couple of reasons:

If someone goes to the effort of reaching out to their admin - it shows higher demand for the feature.
It spreads the word of the feature by default - they'll tell their admin...which is one more user to share this about.

Are we saying we could render different views of the splash screen for admin vs normal users? If so, that would be great! Even greater would be passing into the telemetry which version of the page they see so we can try and understand if we're seeing much drop-off. I don't want to increase scope though if capturing this is tough...

crespocarlos commented 1 year ago

Thanks @roshan-elastic . normal users will see the same thing, but the button would be disabled - and maybe there should be something in there asking them to reach out to an admin. While admins will see the same page as in the screenshot.

I guess we need @formgeist to help us with how we can communicate that with normal users.

formgeist commented 1 year ago

We can disable the button and show a tooltip when the user hovers on the button similar to Integrations when the user doesn't have permission to add the integration.

roshan-elastic commented 1 year ago

Great - so we just need copy for the tooltip?

"You don't have sufficient privileges to enable this feature - please reach out to your Kibana Administrator and ask them to visit this page to enable this feature"

Ideally, we'd mentioned the exact privilege they need and link through to the docs on this:

"You don't have sufficient privileges to enable this feature - please reach out to your Kibana Administrator and ask them to visit this page to enable this feature.

They will need a role with Kibana > Management > Advanced Settings > All permissions."

Would that be possible?

@crespocarlos @formgeist feel free to suggest alternatives/point out if this is incorrect - I'm working off the basis of this within roles:

formgeist commented 1 year ago

Links aren't possible in those tooltips. Then we might be better off displaying a message on the page that has the link. Something along the lines of

jennypavlova commented 1 year ago

Thank you all for the comments!

A user without permission to get will most likely be unable to use anything that's behind a feature flag, I ran a test here and was able to get the ff value. Maybe I did something different?

@crespocarlos I think it depends if the role is configured to have all or none permission for the advanced settings (if it's none you can see it only if it's read, I just tested that). I think the read access to everything is most likely to be the case for all non-admin users but I am not sure what's the real case in most of the user setups.

@formgeist I like the new screen but I am wondering if we still want to show the disabled button in this case (when we have the message to contact the admin and no tooltip)?

tonyghiani commented 1 year ago

@crespocarlos I just updated the naming for the events to be consistent with those already defined, let me know if you want to add and check together these definitions into the Telemetry Tracking Plan spreadsheet.

formgeist commented 1 year ago

@formgeist I like the new screen but I am wondering if we still want to show the disabled button in this case (when we have the message to contact the admin and no tooltip)?

You're right, we should opt to hide the disabled button if we're relying on a callout for the message 👍

roshan-elastic commented 1 year ago

Looking great everyone!

@formgeist - I'll let you decide on the best design but feel free to reach out if anyone has any Qs/decisions I need to make. You seem to be running this pretty well :)

jennypavlova commented 1 year ago

@crespocarlos Regarding the case with overrides we discussed - When the setting is defined in the kibana.yml file as an override (so when uiSettings.overrides.observability:enableInfrastructureHostsView is set to false, if it's set to true then the user should see the host view directly ) It won't be possible to change it with the enable button ( I tested it with a functional test ). We can still check the case using uiSettings.isOverridden to check and show again a callout instead of button. The callout text should be different because it won't be possible to change it in advance settings - only in the yml config. I would consider this a rare case but we can still implement a condition/new callout for it. Wdyt?

PS. This is my WIP branch - in case you want to check something there (I started with functional tests)

roshan-elastic commented 1 year ago

Hey @jennypavlova @crespocarlos this is looking great.

Quick Q - I shared your video in the internal observability newsletter and Vinay noticed the technical preview badge wasn't there on the splash screen:

Just checking that it will be added?

Cheers

crespocarlos commented 1 year ago

We can still check the case using uiSettings.isOverridden to check and show again a callout instead of button. The callout text should be different because it won't be possible to change it in advance settings - only in the yml config. I would consider this a rare case but we can still implement a condition/new callout for it. Wdyt?

I agree, this would be unlikely to happen. Especially now in 8.7. I suppose users won't know about this configuration key. But it's a scenario that if we cover, could avoid some headache.

jennypavlova commented 1 year ago

Quick Q - I shared your video in the internal observability newsletter and Vinay noticed the technical preview badge wasn't there on the splash screen:

@roshan-elastic I know about that - As I mentioned the purpose of the video was just to show the navigation and to make the routing question (about using the same path) more understandable. I mentioned that just before the video: "(It's still WIP state so it's for showing the navigation)" - I am sorry if it wasn't clear. I added it already for both cases 🙂:

I agree, this would be unlikely to happen. Especially now in 8.7. I suppose users won't know about this configuration key. But it's a scenario that if we cover, could avoid some headache.

@crespocarlos Then maybe it will be better to create a separate issue and prioritize it for this case if that won't be a priority for 8.7 ( we can still work on it after we are ready with this issue)- Wdyt?

crespocarlos commented 1 year ago

Then maybe it will be better to create a separate issue and prioritize it for this case if that won't be a priority for 8.7 ( we can still work on it after we are ready with this issue)- Wdyt?

Absolutely :)!

roshan-elastic commented 1 year ago

Sorry @jennypavlova - absolutely not your fault. I pulled this video out of context so understandable!

Thanks for this!

jennypavlova commented 1 year ago

@roshan-elastic I created a new issue about the case we discussed in this comment. Please let me know what is your opinion (I will appreciate some help with the message there so it won't sound too technical 🙂 ). Feel free to edit it and prioritize it when you have time - it's not urgent as it will be the next step after this issue is closed.

jennypavlova commented 1 year ago

@crespocarlos, @roshan-elastic Do you know if there is any update regarding the full-story staging access? (last point from the list):

It may help to have access to Fullstory staging (org_id: "1397FY") in order to test the events above: https://github.com/elastic/fullstory

tonyghiani commented 1 year ago

@crespocarlos, @roshan-elastic Do you know if there is any update regarding the full-story staging access? (last point from the list):

This is the issue created by @crespocarlos asking for access for our team, but I assume is still pending.

crespocarlos commented 1 year ago

Yeah. I've also created a ticket on another project. No answers yet.

elastic / kibana