feat(metrics): add block transaction count histogram for empty block monitoring#6602
feat(metrics): add block transaction count histogram for empty block monitoring#6602ToXMon wants to merge 5 commits intotronprotocol:developfrom
Conversation
…monitoring Replace the dedicated tron:block_empty_total counter with a more comprehensive tron:block_transaction_count histogram that tracks the distribution of transaction counts per block. Changes: - Add overloaded init() method in MetricsHistogram to support custom buckets - Add BLOCK_TRANSACTION_COUNT histogram with buckets [0, 10, 50, 100, 200, 500, 1000, 2000, 5000, 10000] - Record transaction count for all blocks (including empty blocks with txCount=0) - Empty blocks can be queried via bucket le=0.0 - Remove unused BLOCK_EMPTY counter - Keep SR_SET_CHANGE counter for SR set change monitoring This provides richer insights for network analysis while still supporting empty block monitoring via histogram bucket queries. Closes tronprotocol#6590
There was a problem hiding this comment.
Hi @ToXMon and maintainers,
I actually have an alternative implementation for Issue #6590 that addresses some of the concerns mentioned above:
Key differences in my approach:
- SR Change Detection: Only triggers during maintenance period changes (using
nextMaintenanceTimecheck) to avoid per-block overhead - Label Alignment: Uses
action(add/remove) as specified in the original issue - Implementation: Uses standard Set operations without additional Guava dependencies
My implementation includes:
- Histogram with buckets
[0, 10, 50, 100, 200, 500, 1000, 2000, 5000, 10000]for transaction counts - SR set change Counter with proper maintenance period detection
- Comprehensive tests for histogram bucket queries (
le="0.0")
The branch is ready and passes all CI checks. I'm happy to:
- Submit a separate PR for comparison
- Collaborate with @xl-2055 to merge the best aspects of both implementations
Let me know how you'd like to proceed!
| StringUtil.encode58Check(address)); | ||
|
|
||
| // SR set change detection | ||
| List<ByteString> currentSrList = |
There was a problem hiding this comment.
Hi @ToXMon, thanks for the contribution! I've identified a few issues that need attention:
1. SR Set Change Detection Logic Issue
The current implementation checks for SR changes on every applyBlock() call without considering the maintenance period:
// Current: runs on EVERY block
List<ByteString> currentSrList = chainBaseManager.getWitnessScheduleStore().getActiveWitnesses();However, SR set changes only happen during maintenance periods. This could cause:
- Unnecessary performance overhead (set comparison on every block)
- False positives if there are temporary witness list fluctuations
2. Label Naming Inconsistency
The Issue #6590 specification suggests using action (add/remove) for the label key, but this PR uses change_type (added/removed). While both work, aligning with the original issue specification would be better for consistency.
3. GitHub CI Check
It looks like the PR may not pass all CI checks yet. Please ensure:
- All tests pass (
./gradlew test) - Checkstyle passes (
./gradlew checkstyleMain checkstyleTest) - No compilation warnings
| private long failProcessBlockNum = 0; | ||
| @Setter | ||
| private String failProcessBlockReason = ""; | ||
| private Set<String> previousSrSet = new HashSet<>(); |
There was a problem hiding this comment.
Even if the applyBlock() call path is currently single-threaded
(protected by the Manager lock), writes to previousSrSet are not guaranteed to be visible to other threads (e.g., Metrics query threads) without proper synchronization.
4 participants
Summary
Implements Issue #6590 — adds Prometheus metrics for empty block detection and SR set change monitoring:
tron:block_transaction_count— Histogram tracking distribution of transaction counts per block (enables empty block detection via bucketle="0.0")tron:sr_set_change_total— Counter tracking SR set changes withadded/removedlabelsChanges
New Metrics
tron:block_transaction_countminerle="0.0")tron:sr_set_change_totalwitness,change_typeKey Implementation Details
[0, 10, 50, 100, 200, 500, 1000, 2000, 5000, 10000]tron:block_transaction_count_bucket{le="0.0"}rate(tron:block_transaction_count_bucket{le="0.0"}[1h]) / rate(tron:block_transaction_count_count[1h])Files Modified
common/src/main/java/org/tron/common/prometheus/MetricKeys.java— removed BLOCK_EMPTY counter, added BLOCK_TRANSACTION_COUNT histogramcommon/src/main/java/org/tron/common/prometheus/MetricsCounter.java— removed BLOCK_EMPTY registrationcommon/src/main/java/org/tron/common/prometheus/MetricsHistogram.java— added overloaded init() for custom bucketsframework/src/main/java/org/tron/core/metrics/blockchain/BlockChainMetricManager.java— histogramObserve() for all blocks, keeps SR counterframework/src/test/java/org/tron/core/metrics/prometheus/PrometheusApiServiceTest.java— updated tests for histogramTesting
testMetric()— PASSEDtestEmptyBlockMetric()— PASSEDtestSrSetChangeMetric()— PASSEDAdvantages Over Simple Counter
le="0.0"bucketBackward Compatibility
Fully backward compatible. Zero protocol changes, zero API changes, zero config changes.
Closes #6590