Skip to content

Fix initialization of Logstash service URL#17497

Merged
ycombinator merged 17 commits intoelastic:masterfrom
ycombinator:mb-ls-xp-init-bugfix
Apr 7, 2020
Merged

Fix initialization of Logstash service URL#17497
ycombinator merged 17 commits intoelastic:masterfrom
ycombinator:mb-ls-xp-init-bugfix

Conversation

@ycombinator
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator commented Apr 4, 2020

What does this PR do?

This PR fixes the initialization of the Logstash service's URI in the logstash/node_stats metricset.

Why is it important?

Prior to this fix, when the logstash-xpack Metricbeat module was enabled, users would need to start up Metricbeat after Logstash was started up in order for the logstash/node_stats metricset to report the correct data.

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
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@ycombinator ycombinator added bug needs_backport PR is waiting to be backported to other branches. Feature:Stack Monitoring Team:Services (Deprecated) Label for the former Integrations-Services team v7.6.3 v7.7.0 v7.8.0 v8.0.0 labels Apr 4, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring (Stack monitoring)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/integrations-services (Team:Services)

@ycombinator
Copy link
Copy Markdown
Contributor Author

Temporarily taking this PR out of review. I want to look into automating the testing for this bugfix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: massaging :) ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually I would cover this method with a unit test to shorten debugging time (missing query parameters, etc.).

I'm not sure if the method name is not misleading (looks like a setter), but it enables Graph API, right?

@ycombinator
Copy link
Copy Markdown
Contributor Author

@mtojek Thanks for the review. I've addressed your feedback. Mind taking another look when you get a chance, please?

@ycombinator
Copy link
Copy Markdown
Contributor Author

CI failures are unrelated. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logstash monitoring data collection requires Metricbeat to be started after Logstash

5 participants