Logging refactoring #1 Subscription and Content Blocking (#3091)

Task/Issue URL: https://app.asana.com/0/1205842942115003/1206750146033742/f Tech Design URL: https://app.asana.com/0/1200194497630846/1206777133329590/f **Description**: Step 1 of the Logging refactoring Subscription Content Blocking User Scripts SecureVault History Remote messages General (removed and create local UserDefaultCache logger)
duckduckgo · Aug 20, 2024 · 94409e4 · 94409e4
1 parent 2fafe3c
commit 94409e4
Show file tree

Hide file tree

Showing 33 changed files with 47 additions and 140 deletions.
diff --git a/.swiftlint.yml b/.swiftlint.yml
@@ -33,13 +33,6 @@ custom_rules:
       - keyword
     message: "Classes should be `final` by default, use explicit `internal` or `public` for non-final classes."
     severity: error
-  enforce_os_log_wrapper:
-    included: ".*\\.swift"
-    name: "Use `import Common` for os_log instead of `import os.log`"
-    regex: "^(import (?:os\\.log|os|OSLog))$"
-    capture_group: 0
-    message: "os_log wrapper ensures log args are @autoclosures (computed when needed) and to be able to use String Interpolation."
-    severity: error
 
 # Rule Config
 cyclomatic_complexity:

diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj
@@ -13453,7 +13453,7 @@
 			repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
 			requirement = {
 				kind = exactVersion;
-				version = 184.0.3;
+				version = 185.0.0;
 			};
 		};
 		9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {

diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -32,8 +32,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/BrowserServicesKit",
       "state" : {
-        "revision" : "f166fbddeb8c368f82b0c150a7fa0ff06031196d",
-        "version" : "184.0.3"
+        "revision" : "2efee14db4d30ac73916794de4d262b745a5f413",
+        "version" : "185.0.0"
       }
     },
     {
@@ -75,7 +75,7 @@
     {
       "identity" : "lottie-spm",
       "kind" : "remoteSourceControl",
-      "location" : "https://github.com/airbnb/lottie-spm",
+      "location" : "https://github.com/airbnb/lottie-spm.git",
       "state" : {
         "revision" : "1d29eccc24cc8b75bff9f6804155112c0ffc9605",
         "version" : "4.4.3"

diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1530"
-   version = "1.8">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"

diff --git a/DuckDuckGo/Common/Logging/Logging.swift b/DuckDuckGo/Common/Logging/Logging.swift
@@ -16,9 +16,8 @@
 //  limitations under the License.
 //
 
-import Common
 import Foundation
-import os.log // swiftlint:disable:this enforce_os_log_wrapper
+import os.log
 
 extension OSLog {
 
@@ -29,7 +28,6 @@ extension OSLog {
         case fire = "Fire"
         case dataImportExport = "Data Import/Export"
         case pixel = "Pixel"
-        case contentBlocking = "Content Blocking"
         case httpsUpgrade = "HTTPS Upgrade"
         case favicons = "Favicons"
         case autoLock = "Auto-Lock"
@@ -48,7 +46,7 @@ extension OSLog {
 
     enum AllCategories {
         static var allCases: [String] {
-            Categories.allCases.map(\.rawValue) + AppCategories.allCases.map(\.rawValue)
+            AppCategories.allCases.map(\.rawValue)
         }
     }
 

diff --git a/DuckDuckGo/Configuration/ConfigurationManager.swift b/DuckDuckGo/Configuration/ConfigurationManager.swift
@@ -68,7 +68,6 @@ final class ConfigurationManager {
     private var lastRefreshCheckTime: Date = Date()
 
     private lazy var fetcher = ConfigurationFetcher(store: ConfigurationStore.shared,
-                                                    log: .config,
                                                     eventMapping: Self.configurationDebugEvents)
 
     static let configurationDebugEvents = EventMapping<ConfigurationDebugEvents> { event, error, _, _ in

diff --git a/DuckDuckGo/ContentBlocker/ContentBlocking.swift b/DuckDuckGo/ContentBlocker/ContentBlocking.swift
@@ -80,8 +80,7 @@ final class AppContentBlocking {
         contentBlockingManager = ContentBlockerRulesManager(rulesSource: contentBlockerRulesSource,
                                                             exceptionsSource: exceptionsSource,
                                                             cache: ContentBlockingRulesCache(),
-                                                            errorReporting: Self.debugEvents,
-                                                            log: .contentBlocking)
+                                                            errorReporting: Self.debugEvents)
         userContentUpdating = UserContentUpdating(contentBlockerRulesManager: contentBlockingManager,
                                                   privacyConfigurationManager: privacyConfigurationManager,
                                                   trackerDataManager: trackerDataManager,

diff --git a/DuckDuckGo/History/Services/EncryptedHistoryStore.swift b/DuckDuckGo/History/Services/EncryptedHistoryStore.swift
@@ -22,6 +22,7 @@ import CoreData
 import Combine
 import History
 import PixelKit
+import os.log
 
 final class EncryptedHistoryStore: HistoryStoring {
 
@@ -90,7 +91,7 @@ final class EncryptedHistoryStore: HistoryStoring {
                 for entry in entriesToDelete {
                     context.delete(entry)
                 }
-                os_log("%d items cleaned from history", log: .history, entriesToDelete.count)
+                Logger.history.debug("\(entriesToDelete.count) items cleaned from history")
             } catch {
                 PixelKit.fire(DebugEvent(GeneralPixel.historyRemoveFailed, error: error))
                 self.context.reset()
@@ -114,7 +115,7 @@ final class EncryptedHistoryStore: HistoryStoring {
         fetchRequest.returnsObjectsAsFaults = false
         do {
             let historyEntries = try context.fetch(fetchRequest)
-            os_log("%d entries loaded from history", log: .history, historyEntries.count)
+            Logger.history.debug("\(historyEntries.count) entries loaded from history")
             let history = BrowsingHistory(historyEntries: historyEntries)
             return .success(history)
         } catch {

diff --git a/DuckDuckGo/Menus/MainMenu.swift b/DuckDuckGo/Menus/MainMenu.swift
@@ -20,7 +20,7 @@ import BrowserServicesKit
 import Cocoa
 import Common
 import Combine
-import OSLog // swiftlint:disable:this enforce_os_log_wrapper
+import OSLog
 import SwiftUI
 import WebKit
 import Configuration
@@ -408,7 +408,6 @@ final class MainMenu: NSMenu {
         updateHomeButtonMenuItem()
         updateBookmarksBarMenuItem()
         updateShortcutMenuItems()
-        updateLoggingMenuItems()
         updateInternalUserItem()
         updateRemoteConfigurationInfo()
         updateAutofillDebugScriptMenuItem()
@@ -664,23 +663,6 @@ final class MainMenu: NSMenu {
     private func setupLoggingMenu() -> NSMenu {
         let menu = NSMenu(title: "")
 
-        menu.addItem(NSMenuItem(title: "Enable All", action: #selector(enableAllLogsMenuItemAction), target: self))
-        menu.addItem(NSMenuItem(title: "Disable All", action: #selector(disableAllLogsMenuItemAction), target: self))
-        menu.addItem(.separator())
-
-        for category in OSLog.AllCategories.allCases.sorted() {
-            let menuItem = NSMenuItem(title: category, action: #selector(loggingMenuItemAction), target: self)
-            menuItem.identifier = .init(category)
-            menu.addItem(menuItem)
-        }
-
-        menu.addItem(autofillDebugScriptMenuItem
-            .targetting(self))
-
-        menu.addItem(.separator())
-        let debugLoggingMenuItem = NSMenuItem(title: OSLog.isRunningInDebugEnvironment ? "Disable DEBUG level logging…" : "Enable DEBUG level logging…", action: #selector(debugLoggingMenuItemAction), target: self)
-        menu.addItem(debugLoggingMenuItem)
-
         if #available(macOS 12.0, *) {
             let exportLogsMenuItem = NSMenuItem(title: "Save Logs…", action: #selector(exportLogs), target: self)
             menu.addItem(exportLogsMenuItem)
@@ -694,17 +676,6 @@ final class MainMenu: NSMenu {
         internalUserItem.title = NSApp.delegateTyped.internalUserDecider.isInternalUser ? "Remove Internal User State" : "Set Internal User State"
     }
 
-    private func updateLoggingMenuItems() {
-        guard let loggingMenu else { return }
-
-        let enabledCategories = OSLog.loggingCategories
-        for item in loggingMenu.items {
-            guard let category = item.identifier.map(\.rawValue) else { continue }
-
-            item.state = enabledCategories.contains(category) ? .on : .off
-        }
-    }
-
     private func updateAutofillDebugScriptMenuItem() {
         autofillDebugScriptMenuItem.state = AutofillPreferences().debugScriptEnabled ? .on : .off
     }
@@ -721,63 +692,12 @@ final class MainMenu: NSMenu {
         customConfigurationUrlMenuItem.title = "Configuration URL:  \(AppConfigurationURLProvider().url(for: .privacyConfiguration).absoluteString)"
     }
 
-    @objc private func loggingMenuItemAction(_ sender: NSMenuItem) {
-        guard let category = sender.identifier?.rawValue else { return }
-
-        if case .on = sender.state {
-            OSLog.loggingCategories.remove(category)
-        } else {
-            OSLog.loggingCategories.insert(category)
-        }
-    }
-
-    @objc private func enableAllLogsMenuItemAction(_ sender: NSMenuItem) {
-        OSLog.loggingCategories = Set(OSLog.AllCategories.allCases)
-    }
-
-    @objc private func disableAllLogsMenuItemAction(_ sender: NSMenuItem) {
-        OSLog.loggingCategories = []
-    }
-
     @objc private func toggleAutofillScriptDebugSettingsAction(_ sender: NSMenuItem) {
         AutofillPreferences().debugScriptEnabled = !AutofillPreferences().debugScriptEnabled
         NotificationCenter.default.post(name: .autofillScriptDebugSettingsDidChange, object: nil)
         updateAutofillDebugScriptMenuItem()
     }
 
-    @objc private func debugLoggingMenuItemAction(_ sender: NSMenuItem) {
-#if APPSTORE
-        if !OSLog.isRunningInDebugEnvironment {
-            let alert = NSAlert()
-            alert.messageText = "Restart with DEBUG logging Enabled not supported for AppStore build"
-            alert.informativeText = """
-            Open terminal and run:
-            export \(ProcessInfo.Constants.osActivityMode)=\(ProcessInfo.Constants.debug)
-            "\(Bundle.main.executablePath!)"
-            """
-            alert.runModal()
-
-            return
-        }
-#endif
-
-        let alert = NSAlert()
-        alert.messageText = "Restart with DEBUG logging \(OSLog.isRunningInDebugEnvironment ? "Disabled" : "Enabled")?"
-        alert.addButton(withTitle: "Restart").tag = NSApplication.ModalResponse.OK.rawValue
-        alert.addButton(withTitle: "Cancel").tag = NSApplication.ModalResponse.cancel.rawValue
-        guard case .OK = alert.runModal() else { return }
-
-        let config = NSWorkspace.OpenConfiguration()
-        config.createsNewApplicationInstance = true
-        config.environment = [ProcessInfo.Constants.osActivityMode: (OSLog.isRunningInDebugEnvironment ? "" : ProcessInfo.Constants.debug)]
-
-        NSWorkspace.shared.openApplication(at: Bundle.main.bundleURL, configuration: config)
-
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) {
-            NSApp.terminate(nil)
-        }
-    }
-
     @available(macOS 12.0, *)
     @objc private func exportLogs(_ sender: NSMenuItem) {
         let displayName = Bundle.main.displayName!.replacingOccurrences(of: " ", with: "")

diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift
@@ -21,7 +21,7 @@ import Combine
 import Common
 import BrowserServicesKit
 import PixelKit
-
+import os.log
 import NetworkProtection
 import NetworkProtectionIPC
 import NetworkProtectionUI
@@ -866,23 +866,23 @@ final class NavigationBarViewController: NSViewController {
         let autofillPreferences = AutofillPreferences()
 
         if autofillPreferences.askToSaveUsernamesAndPasswords, let credentials = data.credentials {
-            os_log("Presenting Save Credentials popover", log: .passwordManager)
+            Logger.passwordManager.debug("Presenting Save Credentials popover")
             popovers.displaySaveCredentials(credentials,
                                             automaticallySaved: data.automaticallySavedCredentials,
                                             usingView: passwordManagementButton,
                                             withDelegate: self)
         } else if autofillPreferences.askToSavePaymentMethods, let card = data.creditCard {
-            os_log("Presenting Save Payment Method popover", log: .passwordManager)
+            Logger.passwordManager.debug("Presenting Save Payment Method popover")
             popovers.displaySavePaymentMethod(card,
                                               usingView: passwordManagementButton,
                                               withDelegate: self)
         } else if autofillPreferences.askToSaveAddresses, let identity = data.identity {
-            os_log("Presenting Save Identity popover", log: .passwordManager)
+            Logger.passwordManager.debug("Presenting Save Identity popover")
             popovers.displaySaveIdentity(identity,
                                          usingView: passwordManagementButton,
                                          withDelegate: self)
         } else {
-            os_log("Received save autofill data call, but there was no data to present", log: .passwordManager)
+            Logger.passwordManager.error("Received save autofill data call, but there was no data to present")
         }
     }
 

diff --git a/...tection/NetworkExtensionTargets/NetworkExtensionTargets/MacTransparentProxyProvider.swift b/...tection/NetworkExtensionTargets/NetworkExtensionTargets/MacTransparentProxyProvider.swift
@@ -22,7 +22,7 @@ import Foundation
 import Networking
 import NetworkExtension
 import NetworkProtectionProxy
-import os.log // swiftlint:disable:this enforce_os_log_wrapper
+import os.log
 import PixelKit
 
 final class MacTransparentProxyProvider: TransparentProxyProvider {

diff --git a/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/VPNLogger.swift b/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/VPNLogger.swift
@@ -18,8 +18,7 @@
 
 import Foundation
 import NetworkProtection
-// swiftlint:disable:next enforce_os_log_wrapper
-import OSLog
+import os.log
 
 /// Logger for the VPN
 ///

diff --git a/DuckDuckGo/RemoteMessaging/ActiveRemoteMessageModel.swift b/DuckDuckGo/RemoteMessaging/ActiveRemoteMessageModel.swift
@@ -22,6 +22,7 @@ import Common
 import Foundation
 import PixelKit
 import RemoteMessaging
+import os.log
 
 /**
  * This is used to feed a remote message to the home page view.
@@ -137,12 +138,12 @@ final class ActiveRemoteMessageModel: ObservableObject {
         guard let remoteMessage, let store = store() else {
             return
         }
-        os_log("Remote message shown: %s", log: .remoteMessaging, type: .info, remoteMessage.id)
+        Logger.remoteMessaging.info("Remote message shown: \(remoteMessage.id, privacy: .public)")
         if remoteMessage.isMetricsEnabled {
             PixelKit.fire(GeneralPixel.remoteMessageShown, withAdditionalParameters: ["message": remoteMessage.id])
         }
         if !store.hasShownRemoteMessage(withID: remoteMessage.id) {
-            os_log("Remote message shown for first time: %s", log: .remoteMessaging, type: .info, remoteMessage.id)
+            Logger.remoteMessaging.info("Remote message shown for first time: \(remoteMessage.id, privacy: .public)")
             if remoteMessage.isMetricsEnabled {
                 PixelKit.fire(GeneralPixel.remoteMessageShownUnique, withAdditionalParameters: ["message": remoteMessage.id])
             }

diff --git a/DuckDuckGo/RemoteMessaging/RemoteMessagingClient.swift b/DuckDuckGo/RemoteMessaging/RemoteMessagingClient.swift
@@ -36,8 +36,7 @@ struct DefaultRemoteMessagingStoreProvider: RemoteMessagingStoreProviding {
             database: database,
             notificationCenter: .default,
             errorEvents: RemoteMessagingStoreErrorHandling(),
-            remoteMessagingAvailabilityProvider: availabilityProvider,
-            log: .remoteMessaging
+            remoteMessagingAvailabilityProvider: availabilityProvider
         )
     }
 }
@@ -98,7 +97,6 @@ final class RemoteMessagingClient: RemoteMessagingProcessing {
             configurationFetcher: ConfigurationFetcher(
                 store: configurationStore,
                 urlSession: .session(),
-                log: .remoteMessaging,
                 eventMapping: ConfigurationManager.configurationDebugEvents
             ),
             configurationStore: ConfigurationStore.shared

diff --git a/DuckDuckGo/Sync/SyncCredentialsAdapter.swift b/DuckDuckGo/Sync/SyncCredentialsAdapter.swift
@@ -38,8 +38,7 @@ final class SyncCredentialsAdapter {
         databaseCleaner = CredentialsDatabaseCleaner(
             secureVaultFactory: secureVaultFactory,
             secureVaultErrorReporter: SecureVaultReporter.shared,
-            errorEvents: CredentialsCleanupErrorHandling(),
-            log: .passwordManager
+            errorEvents: CredentialsCleanupErrorHandling()
         )
     }
 

diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift
@@ -25,6 +25,7 @@ import UserScript
 import WebKit
 import History
 import PixelKit
+import os.log
 
 protocol TabDelegate: ContentOverlayUserScriptDelegate {
     func tabWillStartNavigation(_ tab: Tab, isUserInitiated: Bool)
@@ -1012,7 +1013,7 @@ extension Tab: UserContentControllerDelegate {
 
     @MainActor
     func userContentController(_ userContentController: UserContentController, didInstallContentRuleLists contentRuleLists: [String: WKContentRuleList], userScripts: UserScriptsProvider, updateEvent: ContentBlockerRulesManager.UpdateEvent) {
-        os_log("didInstallContentRuleLists", log: .contentBlocking, type: .info)
+        Logger.contentBlocking.info("didInstallContentRuleLists")
         guard let userScripts = userScripts as? UserScripts else { fatalError("Unexpected UserScripts") }
 
         userScripts.debugScript.instrumentation = instrumentation

diff --git a/DuckDuckGo/Tab/TabExtensions/ContentBlockingTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/ContentBlockingTabExtension.swift
@@ -22,6 +22,7 @@ import Common
 import ContentBlocking
 import Foundation
 import Navigation
+import os.log
 
 struct DetectedTracker {
     enum TrackerType {
@@ -109,7 +110,7 @@ extension ContentBlockingTabExtension: NavigationResponder {
         // Ensure Content Blocking Assets (WKContentRuleList&UserScripts) are installed
         if userContentController?.contentBlockingAssetsInstalled == false
             && privacyConfigurationManager.privacyConfig.isEnabled(featureKey: .contentBlocking) {
-            os_log("%d: tabWillWaitForRulesCompilation", log: .contentBlocking, identifier)
+            Logger.contentBlocking.log("\(self.identifier) tabWillWaitForRulesCompilation")
             cbaTimeReporter?.tabWillWaitForRulesCompilation(identifier)
 
             disableLongDecisionMakingChecks()
@@ -118,7 +119,7 @@ extension ContentBlockingTabExtension: NavigationResponder {
             }
 
             await userContentController?.awaitContentBlockingAssetsInstalled()
-            os_log("%d: Rules Compilation done", log: .contentBlocking, identifier)
+            Logger.contentBlocking.log("\(self.identifier) Rules Compilation done")
             cbaTimeReporter?.reportWaitTimeForTabFinishedWaitingForRules(identifier)
         } else {
             cbaTimeReporter?.reportNavigationDidNotWaitForRules()