Skip to content

Commit

Permalink
Improve VPN logging logic (#2939)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1206580121312550/1207223772590802/f

iOS PR: duckduckgo/iOS#3032
BSK PR: duckduckgo/BrowserServicesKit#877

## Description

Improves the logging logic for the VPN.
  • Loading branch information
diegoreymendez authored Jul 5, 2024
1 parent e581911 commit 53e02ea
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 12 deletions.
10 changes: 7 additions & 3 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,6 @@
4B4BEC482A11B61F001D9AC5 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 4B4BEC342A11B509001D9AC5 /* Assets.xcassets */; };
4B4D603F2A0B290200BCD287 /* NetworkExtension.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4B4D603E2A0B290200BCD287 /* NetworkExtension.framework */; };
4B4D60892A0B2A1C00BCD287 /* NetworkProtectionUNNotificationsPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4D60762A0B29FA00BCD287 /* NetworkProtectionUNNotificationsPresenter.swift */; };
4B4D609F2A0B2C7300BCD287 /* Logging.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85799C1725DEBB3F0007EC87 /* Logging.swift */; };
4B4D60A02A0B2D5B00BCD287 /* Bundle+VPN.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4D605E2A0B29FA00BCD287 /* Bundle+VPN.swift */; };
4B4D60A12A0B2D6100BCD287 /* NetworkProtectionOptionKeyExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4D605F2A0B29FA00BCD287 /* NetworkProtectionOptionKeyExtension.swift */; };
4B4D60A52A0B2EC000BCD287 /* UserText+NetworkProtectionExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4D607C2A0B29FA00BCD287 /* UserText+NetworkProtectionExtensions.swift */; };
Expand Down Expand Up @@ -1511,6 +1510,8 @@
7B00997D2B6508B700FE7C31 /* NetworkProtectionProxy in Frameworks */ = {isa = PBXBuildFile; productRef = 7B00997C2B6508B700FE7C31 /* NetworkProtectionProxy */; };
7B00997F2B6508C200FE7C31 /* NetworkProtectionProxy in Frameworks */ = {isa = PBXBuildFile; productRef = 7B00997E2B6508C200FE7C31 /* NetworkProtectionProxy */; };
7B0099822B65C6B300FE7C31 /* MacTransparentProxyProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B0099802B65C6B300FE7C31 /* MacTransparentProxyProvider.swift */; };
7B01AC6E2C36BB7E004FADC7 /* VPNLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B01AC6D2C36BB7E004FADC7 /* VPNLogger.swift */; };
7B01AC6F2C36BB7E004FADC7 /* VPNLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B01AC6D2C36BB7E004FADC7 /* VPNLogger.swift */; };
7B0694982B6E980F00FA4DBA /* VPNProxyLauncher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B0694972B6E980F00FA4DBA /* VPNProxyLauncher.swift */; };
7B09CBA92BA4BE8100CF245B /* NetworkProtectionPixelEventTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B09CBA72BA4BE7000CF245B /* NetworkProtectionPixelEventTests.swift */; };
7B09CBAA2BA4BE8200CF245B /* NetworkProtectionPixelEventTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B09CBA72BA4BE7000CF245B /* NetworkProtectionPixelEventTests.swift */; };
Expand Down Expand Up @@ -3428,6 +3429,7 @@
56D145F029E6F06D00E3488A /* MockBookmarkManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockBookmarkManager.swift; sourceTree = "<group>"; };
56D6A3D529DB2BAB0055215A /* ContinueSetUpView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ContinueSetUpView.swift; sourceTree = "<group>"; };
7B0099802B65C6B300FE7C31 /* MacTransparentProxyProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MacTransparentProxyProvider.swift; sourceTree = "<group>"; };
7B01AC6D2C36BB7E004FADC7 /* VPNLogger.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VPNLogger.swift; sourceTree = "<group>"; };
7B05829D2A812AC000AC3F7C /* NetworkProtectionOnboardingMenu.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NetworkProtectionOnboardingMenu.swift; sourceTree = "<group>"; };
7B0694972B6E980F00FA4DBA /* VPNProxyLauncher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNProxyLauncher.swift; sourceTree = "<group>"; };
7B09CBA72BA4BE7000CF245B /* NetworkProtectionPixelEventTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionPixelEventTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5311,6 +5313,7 @@
EEF12E6D2A2111880023E6BF /* MacPacketTunnelProvider.swift */,
7B0099802B65C6B300FE7C31 /* MacTransparentProxyProvider.swift */,
EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */,
7B01AC6D2C36BB7E004FADC7 /* VPNLogger.swift */,
);
path = NetworkExtensionTargets;
sourceTree = "<group>";
Expand Down Expand Up @@ -10868,6 +10871,7 @@
4B2D06322A11C1D300DE1F49 /* NSApplicationExtension.swift in Sources */,
F1FDC93B2BF51F41006B1435 /* VPNSettings+Environment.swift in Sources */,
4BF0E50B2AD2552200FFEC9E /* NetworkProtectionPixelEvent.swift in Sources */,
7B01AC6F2C36BB7E004FADC7 /* VPNLogger.swift in Sources */,
4B41EDA12B15437A001EEDF4 /* NetworkProtectionNotificationsPresenterFactory.swift in Sources */,
7B0099822B65C6B300FE7C31 /* MacTransparentProxyProvider.swift in Sources */,
B65DA5F32A77D3C700CBEE8D /* UserDefaultsWrapper.swift in Sources */,
Expand Down Expand Up @@ -10982,7 +10986,6 @@
buildActionMask = 2147483647;
files = (
4B41EDA02B15437A001EEDF4 /* NetworkProtectionNotificationsPresenterFactory.swift in Sources */,
4B4D609F2A0B2C7300BCD287 /* Logging.swift in Sources */,
EE66418C2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift in Sources */,
7B7DFB202B7E736B009EA1A3 /* MacPacketTunnelProvider.swift in Sources */,
F1DA51962BF6083700CF29FA /* PrivacyProPixel.swift in Sources */,
Expand All @@ -10996,6 +10999,7 @@
4B4D60A02A0B2D5B00BCD287 /* Bundle+VPN.swift in Sources */,
4B4D60AD2A0C807300BCD287 /* NSApplicationExtension.swift in Sources */,
F1DA51922BF6081C00CF29FA /* AttributionPixelHandler.swift in Sources */,
7B01AC6E2C36BB7E004FADC7 /* VPNLogger.swift in Sources */,
4B4D60A52A0B2EC000BCD287 /* UserText+NetworkProtectionExtensions.swift in Sources */,
4BF0E50C2AD2552300FFEC9E /* NetworkProtectionPixelEvent.swift in Sources */,
B65DA5F22A77D3C600CBEE8D /* UserDefaultsWrapper.swift in Sources */,
Expand Down Expand Up @@ -13178,7 +13182,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 165.0.0;
version = 165.0.1;
};
};
9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
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" : "777e5ae1ab890d9ec22e069bc5dc0f0ada4b35af",
"version" : "165.0.0"
"revision" : "2df7f9d9063c9f8f8f07ccb80c95d7e35738d1ea",
"version" : "165.0.1"
}
},
{
Expand Down
2 changes: 0 additions & 2 deletions DuckDuckGo/Common/Logging/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ extension OSLog {
case duckPlayer = "Duck Player"
case tabSnapshots = "Tab Snapshots"
case sync = "Sync"
case networkProtection = "VPN"
case dbp = "dbp"
}

Expand Down Expand Up @@ -70,7 +69,6 @@ extension OSLog {
@OSLogWrapper(AppCategories.duckPlayer) static var duckPlayer
@OSLogWrapper(AppCategories.tabSnapshots) static var tabSnapshots
@OSLogWrapper(AppCategories.sync) static var sync
@OSLogWrapper(AppCategories.networkProtection) static var networkProtection
@OSLogWrapper(AppCategories.dbp) static var dbp

// Debug->Logging categories will only be enabled for one day
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {

// MARK: - PacketTunnelProvider.Event reporting

private static var vpnLogger = VPNLogger()

private static var packetTunnelProviderEvents: EventMapping<PacketTunnelProvider.Event> = .init { event, _, _, _ in

#if NETP_SYSTEM_EXTENSION
Expand All @@ -153,6 +155,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
withAdditionalParameters: [PixelKit.Parameters.vpnCohort: PixelKit.cohort(from: defaults.vpnFirstEnabled)],
includeAppVersionParameter: true)
case .reportConnectionAttempt(attempt: let attempt):
vpnLogger.log(attempt)

switch attempt {
case .connecting:
PixelKit.fire(
Expand All @@ -171,6 +175,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .reportTunnelFailure(result: let result):
vpnLogger.log(result)

switch result {
case .failureDetected:
PixelKit.fire(
Expand All @@ -186,6 +192,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
break
}
case .reportLatency(let result):
vpnLogger.log(result)

switch result {
case .error:
PixelKit.fire(
Expand All @@ -200,6 +208,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .rekeyAttempt(let step):
vpnLogger.log(step, named: "Rekey")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -218,6 +228,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .tunnelStartAttempt(let step):
vpnLogger.log(step, named: "Tunnel Start")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -236,6 +248,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .tunnelStopAttempt(let step):
vpnLogger.log(step, named: "Tunnel Stop")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -254,6 +268,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .tunnelUpdateAttempt(let step):
vpnLogger.log(step, named: "Tunnel Update")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -272,6 +288,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .tunnelWakeAttempt(let step):
vpnLogger.log(step, named: "Tunnel Wake")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -290,6 +308,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .failureRecoveryAttempt(let step):
vpnLogger.log(step)

switch step {
case .started:
PixelKit.fire(
Expand Down Expand Up @@ -317,6 +337,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
)
}
case .serverMigrationAttempt(let step):
vpnLogger.log(step, named: "Server Migration")

switch step {
case .begin:
PixelKit.fire(
Expand All @@ -335,6 +357,8 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
includeAppVersionParameter: true)
}
case .tunnelStartOnDemandWithoutAccessToken:
vpnLogger.logStartingWithoutAuthToken()

PixelKit.fire(
NetworkProtectionPixelEvent.networkProtectionTunnelStartAttemptOnDemandWithoutAccessToken,
frequency: .dailyAndCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extension NetworkProtectionKeychainTokenStore: SubscriptionTokenStoring {
if token.hasPrefix("ddg:") {
token = token.replacingOccurrences(of: "ddg:", with: "")
}
os_log("🔵 Wrapper successfully fetched token %{token}@", log: .networkProtection, type: .info, token)
os_log("🟢 Wrapper successfully fetched token %{token}@", log: .networkProtection, token)
return token
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//
// VPNLogger.swift
//
// 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 NetworkProtection
// swiftlint:disable:next enforce_os_log_wrapper
import OSLog

/// Logger for the VPN
///
/// Since we'll want to ensure this adheres to our privacy standards, grouping the logging logic to be mostly
/// handled by a single class sounds like a good approach to be able to review what's being logged..
///
public final class VPNLogger {
public typealias AttemptStep = PacketTunnelProvider.AttemptStep
public typealias ConnectionAttempt = PacketTunnelProvider.ConnectionAttempt
public typealias LogCallback = (OSLogType, OSLogMessage) -> Void

public init() {}

public func logStartingWithoutAuthToken() {
os_log("🔴 Starting tunnel without an auth token", log: .networkProtection, type: .error)
}

public func log(_ step: AttemptStep, named name: String) {
let log = OSLog.networkProtection

switch step {
case .begin:
os_log("🔵 %{public}@ attempt begins", log: log, name)
case .failure(let error):
os_log("🔴 %{public}@ attempt failed with error: %{public}@", log: log, type: .error, name, error.localizedDescription)
case .success:
os_log("🟢 %{public}@ attempt succeeded", log: log, name)
}
}

public func log(_ step: ConnectionAttempt) {
let log = OSLog.networkProtection

switch step {
case .connecting:
os_log("🔵 Connection attempt detected", log: log)
case .failure:
os_log("🔴 Connection attempt failed", log: log, type: .error)
case .success:
os_log("🟢 Connection attempt successful", log: log)
}
}

public func log(_ step: FailureRecoveryStep) {
let log = OSLog.networkProtectionTunnelFailureMonitorLog

switch step {
case .started:
os_log("🔵 Failure Recovery attempt started", log: log)
case .failed(let error):
os_log("🔴 Failure Recovery attempt failed with error: %{public}@", log: log, type: .error, error.localizedDescription)
case .completed(let health):
switch health {
case .healthy:
os_log("🟢 Failure Recovery attempt completed", log: log)
case .unhealthy:
os_log("🔴 Failure Recovery attempt ended as unhealthy", log: log, type: .error)
}
}
}

public func log(_ step: NetworkProtectionTunnelFailureMonitor.Result) {
let log = OSLog.networkProtectionTunnelFailureMonitorLog

switch step {
case .failureDetected:
os_log("🔴 Tunnel failure detected", log: log, type: .error)
case .failureRecovered:
os_log("🟢 Tunnel failure recovered", log: log)
case .networkPathChanged:
os_log("🔵 Tunnel recovery detected path change", log: log)
}
}

public func log(_ result: NetworkProtectionLatencyMonitor.Result) {
let log = OSLog.networkProtectionLatencyMonitorLog

switch result {
case .error:
os_log("🔴 There was an error logging the latency", log: log, type: .error)
case .quality(let quality):
os_log("Connection quality is: %{public}@", log: log, quality.rawValue)
}
}
}
2 changes: 1 addition & 1 deletion DuckDuckGoVPN/DuckDuckGoVPNAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ final class DuckDuckGoVPNAppDelegate: NSObject, NSApplicationDelegate {
func applicationDidFinishLaunching(_ aNotification: Notification) {

APIRequest.Headers.setUserAgent(UserAgent.duckDuckGoUserAgent())
os_log("DuckDuckGoVPN started", log: .networkProtectionLoginItemLog, type: .info)
os_log("DuckDuckGoVPN started", log: .networkProtectionLoginItemLog)

setupMenuVisibility()

Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/DataBrokerProtection/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
targets: ["DataBrokerProtection"])
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.1"),
.package(path: "../SwiftUIExtensions"),
.package(path: "../XPCHelper"),
],
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NetworkProtectionMac/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
.library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.1"),
.package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"),
.package(path: "../AppLauncher"),
.package(path: "../UDSHelper"),
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/SubscriptionUI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ let package = Package(
targets: ["SubscriptionUI"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "165.0.1"),
.package(path: "../SwiftUIExtensions")
],
targets: [
Expand Down

0 comments on commit 53e02ea

Please sign in to comment.