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

Only initialize service URL once #16044

Merged
merged 3 commits into from
Feb 4, 2020

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Feb 3, 2020

What does this PR do?

It initializes the URL for the Logstash node stats API to be called by the logstash module when xpack.enabled: true is set exactly once.

Why is it important?

Before this fix, the ?vertices=true query parameter string was getting appended to the Logstash node stats API URL repeatedly, each time the logstash/node_stats's Fetch() method was invoked. This resulted in Logstash eventually returning HTTP 400 responses and Logstash monitoring data not being collected by the Logstash Metricbeat module.

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 that verify correctness of collecting Logstash monitoring data. To test this particular bugfix, manual testing is OK IMO.

How to test this PR locally

  1. Run Logstash with a simple pipeline.

    ./bin/logstash -e 'input { stdin {} }'
    
  2. Enable the logstash-xpack module in Metricbeat.

    ./metricbeat modules enable logstash-xpack
    
  3. Run Metricbeat.

    ./metricbeat -e
    
  4. Open up Wireshark (or similar) and sniff for HTTP traffic on localhost going to port 9600 (the Logstash HTTP API port). Verify that you are seeing calls like GET /_node/stats?vertices=true every 10 seconds. Verify that the ?vertices=true query parameter string is not being appended in each iteration. So the first iteration should have exactly one ?vertices=true, as should the second, third, fourth, and so on.

Related issues

@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@ycombinator ycombinator added needs_backport PR is waiting to be backported to other branches. test-plan Add this PR to be manual test plan labels Feb 3, 2020
@ycombinator ycombinator changed the title Only add vertices param to URL if it doesn't already exist Only initialize service URL once Feb 3, 2020
@ycombinator
Copy link
Contributor Author

Travis CI is green and Jenkins CI failure is unrelated. Merging.

@ycombinator ycombinator merged commit b595220 into elastic:master Feb 4, 2020
@ycombinator ycombinator deleted the mb-ls-xp-url-fix branch February 4, 2020 16:43
@ycombinator ycombinator added v7.6.0 and removed v7.6.1 labels Feb 4, 2020
ycombinator added a commit that referenced this pull request Feb 5, 2020
* Only add vertices param to URL if it doesn't already exist

* Adding CHANGELOG entry

* Use sync.Once instead
ycombinator added a commit that referenced this pull request Feb 5, 2020
* Only add vertices param to URL if it doesn't already exist

* Adding CHANGELOG entry

* Use sync.Once instead
@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
bug Feature:Stack Monitoring Metricbeat Metricbeat module Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.6.0 v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metricbeat 7.5.2 silently stops collecting Logstash monitoring data
4 participants