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

Prometheus metric wallet_balance incorrect #2381

Closed
1 of 5 tasks
mturkia opened this issue Jul 6, 2022 · 5 comments · Fixed by #2440
Closed
1 of 5 tasks

Prometheus metric wallet_balance incorrect #2381

mturkia opened this issue Jul 6, 2022 · 5 comments · Fixed by #2440
Assignees
Labels
A: bug Admin: something isn't working A: good-first-issue Admin: good for newcomers I: telemetry Internal: related to Telemetry & metrics
Milestone

Comments

@mturkia
Copy link

mturkia commented Jul 6, 2022

Summary of Bug

The wallet_balance prometheus metrics converts the account balance by dividing value by 10^6, which works for most Cosmos chains, but does not work e.g. for Crypto.org chain where it should be 10^8, or evmos with 10^18. There are also other chains with base unit something else than micro.

Version

0.15.0+e3d0b108

Steps to Reproduce

Configure Hermes with crypto.org (or fetch, evmos, provenance) and add e.g. 1 cro (100000000basecro) to the account. Observe that wallet_balance gives you 100cro as balance.

Acceptance Criteria

Correct chain specific denom exponent is used to calculate account balance.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@romac romac added A: bug Admin: something isn't working A: good-first-issue Admin: good for newcomers I: telemetry Internal: related to Telemetry & metrics labels Jul 8, 2022
@romac romac added this to the post-v1 milestone Jul 8, 2022
@romac romac changed the title Prometheus metric wallet_balance incorrect Prometheus metric wallet_balance incorrect Jul 13, 2022
@ljoss17 ljoss17 self-assigned this Jul 13, 2022
@ljoss17
Copy link
Contributor

ljoss17 commented Jul 15, 2022

Hi @mturkia we can suggest the following solution. Instead of parametrising the value used to divide the balance, we will correct the description of the wallet_balance metric to be accurate. If I take your example of a wallet balance being 100000000 basecro, the new output would look like this:

wallet_balance{account=<ACCOUNT_ADDRESS>,chain=<CHAIN_ID>,denom="basecro 10E-6"} 100

Would this solution be acceptable ?

@romac
Copy link
Member

romac commented Jul 19, 2022

Some operators are reporting that they would like to keep the raw balance as was previously reported. In fact, they use wallets with small amount of coins which fit into a u64 and are therefore not interested in the divided value which is throwing off their metrics. I would suggest we still add a setting to specify by how much to divide or more simply by how many decimals to remove from the balance (eg. a setting of 6 would divide by 10^6).

@mturkia
Copy link
Author

mturkia commented Jul 19, 2022

I think that from usage point of view, the raw balance would be better than always dividing by 6. So either raw value, or correct base unit value.

@romac
Copy link
Member

romac commented Jul 19, 2022

We cannot do the raw value only because some operators use wallets with with a high balance which overflows the u64 counter (the Rust OpenTelemetry crate currently only support 64-bits counters) but with this change you should be able to get the raw value by eg. setting the exponent to 0 if we take it to mean "divide by 10^{exponent}". Would that work for you?

@mturkia
Copy link
Author

mturkia commented Jul 19, 2022

Should be fine for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: bug Admin: something isn't working A: good-first-issue Admin: good for newcomers I: telemetry Internal: related to Telemetry & metrics
Projects
No open projects
Status: Closed
4 participants