Introduce an Experiments.shared object to add Nimbus experiments #8223

jhugman · 2021-03-25T19:55:34Z

We introduce a singleton Experiments.shared to give Firefox for iOS access to the Nimbus experimentation platform.

This ~~is branched from @rfk's upgrade of Application Services, but~~ will require a version of Application Services' library of at least 75.0.0
(including mozilla/application-services#3901, mozilla/application-services#3930 and mozilla/application-services#3991)
to enable access to the Nimbus.swift wrapper and the Nimbus-SDK.

Because of this, it is not ready to land on main. We should discuss when it is ready, and where to land it in the meantime.

However, it should be ready for review.

~~Because it is building on another branch outside of the main repo, I can't base this PR on that branch. The commit to review is~~

mergify · 2021-03-25T20:02:55Z

This pull request has conflicts when rebasing. Could you fix it @jhugman?

rfk · 2021-03-25T21:08:19Z

This is branched from @rfk's upgrade of Application Services,

FYI those changes are now on main, but via a rebased commit, so I expect you may need to rebase away my commit on your branch here as well.

jhugman · 2021-03-26T17:23:34Z

Client/Application/AppDelegate.swift

@@ -109,6 +109,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UIViewControllerRestorati

        telemetry = TelemetryWrapper(profile: profile)

+        // Start intialzing the Nimbus SDK. This should be done after Glean
+        // has been started.
+        initializeExperiments()


This needs to be initialized after Glean, but as early as possible so it's available to startup experiments.

Technically, it could be initialized before Glean as Glean caches recordings before it is initialized and replays them when it is.

I was seeing Rust errors in FxA client during the initialization of TelemetryWrapper when putting nimbus init earlier.

jhugman · 2021-03-26T19:06:35Z

Client/Experiments/Experiments.swift

+    static var dbPath: String? {
+        let profilePath: String?
+        if AppConstants.IsRunningTest || AppConstants.IsRunningPerfTest {
+            profilePath = (UIApplication.shared.delegate as? TestAppDelegate)?.dirForTestProfile
+        } else {
+            profilePath = FileManager.default.containerURL(
+                forSecurityApplicationGroupIdentifier: AppInfo.sharedContainerIdentifier
+            )?
+                .appendingPathComponent("profile.profile")
+                .path
+        }
+        let dbPath = profilePath.flatMap {
+            URL(fileURLWithPath: $0).appendingPathComponent("nimbus.db").path
+        }
+
+        if let dbPath = dbPath, Logger.logPII {
+            log.info("Nimbus database: \(dbPath)")
+        }
+        return dbPath
+    }


This feels like it should be utility function someplace (it's all static), but I couldn't find one.

For comparison, Glean gets its file directory like this:

func getGleanDirectory() -> URL { let paths = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask) let documentsDirectory = paths[0] return documentsDirectory.appendingPathComponent("glean_data") }

I see this asks for a path to a shared container so that it is the same storage for both the app and any app-extensions. Just FYI, allowing Nimbus to run in an iOS app-extension may deserve some consideration since app-extensions run in their own process, typically have their own storage and memory, and Glean does not support being ran in a app-extension process. This may all work just fine for Nimbus, but beware: here be dragons (from my experiences with Glean on iOS).

I've been worried about extensions.

I had considered working with extensions to be part of another, as yet defined epic, once there was a demand for it. It will require some fundamental work in Nimbus SDK, e.g. understanding accessing rkv across process boundaries, running rkv in different processes to access the same file, or running completely different instances of nimbus and database, that only share ids (e.g. in the keychain).

That Glean doesn't support extensions yet, is just yet another reason for Nimbus not worry about them right now.

travis79 · 2021-03-26T19:29:41Z

Client/Application/AppDelegate.swift

@@ -109,6 +109,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UIViewControllerRestorati

        telemetry = TelemetryWrapper(profile: profile)

+        // Start intialzing the Nimbus SDK. This should be done after Glean
+        // has been started.
+        initializeExperiments()


Technically, it could be initialized before Glean as Glean caches recordings before it is initialized and replays them when it is.

travis79 · 2021-03-26T19:48:19Z

Client/Experiments/Experiments.swift

+    static var dbPath: String? {
+        let profilePath: String?
+        if AppConstants.IsRunningTest || AppConstants.IsRunningPerfTest {
+            profilePath = (UIApplication.shared.delegate as? TestAppDelegate)?.dirForTestProfile
+        } else {
+            profilePath = FileManager.default.containerURL(
+                forSecurityApplicationGroupIdentifier: AppInfo.sharedContainerIdentifier
+            )?
+                .appendingPathComponent("profile.profile")
+                .path
+        }
+        let dbPath = profilePath.flatMap {
+            URL(fileURLWithPath: $0).appendingPathComponent("nimbus.db").path
+        }
+
+        if let dbPath = dbPath, Logger.logPII {
+            log.info("Nimbus database: \(dbPath)")
+        }
+        return dbPath
+    }


For comparison, Glean gets its file directory like this:

func getGleanDirectory() -> URL { let paths = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask) let documentsDirectory = paths[0] return documentsDirectory.appendingPathComponent("glean_data") }

I see this asks for a path to a shared container so that it is the same storage for both the app and any app-extensions. Just FYI, allowing Nimbus to run in an iOS app-extension may deserve some consideration since app-extensions run in their own process, typically have their own storage and memory, and Glean does not support being ran in a app-extension process. This may all work just fine for Nimbus, but beware: here be dragons (from my experiences with Glean on iOS).

travis79 · 2021-03-26T19:56:18Z

Client/Experiments/Experiments.swift

+        // channel. The values given here should match what `Experimenter`
+        // thinks it is.
+        let appSettings = NimbusAppSettings(
+            appName: "fxios",


Note that this should be changed to firefox_ios. Source of truth for this app name is currently defined here: https://github.com/mozilla/probe-scraper/blob/b3730a6d22e70519beeb2c5df6aac6fb8d3ed00f/repositories.yaml#L231

This is useful, thanks.

jhugman requested review from travis79 and boek March 25, 2021 19:56

jhugman force-pushed the jhugman/fxios-hearts-nimbus branch from 6b61588 to 49b5d46 Compare March 26, 2021 19:04

jhugman commented Mar 26, 2021

View reviewed changes

travis79 reviewed Mar 26, 2021

View reviewed changes

jhugman force-pushed the jhugman/fxios-hearts-nimbus branch from 7f4b062 to 7216138 Compare March 29, 2021 15:45

jhugman added 2 commits March 30, 2021 19:52

Introduce a singleton for Nimbus, and initialize at startup

1e84fcf

Address reviewer comments

d814cfb

jhugman force-pushed the jhugman/fxios-hearts-nimbus branch from 7216138 to d814cfb Compare March 30, 2021 19:04

boek approved these changes Mar 30, 2021

View reviewed changes

jhugman merged commit 9e1a42a into main Mar 31, 2021

jhugman deleted the jhugman/fxios-hearts-nimbus branch March 31, 2021 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce an Experiments.shared object to add Nimbus experiments #8223

Introduce an Experiments.shared object to add Nimbus experiments #8223

jhugman commented Mar 25, 2021 •

edited

Loading

mergify bot commented Mar 25, 2021

rfk commented Mar 25, 2021

jhugman Mar 26, 2021

travis79 Mar 26, 2021

jhugman Mar 26, 2021

jhugman Mar 26, 2021

travis79 Mar 26, 2021

jhugman Mar 26, 2021

travis79 Mar 26, 2021

travis79 Mar 26, 2021

travis79 Mar 26, 2021

jhugman Mar 26, 2021

Introduce an Experiments.shared object to add Nimbus experiments #8223

Introduce an Experiments.shared object to add Nimbus experiments #8223

Conversation

jhugman commented Mar 25, 2021 • edited Loading

mergify bot commented Mar 25, 2021

rfk commented Mar 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhugman commented Mar 25, 2021 •

edited

Loading