-
Notifications
You must be signed in to change notification settings - Fork 68
Implement timestamp() for native histograms #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
894d1c4 to
4a960b9
Compare
4a960b9 to
577abd2
Compare
| for _, vector := range in { | ||
| for i := range vector.Samples { | ||
| vector.Samples[i] = float64(vector.T / 1000) | ||
| out := o.pool.GetStepVector(vector.T) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these changes? Can't apply the same pattern to histograms as we do for floats?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we have both samples and histograms in the input StepVector, we need to return a StepVector with only Samples populated.
Eg:
load 30s
http_request_duration_seconds{pod="nginx-1"} {{schema:0 count:3 sum:14.00 buckets:[1 2]}}+{{schema:0 count:4 buckets:[1 2 1]}}x20
http_request_duration_seconds{pod="nginx-2"} 1x10 {{schema:0 count:2 sum:14.00 buckets:[2]}}+{{schema:0 count:6 buckets:[2 2 2]}}x10`
I'm not sure if we should do this in place.
for _, vector := range in {
for i := range vector.Samples {
vector.Samples[i] = float64(vector.T / 1000)
}
for i := range vector.Histograms {
vector.Samples = append(vector.Samples, float64(vector.T)/1000)
vector.SampleIDs = append(vector.SampleIDs, vector.HistogramIDs[i])
}
vector.Histograms = nil
vector.HistogramIDs = nil
}
}
577abd2 to
d2e54d7
Compare
|
Could you take another look @fpetkovski? |
|
@harry671003 would you mind rebasing this so we can merge it? |
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
d2e54d7 to
d4aec07
Compare
Notes
From the docs: https://prometheus.io/docs/prometheus/latest/querying/functions/#timestamp