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

Improve VPN logging logic #2939

Merged
merged 12 commits into from
Jul 5, 2024
12 changes: 8 additions & 4 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,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 @@ -1546,6 +1545,8 @@
7B4D8A222BDA857300852966 /* VPNOperationErrorRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B4D8A202BDA857300852966 /* VPNOperationErrorRecorder.swift */; };
7B4D8A232BDA857300852966 /* VPNOperationErrorRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B4D8A202BDA857300852966 /* VPNOperationErrorRecorder.swift */; };
7B4D8A242BDA857300852966 /* VPNOperationErrorRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B4D8A202BDA857300852966 /* VPNOperationErrorRecorder.swift */; };
7B5174762C35FD8B00B57C86 /* VPNLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B5174752C35FD8B00B57C86 /* VPNLogger.swift */; };
7B5174772C35FD8B00B57C86 /* VPNLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B5174752C35FD8B00B57C86 /* VPNLogger.swift */; };
7B624F172BA25C1F00A6C544 /* NetworkProtectionUI in Frameworks */ = {isa = PBXBuildFile; productRef = 7B624F162BA25C1F00A6C544 /* NetworkProtectionUI */; };
7B6545ED2C0778BB00115BEA /* VPNControllerUDSClient+ConvenienceInitializers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B6545EC2C0778BB00115BEA /* VPNControllerUDSClient+ConvenienceInitializers.swift */; };
7B6545EE2C0779D500115BEA /* VPNControllerUDSClient+ConvenienceInitializers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B6545EC2C0778BB00115BEA /* VPNControllerUDSClient+ConvenienceInitializers.swift */; };
Expand Down Expand Up @@ -3444,6 +3445,7 @@
7B4CE8DA26F02108009134B1 /* UI Tests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "UI Tests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
7B4CE8E626F02134009134B1 /* TabBarTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabBarTests.swift; sourceTree = "<group>"; };
7B4D8A202BDA857300852966 /* VPNOperationErrorRecorder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNOperationErrorRecorder.swift; sourceTree = "<group>"; };
7B5174752C35FD8B00B57C86 /* VPNLogger.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = VPNLogger.swift; path = ../../../../../../../Desktop/VPNLogger.swift; sourceTree = "<group>"; };
7B5291882A1697680022E406 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
7B5291892A169BC90022E406 /* DeveloperID.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = DeveloperID.xcconfig; sourceTree = "<group>"; };
7B6545EC2C0778BB00115BEA /* VPNControllerUDSClient+ConvenienceInitializers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "VPNControllerUDSClient+ConvenienceInitializers.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5306,6 +5308,7 @@
EEBCA0C12BD7CDDA004DF19C /* Pixels */,
4B41ED9F2B15437A001EEDF4 /* NetworkProtectionNotificationsPresenterFactory.swift */,
EEF12E6D2A2111880023E6BF /* MacPacketTunnelProvider.swift */,
7B5174752C35FD8B00B57C86 /* VPNLogger.swift */,
7B0099802B65C6B300FE7C31 /* MacTransparentProxyProvider.swift */,
EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */,
);
Expand Down Expand Up @@ -10854,6 +10857,7 @@
F1FDC93B2BF51F41006B1435 /* VPNSettings+Environment.swift in Sources */,
4B2D06332A11C1E300DE1F49 /* OptionalExtension.swift in Sources */,
4BF0E50B2AD2552200FFEC9E /* NetworkProtectionPixelEvent.swift in Sources */,
7B5174772C35FD8B00B57C86 /* VPNLogger.swift in Sources */,
4B41EDA12B15437A001EEDF4 /* NetworkProtectionNotificationsPresenterFactory.swift in Sources */,
7B0099822B65C6B300FE7C31 /* MacTransparentProxyProvider.swift in Sources */,
B65DA5F32A77D3C700CBEE8D /* UserDefaultsWrapper.swift in Sources */,
Expand Down Expand Up @@ -10970,7 +10974,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 @@ -10984,6 +10987,7 @@
4B4D60A02A0B2D5B00BCD287 /* Bundle+VPN.swift in Sources */,
4B4D60AD2A0C807300BCD287 /* NSApplicationExtension.swift in Sources */,
F1DA51922BF6081C00CF29FA /* AttributionPixelHandler.swift in Sources */,
7B5174762C35FD8B00B57C86 /* VPNLogger.swift in Sources */,
4B4D60A52A0B2EC000BCD287 /* UserText+NetworkProtectionExtensions.swift in Sources */,
4BF0E50C2AD2552300FFEC9E /* NetworkProtectionPixelEvent.swift in Sources */,
4B4D60AC2A0C804B00BCD287 /* OptionalExtension.swift in Sources */,
Expand Down Expand Up @@ -13163,8 +13167,8 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 164.2.0;
kind = revision;
revision = 899da5097f63dc9ba5bee887bbf14f948cc8596e;
};
};
9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "912001a8676345e96a8be360d5a6e3dca6d8e0ec",
"version" : "164.2.0"
"revision" : "899da5097f63dc9ba5bee887bbf14f948cc8596e"
}
},
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really supported so I'm removing it entirely.

@OSLogWrapper(AppCategories.dbp) static var dbp

// Debug->Logging categories will only be enabled for one day
Expand Down
16 changes: 8 additions & 8 deletions DuckDuckGo/InfoPlist.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"localizations" : {
"de" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
Expand All @@ -19,43 +19,43 @@
},
"es" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"fr" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"it" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"nl" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"pl" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"pt" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
},
"ru" : {
"stringUnit" : {
"state" : "translated",
"state" : "needs_review",
"value" : "DuckDuckGo"
}
}
Expand Down
10 changes: 5 additions & 5 deletions DuckDuckGo/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -26917,7 +26917,7 @@
},
"letsmove.alert.message" : {
"comment" : "Message of the alert shown if the app is launched not from the /Applications folder – suggesting to move it there",
"extractionState" : "extracted_with_value",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down Expand Up @@ -26977,7 +26977,7 @@
},
"letsmove.alert.title" : {
"comment" : "Title of the alert shown if the app is launched not from the /Applications folder – suggesting to move it there",
"extractionState" : "extracted_with_value",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down Expand Up @@ -27037,7 +27037,7 @@
},
"letsmove.could.not.move" : {
"comment" : "Error message when moving the app to the /Applications folder failed",
"extractionState" : "extracted_with_value",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down Expand Up @@ -27097,7 +27097,7 @@
},
"letsmove.dont.move.button" : {
"comment" : "Do Not Move to the /Applications folder button title",
"extractionState" : "extracted_with_value",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down Expand Up @@ -27157,7 +27157,7 @@
},
"letsmove.move.button" : {
"comment" : "Move the /Applications folder button title",
"extractionState" : "extracted_with_value",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
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, type: .info, token)
return token
}

Expand Down