feat(rum-core): end spa navigations after browser frame

[vigneshshanmugam]

• Alternative solution to feat(rum-core): end transaction after scheduled browser frames #729
• fixes ending automatic transactions in a graceful way  #404 Capture all spans on route end in angular integration #636
• The solution is based on scheduling the transaction to be ended after the browser paints the new frame ( includes script, layout, style and paint). This ensures that if there is any network activity or if the underlying framework yields to the browser to do the work (style, paint). We include those timings as well which would help us measure most of the layout changes.

There is a detailed blog post explaining different ways to schedule a call after next frame. We are using the rAF + setTimeout technique as we want the call to get executed even if the tab is opened in invisible mode (cmd + click).

Pros

• Simpler and Easier to reason about, don't need to keep the state on the frame running and scheduling problems
• Gets called even if the current tab is invisible but with delays (due to the use of timers). RAF never gets called for invisible tabs.

Cons

• We wont be able to measure if the user does setTimeout(task, 1000) since it could be scheduled by the browser in next to next frames and we only measure the next frame after the navigation. This should be okay as it is not a usual thing.
• we might increase the transaction duration. by 16.67ms worst case due to the use of rAF, but we would be able to capture something the frameworks/user is doing during layout so might be worth it.

Personal Opinion, I like this solution better than the other one due to its simplicity and solving for 99% of the use cases. Was able to work for most of the cases when the call is scheduled in
setImmediate, setTimeout(call, 0), requestAnimationFrame, requestIdleCallback, Promise.resolve, MessageChannel during manual testing.

[codecov-io]

Codecov Report

Merging #730 into master will decrease coverage by 0.50%.
The diff coverage is 100.00%.

@@ Coverage Diff @@ ## master #730 +/- ## ========================================== - Coverage 94.21% 93.71% -0.51%  ========================================== Files 48 49 +1 Lines 2162 2194 +32 Branches 427 437 +10 ========================================== + Hits 2037 2056 +19  - Misses 122 135 +13  Partials 3 3 

Impacted Files	Coverage Δ
packages/rum-core/src/common/utils.js	94.30% <100.00%> (ø)
...e/src/performance-monitoring/capture-navigation.js	99.18% <100.00%> (-0.01%)	⬇️
packages/rum-react/src/get-with-transaction.js	67.79% <0.00%> (-0.63%)	⬇️
packages/rum-core/src/index.js	100.00% <0.00%> (ø)
packages/rum-vue/src/route-hooks.js	100.00% <0.00%> (ø)
packages/rum-core/src/common/after-frame.js	12.50% <0.00%> (ø)
packages/rum-angular/src/apm-service.ts	84.37% <0.00%> (+0.50%)	⬆️
.../src/performance-monitoring/transaction-service.js	91.12% <0.00%> (+0.59%)	⬆️

[hmdhk]

Had a meeting:

• We need to test for afterFrame to investigate behaviour on different browsers
• We also need test for react.lazy

[vigneshshanmugam]

jenkins run the tests please

[hmdhk]

@vigneshshanmugam , Would you please comment about the Chrome 76 scheduling issue here? Let's dig into more detail to see if there's an underlying issue with this approach instead of bumping the chrome version.

[vigneshshanmugam]

@jahtalab

Would you please comment about the Chrome 76 scheduling issue here?

Its not chrome 76 scheduling issue (sorry for the noise), I added more details with comments in the test itself. It's hard to know test this scenario for different browsers, thats why the test checks for 2 or 3 more, Also its not a common scenario - No one of the frameworks that I have tested(React, Vue, Angular, Preact) use this double timeouts.

There is no underlying issue with our approach, instead if the user really uses this way of rendering something, Neither the frameworks not our library would be able to track it. Unless we monkey patch.

[vigneshshanmugam]

Wrote a detailed comment on the double setTimeout tests https://github.com/elastic/apm-agent-rum-js/pull/730/files#diff-c6439d182e960bb8b3e580ba6c8e250aR61-R88 and also moved them to edge case test.

[vigneshshanmugam]

Lazy component mount cannot be solved with this PR as our Route component wont be even kicked in(Transaction will not start until React fetches all underlying dependencies) when the dependencies are being fixed. Also this PR is not for addressing that issue, instead measuring everything that happens before the browser renders the next frame in a better way.

However I added tests when lazy components are fetched inside a component that is child of our ApmRoute component.

Lazy components can be still measured effectively by the technique already mentioned here - https://www.elastic.co/guide/en/apm/agent/rum-js/current/react-integration.html#_instrumenting_lazy_loaded_routes