Skip to content

Commit

Permalink
feat: improve logs
Browse files Browse the repository at this point in the history
  • Loading branch information
joanestebanr committed Nov 13, 2024
1 parent 0ed309e commit aac4ed5
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 38 deletions.
2 changes: 1 addition & 1 deletion agglayer/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ func (c CertificateHeader) String() string {
errors = c.Error.String()
}

return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: %s",
return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: [%s]",
c.Height, c.CertificateID.String(), c.NewLocalExitRoot.String(), c.Status.String(), errors)
}

Expand Down
43 changes: 25 additions & 18 deletions aggsender/aggsender.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ func (a *AggSender) sendCertificates(ctx context.Context) {
select {
case epoch := <-chEpoch:
a.log.Infof("Epoch received: %s", epoch.String())
thereArePendingCerts, err := a.checkPendingCertificatesStatus(ctx)
if err == nil && !thereArePendingCerts {
err := a.checkPendingCertificatesStatus(ctx)
if err == nil {
if _, err := a.sendCertificate(ctx); err != nil {
log.Error(err)
}
} else {
log.Warnf("Skipping epoch %s because there are pending certificates %v or error: %w",
epoch.String(), thereArePendingCerts, err)
log.Infof("Skipping epoch %s because error: %w",
epoch.String(), err)
}
case <-ctx.Done():
a.log.Info("AggSender stopped")
Expand Down Expand Up @@ -177,7 +177,7 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
}

a.saveCertificateToFile(signedCertificate)
a.log.Debugf("certificate ready to be send to AggLayer: %s", signedCertificate.String())
a.log.Infof("certificate ready to be send to AggLayer: %s", signedCertificate.String())

certificateHash, err := a.aggLayerClient.SendCertificate(signedCertificate)
if err != nil {
Expand Down Expand Up @@ -489,45 +489,52 @@ func (a *AggSender) signCertificate(certificate *agglayer.Certificate) (*agglaye
// It returns:
// bool -> if there are pending certificates
// error -> if there was an error
func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) (bool, error) {
func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) error {
pendingCertificates, err := a.storage.GetCertificatesByStatus(nonSettledStatuses)
if err != nil {
err = fmt.Errorf("error getting pending certificates: %w", err)
a.log.Error(err)
return true, err
return err
}
thereArePendingCertificates := false
logErrMsg := ""
a.log.Debugf("checkPendingCertificatesStatus num of pendingCertificates: %d", len(pendingCertificates))
for _, certificate := range pendingCertificates {
certificateHeader, err := a.aggLayerClient.GetCertificateHeader(certificate.CertificateID)
if err != nil {
err = fmt.Errorf("error getting certificate header of %d/%s from agglayer: %w",
certificate.Height, certificate.String(), err)
a.log.Error(err)
return true, err
return err
}
if slices.Contains(nonSettledStatuses, certificateHeader.Status) {
thereArePendingCertificates = true
}
a.log.Debugf("aggLayerClient.GetCertificateHeader status [%s] of certificate %s ",
elapsedTime := time.Now().UTC().Sub(time.UnixMilli(certificate.CreatedAt))
a.log.Debugf("aggLayerClient.GetCertificateHeader status [%s] of certificate %s elapsed_time:%s",
certificateHeader.Status,
certificateHeader.String())
certificateHeader.String(),

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
elapsedTime)

if certificateHeader.Status != certificate.Status {
a.log.Infof("certificate %s changed status from [%s] to [%s]",
certificateHeader.String(), certificate.Status, certificateHeader.Status)
a.log.Infof("certificate %s changed status from [%s] to [%s] elapsed_time: %s",
certificateHeader.String(), certificate.Status, certificateHeader.Status, elapsedTime)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.

certificate.Status = certificateHeader.Status
certificate.UpdatedAt = time.Now().UTC().UnixMilli()

if err := a.storage.UpdateCertificateStatus(ctx, *certificate); err != nil {
err = fmt.Errorf("error updating certificate %s status in storage: %w", certificateHeader.String(), err)
a.log.Error(err)
return true, err
return err
}
}
if slices.Contains(nonSettledStatuses, certificateHeader.Status) {

Check failure on line 528 in aggsender/aggsender.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary leading newline (whitespace)

logErrMsg += fmt.Sprintf("certificate %s is still pending, elapsed_time:%s ", certificateHeader.String(), elapsedTime)
}
}
if logErrMsg != "" {
a.log.Infof(logErrMsg)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
return fmt.Errorf(logErrMsg)
}
return thereArePendingCertificates, nil
return nil
}

// shouldSendCertificate checks if a certificate should be sent at given time
Expand Down
31 changes: 13 additions & 18 deletions aggsender/aggsender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,16 +751,15 @@ func generateTestProof(t *testing.T) treeTypes.Proof {

func TestCheckIfCertificatesAreSettled(t *testing.T) {
tests := []struct {
name string
pendingCertificates []*aggsendertypes.CertificateInfo
certificateHeaders map[common.Hash]*agglayer.CertificateHeader
getFromDBError error
clientError error
updateDBError error
expectedErrorLogMessages []string
expectedInfoMessages []string
expectedThereArePendingCerts bool
expectedError bool
name string
pendingCertificates []*aggsendertypes.CertificateInfo
certificateHeaders map[common.Hash]*agglayer.CertificateHeader
getFromDBError error
clientError error
updateDBError error
expectedErrorLogMessages []string
expectedInfoMessages []string
expectedError bool
}{
{
name: "All certificates settled - update successful",
Expand Down Expand Up @@ -796,8 +795,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) {
expectedErrorLogMessages: []string{
"error getting pending certificates: %w",
},
expectedThereArePendingCerts: true,
expectedError: true,
expectedError: true,
},
{
name: "Error getting certificate header",
Expand All @@ -811,8 +809,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) {
expectedErrorLogMessages: []string{
"error getting header of certificate %s with height: %d from agglayer: %w",
},
expectedThereArePendingCerts: true,
expectedError: true,
expectedError: true,
},
{
name: "Error updating certificate status",
Expand All @@ -829,8 +826,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) {
expectedInfoMessages: []string{
"certificate %s changed status to %s",
},
expectedThereArePendingCerts: true,
expectedError: true,
expectedError: true,
},
}

Expand Down Expand Up @@ -864,8 +860,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) {
}

ctx := context.TODO()
thereArePendingCerts, err := aggSender.checkPendingCertificatesStatus(ctx)
require.Equal(t, tt.expectedThereArePendingCerts, thereArePendingCerts)
err := aggSender.checkPendingCertificatesStatus(ctx)
require.Equal(t, tt.expectedError, err != nil)
mockAggLayerClient.AssertExpectations(t)
mockStorage.AssertExpectations(t)
Expand Down
2 changes: 1 addition & 1 deletion config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,5 +343,5 @@ URLRPCL2="{{L2URL}}"
CheckSettledInterval = "2s"
BlockFinality = "LatestBlock"
EpochNotificationPercentage = 50
SaveCertificatesToFiles = false
SaveCertificatesToFilesPath = ""
`

0 comments on commit aac4ed5

Please sign in to comment.