From 3f4fd0f8f59f81130c1e05c5c1d3937063378042 Mon Sep 17 00:00:00 2001 From: Ben Sully Date: Wed, 1 May 2024 22:19:35 +0100 Subject: [PATCH 1/3] fix(traceql): fix extract matcher regexes to work with regexp-type matchers Partially fixes #3635. I'm going to add a further test case which this doesn't fix. --- pkg/traceql/extractmatcher.go | 5 +++-- pkg/traceql/extractmatcher_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/traceql/extractmatcher.go b/pkg/traceql/extractmatcher.go index cfd0420fa14..ee23905b79f 100644 --- a/pkg/traceql/extractmatcher.go +++ b/pkg/traceql/extractmatcher.go @@ -18,7 +18,7 @@ import ( // 3. The boolean values "true" or "false". // // Example: "http.status_code = 200" from the query "{ .http.status_code = 200 && .http.method = }" -var matchersRegexp = regexp.MustCompile(`[\p{L}\p{N}._\-" ]+\s*[=|<=|>=|=~|!=|>|<|!~]\s*(?:"[\p{L}\p{N}\p{P}\p{M}\p{S}]+"|true|false|[a-z]+|[0-9smh]+)`) +var matchersRegexp = regexp.MustCompile(`[\p{L}\p{N}._\-" ]+\s*(=|<=|>=|=~|!=|>|<|!~)\s*(?:"[\p{L}\p{N}\p{P}\p{M}\p{S}]+"|true|false|[a-z]+|[0-9smh]+)`) // TODO: Merge into a single regular expression @@ -29,11 +29,12 @@ var matchersRegexp = regexp.MustCompile(`[\p{L}\p{N}._\-" ]+\s*[=|<=|>=|=~|!=|>| // Query | Match // // { .bar = "foo" } | Yes +// { .bar =~ "foo|bar" } | Yes // { .bar = "foo" && .foo = "bar" } | Yes // { .bar = "foo" || .foo = "bar" } | No // { .bar = "foo" } && { .foo = "bar" } | No // { .bar = "foo" } || { .foo = "bar" } | No -var singleFilterRegexp = regexp.MustCompile(`^\{[^|{}]*[^|{}]}?$`) +var singleFilterRegexp = regexp.MustCompile(`^(\{[^|{}]*[^|{}]}?|\{[^|{}]*=~[^{}]*})$`) const emptyQuery = "{}" diff --git a/pkg/traceql/extractmatcher_test.go b/pkg/traceql/extractmatcher_test.go index fa631616544..57c87113214 100644 --- a/pkg/traceql/extractmatcher_test.go +++ b/pkg/traceql/extractmatcher_test.go @@ -100,6 +100,16 @@ func TestExtractMatchers(t *testing.T) { query: `{ span."foo bar" = "baz" }`, expected: `{span."foo bar" = "baz"}`, }, + { + name: "query with trivial regex matcher", + query: `{ .foo =~ "a" }`, + expected: `{.foo =~ "a"}`, + }, + { + name: "query with regex matcher", + query: `{ .foo =~ "(a|b)" }`, + expected: `{.foo =~ "(a|b)"}`, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From f28f5de05037ee4f6c5a52e326cf73702b783f1e Mon Sep 17 00:00:00 2001 From: Ben Sully Date: Wed, 1 May 2024 22:22:31 +0100 Subject: [PATCH 2/3] Add more test cases for extract matcher --- pkg/traceql/extractmatcher_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/traceql/extractmatcher_test.go b/pkg/traceql/extractmatcher_test.go index 57c87113214..dba3c6f049f 100644 --- a/pkg/traceql/extractmatcher_test.go +++ b/pkg/traceql/extractmatcher_test.go @@ -110,6 +110,16 @@ func TestExtractMatchers(t *testing.T) { query: `{ .foo =~ "(a|b)" }`, expected: `{.foo =~ "(a|b)"}`, }, + { + name: "query with multiple regex matchers", + query: `{ .foo =~ "(a|b)" && .bar =~ "(c|d)" }`, + expected: `{.foo =~ "(a|b)" && .bar =~ "(c|d)"}`, + }, + { + name: "query with mixed equal and regex matchers", + query: `{ .foo = "a" && .bar =~ "(c|d)" }`, + expected: `{.foo = "a" && .bar =~ "(c|d)"}`, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 1539758b9021cf8757c8259505fc1a7dc8d4efb5 Mon Sep 17 00:00:00 2001 From: Ben Sully Date: Thu, 2 May 2024 18:51:50 +0100 Subject: [PATCH 3/3] Add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf5fb0d620..5960338dfd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## main / unreleased +* [BUGFIX] Fix handling of regex matchers in autocomplete endpoints [#3641](https://github.com/grafana/tempo/pull/3641) (@sd2k) * [ENHANCEMENT] Surface new labels for uninstrumented services and systems [#3543](https://github.com/grafana/tempo/pull/3543) (@t00mas) * [FEATURE] Add TLS support for Memcached Client [#3585](https://github.com/grafana/tempo/pull/3585) (@sonisr) * [FEATURE] TraceQL metrics queries: add quantile_over_time [#3605](https://github.com/grafana/tempo/pull/3605) [#3633](https://github.com/grafana/tempo/pull/3633) (@mdisibio)