Skip to content

Commit

Permalink
Logging refactoring phase #2 (#3154)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1205842942115003/1206750146033742/f
Tech Design URL:
https://app.asana.com/0/1200194497630846/1206777133329590/f

**Description**:
Now logging is aligned with our guideline:
https://app.asana.com/0/1202500774821704/1208001254061393/f
- all OSLog wrappers removed
- all os_log uses removed and replaced with Logger 
- ability to disable logs from the app removed
- Local and shared Logger improved
  • Loading branch information
federicocappelli authored Aug 28, 2024
1 parent d37e699 commit e50b5e4
Show file tree
Hide file tree
Showing 185 changed files with 1,166 additions and 1,391 deletions.
68 changes: 39 additions & 29 deletions DuckDuckGo.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "db0a7b47918ccfea0946d88424415c104f67c37d",
"version" : "187.0.0"
"revision" : "faf25f57d1d61ff855216178c454616031585c07",
"version" : "188.0.0"
}
},
{
Expand Down Expand Up @@ -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"
Expand Down
10 changes: 5 additions & 5 deletions DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import Subscription
import NetworkProtectionIPC
import DataBrokerProtection
import RemoteMessaging
import os.log

final class AppDelegate: NSObject, NSApplicationDelegate {

Expand All @@ -65,7 +66,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
let fileStore: FileStore

#if APPSTORE
private let crashCollection = CrashCollection(platform: .macOSAppStore, log: .default)
private let crashCollection = CrashCollection(platform: .macOSAppStore)
#else
private let crashReporter = CrashReporter()
#endif
Expand Down Expand Up @@ -164,7 +165,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
let encryptionKey = NSApplication.runType.requiresEnvironment ? try keyStore.readKey() : nil
fileStore = EncryptedFileStore(encryptionKey: encryptionKey)
} catch {
os_log("App Encryption Key could not be read: %s", "\(error)")
Logger.general.error("App Encryption Key could not be read: \(error.localizedDescription)")
fileStore = EncryptedFileStore()
}

Expand Down Expand Up @@ -545,7 +546,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
dataProvidersSource: syncDataProviders,
errorEvents: SyncErrorHandler(),
privacyConfigurationManager: ContentBlocking.shared.privacyConfigurationManager,
log: OSLog.sync,
environment: environment
)
syncService.initializeIfNeeded()
Expand Down Expand Up @@ -620,10 +620,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
return
}
if isLocked {
os_log(.debug, log: .sync, "Screen is locked")
Logger.sync.debug("Screen is locked")
syncService.scheduler.cancelSyncAndSuspendSyncQueue()
} else {
os_log(.debug, log: .sync, "Screen is unlocked")
Logger.sync.debug("Screen is unlocked")
syncService.scheduler.resumeSyncQueue()
}
}
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/Application/DockCustomizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import Foundation
import Common
import os.log

protocol DockCustomization {
var isAddedToDock: Bool { get }
Expand Down Expand Up @@ -104,7 +105,7 @@ final class DockCustomizer: DockCustomization {
}
return true
} catch {
os_log(.error, "Error writing to Dock plist: %{public}@", error.localizedDescription)
Logger.general.error("Error writing to Dock plist: \(error.localizedDescription, privacy: .public)")
return false
}
}
Expand Down
6 changes: 4 additions & 2 deletions DuckDuckGo/Application/URLEventHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import Subscription
import NetworkProtectionUI
import VPNAppLauncher
import DataBrokerProtection
import os.log
import BrowserServicesKit

// @MainActor
final class URLEventHandler {
Expand Down Expand Up @@ -64,14 +66,14 @@ final class URLEventHandler {

@objc func handleUrlEvent(event: NSAppleEventDescriptor, reply: NSAppleEventDescriptor) {
guard let stringValue = event.paramDescriptor(forKeyword: keyDirectObject)?.stringValue else {
os_log("UrlEventListener: unable to determine path", type: .error)
Logger.general.error("UrlEventListener: unable to determine path")
let error = NSError(domain: "CouldNotGetPath", code: -1, userInfo: nil)
PixelKit.fire(DebugEvent(GeneralPixel.appOpenURLFailed, error: error))
return
}

guard let url = URL.makeURL(from: stringValue) else {
os_log("UrlEventListener: failed to construct URL from path %s", type: .error, stringValue)
Logger.general.debug("UrlEventListener: failed to construct URL from path \(stringValue)")
let error = NSError(domain: "CouldNotConstructURL", code: -1, userInfo: nil)
PixelKit.fire(DebugEvent(GeneralPixel.appOpenURLFailed, error: error))
return
Expand Down
7 changes: 4 additions & 3 deletions DuckDuckGo/Autoconsent/AutoconsentExperiment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@

import Foundation
import Common
import os.log

enum AutoconsentFilterlistExperiment: String, CaseIterable {
static var logic = AutoconsentExperimentLogic()
static var cohort: AutoconsentFilterlistExperiment? {
os_log("🚧 requesting CPM cohort", log: .autoconsent, type: .debug)
Logger.autoconsent.debug("🚧 requesting CPM cohort")
return logic.experimentCohort
}

Expand All @@ -36,11 +37,11 @@ final internal class AutoconsentExperimentLogic {
// if the stored cohort doesn't match, allocate a new one
let cohort = AutoconsentFilterlistExperiment(rawValue: allocatedExperimentCohort)
{
os_log("🚧 existing CPM cohort: %s", log: .autoconsent, type: .debug, String(describing: cohort.rawValue))
Logger.autoconsent.debug("🚧 existing CPM cohort: \(String(describing: cohort.rawValue))")
return cohort
}
let cohort = AutoconsentFilterlistExperiment.allCases.randomElement()!
os_log("🚧 new CPM cohort: %s", log: .autoconsent, type: .debug, String(describing: cohort.rawValue))
Logger.autoconsent.debug("🚧 new CPM cohort: \(String(describing: cohort.rawValue))")
allocatedExperimentCohort = cohort.rawValue
return cohort
}
Expand Down
35 changes: 18 additions & 17 deletions DuckDuckGo/Autoconsent/AutoconsentUserScript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import BrowserServicesKit
import Common
import UserScript
import PrivacyDashboard
import os.log

protocol AutoconsentUserScriptDelegate: AnyObject {
func autoconsentUserScript(consentStatus: CookieConsentInfo)
Expand Down Expand Up @@ -50,7 +51,7 @@ final class AutoconsentUserScript: NSObject, WKScriptMessageHandlerWithReply, Us
weak var delegate: AutoconsentUserScriptDelegate?

init(scriptSource: ScriptSourceProviding, config: PrivacyConfiguration) {
os_log("Initialising autoconsent userscript", log: .autoconsent, type: .debug)
Logger.autoconsent.debug("Initialising autoconsent userscript")
source = Self.loadJS("autoconsent-bundle", from: .main, withReplacements: [:])
self.config = config
}
Expand All @@ -65,15 +66,15 @@ final class AutoconsentUserScript: NSObject, WKScriptMessageHandlerWithReply, Us
let consentStatus = CookieConsentInfo(
consentManaged: consentManaged, cosmetic: cosmetic, optoutFailed: optoutFailed, selftestFailed: selftestFailed
)
os_log("Refreshing dashboard state: %s", log: .autoconsent, type: .debug, String(describing: consentStatus))
Logger.autoconsent.debug("Refreshing dashboard state: \(String(describing: consentStatus))")
self.delegate?.autoconsentUserScript(consentStatus: consentStatus)
}

@MainActor
func userContentController(_ userContentController: WKUserContentController,
didReceive message: WKScriptMessage,
replyHandler: @escaping (Any?, String?) -> Void) {
os_log("Message received: %s", log: .autoconsent, type: .debug, String(describing: message.body))
Logger.autoconsent.debug("Message received: \(String(describing: message.body))")
return handleMessage(replyHandler: replyHandler, message: message)
}
}
Expand Down Expand Up @@ -149,7 +150,7 @@ extension AutoconsentUserScript {
let json = try JSONSerialization.data(withJSONObject: message)
return try JSONDecoder().decode(Target.self, from: json)
} catch {
os_log(.error, "Error decoding message body: %{public}@", error.localizedDescription)
Logger.autoconsent.error("Error decoding message body: \(error.localizedDescription, privacy: .public)")
return nil
}
}
Expand All @@ -170,13 +171,13 @@ extension AutoconsentUserScript {
case MessageName.eval:
handleEval(message: message, replyHandler: replyHandler)
case MessageName.popupFound:
os_log("Autoconsent popup found", log: .autoconsent)
Logger.autoconsent.debug("Autoconsent popup found")
replyHandler([ "type": "ok" ], nil) // this is just to prevent a Promise rejection
case MessageName.optOutResult:
handleOptOutResult(message: message, replyHandler: replyHandler)
case MessageName.optInResult:
// this is not supported in browser
os_log("ignoring optInResult: %s", log: .autoconsent, type: .debug, String(describing: message.body))
Logger.autoconsent.debug("ignoring optInResult: \(String(describing: message.body))")
replyHandler(nil, "opt-in is not supported")
case MessageName.cmpDetected:
// no need to do anything here
Expand All @@ -186,7 +187,7 @@ extension AutoconsentUserScript {
case MessageName.autoconsentDone:
handleAutoconsentDone(message: message, replyHandler: replyHandler)
case MessageName.autoconsentError:
os_log("Autoconsent error: %s", log: .autoconsent, String(describing: message.body))
Logger.autoconsent.debug("Autoconsent error: \(String(describing: message.body))")
replyHandler([ "type": "ok" ], nil) // this is just to prevent a Promise rejection
}
}
Expand All @@ -204,7 +205,7 @@ extension AutoconsentUserScript {

guard url.navigationalScheme?.isHypertextScheme == true else {
// ignore special schemes
os_log("Ignoring special URL scheme: %s", log: .autoconsent, type: .debug, messageData.url)
Logger.autoconsent.debug("Ignoring special URL scheme: \(messageData.url)")
replyHandler([ "type": "ok" ], nil) // this is just to prevent a Promise rejection
return
}
Expand All @@ -217,7 +218,7 @@ extension AutoconsentUserScript {

let topURLDomain = message.webView?.url?.host
guard config.isFeature(.autoconsent, enabledForDomain: topURLDomain) else {
os_log("disabled for site: %s", log: .autoconsent, type: .info, String(describing: url.absoluteString))
Logger.autoconsent.info("disabled for site: \(String(describing: url.absoluteString))")
replyHandler([ "type": "ok" ], nil) // this is just to prevent a Promise rejection
return
}
Expand Down Expand Up @@ -297,7 +298,7 @@ extension AutoconsentUserScript {
replyHandler(nil, "cannot decode message")
return
}
os_log("opt-out result: %s", log: .autoconsent, type: .debug, String(describing: messageData))
Logger.autoconsent.debug("opt-out result: \(String(describing: messageData))")

if !messageData.result {
refreshDashboardState(consentManaged: true, cosmetic: nil, optoutFailed: true, selftestFailed: nil)
Expand All @@ -317,7 +318,7 @@ extension AutoconsentUserScript {
replyHandler(nil, "cannot decode message")
return
}
os_log("opt-out successful: %s", log: .autoconsent, type: .debug, String(describing: messageData))
Logger.autoconsent.debug("opt-out successful: \(String(describing: messageData))")

guard let url = URL(string: messageData.url),
let host = url.host else {
Expand All @@ -329,7 +330,7 @@ extension AutoconsentUserScript {

// trigger popup once per domain
if !management.sitesNotifiedCache.contains(host) {
os_log("bragging that we closed a popup", log: .autoconsent, type: .debug)
Logger.autoconsent.debug("bragging that we closed a popup")
management.sitesNotifiedCache.insert(host)
// post popover notification on main thread
DispatchQueue.main.async {
Expand All @@ -344,22 +345,22 @@ extension AutoconsentUserScript {

if let selfTestWebView = selfTestWebView,
let selfTestFrameInfo = selfTestFrameInfo {
os_log("requesting self-test in: %s", log: .autoconsent, type: .debug, messageData.url)
Logger.autoconsent.debug("requesting self-test in: \(messageData.url)")
selfTestWebView.evaluateJavaScript(
"window.autoconsentMessageCallback({ type: 'selfTest' })",
in: selfTestFrameInfo,
in: WKContentWorld.defaultClient,
completionHandler: { (result) in
switch result {
case.failure(let error):
os_log("Error running self-test: %s", log: .autoconsent, type: .debug, String(describing: error))
Logger.autoconsent.error("Error running self-test: \(error.localizedDescription, privacy: .public)")
case.success:
os_log("self-test requested", log: .autoconsent, type: .debug)
Logger.autoconsent.debug("self-test requested")
}
}
)
} else {
os_log("no self-test scheduled in this tab", log: .autoconsent, type: .debug)
Logger.autoconsent.debug("no self-test scheduled in this tab")
}
selfTestWebView = nil
selfTestFrameInfo = nil
Expand All @@ -372,7 +373,7 @@ extension AutoconsentUserScript {
return
}
// store self-test result
os_log("self-test result: %s", log: .autoconsent, type: .debug, String(describing: messageData))
Logger.autoconsent.debug("self-test result: \(String(describing: messageData))")
refreshDashboardState(consentManaged: true, cosmetic: nil, optoutFailed: false, selftestFailed: messageData.result)
replyHandler([ "type": "ok" ], nil) // this is just to prevent a Promise rejection
}
Expand Down
9 changes: 5 additions & 4 deletions DuckDuckGo/Bookmarks/Model/BookmarkList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Foundation
import BrowserServicesKit
import Common
import Suggestions
import os.log

struct BookmarkList {

Expand Down Expand Up @@ -79,7 +80,7 @@ struct BookmarkList {

mutating func insert(_ bookmark: Bookmark) {
guard itemsDict[bookmark.url] == nil else {
os_log("BookmarkList: Adding failed, the item already is in the bookmark list", type: .error)
Logger.bookmarks.error("BookmarkList: Adding failed, the item already is in the bookmark list")
return
}

Expand Down Expand Up @@ -108,7 +109,7 @@ struct BookmarkList {
guard !newBookmark.isFolder else { return }

guard itemsDict[newBookmark.url] != nil else {
os_log("BookmarkList: Update failed, no such item in bookmark list")
Logger.bookmarks.debug("BookmarkList: Update failed, no such item in bookmark list")
return
}

Expand Down Expand Up @@ -136,7 +137,7 @@ struct BookmarkList {
guard !bookmark.isFolder else { return nil }

guard itemsDict[newURL] == nil else {
os_log("BookmarkList: Update failed, new url already in bookmark list")
Logger.bookmarks.debug("BookmarkList: Update failed, new url already in bookmark list")
return nil
}

Expand All @@ -154,7 +155,7 @@ private extension BookmarkList {

mutating private func updateBookmarkList(newBookmark: Bookmark, oldBookmark bookmark: Bookmark) -> Bookmark? {
guard itemsDict[bookmark.url] != nil, let index = allBookmarkURLsOrdered.firstIndex(of: IdentifiableBookmark(from: bookmark)) else {
os_log("BookmarkList: Update failed, no such item in bookmark list")
Logger.bookmarks.debug("BookmarkList: Update failed, no such item in bookmark list")
return nil
}

Expand Down
Loading

0 comments on commit e50b5e4

Please sign in to comment.