hermes icon indicating copy to clipboard operation
hermes copied to clipboard

Metrics discovery phase

Open AlianBenabdallah opened this issue 2 years ago • 3 comments

Partially closes: #2479

Description

Introduces a discovery phase to initialize the metrics.

Changes :

  • state.rs now contains functions to initialize metrics specific to a chain / client / channel / path.
  • Hermes now scans connections and channels in every case. Previously, the relayer would scan channels only when exact filters were specified in the config, otherwise it would just scan every client.
  • Minor code improvements

Note :

  • tx_latency* metrics are still not initialized because a histogram can only be initialized by adding a value and values can't be removed.
  • The impact of the discovery phase on performances is still unknown. On a small test net, there is no degradation.

PR author checklist:

  • [x] Added changelog entry, using unclog.
  • [ ] Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • [x] Linked to GitHub issue.
  • [ ] Updated code comments and documentation (e.g., docs/).
  • [x] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • [ ] Reviewed Files changed in the GitHub PR explorer.
  • [ ] Manually tested (in case integration/unit/mock tests are absent).

AlianBenabdallah avatar Aug 02 '22 10:08 AlianBenabdallah

Great job @AlianBenabdallah ! It might be better to only discover the metrics which depend on the configuration tx_confirmation = true if the configuration is set. The metrics are ibc_receive_packets, ibc_acknowledgment_packets, ibc_timeout_packets and tx_latency_confirmed

Regarding tx_latency*, do you know how to initialize these metrics without adding a new value ?

AlianBenabdallah avatar Aug 04 '22 17:08 AlianBenabdallah

Great job @AlianBenabdallah ! It might be better to only discover the metrics which depend on the configuration tx_confirmation = true if the configuration is set. The metrics are ibc_receive_packets, ibc_acknowledgment_packets, ibc_timeout_packets and tx_latency_confirmed

Regarding tx_latency*, do you know how to initialize these metrics without adding a new value ?

I haven't found a way to initialise the latencies without adding a new value thus distorting the histogram. But in my opinion it is not critical to discover the two metrics tx_latency_submitted and tx_latency_confirmed.

ljoss17 avatar Aug 08 '22 11:08 ljoss17

Great job @AlianBenabdallah ! It might be better to only discover the metrics which depend on the configuration tx_confirmation = true if the configuration is set. The metrics are ibc_receive_packets, ibc_acknowledgment_packets, ibc_timeout_packets and tx_latency_confirmed

It should be solved in the last commit. I also took into account ibc_client_updates, ibc_client_misbehaviours and clear_* metrics.

AlianBenabdallah avatar Aug 08 '22 12:08 AlianBenabdallah