Refactor MultiProcessCollector.collect() to allow for arbitrary merging. #302
Conversation
|
You've test failures, and need to add a DCO. |
f4cde59 to
34c366d
Compare
prometheus_client/multiprocess.py
Outdated
| try: | ||
| from collections import OrderedDict | ||
| except ImportError: | ||
| OrderedDict = dict |
There was a problem hiding this comment.
The code should still work on 2.6
There was a problem hiding this comment.
The code works, but the order is not preserved on 2.6, so it's different behaviour
There was a problem hiding this comment.
Rather than changing this, how about changing the format of the multiprocess key to be a dict for the labels and using sort_keys=True?
2f12be2 to
afaf04d
Compare
prometheus_client/multiprocess.py
Outdated
| else: | ||
| # The duplicates and labels are fixed in the next for. | ||
| metric.add_sample(name, tuple(zip(labelnames, labelvalues)), value) | ||
| metric.add_sample(name, tuple(labels.items()), value) |
There was a problem hiding this comment.
Is the ordering here deterministic?
There was a problem hiding this comment.
No, good catch.
prometheus_client/core.py
Outdated
| @@ -844,6 +851,11 @@ def _floatToGoString(d): | |||
| return '-Inf' | |||
| elif math.isnan(d): | |||
| return 'NaN' | |||
| elif sys.version_info[:2] == (2, 6): | |||
There was a problem hiding this comment.
It was todo with trying to fix that fact that repr of float in 2.6 is different. Ended up just skipping those tests on 2.6.
|
Can you resolve the conflict? I just merged a large reworking of the library to support openmetrics. |
Factors out a merge() method from the previous collect() method, which is parameterized, and thus can be used for arbitrary merging of samples. For motivation, see discussion in issue prometheus#275 around merging dead workers data into a single mmaped file. This basically allows us to parameterize the files to be merged, and also whether to accumulate histograms or not. Accumulation is on by default, as that is what the prometheus format expects. But it can now be disabled, which allows merged values to be correctly written back to an mmaped file. For the same reason, the order of labels is preserved via OrderedDict. Signed-off-by: Simon Davy <simon.davy@canonical.com>
Signed-off-by: Simon Davy <simon.davy@canonical.com>
Signed-off-by: Simon Davy <simon.davy@canonical.com>
Signed-off-by: Simon Davy <simon.davy@canonical.com>
…ion differences make it too difficult Signed-off-by: Simon Davy <simon.davy@canonical.com>
Signed-off-by: Simon Davy <simon.davy@canonical.com>
Signed-off-by: Simon Davy <simon.davy@canonical.com>
c955f42 to
a4370a5
Compare
|
Rebased on latest master |
|
Thanks! |
|
Awesome, thanks :) |
|
Thanks @bloodearnest for this feature. @brian-brazil : any idea when you might release the next version including this ? |
|
I'm waiting to get all the openmetrics stuff in, and then release. |
…ng. (#302) * Refactor MultiprocessCollector.collect to allow for arbitrary merging. Factors out a merge() method from the previous collect() method, which is parameterized, and thus can be used for arbitrary merging of samples. For motivation, see discussion in issue #275 around merging dead workers data into a single mmaped file. This basically allows us to parameterize the files to be merged, and also whether to accumulate histograms or not. Accumulation is on by default, as that is what the prometheus format expects. But it can now be disabled, which allows merged values to be correctly written back to an mmaped file. Signed-off-by: Simon Davy <simon.davy@canonical.com>
Factors out a merge() method from the previous collect() method, which
is parameterized, and thus can be used for arbitrary merging of samples.
For motivation, see discussion in issue #275 around merging dead worker's
data into a single mmaped file.
This basically allows us to parameterize the files to be merged, and
also whether to accumulate histograms or not. Accumulation is on by
default, as that is what the prometheus format expects. But it can now
be disabled, which allows merged values to be correctly written back to
an mmaped file. For the same reason, the order of labels is preserved
via OrderedDict.