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

Reduce the risk of having unregistered metrics #2204

Closed
pracucci opened this issue Mar 3, 2020 · 10 comments
Closed

Reduce the risk of having unregistered metrics #2204

pracucci opened this issue Mar 3, 2020 · 10 comments
Assignees
Labels
hacktoberfest stale type/chore Something that needs to be done; not a bug or a feature type/observability To help know what is going on inside Cortex

Comments

@pracucci
Copy link
Contributor

pracucci commented Mar 3, 2020

While doing code reviews I've realized a common mistake is to create metrics but not register them to the registry. Functions like promauto.NewXXX() are now discouraged due to #1512 and this makes even easier to forget.

In order to overcome this problem, Prometheus client 1.5.0 has introduced a new factory promauto.With(registerer).NewXXX() (713) which allows to use promauto with a custom registerer (thus honoring the design we picked in #1512).

The proposal to enforce all metrics to be registered is:

  1. Upgrade Prometheus client to 1.5.0
  2. Switch all metrics to promauto.With().NewXXX() taking the opportunity to pass the registerer everywhere so we will also solve Embed metrics into structs and optional initialization #1512
  3. Add a linter rule to forbid the import of github.com/prometheus/client_golang/prometheus (except few cases where it's OK, like the metrics mapping we do for the blocks storage and alertmanager)

For the records, this is the same path Thanos is following to avoid the same problem (actually the idea came from @bwplotka).

@pracucci pracucci changed the title Enforce all metrics to be registered to a registry Reduce the risk of having unregistered metrics Mar 3, 2020
@pracucci pracucci self-assigned this Mar 4, 2020
@stale
Copy link

stale bot commented May 3, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 3, 2020
@pracucci pracucci added keepalive Skipped by stale bot and removed stale labels May 4, 2020
@pracucci
Copy link
Contributor Author

pracucci commented May 4, 2020

It's a very low priority on-going work I continue every while and then.

@pracucci
Copy link
Contributor Author

Help wanted!

@pracucci pracucci added hacktoberfest type/chore Something that needs to be done; not a bug or a feature and removed keepalive Skipped by stale bot labels Sep 28, 2020
@roidelapluie
Copy link
Contributor

I will take this one.

@stale
Copy link

stale bot commented Nov 30, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 30, 2020
@pracucci
Copy link
Contributor Author

pracucci commented Dec 1, 2020

Still valid, even if over the time we've significantly improved it

@stale stale bot removed the stale label Dec 1, 2020
@bboreham bboreham added the type/observability To help know what is going on inside Cortex label Dec 29, 2020
@stale
Copy link

stale bot commented Mar 31, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 31, 2021
@pracucci pracucci removed the stale label Mar 31, 2021
@stale
Copy link

stale bot commented Jun 29, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 29, 2021
@pracucci
Copy link
Contributor Author

Still valid

@stale stale bot removed the stale label Jun 29, 2021
@stale
Copy link

stale bot commented Sep 28, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest stale type/chore Something that needs to be done; not a bug or a feature type/observability To help know what is going on inside Cortex
Projects
None yet
Development

No branches or pull requests

3 participants