Improve VPN logging logic (#877)

Task/Issue URL: https://app.asana.com/0/1206580121312550/1207223772590802/f iOS PR: duckduckgo/iOS#3032 macOS PR: duckduckgo/macos-browser#2939 What kind of version bump will this require?: Major/Minor/Patch ## Description Improves the logging logic for the VPN.
duckduckgo · Jul 5, 2024 · f665aae · f665aae
1 parent 8cf5639
commit f665aae
Show file tree

Hide file tree

Showing 7 changed files with 117 additions and 110 deletions.
diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionBandwidthAnalyzer.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionBandwidthAnalyzer.swift
@@ -90,7 +90,6 @@ final class NetworkProtectionConnectionBandwidthAnalyzer {
 
         os_log("Bytes per second in last time-interval: (rx: %{public}@, tx: %{public}@)",
                log: .networkProtectionBandwidthAnalysis,
-               type: .info,
                String(describing: rx), String(describing: tx))
 
         idle = UInt64(rx) < Self.rxThreshold && UInt64(tx) < Self.txThreshold
@@ -103,7 +102,7 @@ final class NetworkProtectionConnectionBandwidthAnalyzer {
     /// Useful when servers are swapped
     ///
     func reset() {
-        os_log("Bandwidth analyzer reset", log: .networkProtectionBandwidthAnalysis, type: .info)
+        os_log("Bandwidth analyzer reset", log: .networkProtectionBandwidthAnalysis)
         entries.removeAll()
     }
 

diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift
@@ -222,7 +222,7 @@ final class NetworkProtectionConnectionTester {
         // After completing the connection tests we check if the tester is still supposed to be running
         // to avoid giving results when it should not be running.
         guard isRunning else {
-            os_log("Tester skipped returning results as it was stopped while running the tests", log: log, type: .info)
+            os_log("Tester skipped returning results as it was stopped while running the tests", log: log)
             return
         }
 

diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift
@@ -108,31 +108,31 @@ public actor NetworkProtectionTunnelFailureMonitor {
         guard firstCheckSkipped else {
             // Avoid running the first tunnel failure check after startup to avoid reading the first handshake after sleep, which will almost always
             // be out of date. In normal operation, the first check will frequently be 0 as WireGuard hasn't had the chance to handshake yet.
-            os_log("⚫️ Skipping first tunnel failure check", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
+            os_log("⚫️ Skipping first tunnel failure check", log: .networkProtectionTunnelFailureMonitorLog)
             firstCheckSkipped = true
             return
         }
 
         let mostRecentHandshake = (try? await handshakeReporter.getMostRecentHandshake()) ?? 0
 
         guard mostRecentHandshake > 0 else {
-            os_log("⚫️ Got handshake timestamp at or below 0, skipping check", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
+            os_log("⚫️ Got handshake timestamp at or below 0, skipping check", log: .networkProtectionTunnelFailureMonitorLog)
             return
         }
 
         let difference = Date().timeIntervalSince1970 - mostRecentHandshake
-        os_log("⚫️ Last handshake: %{public}f seconds ago", log: .networkProtectionTunnelFailureMonitorLog, type: .debug, difference)
+        os_log("⚫️ Last handshake: %{public}f seconds ago", log: .networkProtectionTunnelFailureMonitorLog, difference)
 
         if difference > Result.failureDetected.threshold, isConnected {
             if failureReported {
-                os_log("⚫️ Tunnel failure already reported", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
+                os_log("⚫️ Tunnel failure already reported", log: .networkProtectionTunnelFailureMonitorLog)
             } else {
-                os_log("⚫️ Tunnel failure reported", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
+                os_log("⚫️ Tunnel failure reported", log: .networkProtectionTunnelFailureMonitorLog)
                 callback(.failureDetected)
                 failureReported = true
             }
         } else if difference <= Result.failureRecovered.threshold, failureReported {
-            os_log("⚫️ Tunnel failure recovery", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
+            os_log("⚫️ Tunnel recovered from failure", log: .networkProtectionTunnelFailureMonitorLog)
             callback(.failureRecovered)
             failureReported = false
         }

diff --git a/Sources/NetworkProtection/KeyManagement/NetworkProtectionKeychainStore.swift b/Sources/NetworkProtection/KeyManagement/NetworkProtectionKeychainStore.swift
@@ -72,7 +72,7 @@ final class NetworkProtectionKeychainStore {
         case errSecItemNotFound:
             return nil
         default:
-            os_log("🔵 SecItemCopyMatching status %{public}@", type: .error, String(describing: status))
+            os_log("🔴 SecItemCopyMatching status %{public}@", type: .error, String(describing: status))
             throw NetworkProtectionKeychainStoreError.keychainReadError(field: name, status: status)
         }
     }

diff --git a/Sources/NetworkProtection/Logging/Logging.swift b/Sources/NetworkProtection/Logging/Logging.swift
@@ -41,10 +41,6 @@ extension OSLog {
         Logging.networkProtectionTunnelFailureMonitorLoggingEnabled ? Logging.networkProtectionTunnelFailureMonitor : .disabled
     }
 
-    public static var networkProtectionServerFailureRecoveryLog: OSLog {
-        Logging.networkProtectionServerFailureRecoveryLoggingEnabled ? Logging.networkProtectionServerFailureRecovery : .disabled
-    }
-
     public static var networkProtectionConnectionTesterLog: OSLog {
         Logging.networkProtectionConnectionTesterLoggingEnabled ? Logging.networkProtectionConnectionTesterLog : .disabled
     }
@@ -84,7 +80,7 @@ extension OSLog {
 
 struct Logging {
 
-    static let subsystem = "com.duckduckgo.macos.browser.network-protection"
+    static let subsystem = Bundle.main.bundleIdentifier!
 
     fileprivate static let networkProtectionLoggingEnabled = true
     fileprivate static let networkProtection: OSLog = OSLog(subsystem: subsystem, category: "Network Protection")