search

go-graphite / carbonapi

Implementation of graphite API (graphite-web) in golang
Other
308 stars 140 forks source link

feat(functions): add support for consolidateBy #793

Closed lordvidex closed 3 months ago

lordvidex commented 11 months ago

consolidateBy should send argument to backend so that metrics can be rollup using the aggregation function specified in the function.

lordvidex commented 11 months ago

The first commit contains changes that should be reviewed while the second are refactor changes to make code compile properly.

npazosmendez commented 11 months ago

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.

deniszh commented 11 months ago

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 https://github.com/go-graphite/carbonapi/pull/793#issuecomment-1690117832, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJLTVWPIGTD6SWIZXUIFX3XWYKZPANCNFSM6AAAAAA33RQWIU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

npazosmendez commented 11 months ago

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.

npazosmendez commented 3 months ago

Probably closable since https://github.com/go-graphite/carbonapi/pull/830 was merged

Civil commented 3 months ago

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.