[logging] Add flag for document visibility #9070
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #9070 +/- ##
=======================================
Coverage 59.45% 59.45%
=======================================
Files 369 369
Lines 11747 11747
Branches 2888 2888
=======================================
Hits 6984 6984
Misses 4584 4584
Partials 179 179
Continue to review full report at Codecov.
|
etr2460
approved these changes
Feb 3, 2020
| @@ -107,6 +107,7 @@ const loggerMiddleware = store => next => action => { | |||
| ...eventData, | |||
| event_type: 'user', | |||
| event_id: lastEventId, | |||
| visibility: document.visibilityState, | |||
There was a problem hiding this comment.
should we move this into the top level eventData so that it's added on timing events too?
There was a problem hiding this comment.
timing event (mostly is data loading and chart render) is triggered by user event (dashboard mount, switch tab, etc). I assume during 1 user event (a few charts will be loading and re-render), the browser's visibilityState will not change.
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CATEGORY
Choose one
SUMMARY
When we measure dashboard performance, we use dashboard's TTI (time to interactive), which is duration from document load till all charts finished render. But when a user opens a dashboard and visits another browser tab right away, the dashboard won’t start loading its slices until the user goes back to focus on this browser tab: [https://superuser.com/questions/779021/google-chrome-background-tabs-dont-load-build-until-selected]
Modern browser supports Page Visibility API to detect current webpage's visibility:
https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API
This PR is to check if user is focused on other browser tab and Superset dashboard is hidden. If so, we should log it as a special case:
TEST PLAN
CI and manual test
REVIEWERS
@etr2460 @serenajiang