[generator] Insert leading zero value for new counter series #2068
Conversation
zalegrala
changed the title
zalegrala
force-pushed
the
remoteWriteCounter0
branch
2 times, most recently
from
272f86a to
b6cc556
Compare
New counters are created with null values, which causes Prometheus to ignore the first value to avoid the comparison of null -> x. Here we allow for first values on new series to be more useful by appending first a zero to the series, followed by a value. This will allow Prometheus queries to use the first value of 0 -> x rather than null -> x.
Signed-off-by: Zach Leslie <[email protected]>
Signed-off-by: Zach Leslie <[email protected]>
Signed-off-by: Zach Leslie <[email protected]>
Signed-off-by: Zach Leslie <[email protected]>
zalegrala
force-pushed
the
remoteWriteCounter0
branch
from
b6cc556 to
dcff162
Compare
zalegrala
requested review from
joe-elliott,
annanay25,
mdisibio and
mapno
as code owners
| _, err = appender.Append(0, lb.Labels(nil), timeMs, 0) | ||
| if err != nil { | ||
| return | ||
| } | ||
| // Increment timeMs to ensure that the next value is not at the same time. | ||
| t = t.Add(insertOffsetDuration) |
There was a problem hiding this comment.
I'm wondering if we could just do timeMs - 1000. Instead of delaying our sample, write a zero sample before it. This might trigger out-of-order writes though... I'm not sure if Prometheus checks for out-of-order writes within the whole batch or just for the serie.
There was a problem hiding this comment.
I wondered about this also, but avoided going negative offset because I wasn't sure about the interplay with Prometheus and was somewhat hard to test. Do you think this will impact the usage having a delay on our sample?
There was a problem hiding this comment.
It adds latency between when the span arrived and when we report it has arrived, so the data becomes more inaccurate. But since we already aggregate data in the collection interval (which is typically 15s) I don't think this will be significant. So I'm fine with going for the safe options and just adding a 1s offset.
Signed-off-by: Zach Leslie <[email protected]>
What this PR does:
Here we allow new values on counter series to be used by Prometehus by inserting a leading 0 into new counter series.
Which issue(s) this PR fixes:
Fixes #2006
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]