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

Init metrics to zero #139

Merged
merged 8 commits into from
Jul 6, 2017
Merged

Init metrics to zero #139

merged 8 commits into from
Jul 6, 2017

Conversation

siimon
Copy link
Owner

@siimon siimon commented Jul 4, 2017

Fixing #118

Added fixes for Counters, Gauges and Histograms.. Summary I'm not sure what to do about tbh.. After a quick look it's not an easy fix.

@siimon siimon requested a review from SimenB July 4, 2017 17:48
@SimenB
Copy link
Collaborator

SimenB commented Jul 5, 2017

@siimon I pushed a fix for summaries, and added a snapshot test for full output.

Working on the case with labels

@siimon
Copy link
Owner Author

siimon commented Jul 5, 2017

Great! Thanks! :)

@SimenB
Copy link
Collaborator

SimenB commented Jul 5, 2017

Ok, should be done now.

Had to revert most of the test changes when considering labels

@siimon
Copy link
Owner Author

siimon commented Jul 5, 2017

LGTM!

@SimenB
Copy link
Collaborator

SimenB commented Jul 5, 2017

@dm3 @paulborza does this look correct? You can check the snapshot test for output with and without labels.

https://github.com/siimon/prom-client/pull/139/files#diff-23be0e4ea533e57b2bd533eaddfed48c

@paulborza
Copy link

@SimenB why initialize the metrics only when there are no labels? The metrics should be initialized to 0 across all labels.

@siimon
Copy link
Owner Author

siimon commented Jul 5, 2017

How would you do that since you don't know the values for the labels? You only know the label names when you create the metric

@paulborza
Copy link

@siimon You're right; my bad. The output looks good for metrics with no labels.
Argh, I just wish there was a way Prometheus would support some sort of wildcard syntax to initialize metrics across all dimensions/labels; but apparently it doesn't.

Thanks for fixing this! :)

@paulborza
Copy link

Once merged, can you also publish the new version to the registry? Thanks

@SimenB
Copy link
Collaborator

SimenB commented Jul 6, 2017

Once merged, can you also publish the new version to the registry? Thanks

Of course 🙂

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