feat: Add Summary metric type #254
Conversation
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
|
cc @mxinden |
| #[cfg(feature = "summary")] | ||
| pub mod summary; |
There was a problem hiding this comment.
if we add this line...
| #[cfg(feature = "summary")] | |
| pub mod summary; | |
| #[cfg(feature = "summary")] | |
| #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] | |
| pub mod summary; |
we'll also make sure that this shows up in the documentation generated on docs.rs, see: https://doc.rust-lang.org/rustdoc/unstable-features.html#doccfg-recording-what-platforms-or-features-are-required-for-code-to-be-present
n.b. we'd need to also add this above in lib.rs, if it's not already present:
#![cfg_attr(docsrs, feature(doc_cfg))]There was a problem hiding this comment.
I use this trick for my crates and I'm glad to include this.
However, it seems a separated topic to this PR so I'd like to listen to maintainer's idea whether we should include it in this PR or add in a follow-up. Both works for me but different maintainers would have different preference.
There was a problem hiding this comment.
Fine either way. Preference for a separate pull request.
There was a problem hiding this comment.
OK. Then let's leave this PR as is.
|
Hi, is this feature still in progress? |
|
@codefever I'd like to. But without a maintainer involved I won't allocate more time since all may be in vain. |
Signed-off-by: tison <[email protected]>
|
Somehow I've switched to opentelemetry's metrics API, like apache/opendal#5524 |
There was a problem hiding this comment.
Thank you for picking up this effort.
Overall looks good to me. A couple of questions.
I am sorry this is not moving faster. Maintaining prometheus-client is a side project of mine and given that I don't have a usecase for Summary myself, I am not prioritizing it.
| @@ -22,8 +23,12 @@ dtoa = "1.0" | |||
| itoa = "1.0" | |||
| parking_lot = "0.12" | |||
| prometheus-client-derive-encode = { version = "0.5.0", path = "derive-encode" } | |||
|
|
|||
| # Optional dependencies | |||
| fastant = { version = "0.1", optional = true } | |||
There was a problem hiding this comment.
I am not familiar with fastant. Can you give a short summary why you chose it? I understand that instant is deprecated.
There was a problem hiding this comment.
why you chose it
We (the fastlabs members) maintain it and use it in large scale production env via fastrace. So far, so good.
It provides a monotonically non-decreasing clock, which is what is required here.
| /// Observe the given value. | ||
| pub fn observe(&self, v: f64) { | ||
| let mut inner = self.inner.write(); | ||
| self.rotate_buckets(&mut inner); |
|
Questions answered. Please let me know if there is any pending items. |
This closes #40.
This follows up #67.
With ...
instantis not maintained any more, usingfastantwhich works on wasm also.