Skip to content
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

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

Merged
merged 2 commits into from
Mar 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Client.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@
39E65D191CA455A900C63CE3 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 391AEFD11C8F11ED00691F84 /* Images.xcassets */; };
39E65D271CA5B92000C63CE3 /* AsyncReducerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39E65D261CA5B92000C63CE3 /* AsyncReducerTests.swift */; };
39EB469A1E26DDB4006346E8 /* FxScreenGraph.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39EB46981E26DDB4006346E8 /* FxScreenGraph.swift */; };
39EF434E260A73950011E22E /* Experiments.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39EF434D260A73950011E22E /* Experiments.swift */; };
39EF4363260CEE5E0011E22E /* ExperimentConstants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39EF4362260CEE5E0011E22E /* ExperimentConstants.swift */; };
39EF4378260CF5A30011E22E /* AppDelegate+Experiments.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39EF4377260CF5A30011E22E /* AppDelegate+Experiments.swift */; };
39F4C0FA2045D87400746155 /* FocusHelper.js in Resources */ = {isa = PBXBuildFile; fileRef = 39F4C0F92045D87400746155 /* FocusHelper.js */; };
39F4C10A2045DB2E00746155 /* FocusHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39F4C1092045DB2E00746155 /* FocusHelper.swift */; };
39F819C61FD70F5D009E31E4 /* TabEventHandlers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 39F819C51FD70F5D009E31E4 /* TabEventHandlers.swift */; };
Expand Down Expand Up @@ -1740,6 +1743,9 @@
39DD4C8BA7FCE677AFDACC2D /* da */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = da; path = da.lproj/Localizable.strings; sourceTree = "<group>"; };
39E65D261CA5B92000C63CE3 /* AsyncReducerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AsyncReducerTests.swift; sourceTree = "<group>"; };
39EB46981E26DDB4006346E8 /* FxScreenGraph.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FxScreenGraph.swift; sourceTree = "<group>"; };
39EF434D260A73950011E22E /* Experiments.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Experiments.swift; sourceTree = "<group>"; };
39EF4362260CEE5E0011E22E /* ExperimentConstants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExperimentConstants.swift; sourceTree = "<group>"; };
39EF4377260CF5A30011E22E /* AppDelegate+Experiments.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AppDelegate+Experiments.swift"; sourceTree = "<group>"; };
39F4C0F92045D87400746155 /* FocusHelper.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = FocusHelper.js; sourceTree = "<group>"; };
39F4C1092045DB2E00746155 /* FocusHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FocusHelper.swift; sourceTree = "<group>"; };
39F819C51FD70F5D009E31E4 /* TabEventHandlers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabEventHandlers.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4518,6 +4524,15 @@
name = PushTests;
sourceTree = "<group>";
};
39EF434C260A736D0011E22E /* Experiments */ = {
isa = PBXGroup;
children = (
39EF434D260A73950011E22E /* Experiments.swift */,
39EF4362260CEE5E0011E22E /* ExperimentConstants.swift */,
);
path = Experiments;
sourceTree = "<group>";
};
39F99FC71E3A6DB700F353B4 /* Push */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -5435,6 +5450,7 @@
isa = PBXGroup;
children = (
F84B21E51A0910F600AAB793 /* AppDelegate.swift */,
39EF4377260CF5A30011E22E /* AppDelegate+Experiments.swift */,
EB9854FD2422686F0040F24B /* AppDelegate+PushNotifications.swift */,
EB98550024226EF60040F24B /* AppDelegate+SyncSentTabs.swift */,
D3BE7B451B054F8600641031 /* TestAppDelegate.swift */,
Expand Down Expand Up @@ -5724,6 +5740,7 @@
F84B21E41A0910F600AAB793 /* Application */,
E60961841B62B7E100DD640F /* Configuration */,
E689C6FF1E0C716F008BAADB /* Entitlements */,
39EF434C260A736D0011E22E /* Experiments */,
28EADE5C1AFC3A6D007FB2FB /* Extensions */,
F84B21F11A0910F600AAB793 /* Frontend */,
39A359BD1BFCCE7B006B9E87 /* Helpers */,
Expand Down Expand Up @@ -7513,6 +7530,7 @@
E68F36981EA694000048CF44 /* PanelDataObservers.swift in Sources */,
31ADB5DA1E58CEC300E87909 /* ClipboardBarDisplayHandler.swift in Sources */,
4368BC6824FF14650021931D /* NewTabUserResearch.swift in Sources */,
39EF4363260CEE5E0011E22E /* ExperimentConstants.swift in Sources */,
745DAB3F1CDAB09E00D44181 /* HistoryBackButton.swift in Sources */,
EB9A179D20E69A7F00B12184 /* Theme.swift in Sources */,
CA03B26A247F1D9E00382B62 /* BreachAlertsClient.swift in Sources */,
Expand All @@ -7521,6 +7539,7 @@
435D660523D794B90046EFA2 /* UpdateViewModel.swift in Sources */,
D0152245229855A8009DE753 /* OneLineCell.swift in Sources */,
D3B6923F1B9F9A58004B87A4 /* FindInPageHelper.swift in Sources */,
39EF4378260CF5A30011E22E /* AppDelegate+Experiments.swift in Sources */,
EB9A179C20E69A7F00B12184 /* DarkTheme.swift in Sources */,
EB1C84BF212EFFBF001489DF /* BrowserViewController+ReaderMode.swift in Sources */,
EBA3B2CD2268F27500728BDB /* PhotonActionSheetWidgets.swift in Sources */,
Expand Down Expand Up @@ -7688,6 +7707,7 @@
D8AA923421A602DC002605C0 /* HomePageSettingViewController.swift in Sources */,
EBC4869D2195F58300CDA48D /* ErrorPageHelper.swift in Sources */,
E660BDD91BB06521009AC090 /* TabsButton.swift in Sources */,
39EF434E260A73950011E22E /* Experiments.swift in Sources */,
63306D3921103EAE00F25400 /* SavedTab.swift in Sources */,
2F44FCCB1A9E972E00FD20CC /* SearchEnginePicker.swift in Sources */,
E68E7ACB1CAC1D4500FDCA76 /* PagingPasscodeViewController.swift in Sources */,
Expand Down
20 changes: 20 additions & 0 deletions Client/Application/AppDelegate+Experiments.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import Shared

private let log = Logger.browserLogger

import Foundation

extension AppDelegate {
func initializeExperiments() {
let defaults = UserDefaults()
let nimbusFirstRun = "NimbusFirstRun"
let firstRun = defaults.object(forKey: nimbusFirstRun) != nil
defaults.set(false, forKey: nimbusFirstRun)

Experiments.intialize(with: nil, firstRun: firstRun)
}
}
4 changes: 4 additions & 0 deletions Client/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// Set up a web server that serves us static content. Do this early so that it is ready when the UI is presented.
setUpWebServer(profile)

Expand Down
20 changes: 20 additions & 0 deletions Client/Experiments/ExperimentConstants.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import Foundation

/// An application specific enum of app features that we are configuring with experiments.
/// This is expected to grow and shrink across releases of the app.
enum FeatureId: String {
case nimbusValidation
}

/// A set of common branch ids used in experiments. Branch ids can be application/experiment specific, so
/// _could_ be an `enum`; however, there is a likelihood that they will become less relevant in the future.
enum ExperimentBranch {
static let a1 = "a1"
static let a2 = "a2"
static let control = "control"
static let treatment = "treatment"
}
176 changes: 176 additions & 0 deletions Client/Experiments/Experiments.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import Foundation
import MozillaAppServices
import Shared
import XCGLogger

private let log = Logger.browserLogger
private let nimbusAppName = "firefox_ios"

/// `Experiments` is the main entry point to use the `Nimbus` experimentation platform in Firefox for iOS.
///
/// This class is a application specific holder for a the singleton `NimbusApi` class.
///
/// It is needed to be initialized early in the startup of the app, so a lot of the heavy lifting of calculating where the
/// database should live, and deriving the `Remote Settings` URL for itself. This should be done with the
/// `initialize(with:,firstRun:)` method.
///
/// Most usage with be made of `Nimbus` by feature developers who wish to make decisions about how
/// to configure their features.
///
/// This should be done with the `withExperiment(featureId:)` method.
/// ```
/// button.text = Exeriments.shared.withExperiment(featureId: .submitButton) { branchId in
/// switch branchId {
/// ExperimentBranch.treatment -> return "Ok then"
/// else -> return "OK"
/// }
/// }
/// ```
///
/// Possible values for `featureId` correspond to the application features under experiment, and are
/// enumerated in the `FeatureId` `enum` in `ExperimentConstants.swift`.
///
/// Branches are left as `String`s as they are an unbounded set of values, but commonly used
/// constants are also defined in `ExperimentConstants`.
///
/// The server components of Nimbus are: `RemoteSettings` which serves the experiment definitions to
/// clients, and `Experimenter`, which is the user interface for creating and administering experiments.
///
/// Rust errors are not expected, but will be reported via Sentry.
enum Experiments {
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
}
Comment on lines +45 to +64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


static var remoteSettingsURL: String? {
// TODO: get URL from build secret. https://jira.mozilla.com/browse/SDK-212
return "https://firefox.settings.services.mozilla.com/"
}

/// The `NimbusApi` object. This is the entry point to do anything with the Nimbus SDK on device.
public static let shared: NimbusApi = {
guard AppConstants.NIMBUS_ENABLED else {
return NimbusDisabled.shared
}

guard let dbPath = Experiments.dbPath else {
log.error("Nimbus didn't get to create, because of a nil dbPath")
return NimbusDisabled.shared
}

// If no URL is specified, or it's not valid continue with as if
// we're enabled. This to allow testing of the app, without standing
// up a `RemoteSettings` server.
let serverSettings = Experiments.remoteSettingsURL.flatMap {
URL(string: $0)
}.flatMap {
NimbusServerSettings(url: $0)
}

// App settings, to allow experiments to target the app name and the
// channel. The values given here should match what `Experimenter`
// thinks it is.
let appSettings = NimbusAppSettings(
appName: nimbusAppName,
channel: AppConstants.BuildChannel.nimbusString
)

let errorReporter: NimbusErrorReporter = { err in
Sentry.shared.sendWithStacktrace(
message: "Error in Nimbus SDK",
tag: SentryTag.nimbus,
severity: .error,
description: err.localizedDescription
)
}

do {
let nimbus = try Nimbus.create(
serverSettings,
appSettings: appSettings,
dbPath: dbPath, errorReporter: errorReporter
)
log.info("Nimbus is now available!")
return nimbus
} catch {
errorReporter(error)
log.error("Nimbus errored during create")
return NimbusDisabled.shared
}
}()

/// A convenience method to initialize the `NimbusApi` object at startup.
///
/// This includes opening the database, connecting to the Remote Settings server, and downloading
/// and applying changes.
///
/// All this is set to run off the main thread.
///
/// - Parameters:
/// - fireURL: an optional file URL that stores the initial experiments document.
/// - firstRun: a flag indicating that this is the first time that the app has been run.
public static func intialize(with fileURL: URL?, firstRun: Bool) {
let nimbus = Experiments.shared

nimbus.initialize()

if let fileURL = fileURL, firstRun {
nimbus.setExperimentsLocally(fileURL)
}
// We should immediately calculate the experiment enrollments
// that we've just acquired from the fileURL, or we fetched last run.
nimbus.applyPendingExperiments()

// In the background, we should download the next version of the experiments
// document.
nimbus.fetchExperiments()

log.info("Nimbus is initializing!")
}
}

extension NimbusApi {
/// The entry point for feature developers configuring their feature with an experiment.
///
/// This may be called from any thread.
///
/// - Parameters:
/// - featureId: the id of the feature, as it is known by `Experimenter`.
/// - transform: the mapping between the experiment branch the user is in and something
/// useful for the feature. If the user is not enrolled in the experiment, the branch is `nil`.
func withExperiment<T>(featureId: FeatureId, transform: (String?) -> T) -> T {
let branch = getExperimentBranch(featureId: featureId.rawValue)
return transform(branch)
}
}

private extension AppBuildChannel {
var nimbusString: String {
switch self {
case .release: return "release"
case .beta: return "beta"
case .developer: return "nightly"
}
}
}
16 changes: 16 additions & 0 deletions Shared/AppConstants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,20 @@ public struct AppConstants {
#endif
}()


/// Use the Nimbus experimentation platform. If this is `true` then
/// `Experiments.shared` provides access to Nimbus. If false, it is a dummy object.
public static let NIMBUS_ENABLED: Bool = {
#if MOZ_CHANNEL_RELEASE
return false
#elseif MOZ_CHANNEL_BETA
return false
#elseif MOZ_CHANNEL_FENNEC
return true
#else
return false
#endif
}()


}
1 change: 1 addition & 0 deletions Shared/SentryIntegration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public enum SentryTag: String {
case tabManager = "TabManager"
case bookmarks = "Bookmarks"
case leanplum = "Leanplum"
case nimbus = "Nimbus"
}

public class Sentry {
Expand Down