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

analytics(app): add telemetry to Evmos modules #637

Merged
merged 25 commits into from
Jun 1, 2022

Conversation

danburck
Copy link
Contributor

@danburck danburck commented May 30, 2022

Description

This PR adds telemetry to Evmos to analyze the performance of the modules. A guide on telemetry in the Cosmos SDK can be found here. By generating this data we can understand how Evmos is being used, e.g. we can observe how many users interact with incentivized contracts and how many tokens were distributed in total through incentives.

Metrics

I implemented the following metrics to observe and added them to the specs:

Metric Description Unit Type
tx_msg_convert_coin_amount_total Total amount of converted coins using a ConvertCoin msg token counter
tx_msg_convert_coin_total Total number of txs with a ConvertCoin msg tx counter
tx_msg_convert_erc20_amount_total Total amount of converted erc20 using a ConvertERC20 msg token counter
tx_msg_convert_erc20_total Total number of txs with a ConvertERC20 msg tx counter
tx_msg_ethereum_tx_total Total number of txs processed via the EVM tx counter
tx_msg_ethereum_tx_gas_used_total Total amount of gas used by an etheruem tx token counter
tx_msg_ethereum_tx_incentives_total Total number of txs with an incentivized contract processed via the EVM tx counter
tx_msg_ethereum_tx_incentives_gas_used_total Total amount of gas used by txs with an incentivized contract processed via the EVM token counter
incentives_distribute_participant_total Total number of participants who received rewards participant counter
incentives_distribute_reward_total Total amount of rewards that are distributed to all incentives' participants token counter
inflation_hook_allocate_total Total amount of tokens allocated through inflation token counter
inflation_hook_allocate_staking_total Total amount of tokens allocated through inflation to staking token counter
inflation_hook_allocate_incentives_total Total amount of tokens allocated through inflation to incentives token counter
inflation_hook_allocate_community_pool_total Total amount of tokens allocated through inflation to community pool token counter
recovery_ibc_on_recv_total Total number of recoveries using the ibc onRecvPacket callback recovery counter
recovery_ibc_on_recv_token_total Total amount of tokens recovered using the ibc onRecvPacket callback token counter

Notes

Reference used for naming endpoints: https://blog.pvincent.io/2017/12/prometheus-blog-series-part-2-metric-types/

Please also note that some endpoints are implemented in ethermint

@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #637 (49c0c39) into main (3be5018) will decrease coverage by 0.08%.
The diff coverage is 80.95%.

❗ Current head 49c0c39 differs from pull request most recent head 68b69a4. Consider uploading reports for the commit 68b69a4 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #637      +/-   ##
==========================================
- Coverage   82.61%   82.53%   -0.09%     
==========================================
  Files         122      122              
  Lines        6725     6870     +145     
==========================================
+ Hits         5556     5670     +114     
- Misses       1029     1055      +26     
- Partials      140      145       +5     
Impacted Files Coverage Δ
app/app.go 84.81% <ø> (ø)
x/incentives/keeper/epoch_hooks.go 73.68% <0.00%> (ø)
x/inflation/keeper/hooks.go 68.93% <9.37%> (-25.59%) ⬇️
x/inflation/keeper/inflation.go 79.78% <71.42%> (-2.44%) ⬇️
x/incentives/keeper/distribution.go 78.57% <94.11%> (+3.39%) ⬆️
x/erc20/keeper/msg_server.go 98.28% <100.00%> (+0.36%) ⬆️
x/incentives/keeper/evm_hooks.go 87.75% <100.00%> (+3.97%) ⬆️
x/recovery/keeper/ibc_callbacks.go 96.07% <100.00%> (+0.31%) ⬆️

@github-actions github-actions bot added the docs label May 30, 2022
@danburck danburck marked this pull request as ready for review May 30, 2022 21:15
@danburck danburck changed the title analytics(app): add telemetry for deliverTx analytics(app): add telemetry to Evmos May 30, 2022
@danburck danburck changed the title analytics(app): add telemetry to Evmos analytics(app): add telemetry to Evmos modules May 30, 2022
Copy link
Contributor

@Prosp3r Prosp3r left a comment

Choose a reason for hiding this comment

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

I've become very eagle-eyed due to constantly looking and the linter errors.
I think we could either refactor as we improve on each module or enter improvements and let the Linter do its work when implemented so we can then add all warnings to the sub issues in ENG-284

x/erc20/keeper/msg_server.go Show resolved Hide resolved
x/erc20/keeper/msg_server.go Show resolved Hide resolved
x/inflation/keeper/hooks.go Show resolved Hide resolved
x/recovery/keeper/ibc_callbacks.go Show resolved Hide resolved
@danburck danburck marked this pull request as draft June 1, 2022 08:09
@danburck danburck marked this pull request as ready for review June 1, 2022 09:48
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

based on our conversation

  • erc20 module:
    • total number of transactions (counter with labels for denom and erc20 contract)
    • total amount converted (counter with labels for denom and erc20 contract)
  • on inflation, we care about
    • the total amount of tokens minted (not epoch count)
  • for incentives, we need to track the
    • total amount of rewards allocated per contract (counter with label)
    • total number of participants (labeled per contract)
    • number of rewards allocated per participant (eg: to track bots)
  • recovery: also add the amount of tokens recovered (counter)
  • fees:
    • total amount fees allocated to devs/contract deployers (label by contract) and validators (no label since it's allocated to the fee collector Module Account)

x/incentives/keeper/distribution.go Show resolved Hide resolved
x/incentives/keeper/evm_hooks.go Show resolved Hide resolved
x/incentives/keeper/evm_hooks.go Show resolved Hide resolved
@danburck
Copy link
Contributor Author

danburck commented Jun 1, 2022

@fedekunze

  • number of rewards allocated per participant (eg: to track bots)

This would have to be implemented here

I'm considering if this will be to nested, what do you think?

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM, Minor comments.

x/incentives/keeper/distribution.go Outdated Show resolved Hide resolved
x/inflation/keeper/hooks.go Outdated Show resolved Hide resolved
x/inflation/keeper/inflation.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
docs/protocol/telemetry.md Outdated Show resolved Hide resolved
x/incentives/keeper/distribution.go Outdated Show resolved Hide resolved
@fedekunze fedekunze requested a review from Prosp3r June 1, 2022 16:52
@fedekunze fedekunze dismissed Prosp3r’s stale review June 1, 2022 16:53

changes requested are not relevant in this PR

Copy link
Contributor

@Prosp3r Prosp3r left a comment

Choose a reason for hiding this comment

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

LGTM

@fedekunze fedekunze enabled auto-merge (squash) June 1, 2022 18:36
@fedekunze fedekunze merged commit 7649ee0 into main Jun 1, 2022
@fedekunze fedekunze deleted the ENG-382-add-telemetry-cosmos-tx branch June 1, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants