Skip to content

Commit dc16714

Browse files
[PM-19577] Add flight recorder and hook up to UI (#1500)
1 parent a702d3a commit dc16714

15 files changed

+291
-42
lines changed
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
@preconcurrency import Combine
2+
3+
// MARK: - FlightRecorder
4+
5+
/// A protocol for a service which can temporarily be enabled to collect logs for debugging to a
6+
/// local file.
7+
///
8+
protocol FlightRecorder: Sendable {
9+
/// Disables the collection of temporary debug logs.
10+
///
11+
func disableFlightRecorder() async
12+
13+
/// Enables the collection of temporary debug logs to a local file for a set duration.
14+
///
15+
/// - Parameter duration: The duration that logging should be enabled for.
16+
///
17+
func enableFlightRecorder(duration: FlightRecorderLoggingDuration) async
18+
19+
/// A publisher which publishes the enabled status of the flight recorder.
20+
///
21+
/// - Returns: A publisher for the enabled status of the flight recorder.
22+
///
23+
func isEnabledPublisher() async -> AnyPublisher<Bool, Never>
24+
}
25+
26+
// MARK: - DefaultFlightRecorder
27+
28+
/// A default implementation of a `FlightRecorder`.
29+
///
30+
@MainActor
31+
final class DefaultFlightRecorder {
32+
// MARK: Private Properties
33+
34+
/// A subject containing the enable status of the flight recorder.
35+
private let isEnabledSubject = CurrentValueSubject<Bool, Never>(false)
36+
37+
/// The service used by the application to manage account state.
38+
private let stateService: StateService
39+
40+
/// The service used to get the present time.
41+
private let timeProvider: TimeProvider
42+
43+
// MARK: Initialization
44+
45+
/// Initialize a `DefaultFlightRecorder`.
46+
///
47+
/// - Parameters:
48+
/// - stateService: The service used by the application to manage account state.
49+
/// - timeProvider: The service used to get the present time.
50+
///
51+
init(stateService: StateService, timeProvider: TimeProvider) {
52+
self.stateService = stateService
53+
self.timeProvider = timeProvider
54+
}
55+
}
56+
57+
// MARK: - DefaultFlightRecorder + FlightRecorder
58+
59+
extension DefaultFlightRecorder: FlightRecorder {
60+
func disableFlightRecorder() async {
61+
isEnabledSubject.send(false)
62+
}
63+
64+
func enableFlightRecorder(duration: FlightRecorderLoggingDuration) async {
65+
isEnabledSubject.send(true)
66+
}
67+
68+
func isEnabledPublisher() async -> AnyPublisher<Bool, Never> {
69+
isEnabledSubject.eraseToAnyPublisher()
70+
}
71+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import XCTest
2+
3+
@testable import BitwardenShared
4+
5+
class FlightRecorderTests: BitwardenTestCase {
6+
// MARK: Properties
7+
8+
var stateService: MockStateService!
9+
var subject: FlightRecorder!
10+
var timeProvider: MockTimeProvider!
11+
12+
// MARK: Setup & Teardown
13+
14+
override func setUp() {
15+
super.setUp()
16+
17+
stateService = MockStateService()
18+
timeProvider = MockTimeProvider(.mockTime(Date(year: 2025, month: 1, day: 1)))
19+
20+
subject = DefaultFlightRecorder(
21+
stateService: stateService,
22+
timeProvider: timeProvider
23+
)
24+
}
25+
26+
override func tearDown() {
27+
super.tearDown()
28+
29+
stateService = nil
30+
subject = nil
31+
timeProvider = nil
32+
}
33+
34+
// MARK: Tests
35+
36+
/// `disableFlightRecorder()` disables the flight recorder.
37+
func test_disableFlightRecorder() async {
38+
var isEnabledValues = [Bool]()
39+
let publisher = await subject.isEnabledPublisher().sink { isEnabledValues.append($0) }
40+
defer { publisher.cancel() }
41+
42+
await subject.enableFlightRecorder(duration: .twentyFourHours)
43+
await subject.disableFlightRecorder()
44+
45+
XCTAssertEqual(isEnabledValues, [false, true, false])
46+
}
47+
48+
/// `enableFlightRecorder(duration:)` enables the flight recorder for the specified duration.
49+
func test_enableFlightRecorder() async {
50+
var isEnabledValues = [Bool]()
51+
let publisher = await subject.isEnabledPublisher().sink { isEnabledValues.append($0) }
52+
defer { publisher.cancel() }
53+
54+
await subject.enableFlightRecorder(duration: .twentyFourHours)
55+
56+
XCTAssertEqual(isEnabledValues, [false, true])
57+
}
58+
59+
/// `isEnabledPublisher()` publishes the enabled status of the flight recorder.
60+
func test_isEnabledPublisher() async throws {
61+
var isEnabled = false
62+
let publisher = await subject.isEnabledPublisher().sink { isEnabled = $0 }
63+
defer { publisher.cancel() }
64+
65+
await subject.enableFlightRecorder(duration: .eightHours)
66+
XCTAssertTrue(isEnabled)
67+
68+
await subject.disableFlightRecorder()
69+
XCTAssertFalse(isEnabled)
70+
}
71+
}

BitwardenShared/Core/Platform/Services/ServiceContainer.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
8989
/// of the `Fido2UserInterface` from the SDK.
9090
let fido2UserInterfaceHelper: Fido2UserInterfaceHelper
9191

92+
/// The service used by the application for recording temporary debug logs.
93+
let flightRecorder: FlightRecorder
94+
9295
/// The repository used by the application to manage generator data for the UI layer.
9396
let generatorRepository: GeneratorRepository
9497

@@ -209,6 +212,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
209212
/// - fido2UserInterfaceHelper: A helper to be used on Fido2 flows that requires user interaction
210213
/// and extends the capabilities of the `Fido2UserInterface` from the SDK.
211214
/// - fido2CredentialStore: A store to be used on Fido2 flows to get/save credentials.
215+
/// - flightRecorder: The service used by the application for recording temporary debug logs.
212216
/// - generatorRepository: The repository used by the application to manage generator data for the UI layer.
213217
/// - importCiphersRepository: The repository used by the application to manage importing credential
214218
/// in Credential Exhange flow.
@@ -264,6 +268,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
264268
exportVaultService: ExportVaultService,
265269
fido2CredentialStore: Fido2CredentialStore,
266270
fido2UserInterfaceHelper: Fido2UserInterfaceHelper,
271+
flightRecorder: FlightRecorder,
267272
generatorRepository: GeneratorRepository,
268273
importCiphersRepository: ImportCiphersRepository,
269274
keychainRepository: KeychainRepository,
@@ -317,6 +322,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
317322
self.exportVaultService = exportVaultService
318323
self.fido2CredentialStore = fido2CredentialStore
319324
self.fido2UserInterfaceHelper = fido2UserInterfaceHelper
325+
self.flightRecorder = flightRecorder
320326
self.generatorRepository = generatorRepository
321327
self.importCiphersRepository = importCiphersRepository
322328
self.keychainService = keychainService
@@ -478,6 +484,11 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
478484
timeProvider: timeProvider
479485
)
480486

487+
let flightRecorder = DefaultFlightRecorder(
488+
stateService: stateService,
489+
timeProvider: timeProvider
490+
)
491+
481492
let sendService = DefaultSendService(
482493
fileAPIService: apiService,
483494
sendAPIService: apiService,
@@ -788,6 +799,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
788799
exportVaultService: exportVaultService,
789800
fido2CredentialStore: fido2CredentialStore,
790801
fido2UserInterfaceHelper: fido2UserInterfaceHelper,
802+
flightRecorder: flightRecorder,
791803
generatorRepository: generatorRepository,
792804
importCiphersRepository: importCiphersRepository,
793805
keychainRepository: keychainRepository,

BitwardenShared/Core/Platform/Services/Services.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ typealias Services = HasAPIService
2929
& HasFido2CredentialStore
3030
& HasFido2UserInterfaceHelper
3131
& HasFileAPIService
32+
& HasFlightRecorder
3233
& HasGeneratorRepository
3334
& HasImportCiphersRepository
3435
& HasLocalAuthService
@@ -232,6 +233,13 @@ protocol HasFileAPIService {
232233
var fileAPIService: FileAPIService { get }
233234
}
234235

236+
/// Protocol for an object that provides a `FlightRecorder`.
237+
///
238+
protocol HasFlightRecorder {
239+
/// The service used by the application for recording temporary debug logs.
240+
var flightRecorder: FlightRecorder { get }
241+
}
242+
235243
/// Protocol for an object that provides a `GeneratorRepository`.
236244
///
237245
protocol HasGeneratorRepository {
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import Combine
2+
3+
@testable import BitwardenShared
4+
5+
@MainActor
6+
final class MockFlightRecorder: FlightRecorder {
7+
var disableFlightRecorderCalled = false
8+
var enableFlightRecorderCalled = false
9+
var enableFlightRecorderDuration: FlightRecorderLoggingDuration?
10+
var isEnabledSubject = CurrentValueSubject<Bool, Never>(false)
11+
12+
nonisolated init() {}
13+
14+
func disableFlightRecorder() {
15+
disableFlightRecorderCalled = true
16+
}
17+
18+
func enableFlightRecorder(duration: FlightRecorderLoggingDuration) async {
19+
enableFlightRecorderCalled = true
20+
enableFlightRecorderDuration = duration
21+
}
22+
23+
func isEnabledPublisher() async -> AnyPublisher<Bool, Never> {
24+
isEnabledSubject.eraseToAnyPublisher()
25+
}
26+
}

BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ extension ServiceContainer {
2929
exportVaultService: ExportVaultService = MockExportVaultService(),
3030
fido2CredentialStore: Fido2CredentialStore = MockFido2CredentialStore(),
3131
fido2UserInterfaceHelper: Fido2UserInterfaceHelper = MockFido2UserInterfaceHelper(),
32+
flightRecorder: FlightRecorder = MockFlightRecorder(),
3233
generatorRepository: GeneratorRepository = MockGeneratorRepository(),
3334
importCiphersRepository: ImportCiphersRepository = MockImportCiphersRepository(),
3435
httpClient: HTTPClient = MockHTTPClient(),
@@ -87,6 +88,7 @@ extension ServiceContainer {
8788
exportVaultService: exportVaultService,
8889
fido2CredentialStore: fido2CredentialStore,
8990
fido2UserInterfaceHelper: fido2UserInterfaceHelper,
91+
flightRecorder: flightRecorder,
9092
generatorRepository: generatorRepository,
9193
importCiphersRepository: importCiphersRepository,
9294
keychainRepository: keychainRepository,

BitwardenShared/UI/Platform/Settings/Settings/About/AboutAction.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ enum AboutAction: Equatable {
2424
/// The toast was shown or hidden.
2525
case toastShown(Toast?)
2626

27-
/// The flight recorder toggle value changed.
28-
case toggleFlightRecorder(Bool)
29-
3027
/// The submit crash logs toggle value changed.
3128
case toggleSubmitCrashLogs(Bool)
3229

BitwardenShared/UI/Platform/Settings/Settings/About/AboutEffect.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,10 @@
55
enum AboutEffect: Equatable {
66
/// The view appeared so the initial data should be loaded.
77
case loadData
8+
9+
/// Stream the enabled status of the flight recorder.
10+
case streamFlightRecorderEnabled
11+
12+
/// The flight recorder toggle value changed.
13+
case toggleFlightRecorder(Bool)
814
}

BitwardenShared/UI/Platform/Settings/Settings/About/AboutProcessor.swift

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ final class AboutProcessor: StateProcessor<AboutState, AboutAction, AboutEffect>
1111
& HasConfigService
1212
& HasEnvironmentService
1313
& HasErrorReporter
14+
& HasFlightRecorder
1415
& HasPasteboardService
1516

1617
// MARK: Properties
@@ -52,6 +53,14 @@ final class AboutProcessor: StateProcessor<AboutState, AboutAction, AboutEffect>
5253
switch effect {
5354
case .loadData:
5455
await loadData()
56+
case .streamFlightRecorderEnabled:
57+
await streamFlightRecorderEnabled()
58+
case let .toggleFlightRecorder(isOn):
59+
if isOn {
60+
coordinator.navigate(to: .enableFlightRecorder)
61+
} else {
62+
await services.flightRecorder.disableFlightRecorder()
63+
}
5564
}
5665
}
5766

@@ -77,13 +86,6 @@ final class AboutProcessor: StateProcessor<AboutState, AboutAction, AboutEffect>
7786
})
7887
case let .toastShown(newValue):
7988
state.toast = newValue
80-
case let .toggleFlightRecorder(isOn):
81-
if isOn {
82-
coordinator.navigate(to: .enableFlightRecorder)
83-
} else {
84-
// TODO: PM-19577 Turn logging off
85-
state.isFlightRecorderToggleOn = isOn
86-
}
8789
case let .toggleSubmitCrashLogs(isOn):
8890
state.isSubmitCrashLogsToggleOn = isOn
8991
services.errorReporter.isEnabled = isOn
@@ -110,4 +112,11 @@ final class AboutProcessor: StateProcessor<AboutState, AboutAction, AboutEffect>
110112
private func loadData() async {
111113
state.isFlightRecorderFeatureFlagEnabled = await services.configService.getFeatureFlag(.flightRecorder)
112114
}
115+
116+
/// Streams the enabled status of the flight recorder.
117+
private func streamFlightRecorderEnabled() async {
118+
for await isEnabled in await services.flightRecorder.isEnabledPublisher().values {
119+
state.isFlightRecorderToggleOn = isEnabled
120+
}
121+
}
113122
}

0 commit comments

Comments
 (0)