Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions core/services/arbiter/arbiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"time"

"google.golang.org/grpc"
"google.golang.org/grpc/health"
healthgrpc "google.golang.org/grpc/health/grpc_health_v1"
"google.golang.org/grpc/reflection"

"github.com/smartcontractkit/chainlink-common/pkg/services"
ringpb "github.com/smartcontractkit/chainlink-protos/ring/go"
Expand All @@ -27,6 +30,7 @@ type arbiter struct {
services.StateMachine

grpcServer *grpc.Server
healthServer *health.Server
grpcHandler *GRPCServer
ringArbiterHandler *RingArbiterHandler
state *State
Expand Down Expand Up @@ -70,8 +74,16 @@ func New(
ringpb.RegisterArbiterServer(grpcServer, grpcHandler)
ringpb.RegisterArbiterScalerServer(grpcServer, ringArbiterHandler)

// Register gRPC health check service
healthServer := health.NewServer()
healthgrpc.RegisterHealthServer(grpcServer, healthServer)

// Register gRPC server reflection (enables grpcurl and other tools)
reflection.Register(grpcServer)
Comment on lines +81 to +82
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +77 to +82
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

return &arbiter{
grpcServer: grpcServer,
healthServer: healthServer,
grpcHandler: grpcHandler,
ringArbiterHandler: ringArbiterHandler,
state: state,
Expand Down Expand Up @@ -103,6 +115,9 @@ func (a *arbiter) Start(ctx context.Context) error {
"grpcAddr", a.grpcAddr,
)

// Mark gRPC health as serving
a.healthServer.SetServingStatus("", healthgrpc.HealthCheckResponse_SERVING)

Comment on lines +118 to +120
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
return nil
})
}
Expand Down Expand Up @@ -145,6 +160,9 @@ func (a *arbiter) Close() error {
// Signal stop
close(a.stopCh)

// Mark health as not serving before stopping
a.healthServer.SetServingStatus("", healthgrpc.HealthCheckResponse_NOT_SERVING)

// Graceful shutdown of gRPC server
a.grpcServer.GracefulStop()
a.lggr.Debug("gRPC server stopped gracefully")
Expand Down
Loading