Skip to content
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

metrics: add performance and application metrics for BTC staking #440

Merged
merged 15 commits into from
Feb 12, 2024

Conversation

SebastianElvis
Copy link
Member

Fixes BM-1169

This PR introduces metrics for BTC staking. The metrics include both performance oriented ones and application specific ones. In particular, it includes

  • performance: execution time of all relevant message handlers
  • BTC staking module: number of finality providers and BTC delegations under each status, and total amount of staked Bitcoins
  • finality module: last finalised height and last height (we can derive the gap of non finalised blocks by using them)

The implementation uses Cosmos SDK's native telemetry module.

Copy link
Member

@filippos47 filippos47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, these will be super handy!
Out of curiosity, how did you choose from which entity to emit the metrics? Afaik, Prom metrics are exposed both from the Telemetry module and a dedicated Prom server (by default listens to 26660).

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very useful! Some questions about performance and types.

Also, let's change the base branch or wait until the new release before merging.

x/btcstaking/keeper/finality_providers.go Outdated Show resolved Hide resolved
x/btcstaking/keeper/keeper.go Outdated Show resolved Hide resolved
x/btcstaking/keeper/keeper.go Show resolved Hide resolved
x/btcstaking/keeper/keeper.go Outdated Show resolved Hide resolved
x/btcstaking/types/metrics.go Show resolved Hide resolved
x/btcstaking/types/metrics.go Show resolved Hide resolved
x/finality/types/metrics.go Outdated Show resolved Hide resolved
@SebastianElvis
Copy link
Member Author

SebastianElvis commented Feb 8, 2024

Afaik, Prom metrics are exposed both from the Telemetry module and a dedicated Prom server (by default listens to 26660).

I think all metrics specified using the telemetry package will be exposed by the prometheus server at :26660. The telemetry module gives all metrics specified by the developer to "github.com/hashicorp/go-metrics" (ref), more specifically to a singleton variable globalMetrics. When enabling metrics, the Metrics object will be created here, which imports metrics from "github.com/hashicorp/go-metrics". If you take a look at the code of Babylon and Cosmos SDK you might find that metrics specified by telemetry are presented in prometheus server.

@SebastianElvis
Copy link
Member Author

Also, let's change the base branch or wait until the new release before merging.

Let's wait until the new release then. This PR will be affected by the new finality provider set rotation logic as well.

@SebastianElvis SebastianElvis marked this pull request as draft February 8, 2024 03:48
@SebastianElvis SebastianElvis marked this pull request as ready for review February 9, 2024 00:40
@SebastianElvis
Copy link
Member Author

Thanks for the comments! This PR is now rebased to the latest dev and revised according to comments. Please feel free to take a look again.

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

x/btcstaking/keeper/keeper.go Outdated Show resolved Hide resolved
x/btcstaking/types/metrics.go Outdated Show resolved Hide resolved
@SebastianElvis SebastianElvis merged commit b775284 into dev Feb 12, 2024
4 checks passed
@SebastianElvis SebastianElvis deleted the btcstaking-metrics branch February 12, 2024 00:04
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.

None yet

3 participants