-
Notifications
You must be signed in to change notification settings - Fork 493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Track number of optimised regexp label matchers #4813
Track number of optimised regexp label matchers #4813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thanks! Just one small suggestion.
CHANGELOG.md
Outdated
@@ -10,6 +10,7 @@ | |||
* [ENHANCEMENT] Go: update to 1.20.3. #4773 | |||
* [ENHANCEMENT] Store-gateway: reduce memory usage in some LabelValues calls. #4789 | |||
* [ENHANCEMENT] Store-gateway: add a `stage` label to the metric `cortex_bucket_store_series_data_touched`. This label now applies to `data_type="chunks"` and `data_type="series"`. The `stage` label has 2 values: `processed` - the number of series that parsed - and `returned` - the number of series selected from the processed bytes to satisfy the query. #4797 | |||
* [ENHANCEMENT] Query-frontend: track number of optimised regexp label matchers. #4813 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [ENHANCEMENT] Query-frontend: track number of optimised regexp label matchers. #4813 | |
* [ENHANCEMENT] Query-frontend: Add `cortex_frontend_regexp_matcher_count` and `cortex_frontend_regexp_matcher_optimized_count` metrics to track optimization of regular expression label matchers. #4813 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! My only comment is that that way it's implemented means it gets tracked only for tenants with query sharding enabled. I think we should track for every query. For this reason, I think a better place where to place this logic is pkg/frontend/querymiddleware/stats.go
. You will need to parse the query there again (in a follow up PR we can do a refactoring to parse the query once and pass around the parsed query along with the Request
).
a773221
to
48dca36
Compare
Signed-off-by: dhanu <andreasdhanu@gmail.com>
cfdaa8d
to
1311a66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Signed-off-by: Marco Pracucci <marco@pracucci.com>
I pushed a commit 5b6e6f9 to ignore any parsing error in |
@dhanusaputra could you sign the CLA (see message above)? Otherwise we can't merge this PR. |
What this PR does
longestRegexpMatcherBytes
so it can return more dataWhich issue(s) this PR fixes or relates to
Fixes #4638
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]