Skip to content

Only initialize service URL once#16044

Merged
ycombinator merged 3 commits intoelastic:masterfrom
ycombinator:mb-ls-xp-url-fix
Feb 4, 2020
Merged

Only initialize service URL once#16044
ycombinator merged 3 commits intoelastic:masterfrom
ycombinator:mb-ls-xp-url-fix

Conversation

@ycombinator
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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
Copy Markdown
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