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

Add health endpoint #752

Merged
merged 1 commit into from
Mar 16, 2021
Merged

Add health endpoint #752

merged 1 commit into from
Mar 16, 2021

Conversation

adebasi
Copy link
Contributor

@adebasi adebasi commented Feb 25, 2021

The existing endpoints expose potentially sensitive data. So they cannot be used without authentication to check from the internet if the services is running.

@delete-merged-branch delete-merged-branch bot deleted the add-health-endpoint branch February 25, 2021 11:10
@SuperQ
Copy link
Member

SuperQ commented Feb 25, 2021

We should add this to the exporter toolkit as part of the landing page boilerplate functions.

Cc @roidelapluie

@roidelapluie
Copy link
Member

roidelapluie commented Feb 25, 2021

Yes, it would go to exporter-toolkit. Also note that Prometheus exposes /-/healthy.

@adebasi
Copy link
Contributor Author

adebasi commented Feb 25, 2021

Hey folks, thanks for your quick reply. Unfortunately, I don't fully understand what you mean.

Would you like to move the endpoint definition to the exporter-toolkit? And how would you like to combine it with the landing page boilerplate? The boiler plate exposes potentially sensitive data because it lists all probes.
@roidelapluie Is your comment about the naming/path for the endpoint?

Copy link
Member

@roidelapluie roidelapluie left a comment

Choose a reason for hiding this comment

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

Let's move forward here for now :)

we were refering to the exporter-toolkit repository but it is unclear when we will have the landing page here.

main.go Outdated
@@ -309,6 +309,10 @@ func run() int {
}
})
http.Handle(path.Join(*routePrefix, "/metrics"), promhttp.Handler())
http.HandleFunc(path.Join(*routePrefix, "/health"), func(w http.ResponseWriter, r *http.Request) {
Copy link
Member

Choose a reason for hiding this comment

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

Please use /-/healthy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

main.go Outdated
@@ -309,6 +309,10 @@ func run() int {
}
})
http.Handle(path.Join(*routePrefix, "/metrics"), promhttp.Handler())
http.HandleFunc(path.Join(*routePrefix, "/health"), func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Write([]byte("OK"))
Copy link
Member

Choose a reason for hiding this comment

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

Can we use "Healthy" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

The root endpoint exposes too much data. So it cannot be used as public
health check endpoint.

Signed-off-by: Mitja Adebahr <[email protected]>
Copy link
Member

@electron0zero electron0zero left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@adebasi
Copy link
Contributor Author

adebasi commented Mar 15, 2021

@SuperQ or @roidelapluie, can you please merge the PR?

@mem mem merged commit b8d3883 into prometheus:master Mar 16, 2021
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.

5 participants