Add TxnManager metrics
#170
Conversation
ian-shim
force-pushed
the
txn-manager-metrics
branch
2 times, most recently
from
dab2090 to
222569e
Compare
ian-shim
force-pushed
the
txn-manager-metrics
branch
from
222569e to
dc518fa
Compare
| @@ -143,6 +149,7 @@ func (t *txnManager) ReceiptChan() chan *ReceiptOrErr { | |||
| // monitorTransaction monitors the transaction and resends it with a higher gas price if it is not mined without a timeout. | |||
| // It returns an error if the transaction fails to be sent for reasons other than timeouts. | |||
| func (t *txnManager) monitorTransaction(ctx context.Context, req *TxnRequest) (*types.Receipt, error) { | |||
| numSpeedUps := 0 | |||
| for { | |||
There was a problem hiding this comment.
nit: should their be some sort of backoff here? to handle case with network congestion and avoid immediate retry
There was a problem hiding this comment.
It always waits for t.txnRefreshInterval for every retry
| @@ -173,6 +181,7 @@ func (t *txnManager) monitorTransaction(ctx context.Context, req *TxnRequest) (* | |||
| t.logger.Error("failed to send txn", "tag", req.Tag, "txn", req.Tx.Hash().Hex(), "err", err) | |||
| continue | |||
| } | |||
| numSpeedUps++ | |||
| } else { | |||
| t.logger.Error("transaction failed", "tag", req.Tag, "txHash", req.Tx.Hash().Hex(), "err", err) | |||
There was a problem hiding this comment.
do we want to track how many Transactions failed because of timeout
There was a problem hiding this comment.
This condition doesn't mean the transaction failed due to timeout as there is no timeout that fails an onchain confirmation anymore. We may get into this condition for any other errors (i.e. RPC error, networking issue, txn failure etc.), hence reporting error and exiting right away.
Since this condition isn't expected, I'm not sure tracking it as a metric makes sense. It should be alerted as any other error logs might.
Why are these changes needed?
Adding the following metrics to
TxnManager:Checks