Add per-user query metrics for series and bytes returned

[56quarters]

What this PR does:

Add stats included in query responses from the querier and distributor
for measuring the number of series and bytes included in successful
queries. These stats are emitted per-user as counters from the query
frontends.

These stats are picked to add visibility into the same resources limited
as part of #4179 and #4216.

Fixes #4259

Notes to reviewers

Open issue:

• The spanlog in pkg/querier/blocks_store_queryable.go [1] computes the number of bytes for the series (countSeriesBytes()) differently than the way chunk bytes are limited. I wasn't sure if I should update the spanlog to use countChunkBytes() the same way I did for the stats emitted or if it's supposed to be measuring something entirely different. As it is, the number bytes emitted by the spanlog is consistently lower than countChunkBytes() since it doesn't include the timestamps for each chunk.

Looking for input:

• The buckets picked for the histograms were chosen based on queries performed by the cortex-mixin dashboards. As they are now, nearly all queries fall into the largest bucket or lower (number of series and chunk bytes). If there are changes you'd like to see, let me know.
• Lots of casting to uint64 from int and vice versa. It seemed from the PRs that added the limits that there wasn't consensus on int vs uint64. Let me know if you'd like to see uint64 here changed to something else.

Signed-off-by: Nick Pillitteri nick.pillitteri@grafana.com

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[pracucci]

I have some concerns doing it a per-user basis. @tomwilkie just yesterday commented here #4259 (comment) on the fact that we have to pay attention each time we add a per-user histogram, cause they easily explode cardinality.

[56quarters]

I have some concerns doing it a per-user basis. @tomwilkie just yesterday commented here #4259 (comment) on the fact that we have to pay attention each time we add a per-user histogram, cause they easily explode cardinality.

Understood. The approach here (to emit the metrics from the query-frontend) was taken based on a suggestion from @pstibrany to reduce the number of series (since there are usually only a few query frontends). I can definitely reduce the number of buckets of the histograms.

[56quarters]

Based on @tomwilkie 's comment it seems like what I should do is:

• Keep emitting these metrics as part of the query log
• Change these histograms to summaries? Or perhaps remove them entirely?

[tomwilkie]

Yes please Nick! I'd probably go with logs in this case, but a summary would also be fine with 1 or 2 quantiles.

[replay]

LGTM with one comment

[pracucci]

Good job @56quarters! I left few comments about naming to try to make it a bit more clear. A part from this LGTM (and no concern about cardinality since you moved away from histograms). Thanks for addressing my initial feedback! 🙏

[pracucci]

LGTM (modulo a couple of final nits). Thanks!

[pstibrany]

LGTM, thank you!

[56quarters]

I think it would be useful to emit this metric in some non-successful queries too. The specific case I have in mind is when the query do indeed hit the limit, we should still have this metric emitted so we can detect if any user hit the limit.

I think it would be useful as well but I'm not going to make the change here since I think it would involve a fairly large refactoring of the code. Nothing preventing us from doing this as a follow-up though.