[Monitoring] Metricbeat migration flyout and instructions by chrisronline · Pull Request #35228 · elastic/kibana

chrisronline · 2019-04-17T13:40:20Z

Step 2 for #39010

Summary

This PR introduces a new "Setup Mode" into the stack monitoring UI that will offer different UI treatments when toggled. Per the steps from #39010, this PR doesn't introduce a way in the UI to toggle this "Setup Mode" but it does introduce the UIs that will be visible once toggled. For testing this PR, toggling this "Setup Mode" will need to be manual. See the testing instructions section for more information.

Scope

This PR affects two pages in the UI, the ES nodes listing page and the Kibana instances listing page. Once "Setup Mode" is enabled, there will be an additional column in each page that shows the Metricbeat migration status, which is either a green badge saying Migrated or a red CTA button saying Migrate. Clicking Migrate open a flyout with instructions on how to migrate, with the ability to check the status directly in the flyout.

TODO

Unit/functional tests, this can be done in a separate PR too
~~Localization~~

Unanswered Questions

~~There are nuances around fetching the monitoring url that are making me think we should not attempt to prefill this and rather require the user to always supply it. Thoughts?~~
There is a nuance visual "bug" where, when in a partially migrated state where you only see two steps, if you check for data and the response comes back indicating the migration is complete, the steps will switch back to the full list of steps. I don't think this is a big issue, but lemme know if it's unjarring at ll.

Testing

The first thing to talk about is enabling this "Setup Mode". The most straight forward way is to just update the query string in the url, by adding this to the global state param: ,inSetupMode:!t. A final, working URL will look something like: http://localhost:5601/app/monitoring#/elasticsearch/nodes?_g=(cluster_uuid:qtoNYcCxQdK-9BGIdPN_lw,inSetupMode:!t)

The second thing is the actual setup and configuration of Metricbeat to collect and ship the monitoring data. It's probably a good idea to not describe that here, and simply rely on the instructions inside of the flyout to educate anyone not currently aware. If they aren't help you or you get stuck, then we need to update the instructions!

There are a couple scenarios I think are worth testing that I'll list here:

Multiple Kibanas, upgrading one at a time
Multiple ES nodes, upgrading one at a time
Testing the visual state when internal and MB collection are both enabled

Screenshots

UI when "Setup Mode" is enabled

Sequential flow for the flyout

Table, when fully migrated

ES state when all nodes are partially migrated

Kibana state when a node is partially migrated

Kibana state when migrating the kibana currently used

…p_step2

elasticmachine · 2019-04-17T14:35:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-17T18:42:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-17T21:10:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-19T21:49:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-22T16:26:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-23T14:09:32Z

Pinging @elastic/stack-monitoring

chrisronline · 2019-04-23T14:30:22Z

x-pack/plugins/monitoring/public/components/metricbeat_migration/flyout/flyout.js

+    };
+  }
+
+  // shouldComponentUpdate(nextProps, nextState) {


I'll remove this

igoristic · 2019-04-23T17:39:22Z

I'm assuming you will also remove these files?

x-pack/plugins/monitoring/public/components/metricbeat_migration/index.js
https://github.com/elastic/kibana/pull/35228/files#diff-93944299482a29bbb76ce4a142fd3842

x-pack/plugins/monitoring/public/components/metricbeat_migration/metricbeat_migration.js
https://github.com/elastic/kibana/pull/35228/files#diff-a60baeaccef05023d94c3551a3074949

chrisronline · 2019-04-23T17:45:46Z

@igoristic Yes, good catch. I pushed the commit up, but the PR is not recognizing it for some reason. You can see it here: https://github.com/chrisronline/kibana/commits/monitoring/mb_setup_step2

cchaos

Hey @chrisronline this looks pretty good, just have some suggestions for cleaning up the flyout.

Remove the dashed border, the "Exit setup mode" link plus the new buttons should be enough to indicate.
Remove the top level wizard steps. Since there are only two, it's not really necessary to have them.
Use a flyout footer to house the "Next" button plus a "Close" button for them to exit the flyout

Remove the outer EuiPanel surrounding the full steps list
Remove the leading paragraph that just tells them to follow the instructions (pretty self-explanatory)
I see an error with the EuiSteps layout that I'll get fixed in EUI, but shouldn't hold up this PR
Be sure to wrap any <p> tags inside of a <EuiText> and watch your extra lines before and after the string inside code blocks.
Use sentence case for the menu links up at the top left

Reiterating some of these below in the code to find the correct reference.

x-pack/plugins/monitoring/public/components/metricbeat_migration/flyout/flyout.js

...oring/public/components/metricbeat_migration/instruction_steps/elasticsearch_instructions.js

...s/monitoring/public/components/metricbeat_migration/instruction_steps/kibana_instructions.js

ycombinator · 2019-04-23T18:29:24Z

Lots of good stuff in this PR! A few initial pieces of feedback, mostly minor:

When the flyout opens I see the two steps at the top: Configure monitoring cluster and Setup Elastisearch|Kibana. I was able to click on step 2 and move to it without completing step 1. Is there some way to prevent this?
Typo: The running metricbeat instance will need to be able to react this location. — I think you meant "reach" instead of "react"?
For the monitoring cluster hostname textbox, WDYT about adding a placeholder with a sample so users know what format to enter it in?
Possible nit: Monitoring cluster URL — Technically the box would contain one (typically) or more (less typically) comma-separate URLs to Elasticsearch hosts in the Monitoring cluster. I'm not sure how to convey all that in the label. Or maybe the label is fine as-is but we should add some help text below the text box? There's plenty of space anyway.
I really like the 1-2 steps at the top of the flyout. WDYT about splitting 2 into 2 and 3, where 2 is about "Set up Elasticsearch|Kibana" and 3 is "Set up Metricbeat"?
Nit: "Set the xpack.monitoring.collection.enabled setting". Would it be possible to format the setting name in monospace?
Typo: "To migrate, following the following instructions:" — "following the following"
In the ES setup, for the "2. Disable the default collection of Elasticsearch monitoring metrics" step, if the cluster is the same as the production cluster Kibana is connected to, could we replace this with a button that performs the API call when clicked? This could be deferred to a follow up PR; I'm more curious how difficult this would be to pull off.
In the ES setup, for the "4. Enable and configure the Elasticsearch module in Metricbeat" why is the hosts setting using the Monitoring cluster hosts? This should be the HTTP address of the node being monitored.
Ideally the user would enter the monitoring hosts only once.

After they enter it the first time can we "save" it somewhere (maybe in the global state?), so its 1) auto-populated in any subsequent flyouts the user opens and b) when a user opens a flyout again, it takes the user directly to step 2? Of course, the user could still then go back to step 1 and edit the monitoring hosts, but they really shouldn't need to do this.

Another option might be to remove this step from the flyout altogether as its more global to all setup. Maybe it's something we can show in a more global location on the page whenever the user is in setup mode?

Finally, if we can't get to a satisfactory UX for getting the monitoring hosts from the user, I'd also be okay with simply not asking for this information (for now). In the Metricbeat YAML snippets we could simply mirror what we do in our docs.

elasticmachine · 2019-06-13T18:08:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline · 2019-06-13T19:40:28Z

FYI, I've updated the screenshots to reflect the most recent version of this PR to assist @cchaos in reviewing this

cchaos

Just saw a couple, mostly minor, UI issues. There's also a lot of new text. I'd highly suggest getting @gchaps to review the copy.

x-pack/plugins/monitoring/public/components/metricbeat_migration/flyout/flyout.js

...nents/metricbeat_migration/instruction_steps/elasticsearch/enable_metricbeat_instructions.js

...c/components/metricbeat_migration/instruction_steps/kibana/enable_metricbeat_instructions.js

...icbeat_migration/instruction_steps/elasticsearch/disable_internal_collection_instructions.js

...ts/metricbeat_migration/instruction_steps/kibana/disable_internal_collection_instructions.js

x-pack/plugins/monitoring/public/components/metricbeat_migration/flyout/flyout.js

chrisronline · 2019-06-14T14:08:37Z

@cchaos Thanks for the great feedback, I've addressed each piece and added screenshots for you tor review.

elasticmachine · 2019-06-14T14:59:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

chrisronline · 2019-06-14T15:01:10Z

retest

elasticmachine · 2019-06-14T16:11:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos

LGTM, thanks for all those UI changes. I'd sill have @gchaps do a once over for copy.

chrisronline · 2019-06-14T16:49:51Z

@gchaps @cchaos I'm wondering if it makes sense to wait sense to wait until the last PR (see steps here) to come in and go over the PR and figure out any copy changes. Again, even if this PR ships without all steps being complete (which is not likely), users will not be able to see any of these UIs until the last step is done.

Thoughts?

cchaos · 2019-06-14T17:17:36Z

Up to @gchaps , but for me it's so long as you promise to get some copy writer to do a final pass before it goes into master 😏

ycombinator · 2019-06-14T18:24:32Z

@cchaos @chrisronline added a TODO item for final copy approval from @gchaps under step 4 of the meta issue here: #39010.

) * Initial attempt at a reactor of how this works * Enter and exiting setup mode with migration buttons working * Adding monitoring url step back in and some small cleanup * Elasticsearch steps * Add missing file * Better organization here * Remove this debug logic * Clean up * PR feedback * Add in monospacing * Persist monitoring url in local storage * Rework the steps * Change node to server, and add missing files * Fix linting issues * Fix api integration tests * PR feedback * Pass down if the product is the "primary" or not, then use that to show certain warnings in the UI (just supported for Kibana right now) * Elasticsearch migration will work slightly differently in that all nodes must be partially migrated before we can disable internal collection * More PR feedback * PR feedback * Better links * Fix tests * This should open in a new tab * PR feedback * Design and PR feedback * Fix these tests * PR feedback * Remove debug * PR feedback * Update the import path * Update this import path too * PR feedback * Fix i18n

elasticmachine · 2019-06-14T19:37:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…39014) * Initial attempt at a reactor of how this works * Enter and exiting setup mode with migration buttons working * Adding monitoring url step back in and some small cleanup * Elasticsearch steps * Add missing file * Better organization here * Remove this debug logic * Clean up * PR feedback * Add in monospacing * Persist monitoring url in local storage * Rework the steps * Change node to server, and add missing files * Fix linting issues * Fix api integration tests * PR feedback * Pass down if the product is the "primary" or not, then use that to show certain warnings in the UI (just supported for Kibana right now) * Elasticsearch migration will work slightly differently in that all nodes must be partially migrated before we can disable internal collection * More PR feedback * PR feedback * Better links * Fix tests * This should open in a new tab * PR feedback * Design and PR feedback * Fix these tests * PR feedback * Remove debug * PR feedback * Update the import path * Update this import path too * PR feedback * Fix i18n

chrisronline · 2019-06-14T20:08:47Z

Backport:

7.x: 75785e9

chrisronline added 2 commits April 17, 2019 09:34

Initial attempt at a reactor of how this works

08a7de1

Enter and exiting setup mode with migration buttons working

22fb160

chrisronline mentioned this pull request Apr 17, 2019

[Monitoring] Metricbeat migration flyout and instructions #35102

Closed

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

964d9da

…p_step2

Adding monitoring url step back in and some small cleanup

fc78ee8

Elasticsearch steps

1a887ab

chrisronline added 2 commits April 18, 2019 12:05

Add missing file

79d1845

Better organization here

f7881f5

Remove this debug logic

2c74202

chrisronline self-assigned this Apr 23, 2019

chrisronline added Team:Monitoring Stack Monitoring team review v7.2.0 v8.0.0 labels Apr 23, 2019

chrisronline requested review from igoristic and ycombinator April 23, 2019 14:09

chrisronline marked this pull request as ready for review April 23, 2019 14:19

chrisronline requested a review from a team as a code owner April 23, 2019 14:19

chrisronline commented Apr 23, 2019

View reviewed changes

Clean up

caa889e

cchaos suggested changes Apr 23, 2019

View reviewed changes

igoristic approved these changes Jun 13, 2019

View reviewed changes

chrisronline added 3 commits June 13, 2019 13:03

Merge in master

9118cd0

Update the import path

7d6cea1

Update this import path too

f45c64d

chrisronline mentioned this pull request Jun 13, 2019

[Monitoring] Use cases to consider for setup mode #38929

Open

cchaos suggested changes Jun 13, 2019

View reviewed changes

PR feedback

3218a7c

chrisronline added the release_note:skip Skip the PR/issue when compiling release notes label Jun 14, 2019

Fix i18n

ee15ead

cchaos approved these changes Jun 14, 2019

View reviewed changes

cchaos requested a review from gchaps June 14, 2019 16:48

chrisronline merged commit b09e3a6 into elastic:master Jun 14, 2019

ycombinator mentioned this pull request Jun 14, 2019

Implement a UI within Stack Monitoring to help users migrate to MB shipping/collecting #39010

Closed

5 tasks

chrisronline mentioned this pull request Jun 14, 2019

[7.x] [Monitoring] Metricbeat migration flyout and instructions (#35228) #39014

Merged

chrisronline deleted the monitoring/mb_setup_step2 branch June 14, 2019 20:08

chrisronline mentioned this pull request Jul 22, 2019

[Monitoring] Metricbeat migration for logstash, beats and apm #40442

Merged

chrisronline mentioned this pull request Aug 6, 2019

[Monitoring] Metricbeat migration improvements #42600

Merged

Conversation

chrisronline commented Apr 17, 2019 • edited by ycombinator Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope

TODO

Unanswered Questions

Testing

Screenshots

UI when "Setup Mode" is enabled

Sequential flow for the flyout

Table, when fully migrated

ES state when all nodes are partially migrated

Kibana state when a node is partially migrated

Kibana state when migrating the kibana currently used

Uh oh!

elasticmachine commented Apr 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Apr 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Apr 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Apr 19, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Apr 22, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Apr 23, 2019

Uh oh!

chrisronline Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

igoristic commented Apr 23, 2019

Uh oh!

chrisronline commented Apr 23, 2019

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ycombinator commented Apr 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jun 13, 2019

💚 Build Succeeded

Uh oh!

chrisronline commented Jun 13, 2019

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chrisronline commented Jun 14, 2019

Uh oh!

elasticmachine commented Jun 14, 2019

💔 Build Failed

Uh oh!

chrisronline commented Jun 14, 2019

chrisronline commented Apr 17, 2019 •

edited by ycombinator

Loading

ycombinator commented Apr 23, 2019 •

edited

Loading

ycombinator commented Jun 14, 2019 •

edited

Loading