feat: Support negative duration in new function ParseDurationAllowNegative #793
Conversation
There was a problem hiding this comment.
Looks good in general. I just have a nit about redundant code comments and overly short variable names.
However, there is also a more general concern: My assumption was this function is just used for templating, but it is used for many other purposes, too. It is not unlikely that those other purposes rely on the return never being negative.
Let's play safe for now and better introduce a new function that allows negative durations (e.g. ParseDurationAllowNegative or something). It will be easy to wire it to parseDuration in the templating.
WDYT?
|
@beorn7 actually has similar question before, i agree. what would you say now? mean what think about |
|
@beorn7 i really like everything, thanks, and i also improved TestParseDuration function in tests |
|
@beorn7 LGTY? |
There was a problem hiding this comment.
Looks great. I'll also tag a new release so that you can use this change in prometheus/prometheus#16619
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
Signed-off-by: Dmitry Ponomaryov <iamhalje@gmail.com>
21391c0 to
c099408
Compare
This PR extend the
ParseDurationfunction to support negative durations.negative durations need be useful in Loki alerts templates where time offsets such as '-5m' are common in range calculations and comparisons.
Changes:
ParseDurationto handle negative values like-1h,-30m, etc.related to promethes/template changes for Loki alerting - see prometheus/prometheus#16669