Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/services: health Checker cleanup #10670

Merged
merged 2 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
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
35 changes: 10 additions & 25 deletions core/services/chainlink/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func NewApplication(opts ApplicationOpts) (Application, error) {
restrictedHTTPClient := opts.RestrictedHTTPClient
unrestrictedHTTPClient := opts.UnrestrictedHTTPClient

// LOOPs can be be created as options, in the case of LOOP relayers, or
// LOOPs can be created as options, in the case of LOOP relayers, or
// as OCR2 job implementations, in the case of Median today.
// We will have a non-nil registry here in LOOP relayers are being used, otherwise
// we need to initialize in case we serve OCR2 LOOPs
Expand Down Expand Up @@ -217,8 +217,6 @@ func NewApplication(opts ApplicationOpts) (Application, error) {
globalLogger.Info("Nurse service (automatic pprof profiling) is disabled")
}

healthChecker := services.NewChecker()

telemetryIngressClient := synchronization.TelemetryIngressClient(&synchronization.NoopTelemetryIngressClient{})
telemetryIngressBatchClient := synchronization.TelemetryIngressBatchClient(&synchronization.NoopTelemetryIngressBatchClient{})
monitoringEndpointGen := telemetry.MonitoringEndpointGenerator(&telemetry.NoopAgent{})
Expand Down Expand Up @@ -443,7 +441,14 @@ func NewApplication(opts ApplicationOpts) (Application, error) {
feedsService = &feeds.NullService{}
}

app := &ChainlinkApplication{
healthChecker := services.NewChecker()
for _, s := range srvcs {
if err := healthChecker.Register(s); err != nil {
Comment on lines +444 to +446
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Relocated and simplified - no need to cast.

return nil, err
}
}

return &ChainlinkApplication{
relayers: opts.RelayerChainInteroperators,
EventBroadcaster: eventBroadcaster,
jobORM: jobORM,
Expand Down Expand Up @@ -472,27 +477,7 @@ func NewApplication(opts ApplicationOpts) (Application, error) {

// NOTE: Can keep things clean by putting more things in srvcs instead of manually start/closing
srvcs: srvcs,
}

for _, service := range app.srvcs {
checkable := service.(services.Checkable)
if err := app.HealthChecker.Register(service.Name(), checkable); err != nil {
return nil, err
}
}

// To avoid subscribing chain services twice, we only subscribe them if OCR2 is not enabled.
// If it's enabled, they are going to be registered with relayers by default.
if !cfg.OCR2().Enabled() {
for _, service := range app.relayers.Services() {
checkable := service.(services.Checkable)
if err := app.HealthChecker.Register(service.Name(), checkable); err != nil {
return nil, err
}
}
}
Comment on lines -484 to -493
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This exception is no longer required since the relayer services are now added unconditionally.


return app, nil
}, nil
}

func (app *ChainlinkApplication) SetLogLevel(lvl zapcore.Level) error {
Expand Down
2 changes: 2 additions & 0 deletions core/services/checkable.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ type Checkable interface {
// HealthReport returns a full health report of the callee including it's dependencies.
// key is the dep name, value is nil if healthy, or error message otherwise.
HealthReport() map[string]error
// Name returns the fully qualified name of the component. Usually the logger name.
Name() string
}
7 changes: 4 additions & 3 deletions core/services/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type (
// Checker provides a service which can be probed for system health.
Checker interface {
// Register a service for health checks.
Register(name string, service Checkable) error
Register(service Checkable) error
// Unregister a service.
Unregister(name string) error
// IsReady returns the current readiness of the system.
Expand Down Expand Up @@ -176,8 +176,9 @@ func (c *checker) update() {
uptimeSeconds.Add(interval.Seconds())
}

func (c *checker) Register(name string, service Checkable) error {
if service == nil || name == "" {
func (c *checker) Register(service Checkable) error {
Copy link
Contributor Author

@jmank88 jmank88 Sep 17, 2023

Choose a reason for hiding this comment

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

Outside of tests, we only check ServiceCtxs, so every Checkable implementation already had a Name() method. Now that method is defined on Checkable instead of ServiceCtx.

name := service.Name()
if name == "" {
return errors.Errorf("misconfigured check %#v for %v", name, service)
}

Expand Down
7 changes: 4 additions & 3 deletions core/services/health_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package services_test

import (
"fmt"
"net/http"
"testing"
"time"
Expand All @@ -22,6 +21,8 @@ type boolCheck struct {
healthy bool
}

func (b boolCheck) Name() string { return b.name }

func (b boolCheck) Ready() error {
if b.healthy {
return nil
Expand Down Expand Up @@ -57,8 +58,8 @@ func TestCheck(t *testing.T) {
}},
} {
c := services.NewChecker()
for i, check := range test.checks {
require.NoError(t, c.Register(fmt.Sprint(i), check))
for _, check := range test.checks {
require.NoError(t, c.Register(check))
}

require.NoError(t, c.Start())
Expand Down
10 changes: 5 additions & 5 deletions core/services/mocks/checker.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions core/services/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,5 @@ type ServiceCtx interface {
// again, you need to build a new Service to do so.
Close() error

// Name returns the fully qualified name of the service
Name() string

Checkable
}
2 changes: 1 addition & 1 deletion plugins/cmd/chainlink-median/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func main() {
p := median.NewPlugin(s.Logger)
defer s.Logger.ErrorIfFn(p.Close, "Failed to close")

s.MustRegister(p.Name(), p)
s.MustRegister(p)

stop := make(chan struct{})
defer close(stop)
Expand Down
2 changes: 1 addition & 1 deletion plugins/cmd/chainlink-solana/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func main() {
p := &pluginRelayer{Base: plugins.Base{Logger: s.Logger}}
defer s.Logger.ErrorIfFn(p.Close, "Failed to close")

s.MustRegister(p.Name(), p)
s.MustRegister(p)

stopCh := make(chan struct{})
defer close(stopCh)
Expand Down
2 changes: 1 addition & 1 deletion plugins/cmd/chainlink-starknet/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func main() {
p := &pluginRelayer{Base: plugins.Base{Logger: s.Logger}}
defer s.Logger.ErrorIfFn(p.Close, "Failed to close")

s.MustRegister(p.Name(), p)
s.MustRegister(p)

stopCh := make(chan struct{})
defer close(stopCh)
Expand Down
7 changes: 3 additions & 4 deletions plugins/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ type Server struct {
}

// MustRegister registers the Checkable with services.Checker, or exits upon failure.
func (s *Server) MustRegister(name string, c services.Checkable) {
err := s.Register(name, c)
if err != nil {
s.Logger.Fatalf("Failed to register %s with health checker: %v", name, err)
func (s *Server) MustRegister(c services.Checkable) {
if err := s.Register(c); err != nil {
s.Logger.Fatalf("Failed to register %s with health checker: %v", c.Name(), err)
}
}

Expand Down
Loading