[Logs UI] Show highlighted log entries in the minimap by Kerry350 · Pull Request #40745 · elastic/kibana

Kerry350 · 2019-07-10T13:06:06Z

Summary

The minimap to the right now shows the distribution of highlighted entries within the configured interval. In addition, an on-hover tooltip gives more details and a click jumps to one of the highlights within the corresponding time bucket.

Preview

Task breakdown

in the logHighlights hook
- add state to track the summary highlights
- add effect to load the summary highlights
in the minimap component
- render highlighted bucket
- jump to top highlight in bucket on click
in the graphql API
- add field to query for highlighted buckets within an interval

Checklist

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

…nto 22916-log-highlighting

…hting

…nto 22916-log-highlighting

* Show a highlight is active * Focus input when menu is opened * Icon for highlights menu * Debounce onChange handler * Loading state for highlights menu * Neaten up Provider

* Breathing room for icon * Better debounce time for typing

…hting

* Ensure width of popover stays consistent * Add a button to clear highlights

While a `phrase_prefix` query feels more natural, it doesn't work on `keyword` type fields. Using just a `phrase` query keeps the highlighting consistent between text and keyword type fields.

jasonrhodes · 2019-08-01T21:17:36Z

+          throw new DependencyError('Failed to load source: No apollo client available.');
+        }
+        if (!start || !end || !highlightTerms.length) {
+          throw new Error();


Should this error have a message? What does this throw do?

It should have a message, even though it never bubbles up beyond the hook. It's used to short-circuit the loading mechanism. Maybe a return Promise.reject() would be more apppriate. 🤔

jasonrhodes

Overall this looks good but I noticed a couple of quirks worth mentioning:

When I first loaded the PR, I searched for a highlight and it all worked except the next/prev buttons were consistently disabled. I added a console.log in the component and the problem went away and stayed away even after I removed the console.log and ever since it's been fine. Probably just some kind of cache quirk or something, but mentioning it in case it comes up again.
I have a video of me clicking through the mini-map and eventually I get to a supposed section with highlights where there aren't any highlights. Sometimes the highlights don't pop up until after a delay. I'll try to attach the video here shortly...

weltenwort · 2019-08-02T08:45:55Z

The highlights showing up with a delay is not unexpected. They are loaded asynchronously so the delay depends on the network connection and the cluster load.

The alternative would be to block displaying anything until the highlights have been loaded, which is probably an even worse experience. We are showing a spinner inside of the highlight popover to indicate the highlights are still loading. Maybe we should move it into the button so it's visible even when the popover is closed?

jasonrhodes · 2019-08-06T14:27:51Z

Maybe we should move it into the button so it's visible even when the popover is closed?

That's a great idea. I don't think it's a blocker for this ticket, but I'd prefer to see that at some point. Up to you all if you want to do it here or separately.

As for the other problem I had, if you all can't reproduce it by clicking through highlights in the minimap, I'm ok to move on and keep an eye out.

jasonrhodes

Left a last comment here: #40745 (comment)

But LGTM for merging.

weltenwort · 2019-08-07T09:38:05Z

I was finally able to reproduce the missing highlights you describe. I'll investigate.

weltenwort · 2019-08-07T11:10:15Z

So it turned out that the query returned hits that didn't have any highlights because the filter clauses took into account additional fields beyond those that were used for highlighting.

Now the fields should be the same, which means these unhighlighted entries should no longer occur.

@jasonrhodes can you confirm that this prevents the situation you ran into?

elasticmachine · 2019-08-07T12:16:51Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

jasonrhodes

Just checked the latest commits and I don't seem to have the same problems anymore so I think it's all set. Great work!

elasticmachine · 2019-08-09T14:39:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

Kerry350 · 2019-08-09T15:30:06Z

Awesome work @weltenwort 🎉

* Commit to allow opening the PR * Add start of the highlights menu * Add terms input * Add a container for highlights state * Start to hook up highlights state effects, and Bridge with Redux * Move highlighted logs to a separate graphql field * Add highlights argument to unit tests * Add initial draft of a log highlights hook * Finish hooking up hook state ready for GraphQL query * Take startKey and endKey from overall collection of log entries * Add sourceVersion * Fetch the data * Use columns ids instead of column indices * merge highlights into stream WIP * Add highlighting for the message column type * Use the serialized filter query * UX / Performance improvements * Show a highlight is active * Focus input when menu is opened * Icon for highlights menu * Debounce onChange handler * Loading state for highlights menu * Neaten up Provider * Tweaks * Breathing room for icon * Better debounce time for typing * Menu changes * Ensure width of popover stays consistent * Add a button to clear highlights * Add highlighting for field columns * Use phrase query for highlighting While a `phrase_prefix` query feels more natural, it doesn't work on `keyword` type fields. Using just a `phrase` query keeps the highlighting consistent between text and keyword type fields. * Factor out commonly used visibility state * Fix debounce to support changing onChange prop * Move clear highlight button inline * Translate labels * Rely on popover ownfocus prop to focus the input * Rename function to avoid i18n checker false positive * Rough and ready WIP next / previous * Add `countBefore` and `countAfter` highlight args * Fix value used * Add a check for entry * Rearrange icons * 0 falsey blurgh * Organise into separate sub-hooks for clarity * Add comment * Use filter and highlight queries also beyond interval * Refine behaviour of next / previous * Stops wrapping * Ensures that when scrolling next / previous takes off where it was left * Ensures the first highlight is scrolled to when changing highlight term * Add log highlights api tests * Fix a few runtime and compiler errors and linter warnings * Refactor prev/next jumping to avoid index errors * Use nearest index for initial jump * Include element type in translation id * Disable clear button when nothing to clear * Initial summary highlights state * Amend component usage * Comment out load call until query exists * Add graphql api for summary highlights * Add the client-side log summary highlight query * Directly use log view configuration without a bridge * Rename log entry highlight hook to maintain symmetry * Re-enable highlight rendering in the minimap * Keep the time cursor from intercepting clicks * tweak minimap highlighting colors and interaction * Clean up after merge * Remove commented-out code * Change cursor to hint at minimap highlighting interaction * Restrict highlight query filter to relevant fields * Add explanatory error message

weltenwort · 2019-08-09T16:54:28Z

@Kerry350 well done yourself 😉 thanks for the great collaboration

* Commit to allow opening the PR * Add start of the highlights menu * Add terms input * Add a container for highlights state * Start to hook up highlights state effects, and Bridge with Redux * Move highlighted logs to a separate graphql field * Add highlights argument to unit tests * Add initial draft of a log highlights hook * Finish hooking up hook state ready for GraphQL query * Take startKey and endKey from overall collection of log entries * Add sourceVersion * Fetch the data * Use columns ids instead of column indices * merge highlights into stream WIP * Add highlighting for the message column type * Use the serialized filter query * UX / Performance improvements * Show a highlight is active * Focus input when menu is opened * Icon for highlights menu * Debounce onChange handler * Loading state for highlights menu * Neaten up Provider * Tweaks * Breathing room for icon * Better debounce time for typing * Menu changes * Ensure width of popover stays consistent * Add a button to clear highlights * Add highlighting for field columns * Use phrase query for highlighting While a `phrase_prefix` query feels more natural, it doesn't work on `keyword` type fields. Using just a `phrase` query keeps the highlighting consistent between text and keyword type fields. * Factor out commonly used visibility state * Fix debounce to support changing onChange prop * Move clear highlight button inline * Translate labels * Rely on popover ownfocus prop to focus the input * Rename function to avoid i18n checker false positive * Rough and ready WIP next / previous * Add `countBefore` and `countAfter` highlight args * Fix value used * Add a check for entry * Rearrange icons * 0 falsey blurgh * Organise into separate sub-hooks for clarity * Add comment * Use filter and highlight queries also beyond interval * Refine behaviour of next / previous * Stops wrapping * Ensures that when scrolling next / previous takes off where it was left * Ensures the first highlight is scrolled to when changing highlight term * Add log highlights api tests * Fix a few runtime and compiler errors and linter warnings * Refactor prev/next jumping to avoid index errors * Use nearest index for initial jump * Include element type in translation id * Disable clear button when nothing to clear * Initial summary highlights state * Amend component usage * Comment out load call until query exists * Add graphql api for summary highlights * Add the client-side log summary highlight query * Directly use log view configuration without a bridge * Rename log entry highlight hook to maintain symmetry * Re-enable highlight rendering in the minimap * Keep the time cursor from intercepting clicks * tweak minimap highlighting colors and interaction * Clean up after merge * Remove commented-out code * Change cursor to hint at minimap highlighting interaction * Restrict highlight query filter to relevant fields * Add explanatory error message

Kerry350 and others added 30 commits June 25, 2019 12:12

Commit to allow opening the PR

bd2e87a

Add start of the highlights menu

c54da94

Add terms input

0b7420a

Add a container for highlights state

a762d02

Start to hook up highlights state effects, and Bridge with Redux

494604c

Move highlighted logs to a separate graphql field

3f2bc2e

Add highlights argument to unit tests

be180ea

Add initial draft of a log highlights hook

abb640b

Finish hooking up hook state ready for GraphQL query

ad207ea

Merge branch '22916-log-highlighting' of github.com:Kerry350/kibana i…

0a43161

…nto 22916-log-highlighting

Take startKey and endKey from overall collection of log entries

87347d3

Add sourceVersion

e4b95f6

Fetch the data

b778172

Use columns ids instead of column indices

653d468

merge highlights into stream WIP

1302ebd

Merge remote-tracking branch 'upstream/master' into 22916-log-highlig…

9565afd

…hting

Add highlighting for the message column type

2a96ef4

Use the serialized filter query

9c03ced

Merge branch '22916-log-highlighting' of github.com:Kerry350/kibana i…

1ce84a4

…nto 22916-log-highlighting

UX / Performance improvements

4f1bc75

* Show a highlight is active * Focus input when menu is opened * Icon for highlights menu * Debounce onChange handler * Loading state for highlights menu * Neaten up Provider

Tweaks

d647570

* Breathing room for icon * Better debounce time for typing

Merge remote-tracking branch 'upstream/master' into 22916-log-highlig…

4ecb669

…hting

Menu changes

32e2073

* Ensure width of popover stays consistent * Add a button to clear highlights

Add highlighting for field columns

f1f75f2

Use phrase query for highlighting

7405d16

While a `phrase_prefix` query feels more natural, it doesn't work on `keyword` type fields. Using just a `phrase` query keeps the highlighting consistent between text and keyword type fields.

Factor out commonly used visibility state

556a349

Fix debounce to support changing onChange prop

534301b

Move clear highlight button inline

c478def

Translate labels

bea0187

Rely on popover ownfocus prop to focus the input

f816eef

weltenwort added the review label Jul 26, 2019

weltenwort marked this pull request as ready for review July 26, 2019 14:35

weltenwort requested a review from a team as a code owner July 26, 2019 14:35

jasonrhodes self-requested a review July 31, 2019 12:25

jasonrhodes assigned Kerry350 Aug 1, 2019

jasonrhodes requested review from jasonrhodes and removed request for jasonrhodes August 1, 2019 17:51

jasonrhodes reviewed Aug 1, 2019

View reviewed changes

jasonrhodes approved these changes Aug 6, 2019

View reviewed changes

Merge branch 'master' into 22916-log-highlighting-phase-3

90dc47a

weltenwort added 2 commits August 7, 2019 13:04

Restrict highlight query filter to relevant fields

eec2358

Add explanatory error message

037b347

weltenwort changed the title ~~[LogsUI] Highlighting: Minimap functionality~~ [Logs UI] Show highlighted log entries in the minimap Aug 7, 2019

jasonrhodes approved these changes Aug 9, 2019

View reviewed changes

Merge branch 'master' into 22916-log-highlighting-phase-3

2820f7f

Kerry350 merged commit abe56c2 into elastic:master Aug 9, 2019

Kerry350 mentioned this pull request Aug 9, 2019

[7.x] [Logs UI] Show highlighted log entries in the minimap (#40745) #43031

Merged

weltenwort mentioned this pull request Sep 23, 2019

[Logs UI] View in context #40808

Closed

Navigation Menu

Repository navigation

[Logs UI] Show highlighted log entries in the minimap - #40745

[Logs UI] Show highlighted log entries in the minimap#40745
Kerry350 merged 73 commits into
elastic:masterfrom
Kerry350:22916-log-highlighting-phase-3

Kerry350 commented Jul 10, 2019 •

edited by weltenwort

Loading

Uh oh!

jasonrhodes Aug 1, 2019

Uh oh!

weltenwort Aug 2, 2019

Uh oh!

jasonrhodes left a comment

Uh oh!

weltenwort commented Aug 2, 2019 •

edited

Loading

Uh oh!

jasonrhodes commented Aug 6, 2019

Uh oh!

jasonrhodes left a comment

Uh oh!

weltenwort commented Aug 7, 2019

Uh oh!

weltenwort commented Aug 7, 2019

Uh oh!

elasticmachine commented Aug 7, 2019

Uh oh!

jasonrhodes left a comment

Uh oh!

elasticmachine commented Aug 9, 2019

Uh oh!

Kerry350 commented Aug 9, 2019

Uh oh!

weltenwort commented Aug 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Kerry350 commented Jul 10, 2019 • edited by weltenwort Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Preview

Task breakdown

Checklist

Uh oh!

jasonrhodes Aug 1, 2019

Choose a reason for hiding this comment

Uh oh!

weltenwort Aug 2, 2019

Choose a reason for hiding this comment

Uh oh!

jasonrhodes left a comment

Choose a reason for hiding this comment

Uh oh!

weltenwort commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasonrhodes commented Aug 6, 2019

Uh oh!

jasonrhodes left a comment

Choose a reason for hiding this comment

Uh oh!

weltenwort commented Aug 7, 2019

Uh oh!

weltenwort commented Aug 7, 2019

Uh oh!

elasticmachine commented Aug 7, 2019

💚 Build Succeeded

Uh oh!

jasonrhodes left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 9, 2019

💚 Build Succeeded

Uh oh!

Kerry350 commented Aug 9, 2019

Uh oh!

weltenwort commented Aug 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kerry350 commented Jul 10, 2019 •

edited by weltenwort

Loading

weltenwort commented Aug 2, 2019 •

edited

Loading