Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Commit

Permalink
Add basic Glean integration (#1129) (#1139)
Browse files Browse the repository at this point in the history
* Add basic Glean integration (#1129)

This includes the minimum initialization for Glean, which will only enable and emit the Glean baseline ping.  This is tied to the "Send Usage Data" preference in order for users to be able to opt-out of telemetry collection.

This PR also adds and sets up a `GleanActionHandler` observer of the `TelemetryActions`, even though it's not doing anything with those actions yet.  This is in preparation for migration of existing telemetry to Glean but the actual collection of events and other metrics will be included in a subsequent PR.

Includes update to metrics.md to add Glean collection and references.

* Add comment clarifying `setUploadEnabled` is called before `initialize`

* Ensure the Glean observer of `recordUsageData` is used on main thread
  • Loading branch information
travis79 authored and kaylagalway committed Dec 6, 2019
1 parent 640b663 commit f483e5d
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 4 deletions.
3 changes: 2 additions & 1 deletion Cartfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ github "getsentry/sentry-cocoa" "4.3.4"
github "jrendel/SwiftKeychainWrapper" "3.3.0"
github "mozilla-mobile/MappaMundi" "02b6f0b404d0a7178c47c073550936016f42981e"
github "mozilla-mobile/telemetry-ios" "1.1.1"
github "mozilla/application-services" "v0.42.3"
github "mozilla/application-services" "v0.42.3"
github "mozilla/glean" "v21.3.0"
1 change: 1 addition & 0 deletions Cartfile.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ github "jrendel/SwiftKeychainWrapper" "3.3.0"
github "mozilla-mobile/MappaMundi" "02b6f0b404d0a7178c47c073550936016f42981e"
github "mozilla-mobile/telemetry-ios" "1.1.1"
github "mozilla/application-services" "v0.42.3"
github "mozilla/glean" "v21.3.0"
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class CredentialProviderPresenter {
private let userDefaultStore: UserDefaultStore
fileprivate let dataStore: DataStore
private let telemetryActionHandler: TelemetryActionHandler
private let gleanActionHandler: GleanActionHandler
private let credentialProviderStore: CredentialProviderStore
private var credentialProvisionBag = DisposeBag()
private let disposeBag = DisposeBag()
Expand All @@ -36,6 +37,7 @@ class CredentialProviderPresenter {
userDefaultStore: UserDefaultStore = .shared,
dataStore: DataStore = .shared,
telemetryActionHandler: TelemetryActionHandler = TelemetryActionHandler(accountStore: AccountStore.shared),
gleanActionHandler: GleanActionHandler = GleanActionHandler(),
credentialProviderStore: CredentialProviderStore = .shared,
sizeClassStore: SizeClassStore = .shared) { // SizeClassStore needs to be initialized
self.view = view
Expand All @@ -45,6 +47,7 @@ class CredentialProviderPresenter {
self.userDefaultStore = userDefaultStore
self.dataStore = dataStore
self.telemetryActionHandler = telemetryActionHandler
self.gleanActionHandler = gleanActionHandler
self.credentialProviderStore = credentialProviderStore

self.accountStore.syncCredentials
Expand Down
6 changes: 6 additions & 0 deletions Lockbox.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@
CD7DFE7D2376322B004FB690 /* FeatureFlags.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD7DFE7B2376322B004FB690 /* FeatureFlags.swift */; };
CD7DFE7E2376322B004FB690 /* FeatureFlags.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD7DFE7B2376322B004FB690 /* FeatureFlags.swift */; };
CD7DFE7F2376322B004FB690 /* FeatureFlags.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD7DFE7B2376322B004FB690 /* FeatureFlags.swift */; };
1F33DE14237E32500002BA6B /* Glean.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 1F33DE0D237E036B0002BA6B /* Glean.framework */; };
/* End PBXBuildFile section */

/* Begin PBXCopyFilesBuildPhase section */
Expand Down Expand Up @@ -359,6 +360,7 @@
/* Begin PBXFileReference section */
08454B9621486AAC00328040 /* CredentialUserDefaultStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CredentialUserDefaultStore.swift; sourceTree = "<group>"; };
08454B9921486BCE00328040 /* TelemetryStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = TelemetryStore.swift; path = Shared/Store/TelemetryStore.swift; sourceTree = SOURCE_ROOT; };
1F33DE0D237E036B0002BA6B /* Glean.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Glean.framework; path = Carthage/Build/iOS/Glean.framework; sourceTree = "<group>"; };
22336C7121111087004E7B50 /* OnboardingConfirmationPresenterSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingConfirmationPresenterSpec.swift; sourceTree = "<group>"; };
224A6C7C212C8C2B008C7A3F /* UIFont+.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIFont+.swift"; sourceTree = "<group>"; };
226E3FEF2127759C00185D11 /* ExternalLinkStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExternalLinkStore.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -731,6 +733,7 @@
7D4396572135D79800B96FA9 /* Sentry.framework */,
7D897EAD2260FFA800257946 /* Reachability.framework */,
7D116242225FE11C009097C7 /* MozillaAppServices.framework */,
1F33DE14237E32500002BA6B /* Glean.framework in Frameworks */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -1058,6 +1061,7 @@
7D1B1A8A1F98F86400C1F5FF /* Frameworks */ = {
isa = PBXGroup;
children = (
1F33DE0D237E036B0002BA6B /* Glean.framework */,
7DF455DB22A1D3D200207285 /* RxRelay.framework */,
7D897EAC2260FFA800257946 /* Reachability.framework */,
7D11623E225FDD27009097C7 /* SystemConfiguration.framework */,
Expand Down Expand Up @@ -1565,6 +1569,7 @@
"$(SRCROOT)/Carthage/Build/iOS/SwiftProtobuf.framework",
"$(SRCROOT)/Carthage/Build/iOS/Reachability.framework",
"$(SRCROOT)/Carthage/Build/iOS/RxRelay.framework",
"$(SRCROOT)/Carthage/Build/iOS/Glean.framework",
);
name = "Run Script";
outputPaths = (
Expand All @@ -1580,6 +1585,7 @@
"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Sentry.framework",
"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Reachability.framework",
"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/RxRelay.framework",
"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Glean.framework",
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
Expand Down
25 changes: 25 additions & 0 deletions Shared/Action/TelemetryAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import Foundation
import Glean
import Telemetry
import RxSwift
import RxCocoa
Expand Down Expand Up @@ -61,6 +62,30 @@ enum ExtraKey: String {
case fxauid, itemid, error
}

class GleanActionHandler: ActionHandler {
private var disposeBag = DisposeBag()

init(glean: Glean = Glean.shared,
store: UserDefaultStore = UserDefaultStore.shared) {
store.recordUsageData
.observeOn(MainScheduler.instance)
.subscribe(
onNext: { uploadEnabled in
// This is invoked once during the call to subscribe on
// the main thread, so it will ensure that the Glean
// requirement to call `setUploadEnabled()` before
// `initialize()` is called below
glean.setUploadEnabled(uploadEnabled)
},
onError: nil,
onCompleted: nil,
onDisposed: nil
).disposed(by: self.disposeBag)

glean.initialize()
}
}

class TelemetryActionHandler: ActionHandler {
private let telemetry: Telemetry
private let accountStore: BaseAccountStore
Expand Down
13 changes: 10 additions & 3 deletions docs/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ _Last Updated: May 1, 2018_
<!-- TOC depthFrom:2 depthTo:6 withLinks:1 updateOnSave:1 orderedList:0 -->

- [Analysis](#analysis)
- [Collection](#collection)
- [Collection](#collection-legacy)
- [List of Proposed Events](#list-of-proposed-events)
- [Mozilla Glean SDK](#mozilla-glean-sdk)
- [References](#references)

<!-- /TOC -->
Expand Down Expand Up @@ -52,7 +53,7 @@ In service to validating the above hypothesis, we plan on answering these specif

In addition to answering the above questions that directly concern actions in the app, we will also be analyzing telemetry emitted from the password manager that exists in the the Firefox desktop browser. These analyses will primarily examine whether users of Lockbox start active curation of their credentials in the desktop browser (Lockbox users will not be able to edit credentials directly from the app).

## Collection
## Collection (legacy)

Data will be collected using this library:

Expand Down Expand Up @@ -157,9 +158,15 @@ https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/c
* `value`: nil
* `extras`: nil

## Mozilla Glean SDK

Lockwise for iOS uses the [Glean SDK](https://mozilla.github.io/glean/book/index.html) to collect telemetry. The Glean SDK provides a handful of [pings and metrics out of the box](https://mozilla.github.io/glean/book/user/pings/index.html). The data review for using the Glean SDK is available at [this link](TODO).

## References

[Library used to collect and send telemetry on iOS](https://github.com/mozilla-mobile/telemetry-ios/)
[Glean SDK repository, used to collect and send telemetry](https://github.com/mozilla/glean/)

[Legacy library used to collect and send telemetry on iOS](https://github.com/mozilla-mobile/telemetry-ios/)

[Description of the "Core" ping](https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/core-ping.html)

Expand Down
3 changes: 3 additions & 0 deletions lockbox-ios/Presenter/RootPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class RootPresenter {
fileprivate let userDefaultStore: UserDefaultStore
fileprivate let lifecycleStore: LifecycleStore
fileprivate let telemetryActionHandler: TelemetryActionHandler
fileprivate let gleanActionHandler: GleanActionHandler
fileprivate let biometryManager: BiometryManager
fileprivate let sentryManager: SentryStore
fileprivate let adjustManager: AdjustManager
Expand All @@ -72,6 +73,7 @@ class RootPresenter {
userDefaultStore: UserDefaultStore = .shared,
lifecycleStore: LifecycleStore = .shared,
telemetryActionHandler: TelemetryActionHandler = TelemetryActionHandler(accountStore: AccountStore.shared),
gleanActionHandler: GleanActionHandler = GleanActionHandler(),
biometryManager: BiometryManager = BiometryManager(),
sentryManager: SentryStore = SentryStore.shared,
adjustManager: AdjustManager = AdjustManager.shared,
Expand All @@ -88,6 +90,7 @@ class RootPresenter {
self.userDefaultStore = userDefaultStore
self.lifecycleStore = lifecycleStore
self.telemetryActionHandler = telemetryActionHandler
self.gleanActionHandler = gleanActionHandler
self.biometryManager = biometryManager
self.sentryManager = sentryManager
self.adjustManager = adjustManager
Expand Down
13 changes: 13 additions & 0 deletions lockbox-iosTests/UserDefaults+Spec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Nimble
import RxSwift
import RxTest
import RxCocoa
import Glean

@testable import Lockbox

Expand Down Expand Up @@ -70,6 +71,18 @@ class UserDefaultSpec: QuickSpec {
UserDefaults.standard.set(false, forKey: LocalUserDefaultKey.recordUsageData.rawValue)
expect(recordUsageDataSettingObserver.events.last!.value.element).to(beFalse())
}

it("triggers Glean.setUploadEnabled() before Glean.initialize()") {
// Set the default value to false since Glean upload enabled flag defaults to true.
UserDefaultStore.shared.userDefaults
.set(false, forKey: LocalUserDefaultKey.recordUsageData.rawValue)
// Create the ActionHandler which will should call `Glean.shared.setUploadEnabled()`
// and `Glean.shared.initialize()`. Since upload defaults to true in Glean, if this
// is false it means that initialize is called after setUploadEnabled has been called
// with the value that was just set above.
_ = GleanActionHandler()
expect(Glean.shared.getUploadEnabled()).to(beFalse())
}
}
}
}

0 comments on commit f483e5d

Please sign in to comment.