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

Don't return a 400 for / #15

Merged
merged 1 commit into from
Feb 19, 2017
Merged

Don't return a 400 for / #15

merged 1 commit into from
Feb 19, 2017

Conversation

joshuaspence
Copy link
Contributor

Currently this exporter doesn't behave like other Prometheus exporters, which return a 200 for a GET HTTP/1.1 / request. It seems that this was intentional so as to not leak data unintentionally, but I don't personally think that this is a valid reason to deviate from the convention. Specifically, if you don't intend for the Prometheus Varnish exporter to be accessed externally then you should restrict access using security groups, a firewall or iptables.

In our use case, we are using / as a health check for the exporter itself. This health check is currently failing because the Varnish exporter returns a 400 Bad Request rather than 200 OK.

Currently this exporter doesn't behave like other Prometheus exporters, which return a 200 for a `GET HTTP/1.1 /` request. It seems that this was intentional so as to not leak data unintentionally, but I don't personally think that this is a valid reason to deviate from the convention. Specifically, if you don't intend for the Prometheus Varnish exporter to be accessed externally then you should restrict access using security groups, a firewall or `iptables`.

In our use case, we are using `/` as a health check for the exporter itself. This health check is currently failing because the Varnish exporter returns a `400 Bad Request` rather than `200 OK`.
@jonnenauha jonnenauha merged commit e15b11e into jonnenauha:master Feb 19, 2017
@jonnenauha
Copy link
Owner

Yes, these are valid points. Sorry the merge took until the weekend. I will make 1.3.1 release.

@jonnenauha
Copy link
Owner

I will also built it with 1.8.0 as its the latest release at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants