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

Fix issue that wrong config panics http metricsets #6205

Merged
merged 1 commit into from
Jan 31, 2018

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Jan 28, 2018

When setting an invalide http config in a metricbeat module / metricset it could happen that the metricset paniced. The reason is that the error when unpacking the config was not properly returned and handled which lead to an empty http instance.

This PR changes the behaviour to return the errors which can then be handled by the metricsets. The issue also affects all metricsets which were based on the prometheus helper.

Closes #6192

@ruflin ruflin changed the title Fix issue that wrong config panice http metricsets Fix issue that wrong config panics http metricsets Jan 28, 2018
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

LGTM, should we backport this?

@ruflin
Copy link
Member Author

ruflin commented Jan 31, 2018

@exekias I don't think we should backport this to 6.2 as the change is too big and the "bug" only happens if there is an invalid configuration. It should definitively be in 6.x which will happen automatically with the next bulk merge.

When setting an invalide http config in a metricbeat module / metricset it could happen that the metricset paniced. The reason is that the error when unpacking the config was not properly returned and handled which lead to an empty http instance.

This PR changes the behaviour to return the errors which can then be handled by the metricsets. The issue also affects all metricsets which were based on the prometheus helper.

Closes elastic#6192
@exekias
Copy link
Contributor

exekias commented Jan 31, 2018

WFG

@exekias exekias merged commit f01e896 into elastic:master Jan 31, 2018
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.

2 participants