grpchealth-go icon indicating copy to clipboard operation
grpchealth-go copied to clipboard

Feature Request: support empty GET request.

Open anzboi opened this issue 2 years ago • 2 comments
trafficstars

grpchealth as is provides a pretty convenient up check that could be used for checking whether a server is running. This is pretty useful for kube or docker compose.

Right now, the "minimal" request needed looks like...

$ curl -X POST http://HOSTNAME/grpc.health.v1.Health/Check -d "{}" -H "Content-Type: application:json"
# 200 OK

It would be nice if minimality could be brought down to...

$ curl -X GET http://HOSTNAME/grpc.health.v1.Health/Check

NOTE: Setting -X GET causes 405 Method Not Allowed Removing the body and content-type causes 415 Unsupported Media Type

anzboi avatar Nov 27 '22 22:11 anzboi

Totally understand wanting health checking to be as simple as possible - shorter's better! Even without built-in GET support, I think you can shorten your k8s and Docker compose configurations a fair bit.

  • Kuberbetes supports gRPC health checks natively - you don't need to write a curl command at all!
  • With recent cURL releases (anything more recent than version 7.82.0), you can shorten the command you have above to curl --json '{}' http://HOSTNAME/grpc.health.v1.Health/Check. (Note that this command is using the Connect RPC protocol, so it won't work with grpc-go, grpc-java, etc. servers.)
  • The cURL commands you're using are only checking that the HTTP server is up - they're not checking the health of any specific protobuf service. If that's all you care about and you'd prefer to health check with cURL, you don't actually need any of the functionality of this package. Why not simplify and use a standard GET /health instead?
func ok(_ http.ResponseWriter, _ *http.Request) {}

func main() {
  mux := http.NewServeMux()
  // mount any other RPC handlers here, and then...
  mux.Handle("/health", http.HandlerFunc(ok))
  http.ListenAndServe(":8080", mux)
}

akshayjshah avatar Nov 29 '22 07:11 akshayjshah

Checked the code and found that if we want to be compatible with http GET, then the changes are significant.

I agree with @akshayjshah that you can make simplifications in the application code. And the connect-go core lib is still simple.

solarhell avatar May 26 '23 09:05 solarhell

It would be nice to be able to hit the Check endpoint in the browser and it just work.

jamesrom avatar Sep 20 '24 04:09 jamesrom

This repo's purpose is to provide an implementation of the grpc.health.v1.Health service, mainly for use with servers that use the connectrpc.com/connect module. This is an RPC endpoint, and the definition of the RPC is provided and maintained by the gRPC organization. It is not intended to be a general HTTP health-check endpoint.

Wrapping a grpchealth.Checker in a simple HTTP GET endpoint is already trivial to do. If that's what you need, you could do something like this in your code:

// HealthEndpoint returns a handler that responds with 200 if the requested
// service is okay and "503 Service Unavailable" otherwise. The service is
// indicated via "service" query parameter. If it is absent, the checker is
// queried with an empty service name. If the checker returns an error
// instead of a status, the result will be "500 Internal Server Error".
func HealthEndpoint(checker grpchealth.Checker) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		resp, err := checker.Check(r.Context(), &grpchealth.CheckRequest{
			Service: r.URL.Query().Get("service"),
		})
		if err != nil {
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}
		if resp.Status != grpchealth.StatusServing {
			w.WriteHeader(http.StatusServiceUnavailable)
		} // else, 200 OK status is implied
	})
}

But, given the purpose of this repo, that is not really in scope for exported API provided by this module.

jhump avatar Sep 20 '24 18:09 jhump