Skip to content

Commit

Permalink
Add connection tester failure pixels (#2948)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1206580121312550/1207743877093953/f

iOS PR: duckduckgo/iOS#3049
BSK PR: duckduckgo/BrowserServicesKit#881

## Description

Adds pixels to track connection tester failures and recovery. These
should give us a better idea about how users are faring.
  • Loading branch information
diegoreymendez authored Jul 6, 2024
1 parent 50b8cc5 commit 5915175
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 6 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -13228,7 +13228,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 167.0.0;
version = 167.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" : "5954412504b0cf294f5c0d90d7a0c8dfcd009558",
"version" : "167.0.0"
"revision" : "0746af01b77d39a1e037bea93b46591534a13b5c",
"version" : "167.0.1"
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ enum NetworkProtectionPixelEvent: PixelKitEventV2 {
case networkProtectionEnableAttemptSuccess
case networkProtectionEnableAttemptFailure

case networkProtectionConnectionTesterFailureDetected(server: String)
case networkProtectionConnectionTesterFailureRecovered(server: String, failureCount: Int)
case networkProtectionConnectionTesterExtendedFailureDetected(server: String)
case networkProtectionConnectionTesterExtendedFailureRecovered(server: String, failureCount: Int)

case networkProtectionTunnelFailureDetected
case networkProtectionTunnelFailureRecovered

Expand Down Expand Up @@ -180,6 +185,18 @@ enum NetworkProtectionPixelEvent: PixelKitEventV2 {
case .networkProtectionEnableAttemptFailure:
return "netp_ev_enable_attempt_failure"

case .networkProtectionConnectionTesterFailureDetected:
return "netp_connection_tester_failure"

case .networkProtectionConnectionTesterFailureRecovered:
return "netp_connection_tester_failure_recovered"

case .networkProtectionConnectionTesterExtendedFailureDetected:
return "netp_connection_tester_extended_failure"

case .networkProtectionConnectionTesterExtendedFailureRecovered:
return "netp_connection_tester_extended_failure_recovered"

case .networkProtectionTunnelFailureDetected:
return "netp_ev_tunnel_failure"

Expand Down Expand Up @@ -316,6 +333,15 @@ enum NetworkProtectionPixelEvent: PixelKitEventV2 {

var parameters: [String: String]? {
switch self {
case .networkProtectionConnectionTesterFailureDetected(let server),
.networkProtectionConnectionTesterExtendedFailureDetected(let server):
return [PixelKit.Parameters.server: server]
case .networkProtectionConnectionTesterFailureRecovered(let server, let failureCount),
.networkProtectionConnectionTesterExtendedFailureRecovered(let server, let failureCount):
return [
PixelKit.Parameters.server: server,
PixelKit.Parameters.count: String(failureCount)
]
case .networkProtectionKeychainErrorFailedToCastKeychainValueToData(let field):
return [PixelKit.Parameters.keychainFieldName: field]
case .networkProtectionKeychainReadError(let field, let status):
Expand Down Expand Up @@ -454,6 +480,10 @@ enum NetworkProtectionPixelEvent: PixelKitEventV2 {
.networkProtectionEnableAttemptConnecting,
.networkProtectionEnableAttemptSuccess,
.networkProtectionEnableAttemptFailure,
.networkProtectionConnectionTesterFailureDetected,
.networkProtectionConnectionTesterFailureRecovered,
.networkProtectionConnectionTesterExtendedFailureDetected,
.networkProtectionConnectionTesterExtendedFailureRecovered,
.networkProtectionTunnelFailureDetected,
.networkProtectionTunnelFailureRecovered,
.networkProtectionLatency,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,39 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
frequency: .legacyDaily,
withAdditionalParameters: [PixelKit.Parameters.vpnCohort: PixelKit.cohort(from: defaults.vpnFirstEnabled)],
includeAppVersionParameter: true)
case .connectionTesterStatusChange(let status, let server):
vpnLogger.log(status, server: server)

switch status {
case .failed(let duration):
let pixel: NetworkProtectionPixelEvent = {
switch duration {
case .immediate:
return .networkProtectionConnectionTesterFailureDetected(server: server)
case .extended:
return .networkProtectionConnectionTesterExtendedFailureDetected(server: server)
}
}()

PixelKit.fire(
pixel,
frequency: .dailyAndCount,
includeAppVersionParameter: true)
case .recovered(let duration, let failureCount):
let pixel: NetworkProtectionPixelEvent = {
switch duration {
case .immediate:
return .networkProtectionConnectionTesterFailureRecovered(server: server, failureCount: failureCount)
case .extended:
return .networkProtectionConnectionTesterExtendedFailureRecovered(server: server, failureCount: failureCount)
}
}()

PixelKit.fire(
pixel,
frequency: .dailyAndCount,
includeAppVersionParameter: true)
}
case .reportConnectionAttempt(attempt: let attempt):
vpnLogger.log(attempt)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import OSLog
public final class VPNLogger {
public typealias AttemptStep = PacketTunnelProvider.AttemptStep
public typealias ConnectionAttempt = PacketTunnelProvider.ConnectionAttempt
public typealias ConnectionTesterStatus = PacketTunnelProvider.ConnectionTesterStatus
public typealias LogCallback = (OSLogType, OSLogMessage) -> Void

public init() {}
Expand Down Expand Up @@ -63,6 +64,17 @@ public final class VPNLogger {
}
}

public func log(_ status: ConnectionTesterStatus, server: String) {
let log = OSLog.networkProtectionConnectionTesterLog

switch status {
case .failed(let duration):
os_log("🔴 Connection tester (%{public}@ - %{public}@) failure", log: log, type: .error, duration.rawValue, server)
case .recovered(let duration, let failureCount):
os_log("🟢 Connection tester (%{public}@ - %{public}@) recovery (after %{public}@ failures)", log: log, duration.rawValue, server, String(failureCount))
}
}

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

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: "167.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "167.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: "167.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "167.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: "167.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "167.0.1"),
.package(path: "../SwiftUIExtensions")
],
targets: [
Expand Down

0 comments on commit 5915175

Please sign in to comment.