From e3e4d7e16cc16928df0166406557ab6531a54f39 Mon Sep 17 00:00:00 2001 From: Matt Cleinman <9295855+mcleinman@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:46:36 -0800 Subject: [PATCH 1/3] add extra to metric and extend the metric --- src/platforms/ios/ConnectionHealth.swift | 4 ++-- src/platforms/ios/PingAnalyzer.swift | 8 ++++---- src/telemetry/metrics.yaml | 9 +++++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/platforms/ios/ConnectionHealth.swift b/src/platforms/ios/ConnectionHealth.swift index 97d986054e..9c7e1a2205 100644 --- a/src/platforms/ios/ConnectionHealth.swift +++ b/src/platforms/ios/ConnectionHealth.swift @@ -72,10 +72,10 @@ class ConnectionHealth { } logger.info(message: "Creating PingAnalyzer") - let _ = PingAnalyzer(pingAddress: pingAddress) { (connectivity) in + let _ = PingAnalyzer(pingAddress: pingAddress) { (connectivity, error) in guard let connectivity = connectivity else { self.logger.error(message: "PingAnalyzer returned error") - GleanMetrics.ConnectionHealth.pingAnalyzerError.record() + GleanMetrics.ConnectionHealth.pingAnalyzerError.record(PingAnalyzerErrorExtra(errorMessage: error.localizedDescription)) return } diff --git a/src/platforms/ios/PingAnalyzer.swift b/src/platforms/ios/PingAnalyzer.swift index 6eed3ee1ce..b9e2eb7592 100644 --- a/src/platforms/ios/PingAnalyzer.swift +++ b/src/platforms/ios/PingAnalyzer.swift @@ -55,7 +55,7 @@ class PingAnalyzer { timer = Timer.scheduledTimer(timeInterval: TimeInterval(checkTime), target: self, selector: #selector(calculateStability), userInfo: nil, repeats: false) } catch { logger.error(message: "Error when sending pings: \(error)") - callback(nil) + callback(nil, error) } } @@ -68,11 +68,11 @@ class PingAnalyzer { // If any pings take too long to return or if (packetLossPercent >= pingLossNoSignalThreshold) { - callback(.noSignal) + callback(.noSignal, nil) } else if (packetLossPercent >= pingLossUnstableThreshold) { - callback(.unstable) + callback(.unstable, nil) } else { - callback(.stable) + callback(.stable, nil) } } } diff --git a/src/telemetry/metrics.yaml b/src/telemetry/metrics.yaml index 6bdaeb652e..d06f36669d 100644 --- a/src/telemetry/metrics.yaml +++ b/src/telemetry/metrics.yaml @@ -1050,10 +1050,15 @@ connection_health: bugs: - https://mozilla-hub.atlassian.net/browse/VPN-6406 data_reviews: - - TBA + - https://github.com/mozilla-mobile/mozilla-vpn-client/pull/9593#issuecomment-2159109154 data_sensitivity: - technical notification_emails: - mcleinman@mozilla.com - vpn-telemetry@mozilla.com - expires: 2024-12-31 + expires: 2025-06-30 + extra_keys: + error_message: + description: | + Error type + type: string From d535ef489008fda02576668ff8a9d5daae04a5b3 Mon Sep 17 00:00:00 2001 From: Matt Cleinman <9295855+mcleinman@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:46:36 -0800 Subject: [PATCH 2/3] extend connection health error metric and add extra --- src/platforms/ios/ConnectionHealth.swift | 4 ++-- src/platforms/ios/PingAnalyzer.swift | 8 ++++---- src/telemetry/metrics.yaml | 9 +++++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/platforms/ios/ConnectionHealth.swift b/src/platforms/ios/ConnectionHealth.swift index 97d986054e..9c7e1a2205 100644 --- a/src/platforms/ios/ConnectionHealth.swift +++ b/src/platforms/ios/ConnectionHealth.swift @@ -72,10 +72,10 @@ class ConnectionHealth { } logger.info(message: "Creating PingAnalyzer") - let _ = PingAnalyzer(pingAddress: pingAddress) { (connectivity) in + let _ = PingAnalyzer(pingAddress: pingAddress) { (connectivity, error) in guard let connectivity = connectivity else { self.logger.error(message: "PingAnalyzer returned error") - GleanMetrics.ConnectionHealth.pingAnalyzerError.record() + GleanMetrics.ConnectionHealth.pingAnalyzerError.record(PingAnalyzerErrorExtra(errorMessage: error.localizedDescription)) return } diff --git a/src/platforms/ios/PingAnalyzer.swift b/src/platforms/ios/PingAnalyzer.swift index 6eed3ee1ce..b9e2eb7592 100644 --- a/src/platforms/ios/PingAnalyzer.swift +++ b/src/platforms/ios/PingAnalyzer.swift @@ -55,7 +55,7 @@ class PingAnalyzer { timer = Timer.scheduledTimer(timeInterval: TimeInterval(checkTime), target: self, selector: #selector(calculateStability), userInfo: nil, repeats: false) } catch { logger.error(message: "Error when sending pings: \(error)") - callback(nil) + callback(nil, error) } } @@ -68,11 +68,11 @@ class PingAnalyzer { // If any pings take too long to return or if (packetLossPercent >= pingLossNoSignalThreshold) { - callback(.noSignal) + callback(.noSignal, nil) } else if (packetLossPercent >= pingLossUnstableThreshold) { - callback(.unstable) + callback(.unstable, nil) } else { - callback(.stable) + callback(.stable, nil) } } } diff --git a/src/telemetry/metrics.yaml b/src/telemetry/metrics.yaml index 6bdaeb652e..d06f36669d 100644 --- a/src/telemetry/metrics.yaml +++ b/src/telemetry/metrics.yaml @@ -1050,10 +1050,15 @@ connection_health: bugs: - https://mozilla-hub.atlassian.net/browse/VPN-6406 data_reviews: - - TBA + - https://github.com/mozilla-mobile/mozilla-vpn-client/pull/9593#issuecomment-2159109154 data_sensitivity: - technical notification_emails: - mcleinman@mozilla.com - vpn-telemetry@mozilla.com - expires: 2024-12-31 + expires: 2025-06-30 + extra_keys: + error_message: + description: | + Error type + type: string From 676160a549869791078160c03bac1b6846bca160 Mon Sep 17 00:00:00 2001 From: Matt Cleinman <9295855+mcleinman@users.noreply.github.com> Date: Thu, 2 Jan 2025 10:40:50 -0800 Subject: [PATCH 3/3] fix syntax --- src/platforms/ios/ConnectionHealth.swift | 2 +- src/platforms/ios/PingAnalyzer.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platforms/ios/ConnectionHealth.swift b/src/platforms/ios/ConnectionHealth.swift index 9c7e1a2205..4441fa33f2 100644 --- a/src/platforms/ios/ConnectionHealth.swift +++ b/src/platforms/ios/ConnectionHealth.swift @@ -75,7 +75,7 @@ class ConnectionHealth { let _ = PingAnalyzer(pingAddress: pingAddress) { (connectivity, error) in guard let connectivity = connectivity else { self.logger.error(message: "PingAnalyzer returned error") - GleanMetrics.ConnectionHealth.pingAnalyzerError.record(PingAnalyzerErrorExtra(errorMessage: error.localizedDescription)) + GleanMetrics.ConnectionHealth.pingAnalyzerError.record(PingAnalyzerErrorExtra(errorMessage: error?.localizedDescription ?? "no error")) return } diff --git a/src/platforms/ios/PingAnalyzer.swift b/src/platforms/ios/PingAnalyzer.swift index b9e2eb7592..74852a45b0 100644 --- a/src/platforms/ios/PingAnalyzer.swift +++ b/src/platforms/ios/PingAnalyzer.swift @@ -27,7 +27,7 @@ class PingAnalyzer { private let pingLossUnstableThreshold: Double = 0.65 // 13 of 20 pings private let pingLossNoSignalThreshold: Double = 1.0 // all pings - private let callback: (ConnectionHealth.ConnectionStability?) -> Void + private let callback: (ConnectionHealth.ConnectionStability?, Error?) -> Void init(pingAddress: String, callback: @escaping (ConnectionHealth.ConnectionStability?) -> Void) { self.callback = callback