[Perf] Add networkInstrumentationEnabled opt-out flag (#8277)#16168
[Perf] Add networkInstrumentationEnabled opt-out flag (#8277)#16168JesusRojass wants to merge 5 commits into
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a networkInstrumentationEnabled property to allow developers to opt-out of automatic URLSession and URLConnection instrumentation while keeping other performance monitoring features active. The setting can be configured via Info.plist, GULUserDefaults, or at runtime, with logic added to FPRConfigurations and FPRClient to handle the conditional registration of network instrumentation. Feedback was provided regarding the documentation style of the new method, suggesting it be made consistent with existing methods for better maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new opt-out mechanism for network instrumentation (URLSession/URLConnection) in the Firebase Performance SDK, configurable via Info.plist or at runtime. The review identified that the getter for the new property should reflect the active state of the SDK rather than just the pending configuration, and suggested simplifying the logic in the isNetworkInstrumentationEnabled method for better readability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a networkInstrumentationEnabled configuration option, allowing developers to selectively disable URLSession and URLConnection instrumentation via Info.plist or at runtime. The implementation includes new properties in FPRConfigurations and FPRClient, along with public API updates in FIRPerformance. Review feedback suggests resetting the networkInstrumentationSwizzled flag within disableInstrumentation to ensure consistency during testing and recommends using a more generic error code when settings are modified after the SDK has been configured.
Discussion
Fixes #8277
Adds an
Info.plistarray key,firebase_performance_swizzle_denylist, that lists class names to skip when registering swizzling instrumentors. Gives developers a way to opt out of swizzling for specific classes that surface leaks through third-party SDKs, without disabling the rest of Firebase Performance.Usage
Add the key to your app's
Info.plist. Each entry is the exact result ofNSStringFromClass. To address the URLSession leak described in #8277, deny-listNSURLSession:You can add as many entries as needed. The deny-list applies to any class the SDK tries to swizzle (URLSession, URLConnection, UIViewController, user-supplied delegate classes).
How it works
Enforced at the central swizzle chokepoint,
FPRInstrument.registerClassInstrumentor:. IfNSStringFromClass(instrumentedClass)is in the deny-list array, the method returnsNOand the caller bails before any swizzle is installed. Three routes go through the chokepoint:registerInstrumentors.+sharedSessionand+sessionWithConfiguration:discover__NSCFURLSessionand friends).NSURLSessionDelegateclasses registered byFPRNSURLSessionDelegateInstrument.Strict
NSStringFromClassequality, no subclass walk. Read fresh per registration call fromInfo.plist, noGULUserDefaultsmirror, no runtime mutation API.Testing
All existing tests pass. No new tests added: the deny-list runs at a single line in
FPRInstrument.registerClassInstrumentor:and uses standardInfo.plistarray reading.API Changes