Add histogram vectors to model#417
Conversation
2c58700 to
63beb45
Compare
e40cdbb to
72dc02e
Compare
|
Cool. I hope I can help with a review soon… |
beorn7
left a comment
There was a problem hiding this comment.
Thanks for doing this. Looks good overall.
I have identified a few issues, I believe. See comments. Let me know what you think.
(I'll do a detailed review once we have clarified those issues.)
codesome
left a comment
There was a problem hiding this comment.
Looking good overall. I just have a lot of nits and agree with Beorn's comment about not requiring the Type field.
PS: this review does not cover the unit tests.
14ed1f8 to
57e36a0
Compare
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
… enum Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
…sion - experimental) Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
57e36a0 to
84e64de
Compare
|
Quick note: Bumping the minimum required Go version is a bit of an issue as prometheus/client_golang has to be conservative in this regard (and it uses prometheus/common). Currently, client_golang requires go1.17. Generally, they try to support the last three Go minor releases at the very least. In short, I think, it would be better at this time to avoid generics and only require go1.17. |
codesome
left a comment
There was a problem hiding this comment.
Nice work! I just have a bunch more nits with some questions for Björn.
…g go version - experimental)" This reverts commit 84e64de. Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
41cdb02 to
6a3d5c0
Compare
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
This reverts commit f451854. Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
6a3d5c0 to
0267435
Compare
beorn7
left a comment
There was a problem hiding this comment.
Great. Thank you. Just two commenting nits.
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
|
I assume if @roidelapluie had issues with this, he would have chimed in by now. So, merging now… Thank you very much, @zenador . |
Addresses prometheus/client_golang#1179
Tested the JSON decoding successfully by scraping a simple server running native histograms with Prometheus, then using the HTTP API in
client_golang(which does not need to be updated, only this library needs updating) to query it.