MX-17032 Added Metrics for Tracked Blocks#7331
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds metrics for tracking the number of blocks in the transaction pool to enhance observability. The implementation exposes the tracked blocks count through metrics and displays it in the terminal UI.
- Added a new metric
MetricNumTrackedBlocksto track the number of blocks monitored by the transaction pool - Implemented
GetNumTrackedBlocks()method across the transaction pool interfaces and implementations - Updated terminal UI to display the tracked blocks metric in the block info widget
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| common/constants.go | Added metric constant for number of tracked blocks |
| dataRetriever/interface.go | Extended interface with GetNumTrackedBlocks method |
| dataRetriever/txpool/shardedTxPool.go | Implemented GetNumTrackedBlocks method delegation |
| dataRetriever/shardedData/shardedData.go | Added stub implementation with warning log |
| process/block/shardblock.go | Added metric update during block commit |
| cmd/termui/view/interface.go | Extended presenter interface |
| cmd/termui/presenter/blockInfoGetters.go | Implemented metric getter |
| cmd/termui/view/termuic/termuiRenders/widgetsRender.go | Added display of tracked blocks count |
| testscommon/*.go | Updated test stubs and mocks |
| Tests | Added test coverage and updated existing tests |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| durationSentOrReceivedBlockToReceivedSignatures), | ||
| } | ||
|
|
||
| rows[9] = []string{fmt.Sprintf("Num tracked blocks: %d", wr.presenter.GetNumTrackedBlocks())} |
There was a problem hiding this comment.
The hardcoded index 9 for the tracked blocks row makes the code fragile. Consider inserting this row sequentially after row 8 or using a variable to track the current row index.
| rows[9] = []string{fmt.Sprintf("Num tracked blocks: %d", wr.presenter.GetNumTrackedBlocks())} | |
| // Insert tracked blocks row after the last used index (which is 8) | |
| rowIdx := 9 | |
| rows[rowIdx] = []string{fmt.Sprintf("Num tracked blocks: %d", wr.presenter.GetNumTrackedBlocks())} |
There was a problem hiding this comment.
moved to sequential order of rows
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| rows[6] = []string{ | ||
| fmt.Sprintf("Received proposed block: %.6f sec | Received signatures: %.6f sec", | ||
| fmt.Sprintf("Received proposed block: %.6f sec | Received proof: %.6f sec", | ||
| durationStartRoundToSentOrReceivedBlock, | ||
| durationSentOrReceivedBlockToReceivedSignatures), |
There was a problem hiding this comment.
is this change ok? we're still printing durationSentOrReceivedBlockToReceivedSignatures
There was a problem hiding this comment.
We made changes, so now the correct term is Proof, I forgot to change here. I have also changed the names of the variables accordingly, now
| return err | ||
| } | ||
|
|
||
| log.Debug("Number of tracked blocks Metrics updated", "num", sp.dataPool.Transactions().GetNumTrackedBlocks()) |
There was a problem hiding this comment.
we don't need to log this here
| } | ||
|
|
||
| log.Debug("Number of tracked blocks Metrics updated", "num", sp.dataPool.Transactions().GetNumTrackedBlocks()) | ||
| sp.appStatusHandler.SetUInt64Value(common.MetricNumTrackedBlocks, sp.dataPool.Transactions().GetNumTrackedBlocks()) |
There was a problem hiding this comment.
should this be called also for metaBlock ?
There was a problem hiding this comment.
No, there is no Mempool on meta, so there is no SelectTransactions. We wouldn't have what to track here
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc/supernova #7331 +/- ##
================================================
- Coverage 76.81% 76.81% -0.01%
================================================
Files 846 846
Lines 113162 113181 +19
================================================
+ Hits 86930 86935 +5
- Misses 20392 20404 +12
- Partials 5840 5842 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
| func (wr *WidgetsRender) prepareBlockInfo() { | ||
| // 8 rows and one column | ||
| numRows := 9 | ||
| numRows := 10 |
There was a problem hiding this comment.
update comment as well on L302
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
| "github.com/multiversx/mx-chain-core-go/data/block" | ||
| "github.com/multiversx/mx-chain-core-go/data/transaction" | ||
| "github.com/multiversx/mx-chain-core-go/marshal" | ||
| "github.com/multiversx/mx-chain-go/config" |
There was a problem hiding this comment.
remove commented code
|
no other things from me |
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?