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

Add blockdiff and blocknumber gauges for Prometheus #2770

Merged
merged 6 commits into from
Mar 28, 2022
Merged

Conversation

joaquincasares
Copy link
Contributor

Description

Implement Prometheus Gauges and add blockdiff and blocknumber metrics for the exporter using a collector function. Collector functions allow us to leave business logic next to related business logic within the same file, while exposing a group of gauges that should only be inspected at export/scrape time.

Tests

How will this change be monitored? Are there sufficient logs?

@dmanjunath
Copy link
Contributor

dmanjunath commented Mar 28, 2022

@joaquincasares just so i understand correctly what's happening here is

  1. Create a new type of collector for gauges
  2. Create and register gauges to run on sweep
  3. When the sweep happens, run get_health and return 1 if unhealthy, 0 otherwise?

Copy link
Contributor

@csjiang csjiang left a comment

Choose a reason for hiding this comment

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

Looks cool! I am still gathering context here but added a comment/q.

# unsure if overloading is supported.
if gauge:
if name not in PrometheusMetric.gauges:
PrometheusMetric.gauges[name] = Gauge(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we abstract these out into addPrometheusMetric('gauges'), addPrometheusMetric('histograms')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Phew! Thanks! I knew I didn't like the gauge=false bit since I would only extend to two types, but figured to punt that design choice until later.

I'll implement your suggestion, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh, I misread your suggestion, but hopefully the latest change is what you had in mind?

@joaquincasares
Copy link
Contributor Author

@joaquincasares just so i understand correctly what's happening here is

  1. Create a new type of collector for gauges
  2. Create and register gauges to run on sweep
  3. When the sweep happens, run get_health and return 1 if unhealthy, 0 otherwise?
  1. Correct.
  2. We don't have to collect gauges at exporter time, but I figured it would be cleaner to create our own "health check exporter" instead of having PrometheusMetrics objects within get_health(). Since health checks are fired routinely a collector isn't required here, but does make the code cleaner. For other gauges, however, a collector may be required.
  3. Whoops! Fixed the double negative logic. Thanks!

@joaquincasares joaquincasares merged commit ad94ec0 into master Mar 28, 2022
@joaquincasares joaquincasares deleted the jc-asi-932 branch March 28, 2022 22:18
joaquincasares added a commit that referenced this pull request Mar 28, 2022
* implement and use gauges for prometheus metrics

* use Callable, not func

* invert 0/1

* update interface for choosing gauges
@linear linear bot mentioned this pull request Jun 14, 2022
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
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.

None yet

3 participants