-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add statistical aggregation functions #4208
Conversation
c5996ca
to
96d4b79
Compare
Could you also add something for median and standard deviation, assuming it's as easy? They often go together. |
Bit harder to do, but certainly not a problem. If I find some more time to hack on code this weekend I'll consider it, otherwise feel free to just use |
96d4b79
to
3fcef64
Compare
mean
aggregation function
Done. Mind giving this another review given that there were major changes since Jannis' initial approval? |
I needed `mean` for an ingress/egress chart and was surprised we didn't have that yet. This was easy enough to implement, so I just went ahead and did it. Based on the initial review feedback this also adds `stddev`, `variance`, `approximate_median`, and `collect`.
3fcef64
to
1ef6c4c
Compare
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.
Great work!
Approving modulo missing integration tests.
I'm a bit worried about the error bounds of using t-digest for incremental median computation, but having even an approximate value is already very helpful. IIRC the error bounds do not hold upon when composing t-digest sketches, but that's not an issue in our case.
I'll add some before merging. For now I mostly verified that this works as expected by manually comparing values. That's why I implemented |
Co-authored-by: Matthias Vallentin <matthias@vallentin.net>
I needed
mean
for an ingress/egress chart and was surprised we didn't have that yet. This was easy enough to implement, so I just went ahead and did it.Based on the initial review feedback this also adds
stddev
,variance
,approximate_median
, andcollect
.