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

Line Filter: Add regex option to line filter #958

Conversation

gtk-grafana
Copy link
Contributor

@gtk-grafana gtk-grafana commented Dec 12, 2024

Part of: #952
Parent branch: #956

Search for IPs:
image

case insensitive regex
image

case sensitive regex
image

@gtk-grafana gtk-grafana changed the base branch from main to gtk-grafana/issues/952/line-filter-ui-updates__case-sensitive-localstorage December 12, 2024 19:58
@gtk-grafana gtk-grafana mentioned this pull request Dec 12, 2024
10 tasks
@gtk-grafana gtk-grafana marked this pull request as ready for review December 12, 2024 20:58
@gtk-grafana gtk-grafana requested a review from a team as a code owner December 12, 2024 20:58
@gtk-grafana gtk-grafana self-assigned this Dec 12, 2024
@gtk-grafana gtk-grafana added the enhancement New feature or request label Dec 12, 2024
@gtk-grafana gtk-grafana added this to the 1.0.5 milestone Dec 12, 2024
src/services/store.ts Outdated Show resolved Hide resolved
super({
lineFilter: state?.lineFilter || '',
caseSensitive: getLineFilterCase(false),
caseSensitive,
regex: getLineFilterRegex(false),
Copy link
Contributor

Choose a reason for hiding this comment

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

One issue with regex is that we're setting it based on the stored preference and not the filter contents.

Copy link
Contributor

@matyax matyax left a comment

Choose a reason for hiding this comment

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

It's almost there. There is an issue to resolve when you share or reuse a URL, because the regex state doesn't depend on the filter contents. Also it seems that even with regex turned out we're producing and interpreting regexes (the URL I open in incognito with the regex correctly produces log results).

Demo.mov

@matyax
Copy link
Contributor

matyax commented Dec 13, 2024

After seeing #956 and this PR, I think we should change the approach:

  • Regex on, show "case sensitive/case insensitive" toggle. Changing it changes the regular expression.
  • Regex off, don't show sensitiveness toggle, always produce a Line filter.
  • Parse URL: if the URL contains a regex, set regex to true, parse the regular expression, parse case sensitiveness and also set the state.

So we either produce regular expression queries (case sensitive or not), or plain text line filters.

@gtk-grafana gtk-grafana changed the base branch from gtk-grafana/issues/952/line-filter-ui-updates__case-sensitive-localstorage to main December 13, 2024 16:12
@gtk-grafana
Copy link
Contributor Author

gtk-grafana commented Dec 13, 2024

Well ****

if the filter is case sensitive && no regex, it has |=
If the filter is case insensitive && no regex, it has |~ (?i)
If the filter is case sensitive && regex: it does not have |~ (?i)
if the filter is case insensitive && regex: it has|~ (?i) <-- uh oh, same as case 2

So we need to store the sensitive, regex options in the URL so shared URLs can be parsed appropriately.

I disagree that making the options mutually exclusive is the solution (undesirable UX), but I agree that the options that are set should be pulled from the URL. The values stored in local storage should only set the default.

We'll need to make sure that when we change the options we don't overwrite the users default for other queries (i.e. only when the user clicks on a button). Also when options change we'll need to convert and escape the value appropriately.

Potential solutions:

  • Serialize options with the value in the URL
  • Store the options in an ad-hoc variable filter

I think the next step is a PoC with the ad-hoc solution.
An ad-hoc variable filter has an index, operator, and value.
Index is unused for line filters.
Operator is |=, |~, !=, !~
Value is whatever the user typed, appropriately escaped based on the operator
Where does case sensitivity go? Drop it into the index? That might not play nice with the default ad-hoc filter renderer.

But yeah I guess that we should find out in the PoC: can we use the existing variables and renderers for line filters with case,regex options (and what provides best support for more options in the future, e.g. ip function, word wrap, other global regex options etc).

@gtk-grafana gtk-grafana mentioned this pull request Dec 13, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants