sentry-react-native icon indicating copy to clipboard operation
sentry-react-native copied to clipboard

Fix(V6): Implement fallback system to screens that aren't reporting on the native layer the time to display. #4042

Open lucas-zimerman opened this issue 1 year ago • 5 comments
trafficstars

This is a port of https://github.com/getsentry/sentry-react-native/pull/4042 to V6

Tested on Android/iOS: https://sentry-sdks.sentry.io/performance/trace/b0125a1564324d17b42156b5edbd4555/?dataset=transactions&field=title&field=project&field=user.display&field=timestamp&field=replayId&field=transaction.duration&fov=0%2C1910.000244140625&id=21705&name=&node=txn-ccda2e67a35c486d801315ad5c5020d9&node=txn-48c53c8b7b4a4335a12082cd81adba27&project=5428561&query=&queryDataset=transaction-like&sort=-timestamp&source=discover&statsPeriod=24h&timestamp=1729179000&topEvents=5&yAxis=count%28%29

https://sentry-sdks.sentry.io/performance/trace/514b002f17874570b2f016f0f78b9953/?dataset=transactions&field=title&field=project&field=user.display&field=timestamp&field=replayId&fov=0%2C75.13809204101562&id=21705&name=&node=span-87836929b3543124&node=txn-400cebc647684a688c5355197859dd40&project=5428561&query=&queryDataset=transaction-like&sort=-timestamp&source=discover&statsPeriod=1h&timestamp=1729198686&topEvents=5&yAxis=count%28%29

:loudspeaker: Type of change

  • [x] Bugfix
  • [ ] New feature
  • [ ] Enhancement
  • [ ] Refactoring

:scroll: Description

Context: the current native implementation doesn't always emit the required event in order to track the required time to display to be set on the time to display span.

To fix that, I introduced an additional fallback emitter, in short, it reuses the same flow by emitting the original event when the fallback emitter received the notification that the page was rendered and the original emitter didn't emit any event on the following 3 seconds.

With those changes, we have the following benefits:

  • Support for React Native web by also having the JavaScript fallback of requestAnimationFrame.
  • Greatly reduces the wrong time to display end timestamp by having a fallback when the original implementation doesn't return any event.
  • Fallback to the JavaScript implementation in case both Native emitters fail.

:bulb: Motivation and Context

Close #3934, https://github.com/getsentry/sentry-react-native/issues/3809

:green_heart: How did you test it?

The playground tab on our sample is a good case for testing, since it doesn't use navigation stack on it, the events on this condition are limited, not generating and required event by us in order to track the time to display.

I used it and other tabs to compare the difference between the original implementation and also requestAnimationFrame, and the time difference between each other was quite low:

1St Tab JS: requestAnimationFrame, "newFrameTimestampInSeconds" is 1724293664.8300002 Original: InitAsync Event received {"newFrameTimestampInSeconds":1724293664.8179998}

2Nd Tab JS: requestAnimationFrame, "newFrameTimestampInSeconds" is 1724293761.688 Original: InitAsync Event received {"newFrameTimestampInSeconds":1724293761.689}

3Rd Tab JS: requestAnimationFrame, "newFrameTimestampInSeconds" is 1724293824.887 Original: Android didn't emit an event for this page so it wasn't measured

Before this change on the playground screen: https://sentry-sdks.sentry.io/performance/trace/10bd4bf28052404592f408f3cf58175a/?dataset=transactions&field=title&field=event.type&field=project&field=user.display&field=timestamp&field=replayId&fov=0%2C15046.88916015625&id=21705&name=&node=trace-root&project=5428561&query=&queryDataset=transaction-like&sort=-timestamp&source=discover&statsPeriod=1h&timestamp=1724358725&topEvents=5&yAxis=count%28%29

After this change on the playground screen: https://sentry-sdks.sentry.io/performance/trace/8cb478b4370e444fa4a7b9be778d51ab/?dataset=transactions&field=title&field=event.type&field=project&field=user.display&field=timestamp&field=replayId&fov=0%2C235&id=21705&name=&node=txn-b5a8ba18aa254fe79743133baf9071b2&project=5428561&query=&queryDataset=transaction-like&sort=-timestamp&source=discover&statsPeriod=1h&timestamp=1724359145&topEvents=5&yAxis=count%28%29

:pencil: Checklist

  • [x] I reviewed submitted code
  • [x] I added tests to verify changes
  • [x] No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • [ ] All tests passing
  • [x] No breaking changes

:crystal_ball: Next steps

lucas-zimerman avatar Oct 17 '24 20:10 lucas-zimerman