-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
chore(storage/bloom): support simplifiable regexp matchers #14622
chore(storage/bloom): support simplifiable regexp matchers #14622
Conversation
This adds support for basic regexps which can be simplified into a sequence of OR matchers, such as: * `key=~"value"` becomes `key="value"` * `key=~"value1|value2"` becomes `key="value1" or key="value2"`. Matchers like `key=~".+"` continue to not be supported because the lack of a key doesn't mean that it doesn't exist as a label. Only the cases above are "officially" supported. However, we technically support basic concatenations and character classes due to how regexp/syntax parses and simplifies expressions such as `value1|value2` into `value[12]`. To prevent unbounded cardinality, we limit regexp expansion to 25 matchers; otherwise a regexp like `value[0-9][0-9][0-9][0-9]` would expand into 10,000 matchers (too many!).
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.
[docs team] Doc part of this PR LGTM (one typo)
Co-authored-by: J Stickler <julie.stickler@grafana.com>
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.
This is great. Left two nits but LGTM 👏
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!
2fb1a06
to
f7df523
Compare
@salvacorts Going to merge since you gave a soft LGTM in your last review, but please let me know if there's any other changes you'd like me to make :) |
This adds support for basic regexps which can be simplified into a sequence of OR matchers, such as: * `key=~"value" becomes key="value" * `key=~"value1|value2" becomes key="value1" or key="value2". * `key=~".+" checks for the presence of key. This is currently the only way to check if a key exists. Only the cases above are "officially" supported. However, we technically support basic concatenations and character classes due to how regexp/syntax parses and simplifies expressions such as `value1|value2` into `value[12]`. To prevent unbounded cardinality, we limit regexp expansion to 25 matchers; otherwise a regexp like `value[0-9][0-9][0-9][0-9]` would expand into 10,000 matchers (too many!). Closes grafana/loki-private#1106. Co-authored-by: J Stickler <julie.stickler@grafana.com>
What this PR does / why we need it:
This adds support for basic regexps which can be simplified into a sequence of OR matchers, such as:
key=~"value"
becomeskey="value"
key=~"value1|value2"
becomeskey="value1" or key="value2"
.key=~".+"
checks for the presence ofkey
.key=~".+"
is currently the only way to check if a key exists.Only the cases above are "officially" supported. However, we technically support basic concatenations and character classes due to how regexp/syntax parses and simplifies expressions such as
value1|value2
intovalue[12]
.To prevent unbounded cardinality, we limit regexp expansion to 25 matchers; otherwise a regexp like
value[0-9][0-9][0-9][0-9]
would expand into 10,000 matchers (too many!).Which issue(s) this PR fixes:
Closes grafana/loki-private#1106.
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR