Skip to content

Commit

Permalink
feat: enhance gRPC client with structured logging and dependency actions
Browse files Browse the repository at this point in the history 
- Added DependencyActionSync and DependencyActionSetup constants to improve dependency management.
- Refactored GrpcClient to utilize a logger interface for consistent logging across connection states and errors.
- Updated Start, Stop, and connection methods to replace direct log calls with logger methods, enhancing log context and readability.
- Simplified test cases by removing error checks on gRPC client start, ensuring cleaner test setup.
  • Loading branch information
Marvin Zhang committed Dec 23, 2024
1 parent 34455dc commit c3f4c4a
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 54 deletions.
4 changes: 4 additions & 0 deletions core/constants/dependency.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ const (
DependencyFileTypeRequirementsTxt = "requirements.txt"
DependencyFileTypePackageJson = "package.json"
)
const (
DependencyActionSync = "sync"
DependencyActionSetup = "setup"
)
45 changes: 20 additions & 25 deletions core/grpc/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"sync"
"time"

"github.com/apex/log"
"github.com/cenkalti/backoff/v4"
"github.com/crawlab-team/crawlab/core/grpc/middlewares"
"github.com/crawlab-team/crawlab/core/interfaces"
Expand All @@ -31,6 +30,7 @@ type GrpcClient struct {
once sync.Once
stopped bool
stop chan struct{}
logger interfaces.Logger

// clients
NodeClient grpc2.NodeServiceClient
Expand All @@ -45,7 +45,7 @@ type GrpcClient struct {
reconnect chan struct{}
}

func (c *GrpcClient) Start() (err error) {
func (c *GrpcClient) Start() {
c.once.Do(func() {
// initialize reconnect channel
c.reconnect = make(chan struct{})
Expand All @@ -54,23 +54,22 @@ func (c *GrpcClient) Start() (err error) {
go c.monitorState()

// connect
err = c.connect()
err := c.connect()
if err != nil {
c.logger.Fatalf("failed to connect: %v", err)
return
}

// register rpc services
c.register()
})

return err
}

func (c *GrpcClient) Stop() (err error) {
// set stopped flag
c.stopped = true
c.stop <- struct{}{}
log.Infof("[GrpcClient] stopped")
c.logger.Infof("stopped")

// skip if connection is nil
if c.conn == nil {
Expand All @@ -79,10 +78,10 @@ func (c *GrpcClient) Stop() (err error) {

// close connection
if err := c.conn.Close(); err != nil {
log.Errorf("grpc client failed to close connection: %v", err)
c.logger.Errorf("failed to close connection: %v", err)
return err
}
log.Infof("grpc client disconnected from %s", c.address)
c.logger.Infof("disconnected from %s", c.address)

return nil
}
Expand All @@ -94,11 +93,11 @@ func (c *GrpcClient) WaitForReady() {
select {
case <-ticker.C:
if c.IsReady() {
log.Debugf("grpc client ready")
c.logger.Debugf("ready")
return
}
case <-c.stop:
log.Errorf("grpc client stopped")
c.logger.Errorf("stopped")
}
}
}
Expand Down Expand Up @@ -143,15 +142,15 @@ func (c *GrpcClient) monitorState() {

if previous != current {
c.setState(current)
log.Infof("[GrpcClient] state changed from %s to %s", previous, current)
c.logger.Infof("state changed from %s to %s", previous, current)

// Trigger reconnect if connection is lost or becomes idle from ready state
if current == connectivity.TransientFailure ||
current == connectivity.Shutdown ||
(previous == connectivity.Ready && current == connectivity.Idle) {
select {
case c.reconnect <- struct{}{}:
log.Infof("[GrpcClient] triggering reconnection due to state change to %s", current)
c.logger.Infof("triggering reconnection due to state change to %s", current)
default:
}
}
Expand Down Expand Up @@ -183,9 +182,9 @@ func (c *GrpcClient) connect() (err error) {
return
case <-c.reconnect:
if !c.stopped {
log.Infof("[GrpcClient] attempting to reconnect to %s", c.address)
c.logger.Infof("attempting to reconnect to %s", c.address)
if err := c.doConnect(); err != nil {
log.Errorf("[GrpcClient] reconnection failed: %v", err)
c.logger.Errorf("reconnection failed: %v", err)
}
}
}
Expand All @@ -207,32 +206,32 @@ func (c *GrpcClient) doConnect() (err error) {
// create new client connection
c.conn, err = grpc.NewClient(c.address, opts...)
if err != nil {
log.Errorf("[GrpcClient] grpc client failed to connect to %s: %v", c.address, err)
c.logger.Errorf("failed to connect to %s: %v", c.address, err)
return err
}

// connect
log.Infof("[GrpcClient] grpc client connecting to %s", c.address)
c.logger.Infof("connecting to %s", c.address)
c.conn.Connect()

// wait for connection to be ready
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
ok := c.conn.WaitForStateChange(ctx, connectivity.Ready)
if !ok {
return fmt.Errorf("[GrpcClient] grpc client failed to connect to %s: timed out", c.address)
return fmt.Errorf("failed to connect to %s: timed out", c.address)
}

// success
log.Infof("[GrpcClient] grpc client connected to %s", c.address)
c.logger.Infof("connected to %s", c.address)

return nil
}
b := backoff.NewExponentialBackOff()
b.InitialInterval = 5 * time.Second
b.MaxElapsedTime = 10 * time.Minute
n := func(err error, duration time.Duration) {
log.Errorf("[GrpcClient] grpc client failed to connect to %s: %v, retrying in %s", c.address, err, duration)
c.logger.Errorf("failed to connect to %s: %v, retrying in %s", c.address, err, duration)
}
return backoff.RetryNotify(op, b, n)
}
Expand All @@ -242,6 +241,7 @@ func newGrpcClient() (c *GrpcClient) {
address: utils.GetGrpcAddress(),
timeout: 10 * time.Second,
stop: make(chan struct{}),
logger: utils.NewServiceLogger("GrpcClient"),
state: connectivity.Idle,
}
}
Expand All @@ -252,12 +252,7 @@ var _clientOnce sync.Once
func GetGrpcClient() *GrpcClient {
_clientOnce.Do(func() {
_client = newGrpcClient()
go func() {
err := _client.Start()
if err != nil {
log.Fatalf("[GrpcClient] failed to start: %v", err)
}
}()
go _client.Start()
})
return _client
}
42 changes: 14 additions & 28 deletions core/models/client/model_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ func TestModelService_GetById(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
res, err := clientSvc.GetById(m.Id)
Expand All @@ -83,8 +82,7 @@ func TestModelService_GetOne(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
res, err := clientSvc.GetOne(bson.M{"name": m.Name}, nil)
Expand All @@ -109,8 +107,7 @@ func TestModelService_GetMany(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
res, err := clientSvc.GetMany(bson.M{"name": m.Name}, nil)
Expand All @@ -136,8 +133,7 @@ func TestModelService_DeleteById(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.DeleteById(m.Id)
Expand All @@ -164,8 +160,7 @@ func TestModelService_DeleteOne(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.DeleteOne(bson.M{"name": m.Name})
Expand All @@ -192,8 +187,7 @@ func TestModelService_DeleteMany(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.DeleteMany(bson.M{"name": m.Name})
Expand All @@ -220,8 +214,7 @@ func TestModelService_UpdateById(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.UpdateById(m.Id, bson.M{"$set": bson.M{"name": "New Name"}})
Expand All @@ -248,8 +241,7 @@ func TestModelService_UpdateOne(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.UpdateOne(bson.M{"name": m.Name}, bson.M{"$set": bson.M{"name": "New Name"}})
Expand Down Expand Up @@ -280,8 +272,7 @@ func TestModelService_UpdateMany(t *testing.T) {
require.Nil(t, err)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
err = clientSvc.UpdateMany(bson.M{"name": "Test Name"}, bson.M{"$set": bson.M{"name": "New Name"}})
Expand All @@ -308,8 +299,7 @@ func TestModelService_ReplaceById(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
m.Name = "New Name"
Expand Down Expand Up @@ -337,8 +327,7 @@ func TestModelService_ReplaceOne(t *testing.T) {
m.SetId(id)
time.Sleep(100 * time.Millisecond)

err = client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
m.Name = "New Name"
Expand All @@ -357,8 +346,7 @@ func TestModelService_InsertOne(t *testing.T) {
go startSvr(svr)
defer stopSvr(svr)

err := client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
m := models.TestModel{
Expand All @@ -379,8 +367,7 @@ func TestModelService_InsertMany(t *testing.T) {
go startSvr(svr)
defer stopSvr(svr)

err := client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
testModels := []models.TestModel{
Expand Down Expand Up @@ -413,8 +400,7 @@ func TestModelService_Count(t *testing.T) {
}
time.Sleep(100 * time.Millisecond)

err := client2.GetGrpcClient().Start()
require.Nil(t, err)
client2.GetGrpcClient().Start()

clientSvc := client.NewModelService[models.TestModel]()
count, err := clientSvc.Count(bson.M{})
Expand Down
2 changes: 1 addition & 1 deletion frontend/packages/crawlab-ui
Submodule crawlab-ui updated 104 files

0 comments on commit c3f4c4a

Please sign in to comment.