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

Metricbeat logstash module: accept override cluster UUID from Logstash #15795

Merged
merged 6 commits into from
Feb 5, 2020

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Jan 23, 2020

What does this PR do?

This PR teaches the logstash Metricbeat module to accept an optional override cluster_uuid field from Logstash APIs called by the module.

If the Logstash pipeline being monitored by this module has an Elasticsearch plugin vertex in it, the cluster UUID associated with that vertex will be used. If no such vertex is found, the override cluster UUID will be used.

Why is it important?

See #15772.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

  1. Configure Logstash to use an override cluster_uuid for monitoring. To do this, add this line to your config/logstash.yml:

    monitoring.cluster_uuid: foobar
    
  2. Start a simple pipeline in Logstash.

    ./bin/logstash -e 'input { stdin {} }'
    
  3. Enable the logstash-xpack module in Metricbeat. This will enable the logstash module with the required configuration for Stack Monitoring.

    ./metricbeat modules enable logstash-xpack
    
  4. For easy inspection of generated events, disable the elasticsearch output and enable the console output in Metricbeat. To do this, edit your metricbeat.yml file and set output.elasticsearch.enabled: false and output.console.enabled: true.

  5. Run Metricbeat and check that the generated Logstash Stack Monitoring events of type:logstash_state and type:logstash_stats both have a top-level field of cluster_uuid with the same value as set in the Logstash configuration file.

    ./metricbeat | jq -c 'select(.service.type == "logstash") | .type,.cluster_uuid'
    

Related issues

@ycombinator ycombinator added enhancement in progress Pull request is currently in progress. Metricbeat Metricbeat module needs_backport PR is waiting to be backported to other branches. review Feature:Stack Monitoring Team:Integrations Label for the Integrations team v7.7.0 v8.0.0 and removed in progress Pull request is currently in progress. labels Jan 23, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@ycombinator ycombinator requested review from a team January 27, 2020 11:35
@ycombinator ycombinator added the test-plan Add this PR to be manual test plan label Jan 27, 2020
@ycombinator ycombinator marked this pull request as ready for review January 27, 2020 11:36
@andresrc andresrc added [zube]: Inbox Team:Services (Deprecated) Label for the former Integrations-Services team [zube]: In Review and removed [zube]: Inbox labels Jan 27, 2020
@urso
Copy link

urso commented Feb 4, 2020

Logstash pipelines are more or less independent entities (unless one forwards between pipelines). Don't we want pipeline 1 be reported with cluster 1, and pipeline 2 with cluster 2? This changes looks like all pipelines will now be reported with cluster 1 only.
In case a pipeline has no cluster UUID, we could use the pre-configured uuid as default. WDYT?

@ycombinator
Copy link
Contributor Author

@urso Yeah, we can do that. We can invert the logic to let the pipeline's cluster UUID (if set) to override the global one. I'll make the change. Thanks!

@sayden sayden self-assigned this Feb 4, 2020
@ycombinator
Copy link
Contributor Author

@urso I've made the change you suggested. Thanks!

@ycombinator
Copy link
Contributor Author

jenkins, test this

Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

LGTM, tested manually and getting the following output:

➜  metricbeat git:(mb-ls-xp-cluster-uuid) ✗ ./metricbeat | jq -c 'select(.service.type == "logstash") | .type,.cluster_uuid'
"logstash_state"
"foobar"
"logstash_stats"
"foobar"
"logstash_state"
"foobar"
"logstash_state"
"foobar"
"logstash_stats"
"foobar"
"logstash_state"
"foobar"
"logstash_state"
"foobar"
"logstash_state"
"foobar"

@ycombinator ycombinator merged commit 2a6d88f into elastic:master Feb 5, 2020
ycombinator added a commit that referenced this pull request Feb 10, 2020
…ash (#15795) (#16105)

* Fetch override cluster UUID from Logstash node pipelines API and use it

* Parse override cluster UUID from Logstash API response and make node_stats use it

* Adding CHANGELOG entry

* Use override cluster UUID as fallback to ES vertex cluster UUID

* Fixing CHANGELOG entry language

* Adding godoc for new helper function
@ycombinator ycombinator removed the needs_backport PR is waiting to be backported to other branches. label Feb 13, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature:Stack Monitoring Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team Team:Services (Deprecated) Label for the former Integrations-Services team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metricbeat logstash module should honor monitoring.cluster_uuid field in Logstash API responses
6 participants