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

[Uptime] Search made easy #88581

Merged
merged 66 commits into from
Feb 24, 2021
Merged

[Uptime] Search made easy #88581

merged 66 commits into from
Feb 24, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jan 18, 2021

Summary

Fixes: elastic/uptime#274

User can search for monitors using kuery bar in a simple mode, entered query will be automatically searched against monitorid, name and url.

It uses elastic search multi match phrase_prefix query to perform search.

User will not have to know ecs schema to perform search

this also means user can user include text to search against any text appearing anywhere in those fields, which is not possible in kuery bar.

image

User can switch to KQL mode using a simple popover
image

Questions

  1. Should we add monitor.type and step name in fields to search?
  2. Save user config in local storage?
  3. By default enable simple mode?

@andrewvc
Copy link
Contributor

While I like this feature, I'm reticent to include step names in the search because it's not obvious looking at the result what matched.

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor Author

@andrewvc i understand your point, it's always a tricky user experience searching against a data which isn't displayed on UI. I am going to hold off on adding new field for this PR.

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

When typing a search phrase, if you hit enter, the search is executed but the search term disappears in the search box. It then becomes unclear how to remove the search term. If you refresh the page, the search term appears in the search box and can be removed with the x symbol.
chrome-capture (2)

Also, when switching between simple search and KQL, if you switch back to simple search it no longer appears to find results properly. Below, I used a unique monitor id to match a monitor, then switched to KQL and immediately back to simple search. The search term then no longer worked appropriately
chrome-capture (3)

Also, the menu button that opens the popover to switch between KQL and simple search is not tabable or accessible by a keyboard

@shahzad31
Copy link
Contributor Author

@dominiqueclarke great catch, i have fixed and updated key events.

@dominiqueclarke
Copy link
Contributor

dominiqueclarke commented Feb 23, 2021

@dominiqueclarke great catch, i have fixed and updated key events.

Thanks! The fix looks great!

I'm still seeing these issues

  1. Switching back and forth between simple, kql, and simple causes simple to no longer work
  2. Switch button to trigger popover is not tabable.

To recreate issue number 1.
Type in a search phrase in the simple search box. Do not remove it
Switch to KQL (search phrase will then be cleared out)
Switch back to simple
Execute the same search as step 1

@paulb-elastic
Copy link
Contributor

Discussed in refinement, this implementation won't include step names (that could be a future enhancement if there's a use case for it)

@shahzad31
Copy link
Contributor Author

@dominiqueclarke have fixed the toggle issue

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 594 599 +5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
uptime 928.5KB 934.8KB +6.2KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @shahzad31

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzad31 shahzad31 merged commit 1be0612 into elastic:master Feb 24, 2021
@shahzad31 shahzad31 deleted the search-box branch February 24, 2021 15:02
@shahzad31 shahzad31 added the auto-backport Deprecated: Automatically backport this PR after it's merged label Feb 24, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 24, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #92632

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Feb 24, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Shahzad <shahzad.muhammad@elastic.co>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 25, 2021
…bana into task-manager/docs-monitoring

* 'task-manager/docs-monitoring' of github.com:gmmorris/kibana: (120 commits)
  Bump the CI Node.js version from 14.15.4 to 14.16.0 (elastic#92578)
  [Uptime] Search made easy (elastic#88581)
  [Lens] Stop requesting stats on Records field (elastic#92508)
  [APM] Correlations style polish in prep for release (elastic#92514)
  [Lens] Fix sorting undefined, null and NaN values (elastic#92575)
  Put environment back in uiFilters (elastic#92561)
  [Discover] Fix sorting by _score behavior (elastic#92132)
  Add metricbeat steps to jenkins_build_load_testing.sh (elastic#90290)
  [Usage Collection] Remove unused UI Metric APIs (elastic#91620)
  Fix timelion deprecation documentation links (elastic#92576)
  [TSVB] Enable `dual mode`, support index patterns and strings (elastic#92395)
  Fix text align in Safari (elastic#92374)
  [Actions][Doc] Clean up Actions README (elastic#91789)
  chore(NA): add missing files to exclude when building bazel prod packages (elastic#92506)
  [Security Solution][Endpoint][Admin] Match Policy Details in Security Solution with Fleet (elastic#92047)
  Revert "[build] Add task skips intended for partial builds (elastic#92076)"
  [Enterprise Search] Change icon for error in EuiCallouts (elastic#92527)
  [Alerts][Docs] Alert types doc update. Added refs to applications specific alerts groups. (elastic#91787)
  [Security Solution][Exceptions] - Update exceptions modal to use existing lists plugin useApi hook (elastic#92348)
  [DOCS] Updates refresh text in index pattern doc (elastic#92469)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Uptime] Simplify kuery bar to use simple keyword search instead of field specific kql
6 participants