Add new query stats metrics to track prometheus querystats #6228

SungJin1212 · 2024-09-23T11:53:33Z

What this PR does:
Add new query stats metrics cortex_query_samples_scanned_total and cortex_query_peak_samples.
These track scannedSamples and PeakSamples for each in https://github.com/prometheus/prometheus/blob/main/util/stats/query_stats.go

Which issue(s) this PR fixes:
Fixes

Checklist

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

yeya24 · 2024-09-25T22:30:09Z

pkg/frontend/transport/handler.go

+
+		h.queryPeakSamples = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
+			Name: "cortex_query_peak_samples_total",
+			Help: "Highest count of samples considered to execute a query.",


Can you please explain the usecase of tracking peak samples using a counter metric?

I think like promQL: sum by (user) (irate(cortex_query_peak_samples_total[5m])), the cortex user can track the rate of how many samples are loaded in memory for each tenant's overall queries.

One usecase is:

The cortex user can track query memory usage (used by loaded samples) for each tenant and turn promQL to reduce query memory usage.

OK. For a counter metric I feel it is more useful to just track total samples, not peak samples.
Would it be more useful to use native histogram instead?

Yes, I think the histogram is the best.

pkg/frontend/transport/handler.go

SungJin1212 · 2024-10-01T04:31:22Z

@yeya24
Could you please take a look?

yeya24

Thanks. I think I am ok. Would be good to have another approval from maintainer before merging this.

SungJin1212 · 2024-10-14T06:50:03Z

@alanprot @friedrichg
could you please take a look? :)

friedrichg

Awesome tests. Small questions related to naming

friedrichg · 2024-10-21T08:51:59Z

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [FEATURE] Ruler: Minimize chances of missed rule group evaluations that can occur due to OOM kills, bad underlying nodes, or due to an unhealthy ruler that appears in the ring as healthy. This feature is enabled via `-ruler.enable-ha-evaluation` flag. #6129
 * [FEATURE] Store Gateway: Add an in-memory chunk cache. #6245
 * [FEATURE] Chunk Cache: Support multi level cache and add metrics. #6249
+* [ENHANCEMENT] Query Frontend: Add new query stats metrics `cortex_query_total_queryable_samples_total` and `cortex_query_peak_samples` to track totalQueryableSamples and peakSample per user. #6228


is the metric cortex_query_samples_processed_total or cortex_query_total_queryable_samples_total?

Can you double check this?

Thank you for catching it. cortex_query_samples_processed_total is right.

friedrichg · 2024-10-21T09:05:14Z

pkg/querier/stats/stats.proto

@@ -39,4 +39,10 @@ message Stats {
  // The total size of postings touched in store gateway for a specific query, in bytes.
  // Only successful requests from querier to store gateway are included.
  uint64 store_gateway_touched_posting_bytes = 12;
+  // The total number of samples scanned while evaluating a query.
+  // Equal to TotalSamples in https://github.com/prometheus/prometheus/blob/main/util/stats/query_stats.go
+  uint64 processed_samples = 13;


I am not sure I understand where the word "processed" comes from. Can you explain?

Maybe scanned_samples sounds better?

Processed means the total number of considered samples when the query is executed.
But now, I feel like scanned is more intuitive to understand. Thanks.

friedrichg · 2024-10-21T17:14:51Z

pkg/frontend/transport/handler.go

+	querySeconds               *prometheus.CounterVec
+	querySeries                *prometheus.CounterVec
+	queryFetchedSamples        *prometheus.CounterVec
+	queryTotalQueryableSamples *prometheus.CounterVec


Suggested change

queryTotalQueryableSamples *prometheus.CounterVec

queryScannedSamples *prometheus.CounterVec

wdyt?

It looks good for consistency. Thanks you!

CHANGELOG.md

Signed-off-by: SungJin1212 <[email protected]>

yeya24

Thanks!

pull-request-size bot added the size/L label Sep 23, 2024

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch 4 times, most recently from 0a571de to d4ddae7 Compare September 23, 2024 12:20

yeya24 reviewed Sep 25, 2024

View reviewed changes

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch 3 times, most recently from afea4c7 to 1e37e70 Compare September 27, 2024 01:47

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch 2 times, most recently from 0cfa84b to 797b6ee Compare October 3, 2024 00:34

yeya24 approved these changes Oct 7, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 7, 2024

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch from 797b6ee to 1ac4463 Compare October 10, 2024 01:09

friedrichg reviewed Oct 21, 2024

View reviewed changes

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch 3 times, most recently from 2853868 to 593582c Compare October 21, 2024 09:46

friedrichg reviewed Oct 21, 2024

View reviewed changes

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch from 593582c to a097955 Compare October 22, 2024 01:14

friedrichg approved these changes Oct 22, 2024

View reviewed changes

friedrichg requested a review from yeya24 October 22, 2024 06:35

friedrichg reviewed Oct 23, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Add new query stats metrics to track prometheus querystats

6c77531

Signed-off-by: SungJin1212 <[email protected]>

SungJin1212 force-pushed the Add-metrics-for-queryable-samples-and-peak-samples branch from a097955 to 6c77531 Compare October 23, 2024 07:42

friedrichg approved these changes Oct 23, 2024

View reviewed changes

yeya24 approved these changes Oct 23, 2024

View reviewed changes

yeya24 merged commit 3b5011b into cortexproject:master Oct 23, 2024
16 checks passed

erlan-z mentioned this pull request Feb 14, 2025

retrieve peakSamples and processedSamples query stats from results_cache #6591

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new query stats metrics to track prometheus querystats #6228

Add new query stats metrics to track prometheus querystats #6228

SungJin1212 commented Sep 23, 2024 •

edited by friedrichg

Loading

yeya24 Sep 25, 2024

SungJin1212 Sep 26, 2024

yeya24 Sep 26, 2024

SungJin1212 Sep 26, 2024 •

edited

Loading

SungJin1212 commented Oct 1, 2024

yeya24 left a comment

SungJin1212 commented Oct 14, 2024

friedrichg left a comment

friedrichg Oct 21, 2024

SungJin1212 Oct 21, 2024

friedrichg Oct 21, 2024

SungJin1212 Oct 21, 2024

friedrichg Oct 21, 2024

SungJin1212 Oct 22, 2024

yeya24 left a comment

	queryTotalQueryableSamples *prometheus.CounterVec
	queryScannedSamples *prometheus.CounterVec

Add new query stats metrics to track prometheus querystats #6228

Add new query stats metrics to track prometheus querystats #6228

Conversation

SungJin1212 commented Sep 23, 2024 • edited by friedrichg Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SungJin1212 Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

SungJin1212 commented Oct 1, 2024

yeya24 left a comment

Choose a reason for hiding this comment

SungJin1212 commented Oct 14, 2024

friedrichg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 left a comment

Choose a reason for hiding this comment

SungJin1212 commented Sep 23, 2024 •

edited by friedrichg

Loading

SungJin1212 Sep 26, 2024 •

edited

Loading