Timeseries: Add "grandTotal" option.#5640
Conversation
| responseContext | ||
| ); | ||
|
|
||
| if (query.isGrandTotal()) { |
There was a problem hiding this comment.
@gianm wouldn't this materialize the sequence before actual streaming of the results starts?
There was a problem hiding this comment.
I don't think so, because Sequences.map is lazy.
|
👍 |
Are you concerning about doing an additional aggregation for grandTotal in brokers instead of, like other metrics, computing pre-merge aggregates in historicals and merging them in brokers? I think it's fine if it's documented. |
|
@jihoonson nah I'm not concerned about that. I was mostly wondering if it was kosher to do the accumulation by stuffing things in a side-array while mapping the results sequence. I didn't see a better way to implement it though. |
|
I see. I think it's fine. |
* Timeseries: Add "grandTotal" option. * Modify whitespace. * Checkstyle workaround.
* Timeseries: Add "grandTotal" option. * Modify whitespace. * Checkstyle workaround.
4 participants
Adds a grandTotal option to timeseries queries. It saves the need for two queries when a UI wants to draw a timeseries but also wants to show the grand total, and also reduces cluster load (since the grand total is cheap to compute when piggybacked on the main query).
I'm not sure if the way I did this is best -- check out
mergeResultsin the tool chest. I'm open to other suggestions.Similar to #5280, but it's for timeseries.