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

Add Child notion to client #35

Closed
brian-brazil opened this issue Nov 3, 2016 · 3 comments
Closed

Add Child notion to client #35

brian-brazil opened this issue Nov 3, 2016 · 3 comments

Comments

@brian-brazil
Copy link

Currently labels are handled by being the first argument to all functions. This gives the incorrect impression that all metrics should labels (in reality most metrics don't have labels) and makes label-less use harder.

This client should follow the structure laid out in https://prometheus.io/docs/instrumenting/writing_clientlibs/#labels

In addition the user should be required to specify all their label names at metric creation time.

@grobie grobie added this to the v0.7.0 milestone Nov 3, 2016
@grobie grobie self-assigned this Nov 3, 2016
@grobie
Copy link
Member

grobie commented Nov 3, 2016

Label-less use does not get harder by having an optional labels parameter. See the first example from the readme. This is a common ruby pattern.

http_requests = prometheus.counter(:http_requests, 'A counter of HTTP requests made')

# start using the counter
http_requests.increment

Using a labels() method and calling metric functions on that will help performance-wise though. I added this to milestone v0.7.0.

@brian-brazil
Copy link
Author

Label-less use does not get harder by having an optional labels parameter.

The labels parameter is first, so if you want a non-default value it makes it harder.

@grobie grobie removed their assignment Nov 3, 2016
@grobie grobie modified the milestones: v0.7.0, v0.8.0 May 5, 2017
@dmagliola
Copy link
Collaborator

@brian-brazil this is now covered as part of the changes we made in PR #95. We require labels at creation time, they're no longer the first parameter, and the Child notion is implemented through the with_labels methods.

I'm going to close this issue due to this but feel free to reopen if you think there's still work to do on this.

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

No branches or pull requests

3 participants