Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Prometheus metrics don't follow best practices #489

Closed
aabouzaid opened this issue Sep 21, 2020 · 2 comments
Closed

Prometheus metrics don't follow best practices #489

aabouzaid opened this issue Sep 21, 2020 · 2 comments

Comments

@aabouzaid
Copy link
Contributor

Hello,

Currently, KES Prometheus metrics (last_state and sync_calls) don't follow Prometheus metric names best practices.

That's because:

A metric name...

  • ...should have a (single-word) application prefix relevant to the domain the metric belongs to. [...]
  • ...should have a suffix describing the unit, in plural form. [...]

Without a prefix, suffix, or unites it's hard to identify KES metrics and work them.

I'd be happy to make a PR for this change, but since it's a breaking change, I'd like to hear which strategy to follow to deprecate the old metrics.

@lwille
Copy link

lwille commented Sep 22, 2020

I'd suggest the following strategy to introduce the new metric and phase out the old one:

  • implement new prefixed metric in addition to the old one
  • a new metric doesn't break anything, so it can be released any time
  • mark the "old" metric deprecated in the code and schedule its deletion for the next major version release

@Flydiverny
Copy link
Member

Flydiverny commented Oct 11, 2020

Sounds good, PR welcome 😄
Ideally we could solve #399 as well, but think we need to introduce some more magic than just renaming for that 😬

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants