-
Notifications
You must be signed in to change notification settings - Fork 26
Add TXM Prometheus metrics #650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized we don't set up the prometheus server on the starknet nodes, even in our internal test environments :/ .Doing so would probably be a step backwards and we should probably look into beholder for Starknet. Between that and also the fact that Dimitris's txm changes alter the nature of the txm metrics we want to report, I think we should keep this in draft mode and revisit it in the near future. we can pull in his branch and use txmv2 metrics for now because the modifications are more aligned with that version of the txm
I would like to revamp the txm metrics and monitoring, so this is great. And I think we should start with Starknet and non-EVMs first. But we should do it via Beholder to get the metrics in prod and also go back to the drawing board to determine the right set of metrics and labels. I wanted to push this through before we tested the Starknet txm changes but given the changes required and priority of pushing the Starknet txm I think this needs to wait a bit to do it in a more robust manner.
| func initMetrics() { | ||
| metricsOnce.Do(func() { | ||
| promNumSuccessfulTxs = promauto.NewCounterVec(prometheus.CounterOpts{ | ||
| Name: "tx_manager_num_successful_transactions", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this marks if a confirmed tx executed successfully and the union of "tx_manager_num_successful_transactions" and "tx_manager_num_tx_reverted" should be all confirmed transactions?
I prefer the txmv2 metrics where there is only a single metric representing the confirmed transaction. We could then add a label to the metric denoting whether it reverted or confirmed.
|
since prom metrics are already enabled on core node, we can just go with the status quo and report these metrics as an improvement over no metrics today. beholder eventually, but stage metrics is better than no metrics. just need to pull in Dimitris PR / change the metrics to align with txmv2 |
|
Closing in favour of: |




Summary
Adds Prometheus metrics to the Starknet transaction manager for monitoring transaction success rates and queue depth. Matches the metrics already used in chainlink-evm.
What changed
Added 4 metrics:
tx_manager_num_successful_transactionstx_manager_num_tx_revertedtx_manager_num_finalized_transactionstx_manager_tx_attempt_countUsed
sync.Onceto avoid duplicate registration issues when the TXM is imported by multiple packages.