Skip to content

Conversation

@cawthorne
Copy link
Contributor

@cawthorne cawthorne commented Oct 14, 2025

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_transactions
  • tx_manager_num_tx_reverted
  • tx_manager_num_finalized_transactions
  • tx_manager_tx_attempt_count

Used sync.Once to avoid duplicate registration issues when the TXM is imported by multiple packages.

@cawthorne cawthorne marked this pull request as ready for review October 14, 2025 02:16
@cawthorne cawthorne requested review from a team as code owners October 14, 2025 02:16
@cawthorne cawthorne marked this pull request as draft October 14, 2025 02:32
@cawthorne cawthorne marked this pull request as ready for review October 14, 2025 13:39
@cl-sonarqube-production
Copy link

Copy link
Collaborator

@augustbleeds augustbleeds left a 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",
Copy link
Collaborator

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.

@augustbleeds
Copy link
Collaborator

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

@cawthorne
Copy link
Contributor Author

Closing in favour of:
#655

@cawthorne cawthorne closed this Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants