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.

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

[Monitoring] Clearer MB migration state on the overview page #41898

Merged

Conversation

chrisronline
Copy link
Contributor

Resolves #41725

Instead of showing 0/1 (or something similar), let's simplify what we're showing by only showing something (a flag icon) if the user should take action on the listing page for the stack product. Also, provide a tooltip that explains this more.

Screen Shot 2019-07-24 at 10 18 29 AM

Screen Shot 2019-07-24 at 10 18 34 AM

Testing

@chrisronline chrisronline added review Team:Monitoring Stack Monitoring team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.4.0 labels Jul 24, 2019
@chrisronline chrisronline self-assigned this Jul 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring

@ycombinator
Copy link
Contributor

I have a 1-node ES cluster running. Initially it was using internal collection (and no Metricbeat collection). So I saw the flag on the Overview page as expected, with this message:

Some nodes are not monitored by Metricbeat. Click the flag icon to visit the nodes listing page and find out more information about the status of each node.

So I went to the nodes listing page and followed the instructions for enabling Metricbeat collection. After I did that successfully the nodes listing page told me that the Setup Status for my one and only node was:

Internal collection and Metricbeat collection

Then I came back to the Overview page (while still in Setup mode). I still see the flag and the same tooltip message. Now that all my nodes are being monitored via Metricbeat, saying that "Some nodes are not monitored by Metricbeat" seems confusing.

@ycombinator
Copy link
Contributor

Apart from the confusing messaging on the flag tooltip when both internal collection and Metricbeat collection are setup for all ES nodes (see previous comment), I like the general idea of the flag tooltip and simpler messaging here.

I also verified that no flag shows up if the user is not in setup more OR if the user is in setup mode but all nodes are being exclusively monitored by Metricbeat.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline
Copy link
Contributor Author

@ycombinator Good point. I've updated the copy to say:

Some nodes are not monitored exclusively by Metricbeat

In the state where one or more nodes/instances are monitored by both.

WDYT?

@ycombinator
Copy link
Contributor

Consider the scenario where we have a 2-node cluster.

  • Node 1 is using internal collection but not Metricbeat collection.
  • Node 2 is using internal collection AND Metricbeat collection.

What should the tooltip message say then?

@chrisronline
Copy link
Contributor Author

I'm trying to avoid this tooltip being too wordy, but maybe that's not the direction we should.

We could lay out all the cases and support longer messaging, if we think that's a better direction:

All nodes are internally monitored

All nodes are not monitored by Metricbeat.

Some nodes are fully monitored by MB, and some are internally monitored

Some nodes are monitored by Metricbeat, but some are not.

Some nodes are partially (meaning both internal and MB) monitored, and some are internally monitored

Some nodes are monitored by both Metricbeat and internal collection, and some are not monitored by Metricbeat at all.

Some nodes are partially (meaning both internal and MB) monitored, and some are monitored only by Metricbeat (for Kibana use case)

Some nodes are monitored by both Metricbeat and internal collection, and some are only monitored by Metricbeat.

We could list more probably, but is this the direction we should go?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ycombinator
Copy link
Contributor

Clearly what we need in the toolip is a venn diagram 😜

Jokes aside, I think we need to take a step back and decide the purpose of this tooltip. If I'm reading previous discussion correctly, I think the idea is to show this flag+tooltip if there's any setup work yet to be done, with the end goal being for all nodes to be monitored exclusively using Metricbeat. I also like the idea that the tooltip message should be simple while directing the user to see the Node Listing page for details.

So I think we only show the flag+tooltip if:

  • There's at least one node that isn't being monitored using Metricbeat, or
  • All nodes are being monitored using Metricbeat but internal collection still needs to be turned off.

When the end goal is met (all nodes are being monitored exclusively using Metricbeat), we don't show the flag+tooltip at all.

WDYT?

@chrisronline
Copy link
Contributor Author

Yup, agreed.

We still need to decide on the copy for the tooltip.

There's at least one node that isn't being monitored using Metricbeat

Not all nodes are exclusively monitored by Metricbeat.

All nodes are being monitored using Metricbeat but internal collection still needs to be turned off.

All nodes are exclusively monitored by Metricbeat, but internal collection needs to be disabled.

WDYT?

@chrisronline
Copy link
Contributor Author

Or maybe more like the earlier examples for the first one:
Some nodes are not exclusively monitored by Metricbeat.

@ycombinator
Copy link
Contributor

ycombinator commented Jul 24, 2019

All nodes are exclusively monitored by Metricbeat, but internal collection needs to be disabled.

This statement cannot be true 😄. If all nodes are exclusively monitored by Metricbeat, it follows that internal collection is disabled.


I wasn't at all clear in my previous comment but the two bullets I posted were also meant to be the copy for the tooltip (pending any cleanup in the future by @gchaps, of course). I'll repost them below:

  • There's at least one node that isn't being monitored using Metricbeat. Click...
  • All nodes are being monitored using Metricbeat but internal collection still needs to be turned off. Click...

@ycombinator
Copy link
Contributor

@chrisronline To move this PR forward, I'm happy with whatever copy you like as long as we're in agreement about the two conditions under which we should show the flag+tooltip. @gchaps will probably have to clean it up later anyway.

@chrisronline
Copy link
Contributor Author

I'm happy with what you suggested. Code is updated now and ready for 👀

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ycombinator
Copy link
Contributor

ycombinator commented Jul 25, 2019

When all nodes are being monitored by both Metricbeat and internal collection, the tooltip message looks like this:

Screen Shot 2019-07-25 at 7 14 58 AM

In this state, the last sentence doesn't really make sense for the user IMO, particularly this bit:

... and find out more information about the status of each node.

Would it be possible to say something like this instead?

... and disable internal collection.

(I'm using "disable" instead of "turn off" only because the button on the Node Listing page is titled "Disable and finish migration".)

@ycombinator
Copy link
Contributor

ycombinator commented Jul 25, 2019

Once I completely finished my ES cluster migration to Metricbeat monitoring and went back to the Overview page, while still in Setup Mode, I noticed that the flag icon has disappeared. Effectively this page now looks the same as if the user was not in Setup Mode.

I wonder if we should show a banner (or some other UI treatment) telling the user that they are in Setup Mode? With a banner we could also provide a link for them to exit Setup Mode. [EDIT] This would be part of a separate PR, of course, but:

Another idea, perhaps in addition to the banner, would be to replace the flag icon with a checkmark icon when the setup looks good? Hovering over it could basically just tell the user exactly that but in a couple of sentences, like:

All nodes are being monitored using Metricbeat. Your Elasticsearch monitoring setup looks good!

(Again, I expect the language to get cleaned up later by @gchaps, of course).

@chrisronline
Copy link
Contributor Author

I wonder if we should show a banner (or some other UI treatment) telling the user that they are in Setup Mode? With a banner we could also provide a link for them to exit Setup Mode.

Yup, this is a good point but we already plan to add something like this in Step 5

@ycombinator
Copy link
Contributor

@chrisronline Understood re: step 5, but WDYT about the rest of my comment?

Another idea, perhaps in addition to the banner, would be to...

@chrisronline
Copy link
Contributor Author

Would it be possible to say something like this instead?

Agreed, that's better. I've updated the PR with this change

@chrisronline
Copy link
Contributor Author

Another idea, perhaps in addition to the banner, would be to replace the flag icon with a checkmark icon when the setup is completed? Hovering over it could basically just tell the user exactly that but in a couple of sentences,

Originally, the plan was to have an icon indicating they need to take action, or an icon indicating they didn't need to take action. After talking it over with @cchaos, we decided that it might be better to limit the number of icons in the UI and only present ones when the user needs to do something. I do think we need some kind of general indicator of setup mode status, but I'm not sure we need a specific icon indicating each stack product is setup correctly. I could probably go either way, and my decision in this PR was heavily swayed by thoughts from @cchaos

@ycombinator
Copy link
Contributor

@chrisronline Thanks for clarifying. I'm good with the direction you agreed upon previously with @cchaos.

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@igoristic
Copy link
Contributor

LGTM 👍

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline chrisronline merged commit 976d976 into elastic:master Jul 31, 2019
@chrisronline chrisronline deleted the monitoring/clearer_setup_state branch July 31, 2019 20:24
chrisronline added a commit that referenced this pull request Aug 1, 2019
…#42462)

* Just show an icon if action is necessary, otherwise show nothing

* Update copy

* Update copy based on state

* Updated logic and copy

* Update copy
@chrisronline
Copy link
Contributor Author

Backport:

7.x: 9441a86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Setup UI] Distinguish between setup states
4 participants