Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@ordian
Copy link

@ordian ordian commented Sep 30, 2020

Part of #1482 (only alerts are missing now).

@ordian ordian added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Sep 30, 2020
@ordian ordian requested review from drahnr and rphmeier and removed request for drahnr September 30, 2020 15:51
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

LGTM, an improvement might be to use a custom enum and use the variants directly, to avoid the noisy (()) clutter (nit).


#[derive(Clone)]
struct MetricsInner {
gossipped_own_availability_bitfields: prometheus::Counter<prometheus::U64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not 100% sure about the prometheus API, but there should Gauges which might be more suitable?

Copy link
Author

Choose a reason for hiding this comment

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

Gauge is allowed to go down as well, here we count the total number of bitfields gossipped over time. Prometheus has query language to ask what would that be in a specific time range.

Any specific reason you think it might be more suitable?

Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

Added a few inline observations / possible impro topics (but really tiny nits and vague ideas)

@ordian ordian merged commit 5bb296b into master Oct 1, 2020
@ordian ordian deleted the ao-remaining-metrics branch October 1, 2020 10:08
ordian pushed a commit that referenced this pull request Oct 6, 2020
* master:
  Make collation an optional return (#1787)
  XCM: Land xcm-handler and xcm-executor (#1771)
  v0.8.25 (#1785)
  add two node local net script (#1781)
  Adjust max nominators down to 128 (from 256) (#1782)
  Companion for substrate/pull/7215 (#1768)
  Remove Stale Upgrades (#1780)
  Update Polkadot Weights for Substrate 2.0 (#1761)
  Parachains v1 registrar module. (#1559)
  Derive `From` for `AllMessages` and simplify `send_msg` (#1774)
  implement remaining subsystem metrics (#1770)
  Companion for paritytech/substrate#7236 (#1773)
  WIP: remove deprecated only/except clauses, build is now manual on PRs (#1769)
  Increase Westend `spec_version` (#1766)
  move Metrics to utils (#1765)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants