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

[Unified Search] Discrepancy between filter pills and query bar for date fields #172097

Open
Heenawter opened this issue Nov 28, 2023 · 7 comments
Assignees
Labels
enhancement New value added to drive a business result Feature:Unified search Unified search related tasks impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:needs-research This issue requires some research before it can be worked on or estimated Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@Heenawter
Copy link
Contributor

Heenawter commented Nov 28, 2023

Kibana version: main

Describe the bug:
The filter pills use a match_phrase filter for date fields, while the query bar constructs a range filter, as can be seen here:

Screenshot 2023-11-28 at 11 34 46 AM

This causes a discrepancy when a time is included in the search term, as shown in this video where both the query and the filter pill are searching for the exact same term:

Screen.Recording.2023-11-28.at.11.38.54.AM.mov

Steps to reproduce:

  1. Create a query on a date field in the format yyyy-MM-dd'T'HH:mm:ss.SSSZ (for a value that exists)
  2. Create a filter pill for the exact same date + time as the query from step 1
  3. Notice that the query bar returns results, while the filter pill does not 🔥

Expected behavior:
The filter pill should return the same documents as the query bar

@Heenawter Heenawter added bug Fixes for quality problems that affect the customer experience loe:needs-research This issue requires some research before it can be worked on or estimated impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Unified search Unified search related tasks labels Nov 28, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@vadimkibana vadimkibana added Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. and removed Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Dec 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@stratoula stratoula added enhancement New value added to drive a business result and removed bug Fixes for quality problems that affect the customer experience labels Dec 5, 2023
@stratoula
Copy link
Contributor

This is how it worked since I remember so I will change it to enhancement. What you are proposing makes sense to me but as I was not (and none from my team) part of the unified search filters implementation I am not sure if this was intended or not.

Not sure who might be the best to answer this question. @lukasolson maybe you know?

@kertal
Copy link
Member

kertal commented Dec 5, 2023

it's related to this issue: #171441, so currently there's a match_phrase filter being added for all filters, but I think we need to change this depending on the type of the field:

@stratoula
Copy link
Contributor

Yes this makes sense, thanx Matthias.

@lukasolson
Copy link
Member

While we could definitely improve the behavior here, the main issue here is that you are using a timestamp that doesn't contain information about the time zone. With the query generated form your KQL expression, we add this time zone parameter, but it looks like we don't for the match_phrase query. As a workaround, you'll want to specify the time zone in your text (i.e. 2023-11-21T00:10:46.350-07:00 for America/Edmonton):

image

I understand this is totally not intuitive, and that we should align the behaviors for both KQL and the "Add a filter" dialog, so I suggest we leave this open.

Heenawter added a commit that referenced this issue Dec 18, 2023
Closes #143587
Closes #126795

## Summary

This PR adds support for numeric options lists - i.e. options list
controls that are created with a numeric field. In order to make this
possible, I had to add support for fields to have multiple, overlapping
compatible control types (however, it is currently only number fields
where this applies). When selecting a field that is compatible with
multiple control types, the user is given the option to select which
control type they want, like so:

<p align="center"><img alt="GIF of multiple control types being
available for a single field"
src="https://github.com/elastic/kibana/assets/8698078/9ebb6795-1206-47c4-aaef-32437d27cf59"/></p>

> [!NOTE]
> This system currently defaults to options list controls, since these
are the most common - this is backed up by our telemetry, which shows
that (in the last 30 days), clusters with at least one options list
control occured **ten times more often** than clusters with at least one
range slider control. However, if we decide to introduce more control
types (such as, for example, a date picker control), this assumption may
no longer be the case - at that point, we would need to reevaluate
whether the default should **always** be options list.

### Video


https://github.com/elastic/kibana/assets/8698078/4a876c94-a041-4228-aab8-7c2c1c871071

### Exact Match Searching

Since numeric fields do not have a "prefix" query equivalent, the only
possibility for searching these fields is either (1) a range query or
(2) an exact match / equality query. In this PR, I added support for
equality search - i.e. in order to find a value in a numeric options
list, you must enter the *full*, exact term in order for it to be found;
in the future, we could extend this to include a range search. This is
the exact match searching in action:

<p align="center"><img alt="GIF of exact match searching example on
number field"
src="https://github.com/elastic/kibana/assets/8698078/491feff3-031f-4367-8018-67aab9be55e3"/></p>


Since exact match searching is a generic search query that works for
**all** field types, I added this as an option for **all** field types
that support searching - i.e. keyword fields, number fields, IP fields,
etc. For example, here is the supported search techniques for a keyword
field:


<p align="center"><img alt="GIF of all available search techniques for
keyword field"
src="https://github.com/elastic/kibana/assets/8698078/dbc12cef-d43e-4d9f-a779-a5fe9e75325c"/></p>

> [!TIP]
> Exact match / equality / term searching on float values can lead to
slightly unexpected results - refer to the [documentation on precision
loss](https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html#_which_type_should_i_use)
for a description of why. Be mindful when using an options list control
for float values, and consider whether a range slider might be a better
solution!

Eventually, we should be able to add exact-match searching to date
fields, as well; however, due to [discrepancies that exist in the
unified search bar](#172097)
with respect to how searching date fields is handled, we should ideally
wait until this is resolved so that we can be consistent across all
three search experiences (query bar, filter pills, and controls).


Depending on how the author expects a given control to be used, this
search technique will return results **faster** than some of the other
search types since "search as you type" will, more often than not,
return zero results in this setting - that is why I chose to add it for
keyword fields, as well.


### Searching when `allowExpensiveQueries` setting is `false`
Previously, we had **two separate versions** of our options list search
queries - one for when `allowExpensiveQueries` was `true`, and the other
for when it was `false`. This was a significant amount of tech debt that
was time consuming to maintain, which made it difficult to justify
keeping this around considering **how much** of Kibana relies on this
setting to be `true` (searching for existing Dashboards by title on the
listing page, saving a brand new dashboard, etc.).

Therefore, while we still have **some** functionality when
`allowExpensiveQueries` is `false`, I have refactored this code
significantly to simplify the logic.

> [!IMPORTANT]
> Specifically, options list controls now only support **exact match
searching** when `allowExpensiveQueries` is `false`.

Since this query is the same regardless of the type of field or the
value of `allowExpensiveQueries`, this means we no longer have to
maintain two slightly different versions ("cheap" and "expensive") of
our search queries. This cleans up our tech debt significantly.

### Bundle Size Changes @elastic/kibana-operations 

Changes to bundle size are primarily due to the changes I made to the
`OptionsListEditorOptions` component - since this component is directly
added to the options list factory (which is not async imported), this
impacts the bundle size.

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Unified search Unified search related tasks impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:needs-research This issue requires some research before it can be worked on or estimated Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

6 participants