Merge pull request onflow#5997 from onflow/supun/migration-stats

Report storage traversing errors during metrics collection

cmd/util/ledger/migrations/cadence.go

@@ -307,6 +307,7 @@ func NewCadence1ValueMigrations(
 				opts.NWorker,
 				[]AccountBasedMigration{
 					NewMetricsCollectingMigration(
+						log,
 						opts.ChainID,
 						rwf,
 						programs,

cmd/util/ledger/migrations/migration_matrics_collector_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/rs/zerolog"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/onflow/cadence/runtime/common"
@@ -32,7 +33,7 @@ func TestMigrationMetricsCollection(t *testing.T) {
 		rwf := &testReportWriterFactory{}
 
 		logWriter := &writer{}
-		logger := zerolog.New(logWriter).Level(zerolog.InfoLevel)
+		logger := zerolog.New(logWriter).Level(zerolog.ErrorLevel)
 
 		const nWorker = 2
 
@@ -71,6 +72,12 @@ func TestMigrationMetricsCollection(t *testing.T) {
 
 		require.NoError(t, err)
 
+		// Should have 1 error:
+		// - The `Test` contract checking error
+		logs := logWriter.logs
+		require.Len(t, logs, 1)
+		assert.Contains(t, logs[0], "`pub` is no longer a valid access keyword")
+
 		reportWriter := rwf.reportWriters["metrics-collecting-migration"]
 		require.Len(t, reportWriter.entries, 1)
 
@@ -117,7 +124,7 @@ func TestMigrationMetricsCollection(t *testing.T) {
 		rwf := &testReportWriterFactory{}
 
 		logWriter := &writer{}
-		logger := zerolog.New(logWriter).Level(zerolog.InfoLevel)
+		logger := zerolog.New(logWriter).Level(zerolog.ErrorLevel)
 
 		const nWorker = 2
 
@@ -163,6 +170,14 @@ func TestMigrationMetricsCollection(t *testing.T) {
 
 		require.NoError(t, err)
 
+		// Should have 2 errors:
+		// - Contract update failure error
+		// - The `Test` contract checking error
+		logs := logWriter.logs
+		require.Len(t, logs, 2)
+		assert.Contains(t, logs[0], `"migration":"StagedContractsMigration","account":"0x01cf0e2f2f715450","contract":"Test"`)
+		assert.Contains(t, logs[1], "`pub` is no longer a valid access keyword")
+
 		reportWriter := rwf.reportWriters["metrics-collecting-migration"]
 		require.Len(t, reportWriter.entries, 1)
 

cmd/util/ledger/migrations/migration_metrics_collector.go

@@ -24,6 +24,7 @@ const metricsCollectingMigrationName = "metrics_collecting_migration"
 type MetricsCollectingMigration struct {
 	name             string
 	chainID          flow.ChainID
+	log              zerolog.Logger
 	mutex            sync.RWMutex
 	reporter         reporters.ReportWriter
 	metricsCollector *MigrationMetricsCollector
@@ -35,13 +36,15 @@ var _ migrations.ValueMigration = &MetricsCollectingMigration{}
 var _ AccountBasedMigration = &MetricsCollectingMigration{}
 
 func NewMetricsCollectingMigration(
+	log zerolog.Logger,
 	chainID flow.ChainID,
 	rwf reporters.ReportWriterFactory,
 	programs map[common.Location]*interpreter.Program,
 ) *MetricsCollectingMigration {
 
 	return &MetricsCollectingMigration{
 		name:             metricsCollectingMigrationName,
+		log:              log,
 		reporter:         rwf.ReportWriter("metrics-collecting-migration"),
 		metricsCollector: NewMigrationMetricsCollector(),
 		chainID:          chainID,
@@ -137,7 +140,7 @@ func (m *MetricsCollectingMigration) MigrateAccount(
 
 	migration.Migrate(
 		migration.NewValueMigrationsPathMigrator(
-			nil, // No need to report
+			NewStorageVisitingErrorReporter(m.log),
 			m,
 		),
 	)
@@ -307,10 +310,10 @@ func (m *MetricsCollectingMigration) Domains() map[string]struct{} {
 
 type MigrationMetricsCollector struct {
 	mutex             sync.RWMutex
-	TotalValues       int                     `json:"totalValues"`
-	TotalErrors       int                     `json:"totalErrors"`
-	ValuesPerContract map[common.Location]int `json:"valuesPerContract"`
-	ErrorsPerContract map[common.Location]int `json:"errorsPerContract"`
+	TotalValues       int
+	TotalErrors       int
+	ValuesPerContract map[common.Location]int
+	ErrorsPerContract map[common.Location]int
 }
 
 func NewMigrationMetricsCollector() *MigrationMetricsCollector {
@@ -379,3 +382,31 @@ type Metrics struct {
 	// Total values related to each contract
 	ValuesPerContract map[string]int `json:"valuesPerContract"`
 }
+
+type storageVisitingErrorReporter struct {
+	log zerolog.Logger
+}
+
+func NewStorageVisitingErrorReporter(log zerolog.Logger) *storageVisitingErrorReporter {
+	return &storageVisitingErrorReporter{
+		log: log,
+	}
+}
+
+var _ migrations.Reporter = &storageVisitingErrorReporter{}
+
+func (p *storageVisitingErrorReporter) Migrated(
+	_ interpreter.StorageKey,
+	_ interpreter.StorageMapKey,
+	_ string,
+) {
+	// Ignore
+}
+
+func (p *storageVisitingErrorReporter) DictionaryKeyConflict(addressPath interpreter.AddressPath) {
+	p.log.Error().Msgf("dictionary key conflict for %s", addressPath)
+}
+
+func (p *storageVisitingErrorReporter) Error(err error) {
+	p.log.Error().Msgf("%s", err.Error())
+}