[Review Only] Google Analytics changes #1146
Conversation
|
|
||
| func setGoogleAnalyticsPref() { | ||
| // Enable/disable GA based on settings. | ||
| let disableGoogleAnalytics = UserDefaults.standard.bool(forKey: "disableGoogleAnalytics") |
There was a problem hiding this comment.
This is due to an iOS limitation, see: https://stackoverflow.com/questions/36253998
There was a problem hiding this comment.
This isn't an iOS limitation but rather a misunderstanding of the User Defaults API. It has been a while since I've used it, but if I remember quickly, default values must be registered using UserDefaults.register(defaults:). Also, be aware that the setting can change while the app is active, so the setting should be observed for changes as well. In fact, using the initial option, the observation itself can be what enables and disables analytics exclusively.
|
@TADraeseke We'll use this PR as a preview - API keys will be added when we release with GA. |
|
|
||
| func setGoogleAnalyticsPref() { | ||
| // Enable/disable GA based on settings. | ||
| let disableGoogleAnalytics = UserDefaults.standard.bool(forKey: "disableGoogleAnalytics") |
There was a problem hiding this comment.
This isn't an iOS limitation but rather a misunderstanding of the User Defaults API. It has been a while since I've used it, but if I remember quickly, default values must be registered using UserDefaults.register(defaults:). Also, be aware that the setting can change while the app is active, so the setting should be observed for changes as well. In fact, using the initial option, the observation itself can be what enables and disables analytics exclusively.
| 00BE7B36275AEFD60012CE95 /* XCRemoteSwiftPackageReference "firebase-ios-sdk" */ = { | ||
| isa = XCRemoteSwiftPackageReference; | ||
| repositoryURL = "https://github.com/firebase/firebase-ios-sdk"; | ||
| requirement = { | ||
| kind = upToNextMajorVersion; | ||
| minimumVersion = 8.10.0; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
I've never used Firebase before. What impact does adding this dependency have on app size (how large is the Firebase framework)? Also, will we need to change the App Privacy section of our App Store page?
For what it is worth, I generally am not in favor of adding third part dependencies unless there is an extremely compelling reason to do so. Each dependency, especially one like this that executes code even before that app is useable by the user, is a failure point. Remember a while back when a bunch of prominent apps stopped working for a while because they used a popular third party analytics framework that had a problem? That is the power that dependencies like this hold. In my opinion, it isn't worth it. But that's up to you and the samples team to decide.
There was a problem hiding this comment.
- app size
The impact is quite minimum. The previous version (100.13.0.1) of sample viewer app is ~85 MB (can't remember exact number), and the current version (100.13.0.2) is 87 MB, according to App Store on iPhone 11. - App Privacy section
I've included usage data (select sample, category, and source code view) and search history (search bar) policies, prior to the release. - third party dependencies
I agree. I also tried to avoid using 3rd party dependencies in my personal project.
We researched a few options: Google Analytics, Amazon Pinpoint, Adobe Analytics, and building an AWS based in house solution. Eventually Trevor decided GA is the easiest way to go for now, as we wanted to use it as an experiment to see what data we can get. In the future we may decide to move to other solutions or remove GA dependency.
Description
This is a review only PR - we'll not merge this branch to
v.nextormain.This branch contains all the changes except the API key file
GoogleService-Info.plist, so that we can review the code changes without leaking secrets. Let's take a brief look to ensure nothing unexpected is included by Ting.Linked Issue(s)
common-samples/issues/3282common-samples/issues/535How To Test
Screenshots
The telemetry design at this stage
Some test data as below
To Discuss