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

Make prometheus metrics follow naming conventions #332

Closed
6 tasks
petethepig opened this issue Aug 11, 2021 · 3 comments
Closed
6 tasks

Make prometheus metrics follow naming conventions #332

petethepig opened this issue Aug 11, 2021 · 3 comments
Labels
backend Mostly go code ops Features that help pyroscope operators

Comments

@petethepig
Copy link
Member

petethepig commented Aug 11, 2021

Link to naming conventions: https://prometheus.io/docs/practices/naming/

Minimally we should do things like:

  • add pyroscope_ prefix
  • add proper suffixes, like _total or _bytes
  • add tags where appropriate, e.g for badger metrics we should have trees and segments be tags, not parts of the name

This would also be a good time to get rid of metrics package I think.


After we update the names we should also update these dashboards:

  • the one in ./benchmark
  • the one in ./examples/grafana-integration
  • the one on grafana website
@petethepig petethepig added ops Features that help pyroscope operators backend Mostly go code labels Aug 11, 2021
@eh-am
Copy link
Collaborator

eh-am commented Aug 11, 2021

BTW

would be nice to add this prometheus metrics linter
https://github.com/yeya24/promlinter

it can be integrated with golangci-lint (yeah I know we are using revive, although it can be added to golangci-lint, see https://github.com/mgechev/revive#golangci-lint)

but anyway, to get the linter working it seems we would need to change/remove the metrics package, since the linter seems to expect a certain format (https://github.com/yeya24/promlinter/blob/master/promlinter.go#L191), plus having greppable metric names is always a good idea :)

@petethepig
Copy link
Member Author

@eh-am this is a great idea, I think we should remove metrics package and then we can add the linter

This was referenced Aug 12, 2021
@eh-am eh-am mentioned this issue Aug 23, 2021
@petethepig
Copy link
Member Author

Closing because this was implemented a while ago

korniltsev pushed a commit that referenced this issue Jul 18, 2023
* Working on agent doc

* Edit agent documentation

* Correct about the agent title

* Update docs/sources/operators-guide/configure-agent/about-the-agent.md

Co-authored-by: Christian Simon <simon@swine.de>

Co-authored-by: Christian Simon <simon@swine.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Mostly go code ops Features that help pyroscope operators
Projects
None yet
Development

No branches or pull requests

2 participants