[FEAT] Adding TTL to cleanup old metrics#61
[FEAT] Adding TTL to cleanup old metrics#61nicolastakashi wants to merge 1 commit intoweaveworks:masterfrom
Conversation
nicolastakashi
force-pushed
the
master
branch
3 times, most recently
from
9917f90 to
ec64fb9
Compare
|
Hi @nicolastakashi |
|
Hey @shlompy thanks for helping me and sorry for the late reply, I was on holiday. |
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
I read through this change. It looks plausible, however (a) I don't run this code and (b) I don't think Weaveworks run this code either, since Weave Cloud was shut down.
I don't work for Weaveworks any more, so not going to say what is best for the project in future.
I see @shlompy suggested a fix, which does not appear to be in this PR. Please can you clarify whether it was accepted or rejected. Either way, I would suggest a test showing that the problem @shlompy reported does not occur.
| histogram_bucket{le="+Inf"} 4 | ||
| histogram_sum{} 2.5 | ||
| histogram_count{} 1 | ||
| histogram_bucket{le="1"} 0 %[1]d |
There was a problem hiding this comment.
All these changes to add %[1]d to metric output are mysterious to me. Perhaps caused by an upstream change?
I prefer when all changes are explained by the commit message.
3 participants
I'm taking over this PR #45