[Monitoring] Metricbeat migration for logstash, beats and apm #40442
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
retest |
💔 Build Failed |
💔 Build Failed |
chrisronline
force-pushed
the
monitoring/mb_setup_wizard_step3
branch
from
1be61d7 to
bc03b08
Compare
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/apm/instances/instances.js
Outdated
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/cluster/overview/apm_panel.js
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/cluster/overview/apm_panel.js
Outdated
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/cluster/overview/beats_panel.js
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/cluster/overview/logstash_panel.js
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
x-pack/legacy/plugins/monitoring/public/components/logstash/listing/listing.js
Outdated
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
| <EuiText> | ||
| <FormattedMessage | ||
| id="xpack.monitoring.metricbeatMigration.apmInstructions.metricbeatSecuritySetup" | ||
| defaultMessage="If security features are enabled, there may be more setup required.{link}" |
There was a problem hiding this comment.
Looks like you might need a space after the dot.
There was a problem hiding this comment.
There is a space in the link fragment actually that should get the job done, unless you're not seeing a space when running it?
There was a problem hiding this comment.
Was just looking from the code point of view, but I understand now. Will test in a little bit 👍
igoristic
reviewed
Jul 31, 2019
|
|
||
| const enableMetricbeatModuleStep = { | ||
| title: i18n.translate('xpack.monitoring.metricbeatMigration.apmInstructions.enableMetricbeatModuleTitle', { | ||
| defaultMessage: 'Enable and configure the beat x-pack module in Metricbeat' |
There was a problem hiding this comment.
I'm not sure in this case as the module is called beat-xpack. I can go either way I guess. Anyone else have thoughts here?
There was a problem hiding this comment.
IMHO, Beat is still a proper name in the same way that we capitalize Metricbeat. Though, I will grant you that the naming scheme that was chosen makes this feel a little awkward. :)
igoristic
reviewed
Jul 31, 2019
...blic/components/metricbeat_migration/instruction_steps/apm/enable_metricbeat_instructions.js
Outdated
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
| <EuiText> | ||
| <FormattedMessage | ||
| id="xpack.monitoring.metricbeatMigration.beatsInstructions.metricbeatSecuritySetup" | ||
| defaultMessage="If security features are enabled, there may be more setup required.{link}" |
There was a problem hiding this comment.
Might need space after the dot
There was a problem hiding this comment.
See my note from the other comment about needing a space after the dot
igoristic
reviewed
Jul 31, 2019
...ic/components/metricbeat_migration/instruction_steps/beats/enable_metricbeat_instructions.js
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
...ic/components/metricbeat_migration/instruction_steps/beats/enable_metricbeat_instructions.js
Outdated
Show resolved
Hide resolved
igoristic
reviewed
Jul 31, 2019
| <p> | ||
| <FormattedMessage | ||
| id="xpack.monitoring.metricbeatMigration.logstashInstructions.disableInternalCollection.note" | ||
| defaultMessage="You'll need to restart logstash after making this change" |
igoristic
reviewed
Jul 31, 2019
| <EuiText> | ||
| <FormattedMessage | ||
| id="xpack.monitoring.metricbeatMigration.logstashInstructions.metricbeatSecuritySetup" | ||
| defaultMessage="If security features are enabled, there may be more setup required.{link}" |
There was a problem hiding this comment.
See my other comment about this
|
Everything looks good! Great job 💪 |
igoristic
approved these changes
Jul 31, 2019
💚 Build Succeeded |
chrisronline
added a commit
to chrisronline/kibana
that referenced
this pull request
Jul 31, 2019
chrisronline
added a commit
to chrisronline/kibana
that referenced
this pull request
Aug 1, 2019
…c#40442) * Support for logstash * Beats support * Fix cherry-pick api issue * Support for logstash * Updates for beats and logstash * APM migration working * Tweaks for beats migration * Update copy for setup new button * If on cloud, disable setup mode * Handle new beat flow better * Better phrasing for APM * Add beat type to disable step * Fix i18n issue * Fix jest tests * Fix api tests * PR feedback * Update copy * Remove unnecessary code * Undo changes that are now in a separate PR * Disable more links * Fix overview link for logstash * PR feedback * Fix tests * PR feedback * PR feedback * Capitalize Beat per PR feedback
chrisronline
added a commit
that referenced
this pull request
Aug 1, 2019
#42455) * Support for logstash * Beats support * Fix cherry-pick api issue * Support for logstash * Updates for beats and logstash * APM migration working * Tweaks for beats migration * Update copy for setup new button * If on cloud, disable setup mode * Handle new beat flow better * Better phrasing for APM * Add beat type to disable step * Fix i18n issue * Fix jest tests * Fix api tests * PR feedback * Update copy * Remove unnecessary code * Undo changes that are now in a separate PR * Disable more links * Fix overview link for logstash * PR feedback * Fix tests * PR feedback * PR feedback * Capitalize Beat per PR feedback
|
Backport: 7.x: 1597b30 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is step 3 of the Metricbeat setup wizard.
It adds MB migration UX for logstash, beats and APM, including net new user experience as well as migration from internal collection.
This PR builds upon the functionality added in the two step 2 PRs, part 1 and part 2.
In addition, this PR also adds a button to the bottom of each stack products' listing page that allows the user to access the setup wizard to add monitoring for a new node/instance:
Also, this PR takes advantage of the fact that we're attempting to detect if certain stack products exist, based on known indices. This wasn't something that mattered in previous PRs as we can know for certain that a Kibana and ES exist if they are in the stack monitoring UI.
Lastly, this PR adds code to disable entering setup mode if on cloud.
During testing, we realized that logstash and beats can potentially contain mb monitoring documents without a
cluster_uuid(which would put those instances in the standalone cluster) so we built a flow in the wizard to let the user know the location of this instance/node will change. In this flow, the user is required to check the checkbox to finish the wizard. Here is what that looks like: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)yarn add elastic-apm-nodescripts/kibana.js:require('elastic-apm-node').start({})Existing Issues
cluster_uuidas a result). This can be addressed in testing by deleting thedatadirectory in the beat and restarting.