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

fix: Use PerformanceObserver to act as a second check to prevent returning 0 for duration and loadEventEnd #101

Merged
merged 3 commits into from
Feb 25, 2022

Conversation

adebayor123
Copy link
Member

@adebayor123 adebayor123 commented Feb 24, 2022

Version 1.1.0 and later has a bug which causes com.amazon.rum.performance_navigation_event.duration to be 0 when the web client loads before window.load has been fired.

Root Cause:

  • Window fires load when the page has finished loading according to documentations, but there seems to be cases where it is actually fired before the loadEventEnd is fired.
  • PR fix: capture PerformanceNavigationTiming events even when window.load fires before plugin loads #81 correctly fixes the bug that did not capture when the page loads before the plugin does, but by removing PerformanceObserver, we started emitting navigation_events as soon as window fired load.
  • As a result, there are times when navigation_events emits 0 for fields such as duration, loadEventEnd, etc.

This PR addresses the issue by doing the following:

  1. No longer rely on callback when the page has already loaded (we determine by using loadEventEnd) and directly record the values.
  2. Restore PerformanceObserver for regular scenarios where page loads after the plugin.

Verifications:

  1. Unit Test
  2. Integ Test
  3. https://paste.amazon.com/show/renoseo/1645745904

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Comment on lines +26 to 30
* loadEventEnd is populated as 0 if the web page has not loaded completely, even though LOAD has been fired.
* As a result, if loadEventEnd is populated, we can ignore eventListener and record the data directly.
* On the other hand, if not, we have to use eventListener to initializes PerformanceObserver.
* PerformanceObserver will act as a second check for the final load timings.
*/
Copy link
Member

Choose a reason for hiding this comment

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

nit: The method's javadoc comment should explain the behavior of the method and optionally its inputs and outputs. Place implementation details as block comments inside the method.

@qhanam qhanam merged commit cea2c0b into aws-observability:main Feb 25, 2022
@adebayor123 adebayor123 deleted the navTimingFix branch March 2, 2022 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants