[Monitoring] Metricbeat Migration Wizard (last step!!) #45799

chrisronline · 2019-09-16T16:38:28Z

This PR is the culmination of work specified in #39010

It adds the actual UI to enable and disable setup mode, along with various bug fixes and design/copy updates.

This also changes the /no-data page to start recommending users to set up Monitoring with Metricbeat:

The one thing this PR will not have is the tests that go along with this code. I don't want to bloat this PR review with a bunch of test files, but I will open a separate PR with the tests (which are already written) to ensure those go in as well.

The biggest thing for this PR is to really hammer home the testing aspect. This is our last chance to make sure this migration is air-tight and our messaging/design is clear and well understood.

There will continue to be iterative changes in this PR, as we get more feedback from design and tech writers on clearer ways of expressing our intent.

…fferent in setup mode

…p_wizard_step5

elasticmachine · 2019-09-16T16:38:30Z

Pinging @elastic/stack-monitoring

elasticmachine · 2019-09-16T17:13:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

chrisronline · 2019-10-01T13:24:25Z

If I have internal collection setup already and then click the Enter Setup Mode link, I see the Overview and other tabs (e.g. Indices) as clickable. Is this expected? Also, if I click on one of these tabs, I don't see the "Exit Setup Mode" bottom bar. I only see it on the Node/Instance Listing tabs.

IMO tabs other than Node/Instance listing should not be clickable to begin with, in Setup Mode. Also, if a user is on these tabs when not in Setup Mode, they should not be able to enter Setup Mode from here. Or if they do, they should be redirected to the product's Node/Instance listing tab once in Setup Mode.

This is not how it works now, but I can understand the desire for this. It is an inconsistent experience for the users now. I think we can solve it two ways:

Only show setup mode UIs if the page supports setup mode (so if a user is in setup mode and clicks an overview page, they don't see any indication they are in setup mode on that page)
Disable all UIs (disabling links and doing redirects) that do not support setup mode, when setup mode is enabled.

I personally lean towards the first option as it feels less restrictive for the user - they don't have to worry about leaving set up mode to go and look up some monitoring data.

I'm curious to thoughts from @elastic/stack-monitoring

chrisronline · 2019-10-01T13:27:32Z

If I have internal collection already setup, then migrate all my ES nodes to Metricbeat collection and turn off internal collection (all through the Setup Mode UI), then exit Setup Mode, I still see this banner up top on the ES Node Listing page:

Nice catch. Fixed 906c254

chrisronline · 2019-10-01T13:33:21Z

The fundamental problem here is that internal collection was something that was completely hidden from users' perspective. It was an implementation detail of each product (Kibana, ES, etc.). As a result we've never really needed a term for it to explain it to users. It's only now that we're introducing Metricbeat collection, we need a term to identify (and potentially explain) the "other" method.

Yes, this is a good point. We are currently using the term Internal Collection in this PR, but it's true that users will most likely not understand what that means.

What about something like self monitoring? To me, that makes sense in that it worked fine without needing additional tooling, but we are now introducing new tooling to handle the collection of the monitoring data. Or, we could leverage existing terminology and call it local monitoring since that's the same term we use when describing the exporter settings for Elasticsearch.

Also, are we adding complexity/confusion by using the term collection? Should we just stick with the term monitor/monitoring/monitored and avoid using collection at all? I think we are somewhat leaning towards that direction based on recent copy changes, but want to explicitly bring it up

ycombinator · 2019-10-01T13:42:09Z

What about something like self monitoring?

I really like "Self Monitoring" personally over "internal collection". I think it's far more self-explanatory to users than internal collection but I think we might want to include an explainer tooltip or something anyway?

Or, we could leverage existing terminology and call it local monitoring since that's the same term we use when describing the exporter settings for Elasticsearch.

I think this would actually be more confusing since a user can have internal collection with local or http exporters. So I'd vote to stay away from this one.

Also, are we adding complexity/confusion by using the term collection? Should we just stick with the term monitor/monitoring/monitored and avoid using collection at all?

++ to this. Again, we (the developers on the team) know that where the collection happens from is one of the things that has changed between internal collection and Metricbeat collection. However, it's not the only change (also being able to ship directly to the Monitoring cluster is the other change) AND collection is just one part of the overall monitoring process.

As far as a user is concerned I agree that we should try and avoid the term "collection" and use "monitoring" (and its variants) instead.

cchaos · 2019-10-01T13:46:12Z

When I'm not in Setup Mode and visit any page in the UI, I see this text at the top left. It appears to be unstyled. Just want to make sure this is deliberate (the positioning and styling)?

Yes. @cchaos signed off on this

To be clear, I had said the placement is not ideal. That you could consider moving it to a button somewhere on the Overview page which would be better placement but then the button wouldn't be visible on all pages. This might be alright though.

ycombinator · 2019-10-01T13:48:23Z

This might be alright though.

Yeah, in the interest of moving this PR forward I'm alright with the way it is now. We can always tweak it later in a follow up PR.

Mostly I wanted to confirm that this wasn't a TODO item still meant to be addressed in this PR, which sounds like it isn't.

elasticmachine · 2019-10-01T14:37:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

gchaps · 2019-10-01T15:30:41Z

++ to using self monitoring and adding a tooltip to explain.

++ to using monitoring instead of collection. I also found this confusing.

I'll review this text and look for this change.

…ollection' usage

chrisronline · 2019-10-01T17:56:25Z

++ to using self monitoring and adding a tooltip to explain.

++ to using monitoring instead of collection. I also found this confusing.

Both are these are now done -> 05279cf

elasticmachine · 2019-10-01T19:56:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cachedout · 2019-10-02T08:29:14Z

This looks ready to go at this point from my perspective. Great work, @chrisronline !

ycombinator

LGTM.

@chrisronline Not sure if you were expecting to hear from others about #45799 (comment), but I'm okay with the current behavior for now. As you outlined there are pros/cons to either approach so I'm good with shipping the current behavior and adjusting based on feedback.

Great job, not just on this final PR (which had reviews from multiple people and took a lot of iterations) but on the Setup UI as a whole. I'm very happy that we can provide this to our users to aid Metricbeat migration today and do other potential setup tasks in the future!

igoristic

Doesn't look like any of my concerns here cause problems: https://github.com/elastic/kibana/pull/45799/files/d86ebba8ba9cfc453770955ce3ed07bca3d332d9, so I'm going to go ahead and approve.

Nice work 🥇

cachedout · 2019-10-02T15:18:40Z

Only show setup mode UIs if the page supports setup mode (so if a user is in setup mode and clicks an overview page, they don't see any indication they are in setup mode on that page)
Disable all UIs (disabling links and doing redirects) that do not support setup mode, when setup mode is enabled.

I agree with @chrisronline that the first option is the better one.

elasticmachine · 2019-10-02T16:21:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Enable setup mode UI toggles * We want to keep the no data page but update the copy * More updated copy * Remove manual checks for logstash, beats, apm and kibana * Hide the setup mode controls on the no data page. There is nothing different in setup mode * Setup mode test * Fix bug with disabling internal collection for ES * First steps towards the redesign of setup mode * Consolidate UI code, design changes, use constants defined in our plugin * Fix tooltips * Design/copy feedback * Use badge and onClick * More feedback * Only detect usage on the live cluster * Fix existing tests, remove test that will be added in other PR * Fix failing test * Fix issue with wrong callout showing * Ensure we check for live nodes if no cluster uuid is provided * We need a custom listing callout for ES * Custom callout for kibana instances * More space from the bottom bar * Disable switching if they enabled internal collection * Copy updates * Fix broken tests * Fix more tests * Fix i18n * Update copy * Fix a couple i18n issues * Fixing a couple of missing scenarios * Fix translations * Update snapshots * PR feedback * PR feedback * We also need totalUniqueInternallyCollectedCount to identify when we have detected products but they are not monitored (thinking ES and Kibana) * Remove why documentation link until we have the resource available * Ensure tabs are properly disabled * Address issue with the ES nodes callout not working at times * Ensure we check if setup mode is enabled * Change internal collection to self monitoring, and remove the word 'collection' usage * Only show Enter setup mode on pages with valid setup mode options * Copy updates * Copy updates * Ensure we update the top nav item when we toggle setup mode on or off

chrisronline · 2019-10-02T17:47:38Z

Backport:

7.x: 8751538

chrisronline added 15 commits August 19, 2019 14:58

Enable setup mode UI toggles

bc66425

We want to keep the no data page but update the copy

3f0c65a

More updated copy

8c7e679

Remove manual checks for logstash, beats, apm and kibana

10aeb2b

Hide the setup mode controls on the no data page. There is nothing di…

3b33637

…fferent in setup mode

Setup mode test

100da8b

Fix bug with disabling internal collection for ES

236b08b

First steps towards the redesign of setup mode

fa80d10

Consolidate UI code, design changes, use constants defined in our plugin

cfb67e6

Fix tooltips

2e72734

Merge remote-tracking branch 'elastic/master' into monitoring/mb_setu…

679ff23

…p_wizard_step5

Design/copy feedback

da0e658

Use badge and onClick

2241135

More feedback

faea4c2

Only detect usage on the live cluster

7bc14c8

chrisronline added review Team:Monitoring Stack Monitoring team v8.0.0 v7.5.0 Feature:Stack Monitoring labels Sep 16, 2019

chrisronline requested review from ycombinator, cachedout and igoristic September 16, 2019 16:38

chrisronline self-assigned this Sep 16, 2019

chrisronline added the release_note:enhancement label Sep 16, 2019

chrisronline mentioned this pull request Sep 16, 2019

[Monitoring] Metricbeat Migration Wizard unit tests #45806

Closed

chrisronline added 2 commits September 16, 2019 13:18

Fix existing tests, remove test that will be added in other PR

17991ab

Merge in master

bfe2b49

Address issue with the ES nodes callout not working at times

cf32128

Ensure we check if setup mode is enabled

906c254

chrisronline added 2 commits October 1, 2019 13:54

Change internal collection to self monitoring, and remove the word 'c…

05279cf

…ollection' usage

Merge in master

309acef

chrisronline added 2 commits October 1, 2019 14:18

Only show Enter setup mode on pages with valid setup mode options

82c03e3

Copy updates

76f3ae2

cachedout approved these changes Oct 2, 2019

View reviewed changes

ycombinator approved these changes Oct 2, 2019

View reviewed changes

Copy updates

997de3e

igoristic approved these changes Oct 2, 2019

View reviewed changes

Ensure we update the top nav item when we toggle setup mode on or off

ed27251

chrisronline merged commit d935b3d into elastic:master Oct 2, 2019

chrisronline deleted the monitoring/mb_setup_wizard_step5 branch October 2, 2019 16:31

chrisronline mentioned this pull request Oct 2, 2019

[7.x] [Monitoring] Metricbeat Migration Wizard (last step!!) (#45799) #47136

Merged

cachedout added the release highlight label Oct 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Monitoring] Metricbeat Migration Wizard (last step!!) #45799

[Monitoring] Metricbeat Migration Wizard (last step!!) #45799

chrisronline commented Sep 16, 2019

elasticmachine commented Sep 16, 2019

elasticmachine commented Sep 16, 2019

chrisronline commented Oct 1, 2019

chrisronline commented Oct 1, 2019

chrisronline commented Oct 1, 2019

ycombinator commented Oct 1, 2019

cchaos commented Oct 1, 2019

ycombinator commented Oct 1, 2019

elasticmachine commented Oct 1, 2019

gchaps commented Oct 1, 2019

chrisronline commented Oct 1, 2019

elasticmachine commented Oct 1, 2019

cachedout commented Oct 2, 2019

ycombinator left a comment

igoristic left a comment

cachedout commented Oct 2, 2019

elasticmachine commented Oct 2, 2019

chrisronline commented Oct 2, 2019

[Monitoring] Metricbeat Migration Wizard (last step!!) #45799

[Monitoring] Metricbeat Migration Wizard (last step!!) #45799

Conversation

chrisronline commented Sep 16, 2019

elasticmachine commented Sep 16, 2019

elasticmachine commented Sep 16, 2019

💔 Build Failed

chrisronline commented Oct 1, 2019

chrisronline commented Oct 1, 2019

chrisronline commented Oct 1, 2019

ycombinator commented Oct 1, 2019

cchaos commented Oct 1, 2019

ycombinator commented Oct 1, 2019

elasticmachine commented Oct 1, 2019

💚 Build Succeeded

gchaps commented Oct 1, 2019

chrisronline commented Oct 1, 2019

elasticmachine commented Oct 1, 2019

💚 Build Succeeded

cachedout commented Oct 2, 2019

ycombinator left a comment

Choose a reason for hiding this comment

igoristic left a comment

Choose a reason for hiding this comment

cachedout commented Oct 2, 2019

elasticmachine commented Oct 2, 2019

💚 Build Succeeded

chrisronline commented Oct 2, 2019