feat: add gRPC health check and reflection support to arbiter#21410
feat: add gRPC health check and reflection support to arbiter#21410scheibinger wants to merge 1 commit intodevelopfrom
Conversation
|
✅ No conflicts with other open PRs targeting |
|
I see you updated files related to
|
|
There was a problem hiding this comment.
Pull request overview
Risk Rating: LOW. Adds standard gRPC health checking and server reflection to the Arbiter gRPC server to support Kubernetes probes and tooling like grpcurl.
Changes:
- Register
grpc.health.v1.Healthon the Arbiter gRPC server and track serving status during Start/Close. - Register gRPC server reflection for service discovery/introspection.
- Add a
healthServerfield to the arbiter service to manage status.
| // Register gRPC server reflection (enables grpcurl and other tools) | ||
| reflection.Register(grpcServer) |
There was a problem hiding this comment.
reflection.Register(grpcServer) enables server reflection unconditionally. Since this repo otherwise appears not to enable gRPC reflection anywhere else, consider gating this behind a config/env flag (or restricting it to non-production) to avoid exposing service/method metadata on any reachable gRPC port.
| // Mark gRPC health as serving | ||
| a.healthServer.SetServingStatus("", healthgrpc.HealthCheckResponse_SERVING) | ||
|
|
There was a problem hiding this comment.
Health status is first set to SERVING after the gRPC Serve goroutine is launched. During the small window before SetServingStatus is called, the health service will typically respond with SERVICE_UNKNOWN, which can cause flaky readiness/health probes right after startup. Consider setting an explicit NOT_SERVING status before starting the gRPC server (e.g., in New or at the beginning of Start) and only switching to SERVING once startup is complete.
| // Register gRPC health check service | ||
| healthServer := health.NewServer() | ||
| healthgrpc.RegisterHealthServer(grpcServer, healthServer) | ||
|
|
||
| // Register gRPC server reflection (enables grpcurl and other tools) | ||
| reflection.Register(grpcServer) |
There was a problem hiding this comment.
This change introduces new externally visible gRPC behaviors (health check + reflection registration) but there are existing tests for the arbiter service lifecycle in this package. Please add unit/integration tests that (1) call grpc.health.v1.Health/Check and assert SERVING/NOT_SERVING across Start/Close, and (2) verify reflection can list the registered services.
Add gRPC Health Check and Server Reflection to Arbiter Service
Problem
The arbiter gRPC server did not support the standard
grpc.health.v1.Healthservice or server reflection, causing health check probes to fail:Neither the health check service nor reflection were registered on the gRPC server — they simply weren't implemented.
Solution
Registered the standard gRPC health check service and server reflection on the arbiter's gRPC server.
Changes in
core/services/arbiter/arbiter.go:google.golang.org/grpc/health,google.golang.org/grpc/health/grpc_health_v1, andgoogle.golang.org/grpc/reflectionhealthServer *health.Serverfield to thearbiterstructgrpc.health.v1.Healthservice viahealthgrpc.RegisterHealthServer()— enables standard gRPC health check probes (used by Kubernetes,grpcurl, load balancers, etc.)reflection.Register()— enables service discovery tools likegrpcurlto introspect available servicesSERVINGinStart()after the service is fully initializedNOT_SERVINGinClose()before initiating graceful shutdown, ensuring in-flight health probes get an accurate response during drainingRisk: LOW
Additive change — registers two well-known gRPC services on an existing server. No changes to business logic, existing RPCs, or the arbiter's core behavior. All existing tests continue to pass.