-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
feat(performance): Add Time to Full Display and manual API for TTID #3654
Conversation
…to @krystofwoldrich/add-ttid
iOS (new) Performance metrics 🚀
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cocoa code looks good.
There is one thing to fix in the TypeScript behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, left a few minor comments
@Override | ||
public List<ViewManager> createViewManagers( | ||
ReactApplicationContext reactContext) { | ||
return List.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m
: List.of
is only available in Java 9 or newer, I think it would be safer to use Arrays.asList()
from the java.util.Arrays
package.
android/src/main/java/io/sentry/react/RNSentryOnDrawReporterManager.java
Outdated
Show resolved
Hide resolved
android/src/main/java/io/sentry/react/RNSentryOnDrawReporterManager.java
Outdated
Show resolved
Hide resolved
android/src/main/java/io/sentry/react/RNSentryOnDrawReporterManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
📢 Type of change
📜 Description
This PR adds manual API for measuring TTID and TTFD.
TTID and TTFD use React Native Native Components,
android.view.View
on Android andUIView
on iOS. This enables the SDK to reliably hook into the rendering loop and wait for the next frame after the native component is created.Why the need for native components?
We tested exposing the hook for the next frame to JS runtime, which turned out unreliable. When waiting for the component to mount and receiving the mount event back JS and based on that starting listening for the next frame, depending on the JS loop, the listen for the next frame might be significantly (10s, 100s ms) delayed. This only works in ideal scenarios, when JS runtime starts the listen immediately after receiving the mount event.
What does the API look like?
When combined with auto instrumentation only the components are needed. The Time To Display is only measured when
record=true
, any consequent re-renders are ignored until a newactiveSpan
.The Time To Display spans can be started manually.
💚 How did you test it?
sample app, unit and integration tests
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps