Feat: gas metrics #13460
Feat: gas metrics #13460
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Please update the PR title to match https://github.com/filecoin-project/lotus/blob/master/CONTRIBUTING.md#pr-title-conventions
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive gas-related metrics to monitor both the message pool (mpool) and synced blocks, enabling better observability of gas usage patterns, fees, and limits across the Filecoin network.
Key changes:
- Added gas distribution buckets for message gas limits and gas fee distributions for premium and fee cap metrics
- Implemented mpool gas metrics collection on every head change, tracking gas premium, fee cap, gas limits, and various statistical measures (mean, median, gas-weighted median)
- Added synced block gas metrics during block application to track gas usage patterns in executed blocks
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| metrics/metrics.go | Defines new gas-related metrics including BaseFee, mpool metrics (gas premium, fee cap, gas limits), and synced block metrics (gas limits, gas used, message counts), along with appropriate distribution buckets and view configurations |
| chain/messagepool/messagepool.go | Implements gas metrics collection in the message pool, recording metrics on every head change including mean/median calculations and gas-weighted medians for premium and fee cap |
| chain/consensus/compute_state.go | Adds gas metrics tracking during block execution, collecting per-message gas limits and usage, and computing various statistical measures including medians |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Gas-weighted median of gas used (what gas used the median gas unit had) | ||
| accumulatedGas = 0 | ||
| for _, m := range msgGasList { | ||
| accumulatedGas += m.gasUsed | ||
| if accumulatedGas >= halfGasUsed { | ||
| stats.Record(ctx, metrics.SyncedBlockGasUsedMedianByGasUnits.M(m.gasUsed)) | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
The gas-weighted median calculation for gas used is incorrect. The code sorts by gasUsed and then accumulates gasUsed, which just computes the regular median of gasUsed, not a gas-weighted median. For a proper gas-weighted median, the list should be sorted by gasUsed but the accumulated weight should use a different field (like gasLimit). Currently, this metric is redundant with SyncedBlockGasUsedMedian. Consider either removing this metric or fixing the weighting logic to accumulate by gasLimit instead of gasUsed.
| // 10 nanoFIL = 10 * 1e9 attoFIL = 1e10 attoFIL | ||
| var gasFeeDistribution = view.Distribution( | ||
| 100, 200, 500, // very low fees (100-500 attoFIL) | ||
| 1e3, 500e3, // femtoFIL range (1e3 = 1 femtoFIL) |
There was a problem hiding this comment.
The range comment is misleading. The value 500e3 (500,000 attoFIL) equals 500 femtoFIL, which is 0.5 picoFIL. This is transitioning between femtoFIL and picoFIL ranges rather than being purely in the femtoFIL range. Consider updating the comment to better reflect the actual range or moving this bucket to a more appropriate position.
| 1e3, 500e3, // femtoFIL range (1e3 = 1 femtoFIL) | |
| 1e3, 500e3, // femtoFIL to sub-picoFIL range (1e3 = 1 femtoFIL, 500e3 = 0.5 picoFIL) |
| MpoolGasFeeCap = stats.Float64("mpool/gas_fee_cap", "Gas fee cap of messages in mpool in attoFIL (histogram)", stats.UnitDimensionless) | ||
| MpoolGasFeeCapMean = stats.Float64("mpool/gas_fee_cap_mean", "Mean gas fee cap of messages in mpool in attoFIL", stats.UnitDimensionless) | ||
| MpoolGasFeeCapMedian = stats.Float64("mpool/gas_fee_cap_median", "Median gas fee cap of messages in mpool in attoFIL", stats.UnitDimensionless) | ||
| MpoolGasFeeCapMedianByGasUnits = stats.Float64("mpool/gas_fee_cap_median_by_gas_units", "Median gas fee cap weighted by gas limit", stats.UnitDimensionless) |
There was a problem hiding this comment.
The description is less detailed than similar metrics. For consistency with MpoolGasPremiumMedianByGasUnits (line 166), consider adding a clarifying phrase such as "Median gas fee cap weighted by gas limit (what fee cap the median gas unit has)" to help users better understand what this weighted median represents.
| MpoolGasFeeCapMedianByGasUnits = stats.Float64("mpool/gas_fee_cap_median_by_gas_units", "Median gas fee cap weighted by gas limit", stats.UnitDimensionless) | |
| MpoolGasFeeCapMedianByGasUnits = stats.Float64("mpool/gas_fee_cap_median_by_gas_units", "Median gas fee cap weighted by gas limit (what fee cap the median gas unit has)", stats.UnitDimensionless) |
| SyncedBlockTotalGasLimit = stats.Int64("chain/synced_block_total_gas_limit", "Total gas limit of all messages in synced block", stats.UnitDimensionless) | ||
| SyncedBlockMessageGasLimit = stats.Int64("chain/synced_block_message_gas_limit", "Gas limit of messages in synced blocks (histogram)", stats.UnitDimensionless) | ||
| SyncedBlockGasLimitMedian = stats.Int64("chain/synced_block_gas_limit_median", "Median gas limit of messages in synced block", stats.UnitDimensionless) | ||
| SyncedBlockGasLimitMedianByGasUnits = stats.Int64("chain/synced_block_gas_limit_median_by_gas_units", "Median gas limit weighted by gas used", stats.UnitDimensionless) |
There was a problem hiding this comment.
The description is less detailed than similar metrics. For consistency with MpoolGasPremiumMedianByGasUnits (line 166), consider adding a clarifying phrase such as "Median gas limit weighted by gas used (what gas limit the median executed gas unit had)" to help users better understand what this weighted median represents.
| SyncedBlockGasLimitMedianByGasUnits = stats.Int64("chain/synced_block_gas_limit_median_by_gas_units", "Median gas limit weighted by gas used", stats.UnitDimensionless) | |
| SyncedBlockGasLimitMedianByGasUnits = stats.Int64("chain/synced_block_gas_limit_median_by_gas_units", "Median gas limit weighted by gas used (what gas limit the median executed gas unit had)", stats.UnitDimensionless) |
| MessageFetchDuration = stats.Float64("message/fetch_duration_ms", "Duration of message fetch operations", stats.UnitMilliseconds) | ||
|
|
||
| // gas metrics | ||
| // Note: attoFIL metrics use Float64 because values can exceed int64 range (atto = 1e-18) |
There was a problem hiding this comment.
The comment is misleading. The reason Float64 is used isn't because "atto = 1e-18" per se, but because large FIL amounts expressed in attoFIL can exceed int64's maximum value. For example, just 10 FIL equals 1e19 attoFIL, which exceeds int64 max (approximately 9.2e18). Consider clarifying: "attoFIL metrics use Float64 because large FIL amounts in attoFIL units can exceed int64 range (e.g., 10 FIL = 1e19 attoFIL)".
| // Note: attoFIL metrics use Float64 because values can exceed int64 range (atto = 1e-18) | |
| // Note: attoFIL metrics use Float64 because large FIL amounts in attoFIL units can exceed int64 range | |
| // (e.g., 10 FIL = 1e19 attoFIL, which exceeds int64 max ≈ 9.2e18) |
Related Issues
Proposed Changes
Add some useful metrics
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that: