Skip to content

fix: LineFilterLabelFilter.String() regexp correct delimiters #17129

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kelnage
Copy link
Contributor

@kelnage kelnage commented Apr 11, 2025

What this PR does / why we need it:
When the querysharder executes, it converts a query into an AST representation before forwarding it on to the queriers. Unfortunately, when this AST representation is converted back into a string, it does not handle the case where a regexp can contain backticks - it always tries to wrap the regexp in backticks, which can lead to parser errors. This PR updates the String() method that converted the filter back into a query string and adapts the delimiters based on the content of the regexp.

Which issue(s) this PR fixes:
Fixes #16674

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@kelnage kelnage changed the title fix: query fix: query AST representation of regexp surrounded with backticks Apr 11, 2025
@kelnage kelnage self-assigned this Apr 11, 2025
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 11, 2025
@kelnage kelnage changed the title fix: query AST representation of regexp surrounded with backticks fix: label filter representation of regexp always surrounded with backticks Apr 11, 2025
@kelnage kelnage force-pushed the parser-backtick-regexp-error branch from 1c8e5f1 to f515ed1 Compare April 11, 2025 15:52
@kelnage kelnage changed the title fix: label filter representation of regexp always surrounded with backticks fix: LineFilterLabelFilter.String() regexp not always surrounded with backticks Apr 11, 2025
@kelnage kelnage marked this pull request as ready for review April 11, 2025 16:03
@kelnage kelnage requested a review from a team as a code owner April 11, 2025 16:03
@kelnage kelnage changed the title fix: LineFilterLabelFilter.String() regexp not always surrounded with backticks fix: LineFilterLabelFilter.String() regexp correct delimiters Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Label regular expression filters misinterpreting backticks
1 participant