grpc-health-probe icon indicating copy to clipboard operation
grpc-health-probe copied to clipboard

Multiple services check support

Open jvmlet opened this issue 3 years ago • 9 comments

like -service=service1,service2 Another option is to support -discoverServices flag that will use reflection API to discover services and then runs check for each discovered service.

jvmlet avatar Nov 04 '21 14:11 jvmlet

The Check(serviceName) is a standard RPC defined by gRPC. We just mirror that in this tool and so far this never came up, that's why I am inclined to wait for more priority this.

In the meantime, you can use GNU Parallel or a similar bash script equivalent to achieve the same task?

ahmetb avatar Nov 04 '21 22:11 ahmetb

I understand that grpc check supports single service check. My proposition is to loop over passed/discovered services names and call the check for each service.

jvmlet avatar Nov 05 '21 06:11 jvmlet

I am inclined to say that you can probably do that loop yourself in a little wrapper script.

It's mostly a philosophical question as to whether CLI tools should support multiple arguments instead of making the caller write that loop. Some tools like curl, wc, ... accept multiple arguments and do the same task on them.

One can also make an argument that since you control the implementation of the HealthService.Check rpc, you know which services exist in that server, so you can also loop over them. Furthermore, you can perhaps even control the concurrency around how multiple services are checked (perhaps in parallel). It is really up to you how you want to interpret the HealthCheckRequest.service field.

I'll keep this issue open, but I highly doubt we'd implement something that discovers services and loop over them.

ahmetb avatar Nov 09 '21 18:11 ahmetb

I'll keep this issue open, but I highly doubt we'd implement something that discovers services and loop over them.

No need to implement something that discovers services. Its already implemented, you just need to make standard grpc reflection call, same way you call health check. https://github.com/grpc/grpc/blob/master/src/proto/grpc/reflection/v1alpha/reflection.proto

jvmlet avatar Nov 09 '21 18:11 jvmlet

I think adding something like this requires more thought. Not all gRPC servers offer reflection (it's a minority that I'd say allows reflection API in practice), furthermore even the HealthService itself is included in the reflection. Furthermore, I am not sure why this can't be implemented in the server by leaving the service="" empty (indicating all services registered), as the server code has access to the service list (without needing reflection). That's why I am not fully convinced that this is a common scenario that people need and cannot achieve without this tool supporting it.

ahmetb avatar Nov 09 '21 19:11 ahmetb

If user passes -discoverServices then he probably understands what he is doing and he has reflection enabled. It's not hard to exclude reflection service by its name from the list of discovered services, as well as health check service. Implementing service check one by one is easier, if you have a look at default implementation of grpc health manager service in grpc-java, for example.

jvmlet avatar Nov 09 '21 20:11 jvmlet

@ahmetb wrote:

I am inclined to say that you can probably do that loop yourself in a little wrapper script.

If you're shipping your application in a shell-free image like gcr/distroless, the typical for/in/do/done shell scripts won't work -- there is literally no sh to interpret them. It'd be much, much easier if the probe binary accepted multiple service names.

So, my two cents: -service=service1,service2 would be very valuable. -discoverServices would not.

abscondment avatar Sep 07 '22 20:09 abscondment

If you're using Kubernetes, I recommend giving this feedback to the builtin grpc health probe in Kubernetes. This project is largely in maintenance mode at this point.

ahmetb avatar Sep 07 '22 21:09 ahmetb

We're not on 1.23 yet, so we're stuck with maintenance mode :)

abscondment avatar Sep 07 '22 21:09 abscondment