Skip to content

Commit

Permalink
use zap in proposals package (#6146)
Browse files Browse the repository at this point in the history
## Motivation

Part of the effort to migrate from log to zap.
  • Loading branch information
poszu committed Jul 18, 2024
1 parent d9e3655 commit 5c00c97
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 95 deletions.
20 changes: 10 additions & 10 deletions beacon/beacon.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (pd *ProtocolDriver) UpdateBeacon(epoch types.EpochID, beacon types.Beacon)
pd.beacons[epoch] = beacon
pd.logger.Info("using fallback beacon",
zap.Uint32("epoch", epoch.Uint32()),
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
)
pd.onResult(epoch, beacon)
return nil
Expand Down Expand Up @@ -410,7 +410,7 @@ func (pd *ProtocolDriver) recordBeacon(
pd.logger.Debug("added beacon from ballot",
zap.Uint32("epoch", epochID.Uint32()),
zap.Stringer("ballotID", ballot.ID()),
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
zap.Stringer("weight_per", weightPer),
zap.Int("num_eligibility", numEligibility),
zap.Stringer("weight", ballotWeight),
Expand All @@ -433,7 +433,7 @@ func (pd *ProtocolDriver) recordBeacon(
pd.logger.Debug("added beacon from ballot",
zap.Uint32("epoch", epochID.Uint32()),
zap.Stringer("ballotID", ballot.ID()),
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
zap.Stringer("weight_per", weightPer),
zap.Int("num_eligibility", numEligibility),
zap.Stringer("weight", ballotWeight),
Expand Down Expand Up @@ -469,7 +469,7 @@ func (pd *ProtocolDriver) findMajorityBeacon(epoch types.EpochID) types.Beacon {
for beacon, bw := range epochBeacons {
if bw.totalWeight.GreaterThan(majorityWeight) {
logger.Info("beacon determined for epoch",
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
zap.Int("total_weight_unit", totalEligibility),
zap.Stringer("total_weight", totalWeight),
zap.Stringer("beacon_weight", bw.totalWeight),
Expand All @@ -482,7 +482,7 @@ func (pd *ProtocolDriver) findMajorityBeacon(epoch types.EpochID) types.Beacon {
}
}
logger.Warn("beacon determined for epoch by plurality, not majority",
log.ZShortStringer("beacon", bPlurality),
zap.Stringer("beacon", bPlurality),
zap.Int("total_weight_unit", totalEligibility),
zap.Stringer("total_weight", totalWeight),
zap.Stringer("beacon_weight", maxWeight),
Expand Down Expand Up @@ -539,7 +539,7 @@ func (pd *ProtocolDriver) setBeacon(targetEpoch types.EpochID, beacon types.Beac
if err := beacons.Add(pd.cdb, targetEpoch, beacon); err != nil && !errors.Is(err, sql.ErrObjectExists) {
pd.logger.Error("failed to persist beacon",
zap.Uint32("target_epoch", targetEpoch.Uint32()),
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
zap.Error(err),
)
return fmt.Errorf("persist beacon: %w", err)
Expand Down Expand Up @@ -837,7 +837,7 @@ func (pd *ProtocolDriver) runProtocol(ctx context.Context, epoch types.EpochID,
return
}

logger.Info("beacon set for epoch", log.ZShortStringer("beacon", beacon))
logger.Info("beacon set for epoch", zap.Stringer("beacon", beacon))
}

func calcBeacon(logger *zap.Logger, set proposalSet) types.Beacon {
Expand All @@ -847,7 +847,7 @@ func calcBeacon(logger *zap.Logger, set proposalSet) types.Beacon {
// to the same size as the proposal
beacon := types.BytesToBeacon(allProposals.hash().Bytes())
logger.Info("calculated beacon",
log.ZShortStringer("beacon", beacon),
zap.Stringer("beacon", beacon),
zap.Int("num_hashes", len(allProposals)),
zap.Array("proposals", allProposals),
)
Expand Down Expand Up @@ -1256,7 +1256,7 @@ func (pd *ProtocolDriver) gatherMetricsData() ([]*metrics.BeaconStats, *metrics.
for beacon, stats := range epochBeacons {
stat := &metrics.BeaconStats{
Epoch: epoch,
Beacon: beacon.ShortString(),
Beacon: beacon.String(),
Weight: stats.totalWeight,
WeightUnit: stats.numEligibility,
}
Expand All @@ -1268,7 +1268,7 @@ func (pd *ProtocolDriver) gatherMetricsData() ([]*metrics.BeaconStats, *metrics.
if beacon, ok := pd.beacons[epoch+1]; ok {
calculated = &metrics.BeaconStats{
Epoch: epoch + 1,
Beacon: beacon.ShortString(),
Beacon: beacon.String(),
}
}

Expand Down
8 changes: 4 additions & 4 deletions beacon/beacon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,8 @@ func TestBeaconWithMetrics(t *testing.T) {
spacemesh_beacons_beacon_observed_total{beacon="%s",epoch="%d"} %d
spacemesh_beacons_beacon_observed_total{beacon="%s",epoch="%d"} %d
`,
beacon1.ShortString(), thisEpoch, count,
beacon2.ShortString(), thisEpoch, count,
beacon1.String(), thisEpoch, count,
beacon2.String(), thisEpoch, count,
)
err := testutil.GatherAndCompare(
prometheus.DefaultGatherer,
Expand All @@ -466,8 +466,8 @@ func TestBeaconWithMetrics(t *testing.T) {
spacemesh_beacons_beacon_observed_weight{beacon="%s",epoch="%d"} %d
spacemesh_beacons_beacon_observed_weight{beacon="%s",epoch="%d"} %d
`,
beacon1.ShortString(), thisEpoch, weight,
beacon2.ShortString(), thisEpoch, weight,
beacon1.String(), thisEpoch, weight,
beacon2.String(), thisEpoch, weight,
)
err = testutil.GatherAndCompare(
prometheus.DefaultGatherer,
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestMain(m *testing.M) {

func checkUpdate1(t *testing.T, got *bootstrap.VerifiedUpdate) {
require.EqualValues(t, 1, got.Data.Epoch)
require.EqualValues(t, "0x6fe7c971", got.Data.Beacon.String())
require.EqualValues(t, "6fe7c971", got.Data.Beacon.String())
require.Len(t, got.Data.ActiveSet, 2)
require.Equal(
t,
Expand Down Expand Up @@ -124,7 +124,7 @@ func checkUpdate2(t *testing.T, got *bootstrap.VerifiedUpdate) {

func checkUpdate3(t *testing.T, got *bootstrap.VerifiedUpdate) {
require.EqualValues(t, 3, got.Data.Epoch)
require.EqualValues(t, "0xf70cf90b", got.Data.Beacon.String())
require.EqualValues(t, "f70cf90b", got.Data.Beacon.String())
require.Nil(t, got.Data.ActiveSet)
}

Expand Down
7 changes: 1 addition & 6 deletions common/types/ballot.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func (b *Ballot) MarshalLogObject(encoder log.ObjectEncoder) error {
encoder.AddString("atx_id", b.AtxID.String())
encoder.AddString("ref_ballot", b.RefBallot.String())
encoder.AddString("active set hash", activeHash.ShortString())
encoder.AddString("beacon", beacon.ShortString())
encoder.AddString("beacon", beacon.String())
encoder.AddObject("votes", &b.Votes)
return nil
}
Expand Down Expand Up @@ -379,11 +379,6 @@ func (id BallotID) AsHash32() Hash32 {
return Hash20(id).ToHash32()
}

// Field returns a log field. Implements the LoggableField interface.
func (id BallotID) Field() log.Field {
return log.String("ballot_id", id.String())
}

// Compare returns true if other (the given BallotID) is less than this BallotID, by lexicographic comparison.
func (id BallotID) Compare(other BallotID) bool {
return bytes.Compare(id.Bytes(), other.Bytes()) < 0
Expand Down
16 changes: 3 additions & 13 deletions common/types/beacon.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package types

import (
"fmt"
"encoding/hex"

"github.com/spacemeshos/go-spacemesh/common/util"
"github.com/spacemeshos/go-spacemesh/log"
Expand All @@ -20,19 +20,9 @@ type Beacon [BeaconSize]byte
// EmptyBeacon is a canonical empty Beacon.
var EmptyBeacon = Beacon{}

// Hex converts a hash to a hex string.
func (b Beacon) Hex() string { return util.Encode(b[:]) }

// String implements the stringer interface and is used also by the logger when
// doing full logging into a file.
func (b Beacon) String() string { return b.Hex() }

// ShortString returns the first 10 characters of the Beacon, usually for logging purposes.
func (b Beacon) ShortString() string {
str := b.Hex()
l := len(str)
return fmt.Sprintf("%.10s", str[min(2, l):])
}
func (b Beacon) String() string { return hex.EncodeToString(b[:]) }

// Bytes gets the byte representation of the underlying hash.
func (b Beacon) Bytes() []byte {
Expand All @@ -41,7 +31,7 @@ func (b Beacon) Bytes() []byte {

// Field returns a log field. Implements the LoggableField interface.
func (b Beacon) Field() log.Field {
return log.String("beacon", b.ShortString())
return log.Stringer("beacon", b)
}

func (b *Beacon) MarshalText() ([]byte, error) {
Expand Down
5 changes: 0 additions & 5 deletions common/types/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,6 @@ func (id ProposalID) AsHash32() Hash32 {
return Hash20(id).ToHash32()
}

// Field returns a log field. Implements the LoggableField interface.
func (id ProposalID) Field() log.Field {
return log.String("proposal_id", id.String())
}

// Compare returns true if other (the given ProposalID) is less than this ProposalID, by lexicographic comparison.
func (id ProposalID) Compare(other ProposalID) bool {
return bytes.Compare(id.Bytes(), other.Bytes()) < 0
Expand Down
4 changes: 2 additions & 2 deletions hare3/hare.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,8 @@ func (h *Hare) selectProposals(session *session) []types.ProposalID {
h.log.Warn("proposal has different beacon value",
zap.Uint32("lid", session.lid.Uint32()),
zap.Stringer("id", p.ID()),
zap.String("proposal_beacon", p.Beacon().ShortString()),
zap.String("epoch_beacon", session.beacon.ShortString()),
zap.Stringer("proposal_beacon", p.Beacon()),
zap.Stringer("epoch_beacon", session.beacon),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ func (app *App) initServices(ctx context.Context) error {
trtl,
vrfVerifier,
app.clock,
proposals.WithLogger(app.addLogger(ProposalListenerLogger, lg)),
proposals.WithLogger(app.addLogger(ProposalListenerLogger, lg).Zap()),
proposals.WithConfig(proposals.Config{
LayerSize: layerSize,
LayersPerEpoch: layersPerEpoch,
Expand Down
18 changes: 9 additions & 9 deletions proposals/eligibility_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import (
"fmt"

"github.com/spacemeshos/fixed"
"go.uber.org/zap"

"github.com/spacemeshos/go-spacemesh/atxsdata"
"github.com/spacemeshos/go-spacemesh/common/types"
"github.com/spacemeshos/go-spacemesh/fetch"
"github.com/spacemeshos/go-spacemesh/log"
"github.com/spacemeshos/go-spacemesh/miner/minweight"
"github.com/spacemeshos/go-spacemesh/p2p/pubsub"
"github.com/spacemeshos/go-spacemesh/system"
Expand All @@ -25,7 +25,7 @@ type Validator struct {
atxsdata *atxsdata.Data
clock layerClock
beacons system.BeaconCollector
logger log.Log
logger *zap.Logger
vrfVerifier vrfVerifier
}

Expand All @@ -40,7 +40,7 @@ func NewEligibilityValidator(
tortoise tortoiseProvider,
atxsdata *atxsdata.Data,
bc system.BeaconCollector,
lg log.Log,
lg *zap.Logger,
vrfVerifier vrfVerifier,
opts ...ValidatorOpt,
) *Validator {
Expand Down Expand Up @@ -116,7 +116,7 @@ func (v *Validator) CheckEligibility(ctx context.Context, ballot *types.Ballot,
return fmt.Errorf(
"%w: proof contains incorrect VRF signature. beacon: %v, epoch: %v, counter: %v, vrfSig: %s",
fetch.ErrIgnore,
data.Beacon.ShortString(),
data.Beacon,
ballot.Layer.GetEpoch(),
proof.J,
proof.Sig,
Expand All @@ -129,11 +129,11 @@ func (v *Validator) CheckEligibility(ctx context.Context, ballot *types.Ballot,
}
}

v.logger.WithContext(ctx).With().Debug("ballot eligibility verified",
ballot.ID(),
ballot.Layer,
ballot.Layer.GetEpoch(),
data.Beacon,
v.logger.Debug("ballot eligibility verified",
zap.Stringer("id", ballot.ID()),
zap.Uint32("layer", ballot.Layer.Uint32()),
zap.Uint32("layer", ballot.Layer.GetEpoch().Uint32()),
zap.Stringer("beacon", data.Beacon),
)

v.beacons.ReportBeaconFromBallot(ballot.Layer.GetEpoch(), ballot, data.Beacon,
Expand Down
5 changes: 2 additions & 3 deletions proposals/eligibility_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (

"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
"go.uber.org/zap/zaptest"

"github.com/spacemeshos/go-spacemesh/atxsdata"
"github.com/spacemeshos/go-spacemesh/common/types"
"github.com/spacemeshos/go-spacemesh/log/logtest"
"github.com/spacemeshos/go-spacemesh/tortoise"
)

Expand Down Expand Up @@ -553,7 +553,6 @@ func TestEligibilityValidator(t *testing.T) {
}
}).AnyTimes()

lg := logtest.New(t)
c := atxsdata.New()
c.EvictEpoch(tc.evicted)
tv := NewEligibilityValidator(
Expand All @@ -564,7 +563,7 @@ func TestEligibilityValidator(t *testing.T) {
ms.md,
c,
ms.mbc,
lg,
zaptest.NewLogger(t),
ms.mvrf,
)
for _, atx := range tc.atxs {
Expand Down
Loading

0 comments on commit 5c00c97

Please sign in to comment.