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 statistics #3935

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@giganteous
Contributor

giganteous commented Jun 6, 2016

This set of commits tries to mirror the graphite stats as a /prometheus endpoint on the API webserver. Prometheus likes to call this directly instrumentated software.

Things I'd like to fix before a merge is

  • naming / documenting all metrics on g_dstates
  • naming / documenting all metrics on g_frontends
  • naming / documenting all metrics on g_pools
  • agreeing on the endpoint. Prometheus has a default of /metrics

But to get a general idea about if I'm solving this the correct way I'm creating the PR now.

To be clear: prometheus stats don't necessarily need a TYPE or HELP string but aids end users. Giving them the same name and structure should keep you from having to maintain 3 different branches where they're used, iiuc.

giganteous added some commits Jun 2, 2016

Add type and verbose information about metrics
Perhaps a real struct is better/easier to read. My opinion
is that creating a tuple is verbose, and reading a tuple in code is
erroneous.
code fixups from previous step
Tried creating a metrics entry in the g_dstates, but meh. Lets get
it working first.
A few bugfixes
- label keys are fine, labelvalues need to be quoted
- metric names cannot contain dash (-)
- added pool label to packetcache
make tests work
- requests.content is the body
- some typos
- test for label sanity
- allow empty lines
Small bug exposed by tests
The HELP should say something about the metric.
@giganteous

This comment has been minimized.

Contributor

giganteous commented Jun 6, 2016

Prometheus was mentioned in issue #2590, but does not fix the graphite/carbon naming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment