prw2.0: Added support for "custom" layouts for native histogram proto #13558
Conversation
bwplotka
changed the title
bwplotka
force-pushed
the
bwplotka/prw-custom1
branch
from
f5912af
to
aedb080
Compare
Result of the discussions: * #13475 (comment) * https://cloud-native.slack.com/archives/C02KR205UMU/p1707301006347199 Signed-off-by: bwplotka <bwplotka@gmail.com>
bwplotka
force-pushed
the
bwplotka/prw-custom1
branch
from
aedb080
to
a41d56b
Compare
Result of the discussions: * #13475 (comment) * https://cloud-native.slack.com/archives/C02KR205UMU/p1707301006347199 Signed-off-by: bwplotka <bwplotka@gmail.com> # Conflicts: # prompb/write/v2/types.pb.go
bwplotka
force-pushed
the
bwplotka/prw-custom1
branch
from
a41d56b
to
6364db3
Compare
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com> Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
prompb/write/v2/types.proto
Outdated
| // The last element is a lower bound for the implicit +Inf bucket (the overflow | ||
| // bucket). |
There was a problem hiding this comment.
That's not consistent with the above (which says that the +Inf bucket is marked by pointing to len(custom_values)).
There was a problem hiding this comment.
OK, I take that back. It is consistent, but it is confusing because that element is also the upper bound of the last non-Inf bucket.
Maybe word that better. Suggestion: "The last element is not only the upper inclusive bound of the last regular bucket, but implicitly the lower exclusive bound of the +Inf bucket."
There was a problem hiding this comment.
Or remove this sentence entirely, as things are explained above for the +Inf bucket.
There was a problem hiding this comment.
I would mention this, will use your suggestion.
prompb/write/v2/types.proto
Outdated
| // custom_values is an additional field used by non-exponential bucketing layouts. | ||
| // | ||
| // For custom buckets (-53 schema value) custom_values specify monotonically | ||
| // increasing upper inclusive boundary for the bucket counts with arbitrary |
Signed-off-by: bwplotka <bwplotka@gmail.com>
|
Addressed all and fixed tests. Thanks everyone for patience! |
|
Yea, this might get replaced with new protobuf (+gogo removal) |
Result of the discussions:
cc @krajorama @beorn7 @zenador