extend histogram cortex_distributor_sample_delay_seconds_bucket to track negative delays #6838
Conversation
Signed-off-by: Mauro Stettler <[email protected]>
replay
changed the title
replay
changed the title
Signed-off-by: Mauro Stettler <[email protected]>
Signed-off-by: Mauro Stettler <[email protected]>
| if delta > 0 { | ||
| d.sampleDelayHistogram.Observe(float64(delta) / 1000) | ||
| } | ||
| d.sampleDelayHistogram.Observe(float64(delta) / 1000) |
There was a problem hiding this comment.
I'm wondering how expensive it is to call Observe() for every single sample. Do you have a benchmark we can run, to compare it before vs after this change?
There was a problem hiding this comment.
I thought that it is safe to assume that the vast majority of samples which we receive have a delta that is > 0, because all the ones with a timestamp that is older than now will have delta > 0.
removing the delta > 0 condition will only make us also observe those where the sample timestamp is in the future relative to now, which I would expect to be a tiny fraction of all samples that we receive. so the increase in the rate of calls to .Observe() is very unlikely to be significant.
There was a problem hiding this comment.
You're totally right. I initially read the code the other way. I'm good, thanks.
| if delta > 0 { | ||
| d.sampleDelayHistogram.Observe(float64(delta) / 1000) | ||
| } | ||
| d.sampleDelayHistogram.Observe(float64(delta) / 1000) |
There was a problem hiding this comment.
You're totally right. I initially read the code the other way. I'm good, thanks.
We would like to track if there are users who send us samples with timestamps which are in the future, relative to wall clock time.
This change doesn't need to be permanent, we can revert it in a few weeks from now.