Skip to content
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

Support to filter traces using !~ #2410

Merged
merged 9 commits into from
May 2, 2023

Conversation

kousikmitra
Copy link
Contributor

What this PR does:

  • Enables parsing !~ in tracql
  • Support for filtering values using negated regex pattern

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

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@mdisibio mdisibio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this. Overall looks great, just have a few small comments.

pkg/parquetquery/predicate_test.go Outdated Show resolved Hide resolved
pkg/parquetquery/predicate_test.go Outdated Show resolved Hide resolved
pkg/parquetquery/predicates.go Show resolved Hide resolved
@mdisibio
Copy link
Contributor

mdisibio commented May 1, 2023

There are some failing tests. I think they're all related and we need to remove the unsupported cases here :

    --- FAIL: TestExamples/unsupported_-_{_"test"_!~_"test"_} (0.00s)
    --- FAIL: TestExamples/unsupported_-_{_.a_!~_"test"_} (0.00s)

    --- FAIL: TestExamplesInEngine/unsupported_-_{_"test"_!~_"test"_} (0.00s)
    --- FAIL: TestExamplesInEngine/unsupported_-_{_.a_!~_"test"_} (0.00s)

=== RUN   TestExamples/unsupported_-_{_"test"_!~_"test"_}
    ast_validate_test.go:60: 
        	Error Trace:	/home/runner/work/tempo/tempo/pkg/traceql/ast_validate_test.go:60
        	Error:      	An error is expected but got nil.
        	Test:       	TestExamples/unsupported_-_{_"test"_!~_"test"_}
=== RUN   TestExamples/unsupported_-_{_.a_!~_"test"_}
    ast_validate_test.go:60: 
        	Error Trace:	/home/runner/work/tempo/tempo/pkg/traceql/ast_validate_test.go:60
        	Error:      	An error is expected but got nil.
        	Test:       	TestExamples/unsupported_-_{_.a_!~_"test"_}

@kousikmitra kousikmitra force-pushed the feature/not-regex-match branch from a5ac505 to 3b1c823 Compare May 1, 2023 14:35
@kousikmitra
Copy link
Contributor Author

e2e passed on my local, might be some flaky test

Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding doc.

Copy link
Contributor

@mdisibio mdisibio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with the map[string] implementation as-is. There is no regression on current performance, so let's not hold up this awesome feature. We can follow up with performance improvements as needed.

@kousikmitra
Copy link
Contributor Author

I'm good with the map[string] implementation as-is. There is no regression on current performance, so let's not hold up this awesome feature. We can follow up with performance improvements as needed.

Sure, then we are good to go 🚀

@mdisibio mdisibio merged commit 6e7cd10 into grafana:main May 2, 2023
@kousikmitra kousikmitra deleted the feature/not-regex-match branch May 2, 2023 18:17
@mdisibio mdisibio mentioned this pull request May 17, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TraceQL] Support Not Regex (!~)
3 participants