Adds pixels to track main VPN funnels (#2304)

Task/Issue URL: https://app.asana.com/0/0/1206737255908394/f iOS PR: duckduckgo/iOS#2543 BSK PR: duckduckgo/BrowserServicesKit#698 ## Description Add pixels to better track all main VPN funnels: - VPN Controller Start (installation, onboarding) - VPN Tunnel Start - VPN Tunnel Update (change location, etc)
duckduckgo · Mar 7, 2024 · 5838919 · 5838919
1 parent 0dad645
commit 5838919
Show file tree

Hide file tree

Showing 8 changed files with 146 additions and 25 deletions.
diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj
@@ -13612,7 +13612,7 @@
 			repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
 			requirement = {
 				kind = exactVersion;
-				version = 114.1.0;
+				version = "114.1.0-1";
 			};
 		};
 		AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = {

diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -23,8 +23,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/BrowserServicesKit",
       "state" : {
-        "revision" : "045a8782c3dbbf79fc088b38120dea1efadc13e1",
-        "version" : "114.1.0"
+        "revision" : "d9de416d3b77082f818a91b065bee1a2025c8e46",
+        "version" : "114.1.0-1"
       }
     },
     {

diff --git a/...DuckGo/NetworkProtection/AppAndExtensionAndAgentTargets/NetworkProtectionPixelEvent.swift b/...DuckGo/NetworkProtection/AppAndExtensionAndAgentTargets/NetworkProtectionPixelEvent.swift
@@ -27,7 +27,17 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
     case networkProtectionActiveUser
     case networkProtectionNewUser
 
-    case networkProtectionStartFailed
+    case networkProtectionControllerStartAttempt
+    case networkProtectionControllerStartSuccess
+    case networkProtectionControllerStartFailure
+
+    case networkProtectionTunnelStartAttempt
+    case networkProtectionTunnelStartSuccess
+    case networkProtectionTunnelStartFailure
+
+    case networkProtectionTunnelUpdateAttempt
+    case networkProtectionTunnelUpdateSuccess
+    case networkProtectionTunnelUpdateFailure
 
     case networkProtectionEnableAttemptConnecting
     case networkProtectionEnableAttemptSuccess
@@ -77,7 +87,9 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
 
     case networkProtectionNoAuthTokenFoundError
 
+    case networkProtectionRekeyAttempt
     case networkProtectionRekeyCompleted
+    case networkProtectionRekeyFailure
 
     case networkProtectionSystemExtensionActivationFailure
 
@@ -95,8 +107,32 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
         case .networkProtectionNewUser:
             return "m_mac_netp_daily_active_u"
 
-        case .networkProtectionStartFailed:
-            return "m_mac_netp_start_failed"
+        case .networkProtectionControllerStartAttempt:
+            return "m_mac_netp_controller_start_attempt"
+
+        case .networkProtectionControllerStartSuccess:
+            return "m_mac_netp_controller_start_success"
+
+        case .networkProtectionControllerStartFailure:
+            return "m_mac_netp_controller_start_failure"
+
+        case .networkProtectionTunnelStartAttempt:
+            return "m_mac_netp_tunnel_start_attempt"
+
+        case .networkProtectionTunnelStartSuccess:
+            return "m_mac_netp_tunnel_start_success"
+
+        case .networkProtectionTunnelStartFailure:
+            return "m_mac_netp_tunnel_start_failure"
+
+        case .networkProtectionTunnelUpdateAttempt:
+            return "m_mac_netp_tunnel_update_attempt"
+
+        case .networkProtectionTunnelUpdateSuccess:
+            return "m_mac_netp_tunnel_update_success"
+
+        case .networkProtectionTunnelUpdateFailure:
+            return "m_mac_netp_tunnel_update_failure"
 
         case .networkProtectionEnableAttemptConnecting:
             return "m_mac_netp_ev_enable_attempt"
@@ -215,9 +251,15 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
         case .networkProtectionNoAuthTokenFoundError:
             return "m_mac_netp_no_auth_token_found_error"
 
+        case .networkProtectionRekeyAttempt:
+            return "m_mac_netp_rekey_attempt"
+
         case .networkProtectionRekeyCompleted:
             return "m_mac_netp_rekey_completed"
 
+        case .networkProtectionRekeyFailure:
+            return "m_mac_netp_rekey_failure"
+
         case .networkProtectionSystemExtensionActivationFailure:
             return "m_mac_netp_system_extension_activation_failure"
 
@@ -309,7 +351,9 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
              .networkProtectionServerListStoreFailedToEncodeServerList,
              .networkProtectionServerListStoreFailedToDecodeServerList,
              .networkProtectionNoAuthTokenFoundError,
+             .networkProtectionRekeyAttempt,
              .networkProtectionRekeyCompleted,
+             .networkProtectionRekeyFailure,
              .networkProtectionWireguardErrorCannotLocateTunnelFileDescriptor,
              .networkProtectionWireguardErrorFailedDNSResolution,
              .networkProtectionSystemExtensionActivationFailure,
@@ -322,7 +366,15 @@ enum NetworkProtectionPixelEvent: PixelKitEvent {
              .networkProtectionTunnelFailureRecovered,
              .networkProtectionLatency,
              .networkProtectionLatencyError,
-             .networkProtectionStartFailed:
+             .networkProtectionControllerStartAttempt,
+             .networkProtectionControllerStartSuccess,
+             .networkProtectionControllerStartFailure,
+             .networkProtectionTunnelStartAttempt,
+             .networkProtectionTunnelStartSuccess,
+             .networkProtectionTunnelStartFailure,
+             .networkProtectionTunnelUpdateAttempt,
+             .networkProtectionTunnelUpdateSuccess,
+             .networkProtectionTunnelUpdateFailure:
 
             return nil
         }

diff --git a/...uckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionTunnelController.swift b/...uckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionTunnelController.swift
@@ -445,10 +445,18 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
 
     enum StartError: LocalizedError {
         case connectionStatusInvalid
+        case connectionAlreadyStarted
         case simulateControllerFailureError
 
         var errorDescription: String? {
             switch self {
+            case .connectionAlreadyStarted:
+#if DEBUG
+                return "[Debug] Connection already started"
+#else
+                return nil
+#endif
+
             case .connectionStatusInvalid:
 #if DEBUG
                 return "[DEBUG] Connection status invalid"
@@ -464,23 +472,20 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
     /// Starts the VPN connection used for Network Protection
     ///
     func start() async {
+        PixelKit.fire(NetworkProtectionPixelEvent.networkProtectionControllerStartAttempt,
+                      frequency: .dailyAndContinuous)
         controllerErrorStore.lastErrorMessage = nil
 
-#if NETP_SYSTEM_EXTENSION
         do {
+#if NETP_SYSTEM_EXTENSION
             try await activateSystemExtension { [weak self] in
                 // If we're waiting for user approval we wanna make sure the
                 // onboarding step is set correctly.  This can be useful to
                 // help prevent the value from being de-synchronized.
                 self?.onboardingStatusRawValue = OnboardingStatus.isOnboarding(step: .userNeedsToAllowExtension).rawValue
             }
-        } catch {
-            await stop()
-            return
-        }
 #endif
 
-        do {
             let tunnelManager: NETunnelProviderManager
 
             do {
@@ -498,18 +503,30 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
             case .invalid:
                 throw StartError.connectionStatusInvalid
             case .connected:
-                // Intentional no-op
-                break
+                throw StartError.connectionAlreadyStarted
             default:
                 try await start(tunnelManager)
+
+                // It's important to note that we've seen instances where the above call to start()
+                // doesn't throw any errors, yet the tunnel fails to start.  In any case this pixel
+                // should be interpreted as "the controller successfully requrested the tunnel to be
+                // started".  Meaning there's no error caught in this start attempt.  There are pixels
+                // in the packet tunnel provider side that can be used to debug additional logic.
+                //
+                PixelKit.fire(NetworkProtectionPixelEvent.networkProtectionControllerStartSuccess,
+                              frequency: .dailyAndContinuous)
             }
         } catch {
             PixelKit.fire(
-                NetworkProtectionPixelEvent.networkProtectionStartFailed, frequency: .standard, withError: error, includeAppVersionParameter: true
+                NetworkProtectionPixelEvent.networkProtectionControllerStartFailure, frequency: .dailyAndContinuous, withError: error, includeAppVersionParameter: true
             )
 
             await stop()
-            controllerErrorStore.lastErrorMessage = error.localizedDescription
+
+            // Always keep the first error message shown, as it's the more actionable one.
+            if controllerErrorStore.lastErrorMessage == nil {
+                controllerErrorStore.lastErrorMessage = error.localizedDescription
+            }
         }
     }
 

diff --git a/...kProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift b/...kProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift
@@ -198,11 +198,63 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
                     frequency: .dailyAndContinuous,
                     includeAppVersionParameter: true)
             }
-        case .rekeyCompleted:
-            PixelKit.fire(
-                NetworkProtectionPixelEvent.networkProtectionRekeyCompleted,
-                frequency: .dailyAndContinuous,
-                includeAppVersionParameter: true)
+        case .rekeyAttempt(let step):
+            switch step {
+            case .begin:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionRekeyCompleted,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            case .failure(let error):
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionRekeyCompleted,
+                    frequency: .dailyAndContinuous,
+                    withError: error,
+                    includeAppVersionParameter: true)
+            case .success:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionRekeyCompleted,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            }
+        case .tunnelStartAttempt(let step):
+            switch step {
+            case .begin:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelStartAttempt,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            case .failure(let error):
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelStartAttempt,
+                    frequency: .dailyAndContinuous,
+                    withError: error,
+                    includeAppVersionParameter: true)
+            case .success:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelStartSuccess,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            }
+        case .tunnelUpdateAttempt(let step):
+            switch step {
+            case .begin:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelUpdateAttempt,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            case .failure(let error):
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelUpdateFailure,
+                    frequency: .dailyAndContinuous,
+                    withError: error,
+                    includeAppVersionParameter: true)
+            case .success:
+                PixelKit.fire(
+                    NetworkProtectionPixelEvent.networkProtectionTunnelUpdateSuccess,
+                    frequency: .dailyAndContinuous,
+                    includeAppVersionParameter: true)
+            }
         }
     }
 

diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift
@@ -29,7 +29,7 @@ let package = Package(
             targets: ["DataBrokerProtection"])
     ],
     dependencies: [
-        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0"),
+        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0-1"),
         .package(path: "../PixelKit"),
         .package(path: "../SwiftUIExtensions"),
         .package(path: "../XPCHelper"),

diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift
@@ -31,7 +31,7 @@ let package = Package(
         .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"])
     ],
     dependencies: [
-        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0"),
+        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0-1"),
         .package(path: "../XPCHelper"),
         .package(path: "../SwiftUIExtensions"),
         .package(path: "../LoginItems"),

diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift
@@ -12,7 +12,7 @@ let package = Package(
             targets: ["SubscriptionUI"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0"),
+        .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "114.1.0-1"),
         .package(path: "../SwiftUIExtensions")
     ],
     targets: [