From 4013c1f8b5824fc34258fa49cd17256e16a248ea Mon Sep 17 00:00:00 2001 From: akhilanarayanan <45960148+akhilanarayanan@users.noreply.github.com> Date: Fri, 18 Aug 2023 10:55:19 -0700 Subject: [PATCH] Stopped String() from doubly escaping regex characters (#10277) **What this PR does / why we need it**: When queries have regex, characters that needed escaping were escaped twice. Now they're only singly escaped. This is good because it reduces the character count so users can submit queries closer to the 5120 byte limit. **Which issue(s) this PR fixes**: Sort of addresses #10153 **Special notes for your reviewer**: **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [x] Tests updated - [ ] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](https://github.com/grafana/loki/commit/d10549e3ece02120974929894ee333d07755d213) Co-authored-by: Akhila Narayanan --- pkg/logql/log/label_filter.go | 10 ++++++++++ pkg/logql/syntax/parser_test.go | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/pkg/logql/log/label_filter.go b/pkg/logql/log/label_filter.go index 1fda5430b7..279669e5a7 100644 --- a/pkg/logql/log/label_filter.go +++ b/pkg/logql/log/label_filter.go @@ -368,6 +368,16 @@ type lineFilterLabelFilter struct { filter Filterer } +// overrides the matcher.String() function in case there is a regexpFilter +func (s *lineFilterLabelFilter) String() string { + if unwrappedFilter, ok := s.filter.(regexpFilter); ok { + rStr := unwrappedFilter.String() + str := fmt.Sprintf(`%s%s"%s"`, s.Matcher.Name, s.Matcher.Type, rStr) + return str + } + return s.Matcher.String() +} + func (s *lineFilterLabelFilter) Process(_ int64, line []byte, lbs *LabelsBuilder) ([]byte, bool) { v := labelValue(s.Name, lbs) return line, s.filter.Filter(unsafeGetBytes(v)) diff --git a/pkg/logql/syntax/parser_test.go b/pkg/logql/syntax/parser_test.go index 819fdd1ca1..f5a21c21f8 100644 --- a/pkg/logql/syntax/parser_test.go +++ b/pkg/logql/syntax/parser_test.go @@ -3465,3 +3465,13 @@ func TestNoOpLabelToString(t *testing.T) { require.NoError(t, err) require.Len(t, stages, 0) } + +// Tests that the regex part of expr doesn't get doubly escaped when we call expr.String() +func TestParseSampleExpr_String(t *testing.T) { + query := `sum(rate({cluster="beep", namespace="boop"} | msg=~` + "`" + `.*?(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS) /loki/api/(?i)(\d+[a-z]|[a-z]+\d)\w*/query_range` + "`" + `[1d]))` + expr, err := ParseSampleExpr(query) + require.NoError(t, err) + // need to change some backticks to " in order for query to be exactly equal to expr.String(), otherwise it's the same query + expected := `sum(rate({cluster="beep", namespace="boop"} | msg=~".*?(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS) /loki/api/(?i)(\d+[a-z]|[a-z]+\d)\w*/query_range"[1d]))` + require.Equal(t, expected, expr.String()) +}