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

[Logs UI] Improve live streaming behavior when scrolling #44923

Merged
merged 7 commits into from
Sep 9, 2019

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Sep 5, 2019

Summary

May fix #43361

  • Dispatches a stopAutoReload action whenever the user scrolls up or clicks on the minimap while live streaming is turned on
  • If the user scrolls up to stop streaming, When the user stops streaming by clicking on the minimap or the Pause button on the toolbar, prevents any pending live stream request from flashing the Loading Entries screen, or pushing the user back down to the bottom of the stream; new data will instead be appended to the stream without scrolling back down.
  • When the user scrolls up, silently pauses live streaming in the Epic but still indicates a streaming state in the toolbar; shows a prompt to return to the bottom of the stream, and resumes streaming when the user clicks it or scrolls back down to the bottom.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@Zacqary Zacqary added release_note:enhancement v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.5.0 labels Sep 5, 2019
@Zacqary Zacqary requested a review from a team September 5, 2019 18:36
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@simianhacker simianhacker self-requested a review September 5, 2019 22:49
@simianhacker
Copy link
Member

Everything works as described but I wonder a bit about the UX. I feel like we are trying to mimic tail -f but distributed across all servers. When I run tail -f in a terminal and see something that catches my eye, I scroll back to stop the screen from refreshing. In the background,tail is still adding new lines to the terminal. When I scroll back down to the bottom it continues to stream again.

With this implementation, it might not be obvious that it stopped streaming when you scroll back down. I wonder if we could either add a button next to the refresh that says something like "Continue Streaming?" or just start streaming again since we know they stopped it by scrolling.

@Titch990
Copy link
Contributor

Titch990 commented Sep 6, 2019

@simianhacker @Zacqary

Here are my thoughts, assuming I've understood you both correctly, which, of course may not be the case. Happy to be corrected and/or happy to discuss further.

As far as I understand, the interface currently has two states, live streaming and not streaming. If I understand @simianhacker correctly (my apologies if not!) you're discussing a third state (live streaming but not quite) with all the possible transitions implied by that, and another button to control streaming and refreshing (you already have two!).

I think this could make the interface confusing. When the user moves away from the end of the listing during live streaming, either they are still in live streaming mode (but not showing the tail) or they are not.

So, with this change, if the user manually scrolls back to the bottom of the log (or go there via the timeline), does the user see any new entries that have come in since, or do they need to click Load again to see them?

If the former, then I think you are still live streaming, even if the latest results aren't being shown to the user while they are scrolled away, and if the latter, I think you have stopped live streaming.

If you are not stopping live streaming when the user moves away from the end of the listing, but just holding the scroll position, then everything else on that interface should look just as it does when there is live streaming, except that the timeline shows the current scroll time, not the current time.

In this case, I think the current way the status is shown at the top of the screen reflects what is really happening.

image

The status banner at the bottom of the screen could continue to show the "Streaming new entries" message, but instead of the "Last updated" message, simply say "Go to tail". Then the user can click that to get back to the tail when they are ready. There is no need to "Continue" live streaming because it never actually stopped.

If you are actually stopping live streaming when the user moves away from the end of the listing (which is what I think I understand from the description here?) then I think everything else on that interface should look just as it does when there is no live streaming.

The time selector at the top off the screen should reflect what the user is actually seeing, which is a static log display at a specific time, like so.

image

And the status banner at the bottom should change back to the static version.

image

In that case, the user can either click Stream Live to start live streaming, or Load again to load the latest logs without restarting streaming. In both cases, this would take them back to the bottom of the log listing

I think either option could work from the UI perspective, so long the user has consistent cues to show them which state they are in and how to change it. Which is more consistent with other similar log tailing tools?

@Zacqary
Copy link
Contributor Author

Zacqary commented Sep 6, 2019

@Titch990 Thanks for that summary; my implementation does stop live streaming when the user scrolls away, but if we wanted to go with @simianhacker's suggestion then I agree that we should have a banner at the bottom of the stream prompting the user to jump back to the tail.

@simianhacker I like the idea of continuing live streaming when the user scrolls up, but what about if they click or drag somewhere on the log minimap? When not live streaming, there are two different possible expected behaviors when you interact with the minimap:

  1. The user can jump to a time very far away to a point where no log data is currently loaded — e.g, you click on a point several hours ago, and a loading screen appears while we fetch log data from around that point.
  2. The user can jump to a much closer point around which sufficient log data is currently loaded, which simply scrolls the log stream to that point without showing a loading screen.

It seems like 1 should definitely pause the live stream, but I'm on the fence about 2. Technically it doesn't need to, but would it be okay to have the log minimap inconsistently pause live streaming when you interact with it?

@Titch990
Copy link
Contributor

Titch990 commented Sep 6, 2019

@Zacqary Glad my comments made sense, at least. I have no strong feeling about which option you go for, so long as the UI is consistent and the user can easily see what state they are in. Then (and this is the nub of the matter for me) I don't need to write any documentation about what is happening, because the interface will be completely intuitive (at least to someone who is used to streaming logs, which I think we can assume is the case). And what's more, because I don't need to write any documentation, I know the reader won't need to read any documentation to use the functionality. Strange as it may sound, that makes me happy! Then I can concentrate on the docs they do need.

However, now you mention the minimap . . . I struggle with the fact that you can apparently click on a point in the future, and supposedly see the logs from that point. In fact, I don't think you can, because clicking In a future point seems to do some weird stuff, then drops you back approximately where you started. Should the minimap stop at the current time? Or if that's not possible because time rolls on, and you need a future position ready for when the future becomes the present (if that makes sense) could you give the user some audible/visual "oops" feedback if they click in the future?

@Zacqary Zacqary changed the title [Logs UI] Stop live streaming on scroll or minimap click [Logs UI] Improve live streaming behavior when scrolling Sep 6, 2019
@Zacqary
Copy link
Contributor Author

Zacqary commented Sep 6, 2019

Pushed an update:

Screen Shot 2019-09-06 at 4 54 09 PM

  • When the user scrolls up, live streaming will pause in the Epic but the toolbar will still indicate that it's streaming
  • A prompt to jump back to the bottom of the stream appears
  • When the user clicks this prompt or scrolls back to the bottom, streaming will resume
  • Clicking anywhere on the log minimap will still stop live streaming

I tried having the log stream actually continue to add entries after you scroll up, but the vertical scroll panel would slightly move each time additional entries came in. Debugging whatever DOM quirk was causing that seemed like a lot of effort for not that much of a UX gain; scrolling back to the bottom will still retrieve the latest entries after a slight delay.

@Zacqary
Copy link
Contributor Author

Zacqary commented Sep 6, 2019

@Titch990 The reason the future appears on the minimap is because the center of the log stream in the scroll panel and the center of the minimap are always supposed to line up with each other. I agree that it behaves weirdly when you click on a future point in time, but that's regardless of whether or not live streaming is enabled; maybe we can open up another issue around that?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Titch990
Copy link
Contributor

Titch990 commented Sep 9, 2019

@Zacqary This all sounds good to me.

I was about to raise a ticket about the weirdness when you click on a future point in the timeline, and perhaps suggest that the log stream and the timeline lined up at the bottom of the screen (I hadn't realised the centers lined up until your comment). Then I realised it's important for the selected timeline point to be in the centre of the screen if you are looking at historic logs, so you can go forwards as well as backwards.

I'm happy to wait and see what the UI looks like after your changes are incorporated, then add a few words of explanation to the docs if I think it's necessary. Hopefully too many words won't be needed, but I may mention that clicking the minimap timeline stops live streaming, just in case anyone wonders.

Then, if I can still confuse the display by clicking into the future, I'll raise another issue as you suggest.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@simianhacker
Copy link
Member

@Zacqary Good work! This feels so much better!

Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Zacqary Zacqary merged commit 427b507 into elastic:master Sep 9, 2019
@Zacqary Zacqary deleted the 43361-stream-ux-fix branch September 9, 2019 21:23
Zacqary added a commit to Zacqary/kibana that referenced this pull request Sep 9, 2019
* [Logs UI] Stop live streaming on scroll or minimap click

* Silently pause streaming on scroll up

* Fix type checking

* Fix type check again

* Fix i18n
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 10, 2019
…-to-np-ready

* 'master' of github.com:elastic/kibana: (138 commits)
  [Canvas] i18n work on workpad header (and a few header CTAs) and convert to typescript (elastic#44943)
  update close/delete system index modals (elastic#45037)
  TS return type of createIndexPatternSelect (elastic#45107)
  [ML] Fix focus chart updating. (elastic#45146)
  [ML] Data frame transform: Fix progress in wizard create step. (elastic#45116)
  [Graph] Re-enable functional test (elastic#44683)
  [SIEM] unique table id for each top talkers table (elastic#45014)
  [SIEM] ip details heading draggable (elastic#45179)
  [Maps][File upload] Set complete on index pattern creation (elastic#44423)
  [Maps] unmount map embeddable component on destroy (elastic#45183)
  [SIEM] Adds error toasts to MapEmbeddable component (elastic#45088)
  fix redirect to maintain search query string (elastic#45184)
  [APM] One-line trace summary (elastic#44842)
  [Infra UI] Display non-metric details on Node Detail page (elastic#43551)
  [Maps][File upload] Removing bbox from parsed file pending upstream lib fix (elastic#45194)
  [Logs UI] Improve live streaming behavior when scrolling (elastic#44923)
  [APM] Fix indefinite loading state in agent settings for unauthorized user roles (elastic#44970)
  [Reporting] Rewrite addForceNowQuerystring to getFullUrls (elastic#44851)
  [Reporting/ESQueue] Improve logging of doc-update events (elastic#45077)
  [Reporting] Make screenshot capture less noisy by default (elastic#45185)
  ...
Zacqary added a commit that referenced this pull request Sep 23, 2019
…5208)

* [Logs UI] Stop live streaming on scroll or minimap click

* Silently pause streaming on scroll up

* Fix type checking

* Fix type check again

* Fix i18n
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Logs UI] Strange behavior with minimap, scrolling, and live streaming
4 participants