-
Notifications
You must be signed in to change notification settings - Fork 140
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
feat(functions): add support for consolidateBy #793
Conversation
- adding names to fields allows addition of other fields to MetricRequest without breaking backwards compatibility
The first commit contains changes that should be reviewed while the second are refactor changes to make code compile properly. |
Hello!
Are you suggesting to add this as an extension? AFAIK stock Graphite Web (+Whisper) does not support this. The aggregation function is already set and fixed on write time (through |
Graphite web have support for transferring target to backend, and it was
used in some non standard backends. Just FYI.
ср, 23 авг. 2023 г., 16:56 Nicolás Pazos ***@***.***>:
… Hello!
consolidateBy should send argument to backend so that metrics can be
rollup using the aggregation function specified in the function.
Are you suggesting to add this as an extension? AFAIK stock Graphite Web
(+Whisper) does not support this. The aggregation function is already set
and fixed on write time (through storage-aggregation.conf). The
consolidation function is only used for post-rendering aggregation, to obey
the max data points argument.
—
Reply to this email directly, view it on GitHub
<#793 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJLTVWPIGTD6SWIZXUIFX3XWYKZPANCNFSM6AAAAAA33RQWIU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Right, thanks for clarifying @deniszh . So, to be clear, this is indeed a non standard extension, right? BTW I don't think it's bad, but it's good to keep track of how carbonapi diverges from stock graphite, and put feature flags when desirable. |
Probably closable since #830 was merged |
Yeah, I'll close this one. But if you merged PR doesn't do what you think it should do - please create a new one. |
Pull request was closed
consolidateBy should send argument to backend so that metrics can be rollup using the aggregation function specified in the function.