promql: Add warning for too many resets in rate().#16860
Conversation
a8c4df8 to
2e3d3d1
Compare
bboreham
left a comment
There was a problem hiding this comment.
Thanks for this, it looks promising.
Could you add some tests? E.g. that the annotation does appear when you expect, and doesn't appear when it shouldn't.
| } | ||
|
|
||
| resetRatio := float64(resetCount) / float64(numSamplesMinusOne) | ||
| if resetRatio > 0.6 { |
There was a problem hiding this comment.
At least this should be a const, probably configurable.
There was a problem hiding this comment.
probably configurable
the tricky part with a global configuration is that acceptable counter reset ratios might be metric specific?
There was a problem hiding this comment.
Noting that this is a ratio, please give an example of what you're thinking about.
Probably this discussion should be on the issue, which is where 0.6 is proposed.
2e3d3d1 to
da059e2
Compare
| http_requests_total{job="api2"} 10 20 5 6 25 | ||
|
|
||
| eval instant at 5m rate(http_requests_total{job="api"}[5m]) | ||
| expect info |
There was a problem hiding this comment.
can you use the new expect syntax to assert the exact msg too, see ref
NeerajGartia21
left a comment
There was a problem hiding this comment.
@bboreham, Do you think we should apply this reset check for histograms as well? Also, I think we should consider adding a similar annotation for the increase function when excessive resets are detected.
|
|
||
| eval instant at 5m rate(http_requests_total{job="api2"}[5m]) | ||
| expect no_info | ||
| {job="api2"} 0.1388888888888889 |
There was a problem hiding this comment.
Please fix the indentations here.
There was a problem hiding this comment.
weird, I am not seeing any extra indentation in file.
There was a problem hiding this comment.
That might be due to your IDE settings. Check if your tab size is set to 4 or 8 spaces, and ensure you're using either tabs or spaces consistently. That should fix the indentation mismatch.
da059e2 to
c62d412
Compare
fixes: prometheus#16618. Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
c62d412 to
b993120
Compare
|
If the user does not set the flag( |
|
Hello from the bug scrub! This PR seems to have become somewhat stale @sujalshah-bit are you going to return to this? Also please note the work on delta metrics will likely introduce changes to reset detection and new use-cases (e.g. #18344) |
fixes: #16618.