Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging refactoring phase #2 #3268

Merged
merged 15 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Core/BookmarksImporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Common
import Foundation
import Persistence
import SwiftSoup
import os.log

public enum BookmarksImportError: Error {
case invalidHtmlNoDLTag
Expand Down Expand Up @@ -94,7 +95,7 @@ final public class BookmarksImporter {

if isDocumentInSafariFormat(document) {
guard let newDocument = try transformSafariDocument(document: document) else {
os_log("Safari format could not be handled", type: .debug)
Logger.bookmarks.debug("Safari format could not be handled")
throw BookmarksImportError.safariTransformFailure
}
try parse(documentElement: newDocument, importedBookmark: nil)
Expand Down Expand Up @@ -208,7 +209,7 @@ final public class BookmarksImporter {
do {
try await coreDataStorage.importBookmarks(bookmarks)
} catch {
os_log("Failed to save imported bookmarks to core data %s", type: .debug, error.localizedDescription)
Logger.bookmarks.error("Failed to save imported bookmarks to core data: \(error.localizedDescription, privacy: .public)")
throw BookmarksImportError.saveFailure
}
}
Expand Down
6 changes: 2 additions & 4 deletions Core/ContentBlocking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,15 @@ public final class ContentBlocking {
AdClickAttributionDetection(feature: adClickAttribution,
tld: tld,
eventReporting: attributionEvents,
errorReporting: attributionDebugEvents,
log: .adAttributionLog)
errorReporting: attributionDebugEvents)
}

public func makeAdClickAttributionLogic(tld: TLD) -> AdClickAttributionLogic {
AdClickAttributionLogic(featureConfig: adClickAttribution,
rulesProvider: adClickAttributionRulesProvider,
tld: tld,
eventReporting: attributionEvents,
errorReporting: attributionDebugEvents,
log: .adAttributionLog)
errorReporting: attributionDebugEvents)
}

private let attributionEvents = EventMapping<AdClickAttributionEvents> { event, _, parameters, _ in
Expand Down
3 changes: 2 additions & 1 deletion Core/CookieStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import Common
import Foundation
import os.log

/// Class for persisting cookies for fire proofed sites to work around a WKWebView / DataStore bug which does not let data get persisted until the webview has loaded.
///
Expand Down Expand Up @@ -56,7 +57,7 @@ public class CookieStorage {
})

if let cookie = HTTPCookie(properties: properties) {
os_log("read cookie %s %s %s", log: .generalLog, type: .debug, cookie.domain, cookie.name, cookie.value)
Logger.general.debug("read cookie \(cookie.domain) \(cookie.name) \(cookie.value)")
storedCookies.append(cookie)
}
}
Expand Down
3 changes: 2 additions & 1 deletion Core/DebugUserScript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import Common
import WebKit
import UserScript
import os.log

public class DebugUserScript: NSObject, UserScript {

Expand Down Expand Up @@ -56,7 +57,7 @@ public class DebugUserScript: NSObject, UserScript {
}

private func handleLog(message: WKScriptMessage) {
os_log("%s", log: .generalLog, type: .debug, String(describing: message.body))
Logger.general.debug("\(String(describing: message.body))")
}

private func handleSignpost(message: WKScriptMessage) {
Expand Down
7 changes: 4 additions & 3 deletions Core/DefaultVariantManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import Common
import Foundation
import Speech
import BrowserServicesKit
import os.log

extension FeatureName {
// Define your feature e.g.:
Expand Down Expand Up @@ -127,17 +128,17 @@ public class DefaultVariantManager: VariantManager {

public func assignVariantIfNeeded(_ newInstallCompletion: (VariantManager) -> Void) {
guard !storage.hasInstallStatistics else {
os_log("no new variant needed for existing user", log: .generalLog, type: .debug)
Logger.general.debug("no new variant needed for existing user")
return
}

if let variant = currentVariant {
os_log("already assigned variant: %s", log: .generalLog, type: .debug, String(describing: variant))
Logger.general.debug("already assigned variant: \(String(describing: variant))")
return
}

guard let variant = selectVariant() else {
os_log("Failed to assign variant", log: .generalLog, type: .debug)
Logger.general.debug("Failed to assign variant")

// it's possible this failed because there are none to assign, we should still let new install logic execute
_ = newInstallCompletion(self)
Expand Down
3 changes: 2 additions & 1 deletion Core/EtagStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import Common
import Foundation
import Configuration
import os.log

public protocol BlockerListETagStorage {

Expand All @@ -36,7 +37,7 @@ public struct UserDefaultsETagStorage: BlockerListETagStorage {

public func loadEtag(for configuration: Configuration) -> String? {
let etag = defaults?.string(forKey: configuration.storeKey)
os_log("stored etag for %s %s", log: .generalLog, type: .debug, configuration.storeKey, etag ?? "nil")
Logger.general.debug("Stored etag for \(configuration.storeKey) \(etag ?? "nil")")
return etag
}

Expand Down
9 changes: 5 additions & 4 deletions Core/HistoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import BrowserServicesKit
import History
import Common
import Persistence
import os.log

public protocol HistoryManaging {

Expand Down Expand Up @@ -152,7 +153,7 @@ public class HistoryDatabase {

public static var defaultDBLocation: URL = {
guard let url = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first else {
os_log("HistoryDatabase.make - OUT, failed to get application support directory")
Logger.general.fault("HistoryDatabase.make - OUT, failed to get application support directory")
fatalError("Failed to get location")
}
return url
Expand All @@ -163,18 +164,18 @@ public class HistoryDatabase {
}()

public static func make(location: URL = defaultDBLocation, readOnly: Bool = false) -> CoreDataDatabase {
os_log("HistoryDatabase.make - IN - %s", location.absoluteString)
Logger.general.debug("HistoryDatabase.make - IN - \(location.absoluteString)")
let bundle = History.bundle
guard let model = CoreDataDatabase.loadModel(from: bundle, named: "BrowsingHistory") else {
os_log("HistoryDatabase.make - OUT, failed to loadModel")
Logger.general.debug("HistoryDatabase.make - OUT, failed to loadModel")
fatalError("Failed to load model")
}

let db = CoreDataDatabase(name: "History",
containerLocation: location,
model: model,
readOnly: readOnly)
os_log("HistoryDatabase.make - OUT")
Logger.general.debug("HistoryDatabase.make - OUT")
return db
}
}
Expand Down
28 changes: 28 additions & 0 deletions Core/Logger+Multiple.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//
// Logger+Multiple.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import os.log

public extension Logger {
static var adAttribution = { Logger(subsystem: "AD Attribution", category: "") }()
static var lifecycle = { Logger(subsystem: "Lifecycle", category: "") }()
static var configuration = { Logger(subsystem: "Configuration", category: "") }()
static var duckPlayer = { Logger(subsystem: "DuckPlayer", category: "") }()
}
73 changes: 0 additions & 73 deletions Core/Logging.swift

This file was deleted.

8 changes: 3 additions & 5 deletions Core/Pixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import Foundation
import BrowserServicesKit
import Common
import Networking
import os.log

public struct PixelParameters {
public static let url = "url"
Expand Down Expand Up @@ -223,9 +224,7 @@ public class Pixel {
}

guard !isDryRun else {
os_log(.debug, log: .generalLog, "Pixel fired %{public}@ %{public}@",
pixelName.replacingOccurrences(of: "_", with: "."),
params.count > 0 ? "\(params)" : "")
Logger.general.debug("Pixel fired \(pixelName.replacingOccurrences(of: "_", with: "."), privacy: .public) \(params.count > 0 ? "\(params)" : "", privacy: .public)")
// simulate server response time for Dry Run mode
DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
onComplete(nil)
Expand Down Expand Up @@ -256,8 +255,7 @@ public class Pixel {
headers: headers)
let request = APIRequest(configuration: configuration, urlSession: .session(useMainThreadCallbackQueue: true))
request.fetch { _, error in
os_log("Pixel fired %{public}s %{public}s", log: .generalLog, type: .debug,
pixelName.replacingOccurrences(of: "_", with: "."), "\(params)")
Logger.general.debug("Pixel fired \(pixelName, privacy: .public) \(params, privacy: .public)")
onComplete(error)
}
}
Expand Down
7 changes: 5 additions & 2 deletions Core/PrivacyFeatures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import BrowserServicesKit
import Common
import os.log

public final class PrivacyFeatures {

Expand Down Expand Up @@ -54,9 +55,11 @@ public final class PrivacyFeatures {
AppHTTPSUpgradeStore(database: Database.shared,
bloomFilterDataURL: bloomFilterDataURL,
embeddedResources: embeddedBloomFilterResources,
errorEvents: httpsUpgradeDebugEvents)
errorEvents: httpsUpgradeDebugEvents,
logger: Logger.general)
}

public static let httpsUpgrade = HTTPSUpgrade(store: httpsUpgradeStore, privacyManager: ContentBlocking.shared.privacyConfigurationManager)
public static let httpsUpgrade = HTTPSUpgrade(store: httpsUpgradeStore, privacyManager: ContentBlocking.shared.privacyConfigurationManager,
logger: Logger.general)

}
9 changes: 5 additions & 4 deletions Core/StatisticsLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import Common
import Foundation
import BrowserServicesKit
import Networking
import os.log

public class StatisticsLoader {

Expand Down Expand Up @@ -52,7 +53,7 @@ public class StatisticsLoader {

request.fetch { response, error in
if let error = error {
os_log("Initial atb request failed with error %s", log: .generalLog, type: .debug, error.localizedDescription)
Logger.general.error("Initial atb request failed with error: \(error.localizedDescription, privacy: .public)")
completion()
return
}
Expand All @@ -74,7 +75,7 @@ public class StatisticsLoader {

request.fetch { _, error in
if let error = error {
os_log("Exti request failed with error %s", log: .generalLog, type: .debug, error.localizedDescription)
Logger.general.error("Exit request failed with error: \(error.localizedDescription, privacy: .public)")
completion()
return
}
Expand All @@ -96,7 +97,7 @@ public class StatisticsLoader {

request.fetch { response, error in
if let error = error {
os_log("Search atb request failed with error %s", log: .generalLog, type: .debug, error.localizedDescription)
Logger.general.error("Search atb request failed with error: \(error.localizedDescription, privacy: .public)")
completion()
return
}
Expand All @@ -121,7 +122,7 @@ public class StatisticsLoader {

request.fetch { response, error in
if let error = error {
os_log("App atb request failed with error %s", log: .generalLog, type: .debug, error.localizedDescription)
Logger.general.error("App atb request failed with error: \(error.localizedDescription, privacy: .public)")
completion()
return
}
Expand Down
Loading
Loading