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

Prometheus: append postfix only if missing #773

Merged

Conversation

eugenemiretsky
Copy link
Contributor

Append metric name postfix only if missing.
It help

  1. Avoid weird metric names like "my_counter_total_total"
  2. Allow naming metrics in a way that results in same metric name with tools that don't rename metrics (statsD Exportorter)
  3. Standardizes with Micrometer

Copy link
Contributor

@dpsoft dpsoft left a comment

Choose a reason for hiding this comment

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

hey @eugenemiretsky thanks for proposing this PR.

@ivantopo
Copy link
Contributor

ivantopo commented May 6, 2020

Hey @eugenemiretsky, thanks a lot for the PR!

The changes look fine but I do have some questions about items 2. and 3. from your list. Could you please share a bit more details of why 2. and 3. were necessary or what problems were you seeing without this changes?

That info will help us understand if something else should be taken into account before merging and how would this affect users who already have these issues!

@eugenemiretsky
Copy link
Contributor Author

We have the same service deployed in serveral environment

  1. K8s, where the Kamon Prometheus plugin is used to export metrics
  2. AWS, where we set metric to a Prometheus statsD exporter (using Kamon Datadog, though we should probably switch to the statsD plugin ). Because of the renaming, we cannot use the same dashboard in all environments - one way around it is to name all our metrics using the Prometheus naming convention, so we have the same metric names n statsD and as with Kamon Prometheus. Micrometer does something similar to this PR.

@eugenemiretsky
Copy link
Contributor Author

Generally though, I feel like if as a develop I already follow the Prometheus naming convention, it would be weird to see my metrics as my_counter_total_total

@eugenemiretsky
Copy link
Contributor Author

Coming to think of it, would be nice too have Kamon itself follow similar naming convention (or have an option to turn it on )

@ivantopo ivantopo merged commit 1cbdc61 into kamon-io:master May 18, 2020
@ivantopo
Copy link
Contributor

Thanks a lot @eugenemiretsky!

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

Successfully merging this pull request may close these issues.

3 participants