Feat/issue 953 3d pricing metrics #102
Conversation
Add metrics to monitor 3D pricing (extraData driven): P1 metrics: - Node's perception of latest extraData contents - Profitability levels of TxPool contents - Profitability metrics for last sealed block (Sequencer only) Nice to have: - Profitability levels for TransactionProfitabilityCalcuODlator - priorityFeePerGas from TransactionProfitabilityCalculator - compressedTxSize from TransactionProfitabilityCalculator Implement histograms to capture all described metrics. Closes #953 Signed-off-by: Ade Lucas <[email protected]>
Add metrics to monitor 3D pricing (extraData driven): P1 metrics: - Node's perception of latest extraData contents - Profitability levels of TxPool contents - Profitability metrics for last sealed block (Sequencer only) Nice to have: - Profitability levels for TransactionProfitabilityCalcuODlator - priorityFeePerGas from TransactionProfitabilityCalculator - compressedTxSize from TransactionProfitabilityCalculator Implement histograms to capture all described metrics. Closes #953 Signed-off-by: Ade Lucas <[email protected]>
There was a problem hiding this comment.
Did a first pass, without going too much in the details, please see my comments, and it will be great, if you can split this in smaller ones, so it is easier to review, for example one PR for each point:
- Node's perception of extraData (variable cost, fixed cost, eth gas price)
- Profitability levels of the TxPool's contents (low, high, average)
- Profitability levels in the context of the last sealed block
where the next is built on top of the previous, I found this way to be easier to digest and not adding too much time
| double profitabilityRatio = | ||
| payingGasPrice.getAsBigInteger().doubleValue() | ||
| / profitableGasPrice.getAsBigInteger().doubleValue(); | ||
| profitabilityMetrics.recordEvaluatedTxProfitabilityRatio(profitabilityRatio); |
There was a problem hiding this comment.
since this method also calls profitablePriorityFeePerGas we should be sure that some metrics are not calculated and recorded twice
| log.debug( | ||
| "Calculated profitablePriorityFee: {}, profitableGasPrice: {}, payingGasPrice: {}", | ||
| profitablePriorityFee.toHumanReadableString(), | ||
| profitableGasPrice.toHumanReadableString(), | ||
| payingGasPrice.toHumanReadableString()); |
There was a problem hiding this comment.
opt: replace with the lambda version to avoid computation when debug is not enabled
There was a problem hiding this comment.
Good idea.
Apparently Lombok's @slf4j annotation debug method doesn't directly accept a lambda, so I've got this:
if (log.isDebugEnabled()) {
log.debug(
"Calculated profitablePriorityFee: {}, profitableGasPrice: {}, payingGasPrice: {}",
profitablePriorityFee.toHumanReadableString(),
profitableGasPrice.toHumanReadableString(),
payingGasPrice.toHumanReadableString());
}
There was a problem hiding this comment.
it does, look for other usage of log.atDebug()... in the code
sequencer/src/main/java/net/consensys/linea/metrics/TransactionProfitabilityMetrics.java
Show resolved
Hide resolved
| final BlockHeader chainHeadHeader = blockchainService.getChainHeadHeader(); | ||
| final LineaPricingUtils.PricingData pricingData = | ||
| LineaPricingUtils.extractPricingFromExtraData(chainHeadHeader.getExtraData()); | ||
|
|
There was a problem hiding this comment.
I think this is not necessary, since current value are in LineaProfitabilityConfiguration, that is updated every time a new block arrives
There was a problem hiding this comment.
I see what you mean, LineaProfitabilityConfiguration instance has fixedCostWei, variableCostWei. I'll revise the use of PricingData.
There was a problem hiding this comment.
Also the parsing is already done in https://github.com/Consensys/linea-sequencer/blob/main/sequencer/src/main/java/net/consensys/linea/extradata/LineaExtraDataHandler.java so we need to make sure to avoid duplication and in necessary extend what we have to adapt to the new uses
|
|
||
| /** Utility class for handling Linea pricing data extraction from the extraData field. */ | ||
| @Slf4j | ||
| public class LineaPricingUtils { |
There was a problem hiding this comment.
This seems a duplicate to me, since we already have the code to parse extraData field
| @@ -0,0 +1,83 @@ | |||
| @startuml | |||
Add metrics to cover 3D pricing (#953)
This commit introduces monitoring for 3D pricing, including:
Histograms are added to capture profitability metrics, as well as node metrics for evaluating transactions using the TransactionProfitabilityCalculator.
Acceptance Criteria:
Linea-Besu exposes described metrics, and this implementation follows that requirement.
This commit does not include:
Link to design proposal documentation:
https://www.notion.so/consensys/Design-Proposal-Documentation-120fc61a326e809ab07df86a2a69cd32?pvs=4