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] Metricbeat migration net new user experience #39832

Merged

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Jun 27, 2019

This PR is step 2, part 2 of the Metricbeat setup wizard and is also known as the "net new user" experience.

The main purpose of this PR is to provide a UX for new users to enable monitoring for their stack products. Currently, this PR is only concerned with Elasticsearch and Kibana, but a future PR will add support for other stack products (This is because there is/was outstanding work to enable metricbeat collection for these other products).

The current UX for enabling monitoring is extremely simple - it's just a button click in the UI. However, because of the need to install and start a beat, the process is slightly more complicated but the work done in this PR should help the user understand how to enable monitoring, and also guide the user through the experience too.

Testing

Elasticsearch monitoring

  • Start up ES
  • Start up Kibana
  • Load Kibana in the browser
  • Navigate to the stack monitoring application
  • You should land on the no-data page which is the default experience until step 4 is merged
  • To overwrite this behavior to test this PR, manually navigate to http://localhost:5601/app/monitoring#/loading?_g=(inSetupMode:!t) which will route you through the new user setup flow created through this PR
  • Follow instructions on the page to enable Elasticsearch monitoring with Metricbeat

Kibana monitoring

Edge cases

  • Please test with multiple instances of ES
  • I don't really see a need to test multiple instances of Kibana, as there is no way for us to know ahead of time that there is another kibana instance (so we can suggest they add monitoring to it).

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline chrisronline marked this pull request as ready for review July 2, 2019 16:28
@chrisronline chrisronline self-assigned this Jul 2, 2019
@chrisronline chrisronline added release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.4.0 v8.0.0 labels Jul 2, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ycombinator
Copy link
Contributor

Start up ES - ensure any monitoring settings are disabled
Start up Kibana, ensuring the xpack.monitoring.kibana.collection.enabled flag is set to false

I'm not sure these steps should be a prerequisite for testing the UX introduced by this PR. A net new user will start with default settings. So we should make sure the UX introduced by this PR works with these default settings, right?

@ycombinator
Copy link
Contributor

ycombinator commented Jul 3, 2019

Load Kibana in the browser
Navigate to the stack monitoring application
Note that you will no longer see the "No data screen", instead you will be taken to the Elasticsearch nodes listing page

Does this mean that when this PR is merged the user will start seeing the new UX already? If so, I thought we don't want this to be the case until the Step 4 PR is merged, right?

@chrisronline chrisronline removed the request for review from ycombinator July 3, 2019 10:22
@chrisronline
Copy link
Contributor Author

@ycombinator @igoristic I've addressed your feedback and this is ready for another round of review.

@ycombinator In terms of this comment, I've decided to go with this visual, using EUI colors to indicate status. LMK what you think
Screen Shot 2019-07-10 at 9 49 12 AM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ycombinator
Copy link
Contributor

I took another pass at this PR. All my previous feedback has been addressed.

I've decided to go with this visual, using EUI colors to indicate status.

I like this change. I also noticed the tooltips and found them to be helpful. I think the wording in them could be improved but that's for later as part of overall language cleanup anyway.

I noticed #39832 (comment) hasn't been addressed yet but chatting off-PR, I know you were thinking of addressing this in a follow up PR also across the board. Can you add this to the meta issue so we don't lose track of it?

Finally, one use case that I hadn't previously thought of:

What if a net new user has Beats or Logstash or APM Server also running and wants to know how to setup Metricbeat monitoring for those. Should we show sections for these on the cluster overview page with buttons linking to setup instructions (or something else to the same effect)?

@chrisronline
Copy link
Contributor Author

Can you add this to the meta issue so we don't lose track of it?

Added.

What if a net new user has Beats or Logstash or APM Server also running and wants to know how to setup Metricbeat monitoring for those. Should we show sections for these on the cluster overview page with buttons linking to setup instructions (or something else to the same effect)?

Agreed. This will be added/covered in Step 3 from the meta

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.

Functionally LGTM!

@chrisronline
Copy link
Contributor Author

@igoristic I think I addressed all your points, but let me know if I missed something and I'd be happy to fix it up!

@igoristic
Copy link
Contributor

@igoristic I think I addressed all your points, but let me know if I missed something and I'd be happy to fix it up!

@chrisronline Everything looks good for the most part 👍 Also, looks great functionally 😄

Still a little concerned about:
#39832 (comment)
#39832 (comment)

But, might not be a big deal here, and probably just a matter of preference

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline
Copy link
Contributor Author

@ycombinator @igoristic Some changes came in to support our existing api tests that you might want to review here: c63327c

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

@spalger
Copy link
Contributor

spalger commented Jul 19, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline chrisronline merged commit 6356088 into elastic:master Jul 22, 2019
@chrisronline chrisronline deleted the monitoring/mb_setup_wizard_step2_part2 branch July 22, 2019 14:12
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 22, 2019
…b-panel-for-stopping-jobs

* 'master' of github.com:elastic/kibana: (58 commits)
  [DOCS] Timelion cleanup (elastic#41381)
  [Docs] Add simple phrase highlighting to Logs UI (elastic#41610)
  [Maps] Rename modules for clarity (elastic#41608)
  [Monitoring] Metricbeat migration net new user experience (elastic#39832)
  [Maps] Only color legend icon with dynamic color when dynamic config is complete (elastic#41607)
  [TSVB] [Markdown] markdown section do not render after change data parameter (elastic#41576)
  [Vega] (Step 2) Shim new platform - renaming vega -> vis_type_vega (elastic#41565)
  update dark mode tsvb test (elastic#41618)
  [i18n] .i18nrc file as the source of truth and enhance tooling (elastic#39774)
  Reactify Top Nav Menu (kbn_top_nav) (elastic#40262)
  fix(code/frontend): should update search results if search options change (elastic#41232)
  Use kibana-ci-proxy-cache for chrome and gecko drivers (elastic#41581)
  [SIEM] Fix draggables to work with escapeId for the ML severity column (elastic#41621)
  [Canvas] Updates esdocs default count to 1000 (elastic#41604)
  [Uptime] Fix duration chart for Safari (elastic#41619)
  [Canvas] Restores "Today" as a quick time range in time filter (elastic#41528)
  docs: lowercase app (elastic#41612)
  [Code] Update git repository update frequency (elastic#41541)
  Remove language=json on code blocks due to performance hit (elastic#41540)
  [DOCS] Update anchors and links for Elasticserach API relocation. (elastic#41372)
  ...
chrisronline added a commit that referenced this pull request Aug 1, 2019
…41666)

* ES net new user experience

* Adding sorting support

* WIP

* Progress on the ES nodes listing page

* Fix the loading page

* Kibana net new user experience

* Remove unnecessary files

* Remove this one too

* Add this file back in

* Add a way to add the cluster uuid to global state once monitoring is enabled on a stack product

* Ensure this PR can be merged to master by hiding the functionality behind direct links

* Add messaging for net new user experience on Kibana instances page

* Ensure we poll for more data as soon as they are done in the flyout

* PR feedback

* Fix api tests

* Add `skipLiveData` flag to avoid fetching data from the running stack products

* Add comment
@chrisronline
Copy link
Contributor Author

Backport:

7.x: 01d4b69

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.

5 participants