Skip to content

feat: Add new grpc health check#483

Open
deedubs wants to merge 2 commits into
F1bonacc1:mainfrom
uniiverse:feature/grpc-health-checks
Open

feat: Add new grpc health check#483
deedubs wants to merge 2 commits into
F1bonacc1:mainfrom
uniiverse:feature/grpc-health-checks

Conversation

@deedubs
Copy link
Copy Markdown

@deedubs deedubs commented May 11, 2026

We are looking to use process-compose to run grpc processes that don't have http endpoints for health checking.

While we do have a workable solution in using grpcurl 1 it didn't feel like a big lift to add this natively.

This PR adds the ability to use grpc health checks following the grpc standard. It attempts to follow a similar api to that of F1bonacc1/go-health

Footnotes

  1. Example using exec.command: grpcurl -plaintext -connect-timeout 2 localhost:9990 grpc.health.v1.Health/Check

deedubs added 2 commits May 11, 2026 16:06
This PR adds the ability to use grpc health checks following the grpc standard.
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Owner

@F1bonacc1 F1bonacc1 left a comment

Choose a reason for hiding this comment

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

Hi @deedubs - thanks for putting this together, this is a real gap and the implementation is clean. Before going deeper on the details though, I want to raise one architectural question that I think changes how this should be structured.

Should the gRPC checker live in F1bonacc1/go-health (https://github.com/F1bonacc1/go-health) instead?

Looking at how the existing probes are wired:

  • http_get delegates to f1bonacc1/go-health/v2/checkers.NewHTTP - the actual checker lives upstream
  • exec is local to process-compose because it depends on command.ShellConfig, env, etc.
  • grpc is local, but the grpcChecker type has no process-compose-specific dependencies - it's a pure dial + grpc.health.v1.Health/Check implementation

go-health already ships pre-built checkers for HTTP, SQL, Redis, Mongo, Memcache, disk, reachable - gRPC fits that pattern much more naturally than it fits here. Some concrete benefits to splitting it:

  1. Dependency placement - google.golang.org/grpc is a sizable tree. Putting it in go-health means library consumers opt into it; in process-compose every user pays the cost regardless of whether they use gRPC probes.
  2. Consistency - future maintainers won't have to wonder why two of three probes delegate to go-health and one doesn't.
  3. Reusability - go-health has its own users who'd benefit.
  4. Smaller surface here - this PR shrinks to roughly the GrpcProbe config struct, validateAndSetGrpcDefaults, and a getGrpcChecker that calls checkers.NewGRPC(...) - maybe ~30 lines plus the YAML/JSON wiring.

Would you be open to splitting this into two PRs?

  1. F1bonacc1/go-health: add checkers/grpc.go (or checkers/grpc/) exposing NewGRPC(*GRPCConfig) (ICheckable, error). TLS support, connection lifecycle, etc. get designed there in the right context.
  2. F1bonacc1/process-compose: bump go-health and add the config + delegation.

Happy to give detailed feedback on the current diff (schema/docs updates, go mod tidy for the // indirect grpc dep, TLS, port validation, connection-per-probe) once we settle on the structural direction - didn't want to dive into those if the architecture itself is going to shift.

Either way, thanks again for the contribution - solving the "gRPC service with no HTTP endpoint" case natively is a clear win.

Comment thread src/health/probe.go
g.NumPort, _ = strconv.Atoi(g.Port)
}
if g.NumPort < 1 || g.NumPort > 65535 {
g.NumPort = 0
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This will fail silently with an unclear message to the user.
It looks the same as how it is handled for HTTP, but in fact, in HTTP, no port is a valid case as it falls back to the scheme default:

if h.NumPort == 0 {
	urlStr = fmt.Sprintf("%s://%s%s", h.Scheme, h.Host, h.Path)
}

Comment thread go.mod
golang.org/x/sync v0.20.0 // indirect
golang.org/x/text v0.36.0 // indirect
golang.org/x/tools v0.44.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260226221140-a57be14db171 // indirect
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is a direct dependency.
Did you run go mod tidy?

}
defer conn.Close()

client := healthpb.NewHealthClient(conn)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do you think we can preserve the connection and not recreate it on every status tick?

healthServer.SetServingStatus("my.service", healthpb.HealthCheckResponse_SERVING)
healthpb.RegisterHealthServer(srv, healthServer)

go srv.Serve(lis)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please check the err here.
Also, there is a chance the test will start running before the server serves. Consider a WaitGroup here

healthServer.SetServingStatus("", healthpb.HealthCheckResponse_NOT_SERVING)
healthpb.RegisterHealthServer(srv, healthServer)

go srv.Serve(lis)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Same comments here

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