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

Omit monitoring object from logstash_stats.logstash object #16198

Merged
merged 4 commits into from
Feb 12, 2020

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Feb 7, 2020

What does this PR do?

It removes a redundant field from Logstash monitoring data being indexed by Metricbeat for Stack Monitoring.

Why is it important?

It brings parity between Metricbeat collection and internal collection for Logstash monitoring data, specifically documents of type:logstash_stats.

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 There are existing tests (stack monitoring parity tests) that are currently failing and should start passing once this PR is merged.

How to test this PR locally

  1. Install Logstash 7.6.0 or higher.

  2. Configure Logstash to set the override cluster UUID. Edit config/logstash.yml and set monitoring.cluster_uuid: foobar.

  3. Build Metricbeat with this PR.

  4. Enable the Logstash module for Stack Monitoring.

    ./metricbeat modules enable logstash-xpack
    
  5. Run Metricbeat, outputting events to the Console so we can inspect them easily.

    /metricbeat -E output.elasticsearch.enabled=false -E output.console.enabled=true | jq '.'
    
  6. Check that events with "type": "logstash_stats" or "type": "logstash_state" both have a top-level "cluster_uuid": "foobar" field in them.

  7. Also check that events with "type": "logstash_stats" has a "logstash_stats.logstash" field which is an object. Check that this object does not contain a "monitoring" field.

Related issues

@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

Copy link
Contributor

@chrisronline chrisronline 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!

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

left a minor. Beyond this it lgtm codewise.

@ycombinator
Copy link
Contributor Author

Travis CI is green and Jenkins CI failures are unrelated. Merging.

@ycombinator ycombinator merged commit 69cd8d8 into elastic:master Feb 12, 2020
@ycombinator ycombinator deleted the mb-ls-xp-parity branch February 12, 2020 13:59
@ycombinator ycombinator removed the needs_backport PR is waiting to be backported to other branches. label Feb 13, 2020
ycombinator added a commit to ycombinator/beats that referenced this pull request Feb 14, 2020
…6198)

* Omit monitoring object from logstash_stats.logstash object

* Removing unnecessary struct tag info

* Adding CHANGELOG entry

* Adding explanatory comment for nodeInfo
ycombinator added a commit that referenced this pull request Feb 19, 2020
…16272)

* Omit monitoring object from logstash_stats.logstash object

* Removing unnecessary struct tag info

* Adding CHANGELOG entry

* Adding explanatory comment for nodeInfo
kvch pushed a commit to kvch/beats that referenced this pull request Feb 20, 2020
…6198)

* Omit monitoring object from logstash_stats.logstash object

* Removing unnecessary struct tag info

* Adding CHANGELOG entry

* Adding explanatory comment for nodeInfo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants