From 64ba63ef35348e4ef42d6b892c0811420d4a8a2d Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 11 Oct 2022 14:51:36 +1100 Subject: [PATCH 1/5] Initialize `WCCrashLoggingDataProvider` with a `FeatureFlagService` --- .../Classes/ServiceLocator/ServiceLocator.swift | 4 +++- .../Classes/Tools/Logging/WooCrashLoggingStack.swift | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift b/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift index 55c42c51a1e..6d5d0e28a31 100644 --- a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift +++ b/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift @@ -66,7 +66,9 @@ final class ServiceLocator { /// Crash Logging Stack /// - private static var _crashLogging: CrashLoggingStack = WooCrashLoggingStack() + private static var _crashLogging: CrashLoggingStack = WooCrashLoggingStack( + featureFlagService: featureFlagService + ) /// Support for external Card Readers /// diff --git a/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift b/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift index abcbb8fe4c3..0ca709412b9 100644 --- a/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift +++ b/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift @@ -12,14 +12,15 @@ struct WooCrashLoggingStack: CrashLoggingStack { let crashLogging: AutomatticTracks.CrashLogging let eventLogging: EventLogging - private let crashLoggingDataProvider = WCCrashLoggingDataProvider() + private let crashLoggingDataProvider: WCCrashLoggingDataProvider private let eventLoggingDataProvider = WCEventLoggingDataSource() private let eventLoggingDelegate = WCEventLoggingDelegate() - init() { + init(featureFlagService: FeatureFlagService) { let eventLogging = EventLogging(dataSource: eventLoggingDataProvider, delegate: eventLoggingDelegate) self.eventLogging = eventLogging + self.crashLoggingDataProvider = WCCrashLoggingDataProvider(featureFlagService: featureFlagService) self.crashLogging = AutomatticTracks.CrashLogging(dataProvider: crashLoggingDataProvider, eventLogging: eventLogging) /// Upload any remaining files any time the app becomes active @@ -87,7 +88,11 @@ class WCCrashLoggingDataProvider: CrashLoggingDataProvider { /// Indicates that app is in an inconsistent state and we don't want to start asking it for metadata fileprivate var appIsCrashing = false - init() { + let featureFlagService: FeatureFlagService + + init(featureFlagService: FeatureFlagService) { + self.featureFlagService = featureFlagService + NotificationCenter.default.addObserver(self, selector: #selector(updateCrashLoggingSystem(_:)), name: .defaultAccountWasUpdated, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(updateCrashLoggingSystem(_:)), name: .logOutEventReceived, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(updateCrashLoggingSystem(_:)), name: .StoresManagerDidUpdateDefaultSite, object: nil) From e7727ff97dd4a2d4b534ac18fc5ab7accf62c53a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 11 Oct 2022 15:07:21 +1100 Subject: [PATCH 2/5] Define `FeatureFlag` cases for performance monitoring --- Experiments/Experiments/FeatureFlag.swift | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Experiments/Experiments/FeatureFlag.swift b/Experiments/Experiments/FeatureFlag.swift index fac8751c95b..2424c6d3ca4 100644 --- a/Experiments/Experiments/FeatureFlag.swift +++ b/Experiments/Experiments/FeatureFlag.swift @@ -98,4 +98,39 @@ public enum FeatureFlag: Int { /// Hides products onboarding development. /// case productsOnboarding + + // MARK: - Performance Monitoring + // + // These flags are not transient. That is, they are not here to help us rollout a feature, + // but to serve a safety switches to granularly turn off performance monitoring if it looks + // like we are consuming too many events. + + /// Whether to enable performance monitoring. + /// + case performanceMonitoring + + /// Whether to enable performance monitoring for Core Data operations. + /// + /// - Note: The app will ignore this if `performanceMonitoring` is `false` + case performanceMonitoringCoreData + + /// Whether to enable performance monitoring for file IO operations. + /// + /// - Note: The app will ignore this if `performanceMonitoring` is `false` + case performanceMonitoringFileIO + + /// Whether to enable performance monitoring for networking operations. + /// + /// - Note: The app will ignore this if `performanceMonitoring` is `false` + case performanceMonitoringNetworking + + /// Whether to enable performance monitoring for user interaction events. + /// + /// - Note: The app will ignore this if `performanceMonitoring` is `false` + case performanceMonitoringUserInteraction + + /// Whether to enable performance monitoring for `UIViewController` life-cycle events. + /// + /// - Note: The app will ignore this if `performanceMonitoring` is `false`. + case performanceMonitoringViewController } From f37a36f4782016b94c70fd4da8605a2fbb9d5cf9 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 11 Oct 2022 14:33:16 +1100 Subject: [PATCH 3/5] Add performance monitoring configuration via feature flags --- .../Tools/Logging/WooCrashLoggingStack.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift b/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift index 0ca709412b9..1a62a46804f 100644 --- a/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift +++ b/WooCommerce/Classes/Tools/Logging/WooCrashLoggingStack.swift @@ -134,6 +134,26 @@ class WCCrashLoggingDataProvider: CrashLoggingDataProvider { ServiceLocator.crashLogging.setNeedsDataRefresh() } } + + // MARK: – Performance Monitoring + + var performanceTracking: PerformanceTracking { + guard featureFlagService.isFeatureFlagEnabled(.performanceMonitoring) else { + return .disabled + } + + return .enabled( + .init( + // FIXME: Is there a way to control this via feature flags? + sampler: { 0.1 }, + trackCoreData: featureFlagService.isFeatureFlagEnabled(.performanceMonitoringCoreData), + trackFileIO: featureFlagService.isFeatureFlagEnabled(.performanceMonitoringFileIO), + trackNetwork: featureFlagService.isFeatureFlagEnabled(.performanceMonitoringNetworking), + trackUserInteraction: featureFlagService.isFeatureFlagEnabled(.performanceMonitoringUserInteraction), + trackViewControllers: featureFlagService.isFeatureFlagEnabled(.performanceMonitoringViewController) + ) + ) + } } struct CrashLoggingSettings { From 461d7a435d8b0a8c81ce6ca0ac01a34d2fc0fd56 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 11 Oct 2022 15:20:42 +1100 Subject: [PATCH 4/5] Set performance monitoring to `false` in App Store builds --- Experiments/Experiments/DefaultFeatureFlagService.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Experiments/Experiments/DefaultFeatureFlagService.swift b/Experiments/Experiments/DefaultFeatureFlagService.swift index dce9c7739eb..a65c5d99c9a 100644 --- a/Experiments/Experiments/DefaultFeatureFlagService.swift +++ b/Experiments/Experiments/DefaultFeatureFlagService.swift @@ -51,6 +51,14 @@ public struct DefaultFeatureFlagService: FeatureFlagService { return buildConfig == .localDeveloper || buildConfig == .alpha case .productsOnboarding: return buildConfig == .localDeveloper || buildConfig == .alpha + case .performanceMonitoring, + .performanceMonitoringCoreData, + .performanceMonitoringFileIO, + .performanceMonitoringNetworking, + .performanceMonitoringViewController, + .performanceMonitoringUserInteraction: + // Disabled by default to avoid costs spikes, unless in internal testing builds. + return buildConfig == .alpha default: return true } From 02d928c5d50131ad8a3953c394f3c09d6495c179 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 24 Oct 2022 14:46:24 +1100 Subject: [PATCH 5/5] Add `RELEASE-NOTES.txt` entry for performance monitoring flags --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 510b77135ff..5bac2612171 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -6,6 +6,7 @@ 11.0 ----- +- [internal] Add support for controlling performance monitoring via Sentry. **Off by default**. [https://github.com/woocommerce/woocommerce-ios/pull/7831] 10.9