Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement Summary metric #40

Open
folex opened this issue Jan 24, 2022 · 6 comments · May be fixed by #67
Open

Implement Summary metric #40

folex opened this issue Jan 24, 2022 · 6 comments · May be fixed by #67
Assignees
@palash25
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@folex
Copy link

folex commented Jan 24, 2022

Hi! Thank your for the project :)

I found myself missing an implementation of the Summary metric, so decided to file an issue in case anyone (maybe myself) decides to contribute an implementation.

Open Metrics spec defines a metric type that computes quantiles locally on the client: Summary.

It's quite useful if you want to learn/discover how a system behaves, especially if you don't have much data a-priori. In that sense, Summary is dual to Histogram - both can be used to understand data distribution (e.g. latency data), but with different use-cases and tradeoffs.

Good overview on the differences between Summary and Histogram metrics is given in Prometheus doc https://prometheus.io/docs/practices/histograms/
@mxinden
Copy link
Member

mxinden commented Jan 24, 2022

Thank your for the project :)

Glad that it is useful. ❤️

I found myself missing an implementation of the Summary metric, so decided to file an issue in case anyone (maybe myself) decides to contribute an implementation.

Would be good to support the Summary metric type. Contributions are most certainly welcome!

Whoever is picking this up, let me know if you need any help.

@mxinden mxinden added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 24, 2022
@palash25
Copy link

@mxinden hi i would like to work on this could you assign the issue to me?

@mxinden
Copy link
Member

mxinden commented Jun 10, 2022

Done. Thanks @palash25. Let me know in case you need any help.

@palash25
Copy link

hi @mxinden do you have any preference on what crate to use for the underlying quantile algorithm? I found one that implements CKMS would it be ok to use this? https://github.com/blt/quantiles

@mxinden
Copy link
Member

mxinden commented Jun 22, 2022

@palash25 unfortunately I don't have any experience with quantile algorithms, neither in general nor in Rust. Thus no preference. Sorry.

I found one that implements CKMS would it be ok to use this? https://github.com/blt/quantiles

Looks fine to me.

@palash25 palash25 linked a pull request Jun 26, 2022 that will close this issue
Open
@palash25
Copy link

hi @mxinden sorry for the multiple pings but can you please take a look at this #67 (comment) ? i updated the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@palash25 palash25

Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Summary metric type palash25/client_rust
3 participants
@folex @mxinden @palash25