-
Notifications
You must be signed in to change notification settings - Fork 26
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
native: add query traces to Firebase performance #3852
Conversation
4de0371
to
3949602
Compare
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! curious to see what this does
added some screenshots of what the Firebase dashboards look like – I think the immediate benefit is seeing when a new release significantly changes the duration or volume of queries. We are currently not using HTTP request traces (I don't know if this makes sense for Tlon – maybe if we restrict to only Tlon hosting endpoints?); and we're not using "screen traces", which supposedly reports "slow rendering / frozen frames" (when I tested this with the basic setup, it seemed to only detect generically-named React Native screens, so I think we'd have to adapt it for our usage). |
@react-native-firebase/perf
, disabling automatic instrumentationstate
in the store, not as...state
– so loading/saving multiple times ended up with a recurring nested flags object :( I don't understand why this was okay by Zustand's types – 3949602 fixes this in a way that should avoid similar bugs in the futurecreateQuery
– see thequery.*
traces under Custom traces here: https://console.firebase.google.com/u/2/project/tlon-groups-mobile/performance/app/ios:io.tlon.groups/trendsThe design of the performance monitoring code is a little abstract – this was necessary due to dependency constraints:
@tloncorp/shared
got broken after I added@react-native-firebase/perf
as a dependency@tloncorp/shared
has the feature flag setup, so the code enabling/disabling instrumentation needs to referenceshared
createQuery
(in@tloncorp/shared
) seemed like the only reasonable place to hook in. (It seems likeuse-query
should have some kind of global pre/post hooks, but I could not find anything.) So@tloncorp/shared
needs to be able to call into the performance monitoring controller.To resolve:
@tloncorp/shared
has a performance monitoring controllerusePerformanceMonitoringStore
that is agnostic to the performance monitoring "endpoint", abstracting that as an injectedPerformanceMonitoringEndpoint
interfaceusePerformanceMonitoringStore
is used increateQuery
to measure queries@tloncorp/app
implements aPerformanceMonitoringEndpoint
using@react-native-firebase/perf
, and registers it on theusePerformanceMonitoringStore
via a provider componenttlon-mobile
mounts the context provider near root, hooking everything upHow did I test?
getGroup
on a real group serially, 800 times; ran it once without any instrumentation code, and once with instrumentation enabled. There was less than 1ms of difference for the averaged query duration.Example dashboards