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.

Sign up for GitHub

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

Jump to bottom

Improve UI performance when displaying large logs #5537

Merged
merged 1 commit into from
Oct 31, 2015
Merged

Improve UI performance when displaying large logs #5537

merged 1 commit into from
Oct 31, 2015

Conversation

spadgett
Copy link
Member

Changes:

  • Directly manipulate the DOM rather than using ng-repeat for better performance streaming large log files.
  • Only show last 1000 lines and then follow large logs.
  • Limit the number of bytes we'll stream.
  • Add "Start following" link to auto-scroll streaming logs.
  • Remove download log link which doesn't work in most browsers.
  • Fix problem with container dropdown not working for pod logs.
  • Fix error showing chromeless view of pod log when pod has multiple containers.
  • Move duplicate code from build and pod controllers/view into log-viewer directive.
  • Stop streaming log when user navigates away from Logs tab.

Fixes #5474
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1276212
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1276003 (removes the link)

openshift_web_console

Tested in Chrome, Firefox, IE, and Safari.

@spadgett
Copy link
Member Author

@jwforres

@spadgett
Copy link
Member Author

/cc @sg00dwin @benjaminapetersen

This was referenced Oct 30, 2015
Closed
Closed
Closed
@spadgett
Copy link
Member Author

[test]

@spadgett spadgett force-pushed the large-logs branch 2 times, most recently from a3a0629 to 3b4ba03 Compare October 30, 2015 17:32
@spadgett spadgett mentioned this pull request Oct 30, 2015
Closed
@spadgett
Copy link
Member Author

[test]

jwforres
jwforres reviewed Oct 30, 2015
View reviewed changes
assets/app/views/logs/chromeless-pod-log.html
<alerts alerts="alerts"></alerts>
<div ng-if="largeLog" class="log-size-warning">
<span class="pficon pficon-info"></span>
Only the previous {{options.tailLines || 1000}} log lines and new log
Copy link
Member

Choose a reason for hiding this comment

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

are we only limiting the log on the pod logs?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's both. Default options are set in logViewer.js, but not visible to the view because I didn't want to change the value passed in to the directive.

@jwforres
Copy link
Member

Just the one comment, otherwise LGTM

@spadgett
Copy link
Member Author

Thanks @jwforres. I made the change.

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3883/) (Image: devenv-rhel7_2615)

@spadgett
Copy link
Member Author

@jwforres one other minor change to set $scope.limitReached to false when reloading the log

@spadgett
Copy link
Member Author

[merge]

@spadgett
Copy link
Member Author 

bower bootstrap-hover-dropdown#~2.1.3        ECMDERR Failed to execute "git ls-remote --tags --heads git://github.com/CWSpear/twitter-bootstrap-hover-dropdown.git", exit code of #128 fatal: unable to connect to github.com: github.com[0: 192.30.252.128]: errno=Connection timed out

Additional error details:
fatal: unable to connect to github.com:
github.com[0: 192.30.252.128]: errno=Connection timed out

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 2d8cc03

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6505/)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 2d8cc03

openshift-bot pushed a commit that referenced this pull request Oct 31, 2015
Merge pull request #5537 from spadgett/large-logs
1d53387 
Merged by openshift-bot
@openshift-bot openshift-bot merged commit 1d53387 into openshift:master Oct 31, 2015
@spadgett spadgett deleted the large-logs branch November 4, 2015 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spadgett @jwforres @openshift-bot