Query Sample Statistics #4708
Merged
Query Sample Statistics #4708
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alvinlin123
reviewed
Apr 6, 2022
| result.Samples.TotalQueryableSamples += output[key].Value | ||
| } | ||
|
|
||
| if !hasStats { |
There was a problem hiding this comment.
We can move this check right after the above for loop right?
alvinlin123
reviewed
Apr 7, 2022
| @@ -380,6 +395,46 @@ func (s *SampleStream) MarshalJSON() ([]byte, error) { | |||
| return json.Marshal(stream) | |||
| } | |||
|
|
|||
| // statsMerge merge the stats from 2 responses | |||
There was a problem hiding this comment.
It can merge more than 2 responses right?
alvinlin123
reviewed
Apr 7, 2022
| }, | ||
| }, | ||
| { | ||
| name: "[stats] Merging of samples where there is complete overlap.", |
There was a problem hiding this comment.
I think it will be good idea to add some test cases to show the merge function works with more than 2 series.
alvinlin123
reviewed
Apr 7, 2022
pkg/querier/querier.go
Outdated
| @@ -92,6 +93,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
| f.DurationVar(&cfg.QueryIngestersWithin, "querier.query-ingesters-within", 0, "Maximum lookback beyond which queries are not sent to ingester. 0 means all queries are sent to ingester.") | |||
| f.BoolVar(&cfg.QueryStoreForLabels, "querier.query-store-for-labels-enabled", false, "Query long-term store for series, label values and label names APIs. Works only with blocks engine.") | |||
| f.BoolVar(&cfg.AtModifierEnabled, "querier.at-modifier-enabled", false, "Enable the @ modifier in PromQL.") | |||
| f.BoolVar(&cfg.EnablePerStepStats, "querier.per-step-stats-enabled", false, "Enable returning samples stats per steps in PromQL.") | |||
There was a problem hiding this comment.
The description of the flag is a bit weird, the stats is not returned in PromQL, but PromQL response. Or maybe just "query response".
alvinlin123
approved these changes
Apr 7, 2022
qinxx108
pushed a commit
to qinxx108/cortex
that referenced
this pull request
Apr 7, 2022
* Implementing stats on results_cache * Fix TestResultsCacheRecent * adding some comments * Update Changelog * Add test case with 3 responses * address feedback * Doc update Signed-off-by: Yijie Qin <qinyijie@amazon.com>
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings the functionality implemented on prometheus/prometheus#10369 to cortex.
In order to do so, cortex now cache the per step statistics on the results cache and
extract/mergeto fulfill the request.What this PR does:
-querier.per-step-stats-enabledto enable prometheusPerStepStats-frontend.cache-queryable-samples-statsto cache the samples stats on the results cacheWhich issue(s) this PR fixes:
Bring prometheus feature implemented prometheus/prometheus#10369 to cortex.
Fixes partially: #4630
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]