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.

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

Feature request: add rate #37

Closed
msaf1980 opened this issue Dec 21, 2018 · 6 comments
Closed

Feature request: add rate #37

msaf1980 opened this issue Dec 21, 2018 · 6 comments

Comments

@msaf1980
Copy link

msaf1980 commented Dec 21, 2018

Add
rate as sum / (carbon.interval / 1000)
sample_rate as agg.len() / (carbon.interval / 1000)

@Albibek
Copy link
Collaborator

Albibek commented Dec 21, 2018

We think about adding lua for processing metrics before they are sent to carbon. It should solve most of the post-aggregation problems better than single cases

@msaf1980
Copy link
Author

msaf1980@046382e
Quick and ugly realization. Unsafe was used, because I need global mutable variable (filled with value from config file).

@Albibek
Copy link
Collaborator

Albibek commented Jan 16, 2019

You can use a lazy_static to stay safe.
And I'm still unsure if we should add functionality that can easily be done on storage side, i.e. using some graphite function. This would make bio too bloated and monolithic, and we'd like to avoid this.

@loyd
Copy link

loyd commented Sep 24, 2020

@Albibek
I'm surprised that it's not implemented here, at least as optional settings.

Firstly, modern statsd sends .rate and .count metrics, and it has the behavior like bioyino only with legacyNamespace enabled.

Secondly, such metrics highly rely on an interval of aggregation (that defaults to 10s), and it can be tricky to implement rate calculation based on total count only. Could you explain how you use graphite functions to do it? We use mhr3/brubeck (supports .rate) and graphite, and there is a visible (but not dramatically) difference between .rate and perSecond(.count) even in ideal conditions with uniform distribution. Using data from DB we've detected that the precalculated metric is more precise than produced by graphite. The situation worsens in different cases: missing data points, after retention (obviously with right aggregation settings in graphite), irregular grid etc. Thus, the precalculated metrics are more reliable.

we'd like to avoid this.

We write in rust and want to replace brubeck with bioyino, but the absence of .rate is frustrating. Is your position final?

@Albibek
Copy link
Collaborator

Albibek commented Sep 25, 2020

Considering your request is another one here, I think this can and should be implemented. Also, some time ago I was worrying about increasing the number of unique names for ms, but since 0.7 (not yet released) it's possible to define aggregations in config, so this is not a problem.

@Albibek
Copy link
Collaborator

Albibek commented Oct 24, 2020

@loyd @msaf1980 The rate aggregation has been added into 0.8.0-alpha2 tag

Note, that it is available for all metrics, because of the update counter value being used. So for i.e. set or counter the rate is also available, but for set it will not always be equal to len(set)/aggregation_period.

Could be great if you tested this somewhere so I am sure it works as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants