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

Validate exporter type is HTTP for HTTP exporter #49992

Merged

Conversation

jasontedor
Copy link
Member

Today the HTTP exporter settings without the exporter type having been configured to HTTP. When it is time to initialize the exporter, we can blow up. Since this initialization happens on the cluster state applier thread, it is quite problematic that we do not reject settings updates where the type is not configured to HTTP, but there are HTTP exporter settings configured. This commit addresses this by validating that the exporter type is not only set, but is set to HTTP.

Relates #47246
Relates #47711
Relates #49942

Today the HTTP exporter settings without the exporter type having been
configured to HTTP. When it is time to initialize the exporter, we can
blow up. Since this initialization happens on the cluster state applier
thread, it is quite problematic that we do not reject settings updates
where the type is not configured to HTTP, but there are HTTP exporter
settings configured. This commit addresses this by validating that the
exporter type is not only set, but is set to HTTP.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Monitoring)

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

LGTM pending resolution of the two failing ITs.

@jasontedor
Copy link
Member Author

@elasticmachine update branch

@jasontedor jasontedor merged commit 4f212dd into elastic:master Dec 13, 2019
jasontedor added a commit that referenced this pull request Dec 13, 2019
Today the HTTP exporter settings without the exporter type having been
configured to HTTP. When it is time to initialize the exporter, we can
blow up. Since this initialization happens on the cluster state applier
thread, it is quite problematic that we do not reject settings updates
where the type is not configured to HTTP, but there are HTTP exporter
settings configured. This commit addresses this by validating that the
exporter type is not only set, but is set to HTTP.
@jasontedor jasontedor deleted the http-exporter-type-http-dependency branch December 13, 2019 01:03
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
Today the HTTP exporter settings without the exporter type having been
configured to HTTP. When it is time to initialize the exporter, we can
blow up. Since this initialization happens on the cluster state applier
thread, it is quite problematic that we do not reject settings updates
where the type is not configured to HTTP, but there are HTTP exporter
settings configured. This commit addresses this by validating that the
exporter type is not only set, but is set to HTTP.
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.

4 participants