From 929329faef55847d48a6a65436d398abbba808fa Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2026 17:17:18 +0100 Subject: [PATCH 01/20] Import metrics YAML --- components/remote_settings/metrics.yaml | 51 +++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 components/remote_settings/metrics.yaml diff --git a/components/remote_settings/metrics.yaml b/components/remote_settings/metrics.yaml new file mode 100644 index 0000000000..04319868d6 --- /dev/null +++ b/components/remote_settings/metrics.yaml @@ -0,0 +1,51 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +# This file defines the metrics that will be gathered for the Remote Settings +# component. Changes to these metrics require data review. +# +# We can't record metrics from Rust directly. To work around that we: +# - Define the metrics in application-services +# - Define API calls in application-services that return the metrics +# alongside the normal results. +# - Record the metrics with Glean in the consumer code + +--- +$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 + +remote_settings: + sync: + type: event + description: > + Was the remote content successfully pulled? This uptake telemetry + allows to monitor the behaviour of our clients when it comes to + fetching data from remote servers. + See https://searchfox.org/firefox-main/rev/1427c8863/services/common/metrics.yaml + extra_keys: + value: + description: The synchronization status (success, up_to_date, sync_error, ...) + type: string + source: + description: > + A label to distinguish what is being pulled or updated in the component (eg. recipe id, settings collection name, ...). + type: string + duration: + description: The duration of the synchronization process in milliseconds. + type: string + errorName: + description: An optional string with the error name attribute in case of failure. + bugs: &uptake_remotecontent_result_uptake_bugs + - https://bugzil.la/1517469 + - https://bugzil.la/1617133 + data_reviews: &uptake_remotecontent_result_uptake_data_reviews + - https://bugzil.la/1517469 + - https://bugzil.la/1617133 + notification_emails: &uptake_remotecontent_result_uptake_emails + - mleplatre@mozilla.com + - acottner@mozilla.com + data_sensitivity: + - technical + notification_emails: + - synced-client-integrations@mozilla.com + expires: never From 35e9f5707160c7e5a6cd95325e568d063a5044bc Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2026 17:19:48 +0100 Subject: [PATCH 02/20] Add telemetry module --- components/remote_settings/src/lib.rs | 1 + components/remote_settings/src/telemetry.rs | 136 ++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 components/remote_settings/src/telemetry.rs diff --git a/components/remote_settings/src/lib.rs b/components/remote_settings/src/lib.rs index eb689ee370..4783ca29cf 100644 --- a/components/remote_settings/src/lib.rs +++ b/components/remote_settings/src/lib.rs @@ -16,6 +16,7 @@ pub mod service; #[cfg(feature = "signatures")] pub(crate) mod signatures; pub mod storage; +pub mod telemetry; pub(crate) mod jexl_filter; mod macros; diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs new file mode 100644 index 0000000000..264c11d25b --- /dev/null +++ b/components/remote_settings/src/telemetry.rs @@ -0,0 +1,136 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use std::sync::Arc; + +use crate::error::Error; + +/// Remote Settings sync status. +#[derive(Debug, PartialEq, uniffi::Enum)] +pub enum RemoteSettingsSyncStatus { + /// Sync completed and new data was stored. + Success, + /// Local data is already up to date, no new data was stored. + UpToDate, + /// A network-level error occurred (connection refused, timeout, bad HTTP status, ...) + NetworkError, + /// The server asked the client to back off. + BackoffError, + /// Content signature verification failed. + SignatureError, + /// Server error (5xx status) + ServerError, + /// An unknown error occurred. + UnknownError, +} + +#[derive(Debug, PartialEq, uniffi::Record, Default)] +pub struct SyncStatusExtras { + /// Duration of the sync operation in milliseconds, if available. + pub duration: Option, + /// The name of the error that occurred, if available. + pub errorName: Option, +} + +/// Trait implemented by consumers to record Remote Settings metrics with Glean. +/// +/// Consumers should implement this trait and pass it to +/// [crate::RemoteSettingsService::set_telemetry]. +/// +/// Consumers implement the trait like this (Kotlin example): +/// ``` +/// import mozilla.appservices.remote_settings.RemoteSettingsTelemetry +/// import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettingsClient +/// +/// class RSTelemetry : RemoteSettingsTelemetry { +/// override fun report(source: String, value: RemoteSettingsSyncStatus, extras: SyncStatusExtras) { +/// RemoteSettingsClient.syncStatus.record( +/// RemoteSettingsClient.SyncStatusExtra( +/// value = value, +/// source = source, +/// duration = extras.duration, +/// errorName = extras.errorName +/// ) +/// ) +/// } +/// } +/// +/// service.setTelemetry(RSTelemetry()) +/// ``` +#[uniffi::export(with_foreign)] +pub trait RemoteSettingsTelemetry: Send + Sync { + /// Report uptake event. + fn report(&self, source: String, value: RemoteSettingsSyncStatus, extras: SyncStatusExtras); +} + +struct NoopRemoteSettingsTelemetry; + +impl RemoteSettingsTelemetry for NoopRemoteSettingsTelemetry { + fn report(&self, _source: String, _value: RemoteSettingsSyncStatus, _extras: SyncStatusExtras) { + } +} + +/// Wrapper around [RemoteSettingsTelemetry] used internally. +#[derive(Clone)] +pub struct RemoteSettingsTelemetryWrapper { + inner: Arc, +} + +impl RemoteSettingsTelemetryWrapper { + pub fn new(inner: Arc) -> Self { + Self { inner } + } + + pub fn noop() -> Self { + Self { + inner: Arc::new(NoopRemoteSettingsTelemetry), + } + } + + pub fn report_success(&self, source: &str, duration: Option) { + self.inner.report( + source.to_string(), + RemoteSettingsSyncStatus::Success, + SyncStatusExtras { + duration, + errorName: None, + }, + ); + } + + pub fn report_up_to_date(&self, source: &str, duration: Option) { + self.inner.report( + source.to_string(), + RemoteSettingsSyncStatus::UpToDate, + SyncStatusExtras { + duration, + errorName: None, + }, + ); + } + + pub fn report_sync_error(&self, error: &Error, source: &str) { + self.inner.report( + source.to_string(), + error_to_status(error), + SyncStatusExtras { + duration: None, + errorName: Some(format!("{error:?}")), + }, + ); + } +} + +fn error_to_status(error: &Error) -> RemoteSettingsSyncStatus { + match error { + Error::RequestError(viaduct::ViaductError::NetworkError(_)) + | Error::ResponseError { .. } => RemoteSettingsSyncStatus::NetworkError, + Error::BackoffError(_) => RemoteSettingsSyncStatus::BackoffError, + #[cfg(feature = "signatures")] + Error::IncompleteSignatureDataError(_) => RemoteSettingsSyncStatus::SignatureError, + #[cfg(feature = "signatures")] + Error::SignatureError(_) => RemoteSettingsSyncStatus::SignatureError, + _ => RemoteSettingsSyncStatus::UnknownError, + } +} From efcd64eb629d4a4f03fc3ebf96d7e56ffd13059e Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2026 17:23:53 +0100 Subject: [PATCH 03/20] Leverage Telemetry in client --- components/remote_settings/src/lib.rs | 10 ++++++++ components/remote_settings/src/service.rs | 29 +++++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/components/remote_settings/src/lib.rs b/components/remote_settings/src/lib.rs index 4783ca29cf..024e6a25d5 100644 --- a/components/remote_settings/src/lib.rs +++ b/components/remote_settings/src/lib.rs @@ -25,10 +25,12 @@ pub use client::{Attachment, RemoteSettingsRecord, RemoteSettingsResponse, RsJso pub use config::{BaseUrl, RemoteSettingsConfig, RemoteSettingsConfig2, RemoteSettingsServer}; pub use context::RemoteSettingsContext; pub use error::{trace, ApiResult, RemoteSettingsError, Result}; +pub use telemetry::{RemoteSettingsSyncStatus, RemoteSettingsTelemetry, SyncStatusExtras}; use client::Client; use error::Error; use storage::Storage; +use telemetry::RemoteSettingsTelemetryWrapper; uniffi::setup_scaffolding!("remote_settings"); @@ -61,6 +63,14 @@ impl RemoteSettingsService { } } + /// Set the telemetry implementation used to record Glean metrics. + /// This should be set to a real implementation (eg. Kotlin, Swift). + /// If not set, all metric recording is a no-op. + pub fn set_telemetry(&self, telemetry: Arc) { + self.internal + .set_telemetry(RemoteSettingsTelemetryWrapper::new(telemetry)); + } + /// Create a new Remote Settings client /// /// This method performs no IO or network requests and is safe to run in a main thread that can't be blocked. diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index 97ef57f643..15be6881ef 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -15,8 +15,9 @@ use url::Url; use viaduct::Request; use crate::{ - client::RemoteState, config::BaseUrl, error::Error, storage::Storage, RemoteSettingsClient, - RemoteSettingsConfig2, RemoteSettingsContext, RemoteSettingsServer, Result, + client::RemoteState, config::BaseUrl, error::Error, storage::Storage, + telemetry::RemoteSettingsTelemetryWrapper, RemoteSettingsClient, RemoteSettingsConfig2, + RemoteSettingsContext, RemoteSettingsServer, Result, }; /// Internal Remote settings service API @@ -30,6 +31,7 @@ struct RemoteSettingsServiceInner { bucket_name: String, app_context: Option, remote_state: RemoteState, + telemetry: RemoteSettingsTelemetryWrapper, /// Weakrefs for all clients that we've created. Note: this stores the /// top-level/public `RemoteSettingsClient` structs rather than `client::RemoteSettingsClient`. /// The reason for this is that we return Arcs to the public struct to the foreign code, so we @@ -57,11 +59,16 @@ impl RemoteSettingsService { bucket_name, app_context: config.app_context, remote_state: RemoteState::default(), + telemetry: RemoteSettingsTelemetryWrapper::noop(), clients: vec![], }), } } + pub fn set_telemetry(&self, telemetry: RemoteSettingsTelemetryWrapper) { + self.inner.lock().telemetry = telemetry; + } + pub fn make_client(&self, collection_name: String) -> Arc { let mut inner = self.inner.lock(); // Allow using in-memory databases for testing of external crates. @@ -84,11 +91,16 @@ impl RemoteSettingsService { /// Sync collections for all active clients pub fn sync(&self) -> Result> { + const TELEMETRY_SOURCE_POLL: &str = "settings-changes-monitoring"; // Make sure we only sync each collection once, even if there are multiple clients let mut synced_collections = HashSet::new(); let mut inner = self.inner.lock(); - let changes = inner.fetch_changes()?; + let changes_result = inner.fetch_changes(); + if let Err(ref e) = changes_result { + inner.telemetry.report_sync_error(e, TELEMETRY_SOURCE_POLL); + } + let changes = changes_result?; let change_map: HashMap<_, _> = changes .changes .iter() @@ -99,18 +111,27 @@ impl RemoteSettingsService { for client in inner.active_clients() { let client = &client.internal; let collection_name = client.collection_name(); + let cid = format!("{bucket_name}/{collection_name}"); if let Some(client_last_modified) = client.get_last_modified_timestamp()? { if let Some(server_last_modified) = change_map.get(&(collection_name, &bucket_name)) { if client_last_modified == *server_last_modified { trace!("skipping up-to-date collection: {collection_name}"); + inner.telemetry.report_up_to_date(&cid, None); continue; } } } if synced_collections.insert(collection_name.to_string()) { trace!("syncing collection: {collection_name}"); - client.sync()?; + let start_time = std::time::Instant::now(); + let sync_result = client.sync(); + let duration: u64 = start_time.elapsed().as_millis().try_into().unwrap_or(0); + match &sync_result { + Ok(()) => inner.telemetry.report_success(&cid, Some(duration)), + Err(e) => inner.telemetry.report_sync_error(e, &cid), + } + sync_result?; } } Ok(synced_collections.into_iter().collect()) From 379badd296db90affaa369aad890e2399454d339 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 09:09:17 +0100 Subject: [PATCH 04/20] @acottner review --- components/remote_settings/src/service.rs | 23 +++++++++++---------- components/remote_settings/src/telemetry.rs | 1 + 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index 15be6881ef..a157d7b585 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -91,16 +91,11 @@ impl RemoteSettingsService { /// Sync collections for all active clients pub fn sync(&self) -> Result> { - const TELEMETRY_SOURCE_POLL: &str = "settings-changes-monitoring"; // Make sure we only sync each collection once, even if there are multiple clients let mut synced_collections = HashSet::new(); let mut inner = self.inner.lock(); - let changes_result = inner.fetch_changes(); - if let Err(ref e) = changes_result { - inner.telemetry.report_sync_error(e, TELEMETRY_SOURCE_POLL); - } - let changes = changes_result?; + let changes = inner.fetch_changes()?; let change_map: HashMap<_, _> = changes .changes .iter() @@ -203,18 +198,24 @@ impl RemoteSettingsServiceInner { trace!("make_request: {url}"); self.remote_state.ensure_no_backoff()?; + let start_time = std::time::Instant::now(); let req = Request::get(url); let resp = req.send()?; self.remote_state.handle_backoff_hint(&resp)?; + const TELEMETRY_SOURCE_POLL: &str = "settings-changes-monitoring"; if resp.is_success() { - Ok(resp.json()?) + let body = resp.json()?; + let duration: u64 = start_time.elapsed().as_millis().try_into().unwrap_or(0); + self + .telemetry + .report_success(TELEMETRY_SOURCE_POLL, Some(duration)); + Ok(body) } else { - Err(Error::response_error( - &resp.url, - format!("status code: {}", resp.status), - )) + let e = Error::response_error(&resp.url, format!("status code: {}", resp.status)); + self.telemetry.report_sync_error(&e, TELEMETRY_SOURCE_POLL); + Err(e) } } } diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs index 264c11d25b..6dbd383c8d 100644 --- a/components/remote_settings/src/telemetry.rs +++ b/components/remote_settings/src/telemetry.rs @@ -26,6 +26,7 @@ pub enum RemoteSettingsSyncStatus { } #[derive(Debug, PartialEq, uniffi::Record, Default)] +#[allow(non_snake_case)] pub struct SyncStatusExtras { /// Duration of the sync operation in milliseconds, if available. pub duration: Option, From 061d52a402e54cc8193771c04ee5c28d814138bd Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 09:10:38 +0100 Subject: [PATCH 05/20] Fix comment --- components/remote_settings/src/telemetry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs index 6dbd383c8d..643e45c7f5 100644 --- a/components/remote_settings/src/telemetry.rs +++ b/components/remote_settings/src/telemetry.rs @@ -40,7 +40,7 @@ pub struct SyncStatusExtras { /// [crate::RemoteSettingsService::set_telemetry]. /// /// Consumers implement the trait like this (Kotlin example): -/// ``` +/// ```kotlin /// import mozilla.appservices.remote_settings.RemoteSettingsTelemetry /// import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettingsClient /// From bdeaabe8dce289ca99dae25ac7b0f666fa4f269c Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 10:39:35 +0100 Subject: [PATCH 06/20] Add tests --- components/remote_settings/src/service.rs | 222 +++++++++++++++++++++- 1 file changed, 220 insertions(+), 2 deletions(-) diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index a157d7b585..8ebfa7b859 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -208,8 +208,7 @@ impl RemoteSettingsServiceInner { if resp.is_success() { let body = resp.json()?; let duration: u64 = start_time.elapsed().as_millis().try_into().unwrap_or(0); - self - .telemetry + self.telemetry .report_success(TELEMETRY_SOURCE_POLL, Some(duration)); Ok(body) } else { @@ -234,3 +233,222 @@ struct ChangesCollection { bucket: String, last_modified: u64, } + +#[cfg(test)] +mod test { + use super::*; + use crate::telemetry::{RemoteSettingsSyncStatus, SyncStatusExtras}; + use crate::{RemoteSettingsConfig2, RemoteSettingsServer}; + use mockito::{mock, Matcher}; + use std::sync::Arc; + + type EventTuple = (String, RemoteSettingsSyncStatus, SyncStatusExtras); + + /// Telemetry implementation that records all events for later assertion. + struct FakeTelemetry { + events: std::sync::Mutex>, + } + + impl FakeTelemetry { + fn new() -> Self { + Self { + events: std::sync::Mutex::new(Vec::new()), + } + } + } + + impl crate::telemetry::RemoteSettingsTelemetry for FakeTelemetry { + fn report( + &self, + source: String, + status: RemoteSettingsSyncStatus, + extras: SyncStatusExtras, + ) { + self.events.lock().unwrap().push((source, status, extras)); + } + } + + fn make_service(server_url: &str) -> (RemoteSettingsService, Arc) { + let service = RemoteSettingsService::new( + ":memory:".into(), + RemoteSettingsConfig2 { + server: Some(RemoteSettingsServer::Custom { + url: server_url.into(), + }), + ..Default::default() + }, + ); + let telemetry = Arc::new(FakeTelemetry::new()); + service.set_telemetry(RemoteSettingsTelemetryWrapper::new(telemetry.clone())); + (service, telemetry) + } + + #[test] + fn test_telemetry_network_error_on_changes_failure() { + viaduct_dev::init_backend_dev(); + let _m = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(500) + .with_body("server error") + .create(); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].0, "settings-changes-monitoring"); + assert_eq!(events[0].1, RemoteSettingsSyncStatus::NetworkError); + assert!(events[0].2.errorName.is_some()); + } + + #[test] + fn test_telemetry_on_changes_success() { + viaduct_dev::init_backend_dev(); + let _m = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"changes": []}"#) + .create(); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].0, "settings-changes-monitoring"); + assert_eq!(events[0].1, RemoteSettingsSyncStatus::Success); + assert!(events[0].2.duration.is_some()); + } + + #[test] + fn test_telemetry_on_collection_success() { + viaduct_dev::init_backend_dev(); + let collection = "cid"; + let timestamp = 1774420582054u64; + + let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# + )) + .create(); + + let _changeset = mock( + "GET", + format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), + ) + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [], "timestamp": {timestamp}, "metadata": {{"bucket": "main", "signatures": []}}}}"# + )) + .create(); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _client = service.make_client(collection.into()); + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len(), 2); + // First event is for the changes endpoint + assert_eq!(events[0].0, "settings-changes-monitoring"); + // Second event is for the collection sync + assert_eq!(events[1].0, format!("main/{collection}")); + assert_eq!(events[1].1, RemoteSettingsSyncStatus::Success); + assert!(events[1].2.duration.is_some()); + } + + #[test] + fn test_telemetry_on_collection_up_to_date() { + viaduct_dev::init_backend_dev(); + let collection = "cid"; + let timestamp = 1774420582054u64; + + let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# + )) + // .expect_at_least(2) + .create(); + + let _changeset = mock( + "GET", + format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), + ) + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [], "timestamp": {timestamp}, "metadata": {{"bucket": "main", "signatures": []}}}}"# + )) + // .expect(1) + .create(); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _client = service.make_client(collection.into()); + + // First sync: populates local storage with timestamp. + let _ = service.sync(); + let events_before = telemetry.events.lock().unwrap().len(); + // Second sync. + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len() - events_before, 2); + // First event is for the changes endpoint + assert_eq!(events[events_before].0, "settings-changes-monitoring"); + // Second event is for the collection sync + assert_eq!(events[events_before + 1].0, format!("main/{collection}")); + assert_eq!( + events[events_before + 1].1, + RemoteSettingsSyncStatus::UpToDate + ); + } + + #[test] + fn test_telemetry_on_collection_error() { + viaduct_dev::init_backend_dev(); + let collection = "cid"; + let timestamp = 1774420582054u64; + + let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# + )) + .create(); + + let _changeset = mock( + "GET", + format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), + ) + .match_query(Matcher::Any) + .with_status(500) + .with_header("content-type", "application/json") + .with_body("server error") + .create(); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _client = service.make_client(collection.into()); + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len(), 2); + // First event is for the changes endpoint + assert_eq!(events[0].0, "settings-changes-monitoring"); + assert_eq!(events[0].1, RemoteSettingsSyncStatus::Success); + // Second event is for the collection sync + assert_eq!(events[1].0, format!("main/{collection}")); + assert_eq!(events[1].1, RemoteSettingsSyncStatus::NetworkError); + } +} From 0ce04c3b1643ccb0707cdb150223dddc8e9f4dec Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 10:51:44 +0100 Subject: [PATCH 07/20] Improve tests readability --- components/remote_settings/src/service.rs | 174 +++++++++------------- 1 file changed, 71 insertions(+), 103 deletions(-) diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index 8ebfa7b859..ced4673aeb 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -242,11 +242,15 @@ mod test { use mockito::{mock, Matcher}; use std::sync::Arc; - type EventTuple = (String, RemoteSettingsSyncStatus, SyncStatusExtras); + struct EmittedEvent { + source: String, + status: RemoteSettingsSyncStatus, + extras: SyncStatusExtras, + } /// Telemetry implementation that records all events for later assertion. struct FakeTelemetry { - events: std::sync::Mutex>, + events: std::sync::Mutex>, } impl FakeTelemetry { @@ -264,7 +268,10 @@ mod test { status: RemoteSettingsSyncStatus, extras: SyncStatusExtras, ) { - self.events.lock().unwrap().push((source, status, extras)); + self.events + .lock() + .unwrap() + .push(EmittedEvent { source, status, extras }); } } @@ -283,43 +290,70 @@ mod test { (service, telemetry) } + fn mock_monitor_changes(collection: &str, timestamp: u64) -> mockito::Mock { + mock("GET", "/v1/buckets/monitor/collections/changes/changeset") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"timestamp": {timestamp}, "changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# + )) + .create() + } + + fn mock_changeset(collection: &str, timestamp: u64) -> mockito::Mock { + mock( + "GET", + format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), + ) + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!( + r#"{{"changes": [], "timestamp": {timestamp}, "metadata": {{"bucket": "main", "signatures": []}}}}"# + )) + .create() + } + + fn mock_changeset_error(bucket: &str, collection: &str) -> mockito::Mock { + mock( + "GET", + format!("/v1/buckets/{bucket}/collections/{collection}/changeset").as_str(), + ) + .match_query(Matcher::Any) + .with_status(500) + .with_body("server error") + .create() + } + #[test] fn test_telemetry_network_error_on_changes_failure() { viaduct_dev::init_backend_dev(); - let _m = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") - .match_query(Matcher::Any) - .with_status(500) - .with_body("server error") - .create(); + mock_changeset_error("monitor", "changes"); let (service, telemetry) = make_service(&mockito::server_url()); let _ = service.sync(); let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 1); - assert_eq!(events[0].0, "settings-changes-monitoring"); - assert_eq!(events[0].1, RemoteSettingsSyncStatus::NetworkError); - assert!(events[0].2.errorName.is_some()); + assert_eq!(events[0].source, "settings-changes-monitoring"); + assert_eq!(events[0].status, RemoteSettingsSyncStatus::NetworkError); + assert!(events[0].extras.errorName.is_some()); } #[test] fn test_telemetry_on_changes_success() { viaduct_dev::init_backend_dev(); - let _m = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(r#"{"changes": []}"#) - .create(); + let _changes = mock_monitor_changes("cid", 42); let (service, telemetry) = make_service(&mockito::server_url()); let _ = service.sync(); let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 1); - assert_eq!(events[0].0, "settings-changes-monitoring"); - assert_eq!(events[0].1, RemoteSettingsSyncStatus::Success); - assert!(events[0].2.duration.is_some()); + assert_eq!(events[0].source, "settings-changes-monitoring"); + assert_eq!(events[0].status, RemoteSettingsSyncStatus::Success); + assert!(events[0].extras.duration.is_some()); } #[test] @@ -327,27 +361,8 @@ mod test { viaduct_dev::init_backend_dev(); let collection = "cid"; let timestamp = 1774420582054u64; - - let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(format!( - r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# - )) - .create(); - - let _changeset = mock( - "GET", - format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), - ) - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(format!( - r#"{{"changes": [], "timestamp": {timestamp}, "metadata": {{"bucket": "main", "signatures": []}}}}"# - )) - .create(); + let _changes = mock_monitor_changes(collection, timestamp); + let _changeset = mock_changeset(collection, timestamp); let (service, telemetry) = make_service(&mockito::server_url()); let _client = service.make_client(collection.into()); @@ -355,12 +370,10 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 2); - // First event is for the changes endpoint - assert_eq!(events[0].0, "settings-changes-monitoring"); - // Second event is for the collection sync - assert_eq!(events[1].0, format!("main/{collection}")); - assert_eq!(events[1].1, RemoteSettingsSyncStatus::Success); - assert!(events[1].2.duration.is_some()); + assert_eq!(events[0].source, "settings-changes-monitoring"); + assert_eq!(events[1].source, format!("main/{collection}")); + assert_eq!(events[1].status, RemoteSettingsSyncStatus::Success); + assert!(events[1].extras.duration.is_some()); } #[test] @@ -368,29 +381,8 @@ mod test { viaduct_dev::init_backend_dev(); let collection = "cid"; let timestamp = 1774420582054u64; - - let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(format!( - r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# - )) - // .expect_at_least(2) - .create(); - - let _changeset = mock( - "GET", - format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), - ) - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(format!( - r#"{{"changes": [], "timestamp": {timestamp}, "metadata": {{"bucket": "main", "signatures": []}}}}"# - )) - // .expect(1) - .create(); + let _changes = mock_monitor_changes(collection, timestamp); + let _changeset = mock_changeset(collection, timestamp); let (service, telemetry) = make_service(&mockito::server_url()); let _client = service.make_client(collection.into()); @@ -403,14 +395,9 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len() - events_before, 2); - // First event is for the changes endpoint - assert_eq!(events[events_before].0, "settings-changes-monitoring"); - // Second event is for the collection sync - assert_eq!(events[events_before + 1].0, format!("main/{collection}")); - assert_eq!( - events[events_before + 1].1, - RemoteSettingsSyncStatus::UpToDate - ); + assert_eq!(events[events_before].source, "settings-changes-monitoring"); + assert_eq!(events[events_before + 1].source, format!("main/{collection}")); + assert_eq!(events[events_before + 1].status, RemoteSettingsSyncStatus::UpToDate); } #[test] @@ -418,25 +405,8 @@ mod test { viaduct_dev::init_backend_dev(); let collection = "cid"; let timestamp = 1774420582054u64; - - let _changes = mock("GET", "/v1/buckets/monitor/collections/changes/changeset") - .match_query(Matcher::Any) - .with_status(200) - .with_header("content-type", "application/json") - .with_body(format!( - r#"{{"changes": [{{"collection": "{collection}", "bucket": "main", "last_modified": {timestamp}}}]}}"# - )) - .create(); - - let _changeset = mock( - "GET", - format!("/v1/buckets/main/collections/{collection}/changeset").as_str(), - ) - .match_query(Matcher::Any) - .with_status(500) - .with_header("content-type", "application/json") - .with_body("server error") - .create(); + let _changes = mock_monitor_changes(collection, timestamp); + let _changeset = mock_changeset_error("main", collection); let (service, telemetry) = make_service(&mockito::server_url()); let _client = service.make_client(collection.into()); @@ -444,11 +414,9 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 2); - // First event is for the changes endpoint - assert_eq!(events[0].0, "settings-changes-monitoring"); - assert_eq!(events[0].1, RemoteSettingsSyncStatus::Success); - // Second event is for the collection sync - assert_eq!(events[1].0, format!("main/{collection}")); - assert_eq!(events[1].1, RemoteSettingsSyncStatus::NetworkError); + assert_eq!(events[0].source, "settings-changes-monitoring"); + assert_eq!(events[0].status, RemoteSettingsSyncStatus::Success); + assert_eq!(events[1].source, format!("main/{collection}")); + assert_eq!(events[1].status, RemoteSettingsSyncStatus::NetworkError); } } From 5b0501d2de7007adba84dff0a61fa68ec1db0ef6 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 11:05:55 +0100 Subject: [PATCH 08/20] Do not send whole error str, only type name --- components/remote_settings/src/service.rs | 27 ++++++++++++++++----- components/remote_settings/src/telemetry.rs | 12 ++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index ced4673aeb..f960ad1099 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -268,10 +268,11 @@ mod test { status: RemoteSettingsSyncStatus, extras: SyncStatusExtras, ) { - self.events - .lock() - .unwrap() - .push(EmittedEvent { source, status, extras }); + self.events.lock().unwrap().push(EmittedEvent { + source, + status, + extras, + }); } } @@ -338,6 +339,10 @@ mod test { assert_eq!(events.len(), 1); assert_eq!(events[0].source, "settings-changes-monitoring"); assert_eq!(events[0].status, RemoteSettingsSyncStatus::NetworkError); + assert_eq!( + events[0].extras.errorName, + Some("ResponseError".to_string()) + ); assert!(events[0].extras.errorName.is_some()); } @@ -396,8 +401,14 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len() - events_before, 2); assert_eq!(events[events_before].source, "settings-changes-monitoring"); - assert_eq!(events[events_before + 1].source, format!("main/{collection}")); - assert_eq!(events[events_before + 1].status, RemoteSettingsSyncStatus::UpToDate); + assert_eq!( + events[events_before + 1].source, + format!("main/{collection}") + ); + assert_eq!( + events[events_before + 1].status, + RemoteSettingsSyncStatus::UpToDate + ); } #[test] @@ -418,5 +429,9 @@ mod test { assert_eq!(events[0].status, RemoteSettingsSyncStatus::Success); assert_eq!(events[1].source, format!("main/{collection}")); assert_eq!(events[1].status, RemoteSettingsSyncStatus::NetworkError); + assert_eq!( + events[1].extras.errorName, + Some("ResponseError".to_string()) + ); } } diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs index 643e45c7f5..0389cfe21f 100644 --- a/components/remote_settings/src/telemetry.rs +++ b/components/remote_settings/src/telemetry.rs @@ -112,12 +112,22 @@ impl RemoteSettingsTelemetryWrapper { } pub fn report_sync_error(&self, error: &Error, source: &str) { + // This is a bit hacky and naive, but it allows us to get the original + // error type without needing to add too much machinery to our error types. + // This mimics what we do in the desktop client: + // https://searchfox.org/firefox-main/rev/26c440c6196eb0b4/services/settings/RemoteSettingsClient.sys.mjs#965 + let error_name = format!("{error:?}") + .split("{") + .next() + .unwrap_or("") + .trim() + .to_string(); self.inner.report( source.to_string(), error_to_status(error), SyncStatusExtras { duration: None, - errorName: Some(format!("{error:?}")), + errorName: Some(error_name), }, ); } From ecc2e3af2753256f2458f922bb7105ac621fd405 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 25 Mar 2026 13:17:00 +0100 Subject: [PATCH 09/20] Test and fix for signature errors --- CHANGELOG.md | 5 ++++ components/remote_settings/src/service.rs | 26 ++++++++++++++++++ components/remote_settings/src/telemetry.rs | 2 +- .../rc_crypto/nss/fixtures/profile/logins.db | Bin 40960 -> 45056 bytes 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c98a6acb5..fbfcc64e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ [Full Changelog](In progress) +## ✨ What's New ✨ + +### Remote Settings +* Add uptake telemetry support ([#7288](https://github.com/mozilla/application-services/pull/7288)) + # v150.0 (_2026-03-23_) # General diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index f960ad1099..b0a83dd203 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -361,6 +361,7 @@ mod test { assert!(events[0].extras.duration.is_some()); } + #[cfg(not(feature = "signatures"))] #[test] fn test_telemetry_on_collection_success() { viaduct_dev::init_backend_dev(); @@ -381,6 +382,7 @@ mod test { assert!(events[1].extras.duration.is_some()); } + #[cfg(not(feature = "signatures"))] #[test] fn test_telemetry_on_collection_up_to_date() { viaduct_dev::init_backend_dev(); @@ -434,4 +436,28 @@ mod test { Some("ResponseError".to_string()) ); } + + #[cfg(feature = "signatures")] + #[test] + fn test_telemetry_on_collection_signature_error() { + viaduct_dev::init_backend_dev(); + let collection = "cid"; + let timestamp = 1774420582054u64; + let _changes = mock_monitor_changes(collection, timestamp); + let _changeset = mock_changeset(collection, timestamp); + + let (service, telemetry) = make_service(&mockito::server_url()); + let _client = service.make_client(collection.into()); + let _ = service.sync(); + + let events = telemetry.events.lock().unwrap(); + assert_eq!(events.len(), 2); + assert_eq!(events[0].source, "settings-changes-monitoring"); + assert_eq!(events[1].source, format!("main/{collection}")); + assert_eq!(events[1].status, RemoteSettingsSyncStatus::SignatureError); + assert_eq!( + events[1].extras.errorName, + Some("IncompleteSignatureDataError".to_string()) + ); + } } diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs index 0389cfe21f..19bf309488 100644 --- a/components/remote_settings/src/telemetry.rs +++ b/components/remote_settings/src/telemetry.rs @@ -117,7 +117,7 @@ impl RemoteSettingsTelemetryWrapper { // This mimics what we do in the desktop client: // https://searchfox.org/firefox-main/rev/26c440c6196eb0b4/services/settings/RemoteSettingsClient.sys.mjs#965 let error_name = format!("{error:?}") - .split("{") + .split(&['{', '(']) .next() .unwrap_or("") .trim() diff --git a/components/support/rc_crypto/nss/fixtures/profile/logins.db b/components/support/rc_crypto/nss/fixtures/profile/logins.db index ebe1a36f4b2f23b9b7f98e178376fb9ca60a006f..9253f9b3b7d66435ed5e050539c91b1bcaa20ae0 100644 GIT binary patch delta 294 zcmZoTz|`=7X@ayW3j+fKHxR=B50C*8g8@b$gLPt~ydn#OUY8Ir>jwrd)?@}QZT^e= z^=vBq0enaJ9C`QhHn1jdY<$e(+Q`MlE-o+6*qB_Bn3R*6RFs;SoRM1W17~nM2e~?i zxGID=I{CONz=ahwxD=ouGi7oXmxc<6mztMcR9R4xni7y$TwI=Cl%f#g8W93ksR?wK z$>cAr23#F1EbQXa(v1D_n}4z1VV?YfQ;9Q(5iHbGy;+(|3nZktxq-KhiA9WoUu?6W u0uR4vV>+V{gEfP-tSlodD{Et}Dk~!+!(?4~`^nkzQ#SvTXKGmFAOHYIHAWl& delta 213 zcmZp8z|?SnX@ayWGXnzy7ZAe$Cy>Dc;{a(!AZDJ}C@;^RKeqi7d Date: Tue, 31 Mar 2026 16:33:31 +0200 Subject: [PATCH 10/20] Remove logins.db fixture file --- .../rc_crypto/nss/fixtures/profile/logins.db | Bin 45056 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 components/support/rc_crypto/nss/fixtures/profile/logins.db diff --git a/components/support/rc_crypto/nss/fixtures/profile/logins.db b/components/support/rc_crypto/nss/fixtures/profile/logins.db deleted file mode 100644 index 9253f9b3b7d66435ed5e050539c91b1bcaa20ae0..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 45056 zcmeI*+in_17zgm(*qE5ap-QE4VMUpXMm4fc>@;m!tV+drli)636R=EfR?F@fuf6P& z-33f~(Q;E&Ri*Z}57WE8K;Iz`kVtue&g=nL9}-{F|H=pUFf-r$X6EB$k{A23;ZbsA zI~~;{UkaZHq9}Y%h#&~_?E4n`j<(zE20s>UBD;}@cCPq6FXVo^yOekL%DvLaWK3G z{toNIh3%T1vK@nk886MdL@HJG-H_#=@9XPCI#V^z91`22#6BX&p4Y8W)$9=4@$cgb znrGNnHK@-$wtJ>dT9mld^JAP`KWU6N6x7e}Bx4n@iza7#E_E!mL!TOyRT^xQv)yng zEWEdlv^eHcq5bDeX6;R8Uz|2OGDtVVVZ3+_rR=tf<5t zTH5AU`TPs&^r=(lJf7O=l1|U`jIL?009(V@9AlMiZftB2)zXRQ^em0TWMh}G0b-RY z9a}e!cnz0#F^bA?6`Nh|7`jgR+^d%=O;%z&=gyBuRE`D0YByOwY>OC{W_Jv$O&r!Y zhaDSGqTALN9yzt_0a06a&+|ie?4G52j?vAgtrM!T zk}fMu#_OH9tOlzcuY#RbdKPcaQo_K3F&j6uM)+LfGi2M(O$C!W-j3mR43|&IkSY61 zP4v^i?>|nZ0XfJL@{LPUhdhn>`Zj}(@Iyq=%=t}Zri?3#;e zYjGh{&5D9y>GaHfWwIw@irVvRe_sg}*9+EonR=?S8Q)Kh#GA=XHY<+4n_7|4)$wZ= zFIk__l~c>~ln(v7wCRT#lVsiJ$!DAZ{jO$5LNb|owk(cT{g4g)ER0%KCe})@bi-V6 zGmNEF*^v&(2TS;ER4XJbNmeFx^7+9Z7GzVS#f4<1%tCl_E`%}o-0gfE)!N&n>YBgkt(Lfqh|v00bZa0SG_<0uX=z1Rwwb2>j0kMs4w~uqABW zyC)_RiBac4LKFr5=75`j_;4^72UYB z^-MFW#_p+lSTf||uG#98`TlnKr17Y>QK^)yja*K7d{TU@Y`$!t9>}!#`ejb3tUoWP zMW@?8*>6(GYnJ5gr;b@`J{fk`zqWJFkNdqrrTO}#bNHzC+}>>**aKzY?)2*ied(LN z_xjr)4&g?MjjWh{E3hwY5P$##AOHafKmY;|fB*y_009Whh(LPo!Q%Am0KxnJ?}haH z8R;Op0Rad=00Izz00bZa0SG_<0uX?JFOU-FmKVcMRPg8j>_h+9AOHafKmY;|fB*y_ z009U<00I!0VFCX4|9JjC!vl=2K>z{}fB*y_009U<00Izz00fu-p8sPSKmY;|fB*y_ z009U<00Izz00d@VAo%zH-U#V8v)@ Date: Wed, 1 Apr 2026 18:00:55 +0200 Subject: [PATCH 11/20] Rename and rework types to make Android integration more natural --- components/remote_settings/metrics.yaml | 20 ++- components/remote_settings/src/lib.rs | 2 +- components/remote_settings/src/service.rs | 98 ++++++++------- components/remote_settings/src/telemetry.rs | 127 ++++++++++++-------- 4 files changed, 138 insertions(+), 109 deletions(-) diff --git a/components/remote_settings/metrics.yaml b/components/remote_settings/metrics.yaml index 04319868d6..6fabb6e72f 100644 --- a/components/remote_settings/metrics.yaml +++ b/components/remote_settings/metrics.yaml @@ -15,7 +15,7 @@ $schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 remote_settings: - sync: + uptake_remotesettings: type: event description: > Was the remote content successfully pulled? This uptake telemetry @@ -35,17 +35,25 @@ remote_settings: type: string errorName: description: An optional string with the error name attribute in case of failure. + trigger: + description: > + A label to distinguish what triggered the polling/fetching of remote content (eg. "broadcast", "timer", "forced", "manual") + type: string + age: + description: > + The age of pulled data in seconds (ie. difference between publication time and fetch time). + type: string + timestamp: + description: > + The current timestamp, received during synchronization. + type: string bugs: &uptake_remotecontent_result_uptake_bugs - https://bugzil.la/1517469 - https://bugzil.la/1617133 - data_reviews: &uptake_remotecontent_result_uptake_data_reviews - - https://bugzil.la/1517469 - - https://bugzil.la/1617133 + data_reviews: *uptake_remotecontent_result_uptake_bugs notification_emails: &uptake_remotecontent_result_uptake_emails - mleplatre@mozilla.com - acottner@mozilla.com data_sensitivity: - technical - notification_emails: - - synced-client-integrations@mozilla.com expires: never diff --git a/components/remote_settings/src/lib.rs b/components/remote_settings/src/lib.rs index 024e6a25d5..a95e271d3e 100644 --- a/components/remote_settings/src/lib.rs +++ b/components/remote_settings/src/lib.rs @@ -25,7 +25,7 @@ pub use client::{Attachment, RemoteSettingsRecord, RemoteSettingsResponse, RsJso pub use config::{BaseUrl, RemoteSettingsConfig, RemoteSettingsConfig2, RemoteSettingsServer}; pub use context::RemoteSettingsContext; pub use error::{trace, ApiResult, RemoteSettingsError, Result}; -pub use telemetry::{RemoteSettingsSyncStatus, RemoteSettingsTelemetry, SyncStatusExtras}; +pub use telemetry::{RemoteSettingsTelemetry, SyncStatus, UptakeEventExtras}; use client::Client; use error::Error; diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs index b0a83dd203..7f4aa4320c 100644 --- a/components/remote_settings/src/service.rs +++ b/components/remote_settings/src/service.rs @@ -112,7 +112,7 @@ impl RemoteSettingsService { { if client_last_modified == *server_last_modified { trace!("skipping up-to-date collection: {collection_name}"); - inner.telemetry.report_up_to_date(&cid, None); + inner.telemetry.report_uptake_up_to_date(&cid, None); continue; } } @@ -123,8 +123,8 @@ impl RemoteSettingsService { let sync_result = client.sync(); let duration: u64 = start_time.elapsed().as_millis().try_into().unwrap_or(0); match &sync_result { - Ok(()) => inner.telemetry.report_success(&cid, Some(duration)), - Err(e) => inner.telemetry.report_sync_error(e, &cid), + Ok(()) => inner.telemetry.report_uptake_success(&cid, Some(duration)), + Err(e) => inner.telemetry.report_uptake_error(e, &cid), } sync_result?; } @@ -209,11 +209,12 @@ impl RemoteSettingsServiceInner { let body = resp.json()?; let duration: u64 = start_time.elapsed().as_millis().try_into().unwrap_or(0); self.telemetry - .report_success(TELEMETRY_SOURCE_POLL, Some(duration)); + .report_uptake_success(TELEMETRY_SOURCE_POLL, Some(duration)); Ok(body) } else { let e = Error::response_error(&resp.url, format!("status code: {}", resp.status)); - self.telemetry.report_sync_error(&e, TELEMETRY_SOURCE_POLL); + self.telemetry + .report_uptake_error(&e, TELEMETRY_SOURCE_POLL); Err(e) } } @@ -237,20 +238,14 @@ struct ChangesCollection { #[cfg(test)] mod test { use super::*; - use crate::telemetry::{RemoteSettingsSyncStatus, SyncStatusExtras}; + use crate::telemetry::UptakeEventExtras; use crate::{RemoteSettingsConfig2, RemoteSettingsServer}; use mockito::{mock, Matcher}; use std::sync::Arc; - struct EmittedEvent { - source: String, - status: RemoteSettingsSyncStatus, - extras: SyncStatusExtras, - } - /// Telemetry implementation that records all events for later assertion. struct FakeTelemetry { - events: std::sync::Mutex>, + events: std::sync::Mutex>, } impl FakeTelemetry { @@ -262,17 +257,8 @@ mod test { } impl crate::telemetry::RemoteSettingsTelemetry for FakeTelemetry { - fn report( - &self, - source: String, - status: RemoteSettingsSyncStatus, - extras: SyncStatusExtras, - ) { - self.events.lock().unwrap().push(EmittedEvent { - source, - status, - extras, - }); + fn report_uptake(&self, extras: UptakeEventExtras) { + self.events.lock().unwrap().push(extras); } } @@ -337,13 +323,13 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 1); - assert_eq!(events[0].source, "settings-changes-monitoring"); - assert_eq!(events[0].status, RemoteSettingsSyncStatus::NetworkError); assert_eq!( - events[0].extras.errorName, - Some("ResponseError".to_string()) + events[0].source, + Some("settings-changes-monitoring".to_string()) ); - assert!(events[0].extras.errorName.is_some()); + assert_eq!(events[0].value, Some("network_error".to_string())); + assert_eq!(events[0].errorName, Some("ResponseError".to_string())); + assert!(events[0].errorName.is_some()); } #[test] @@ -356,9 +342,12 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 1); - assert_eq!(events[0].source, "settings-changes-monitoring"); - assert_eq!(events[0].status, RemoteSettingsSyncStatus::Success); - assert!(events[0].extras.duration.is_some()); + assert_eq!( + events[0].source, + Some("settings-changes-monitoring".to_string()) + ); + assert_eq!(events[0].value, Some("success".to_string())); + assert!(events[0].duration.is_some()); } #[cfg(not(feature = "signatures"))] @@ -376,10 +365,13 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 2); - assert_eq!(events[0].source, "settings-changes-monitoring"); - assert_eq!(events[1].source, format!("main/{collection}")); - assert_eq!(events[1].status, RemoteSettingsSyncStatus::Success); - assert!(events[1].extras.duration.is_some()); + assert_eq!( + events[0].source, + Some("settings-changes-monitoring".to_string()) + ); + assert_eq!(events[1].source, Some(format!("main/{collection}"))); + assert_eq!(events[1].value, Some("success".to_string())); + assert!(events[1].duration.is_some()); } #[cfg(not(feature = "signatures"))] @@ -402,14 +394,17 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len() - events_before, 2); - assert_eq!(events[events_before].source, "settings-changes-monitoring"); + assert_eq!( + events[events_before].source, + Some("settings-changes-monitoring".to_string()) + ); assert_eq!( events[events_before + 1].source, - format!("main/{collection}") + Some(format!("main/{collection}")) ); assert_eq!( - events[events_before + 1].status, - RemoteSettingsSyncStatus::UpToDate + events[events_before + 1].value, + Some("up_to_date".to_string()) ); } @@ -427,14 +422,14 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 2); - assert_eq!(events[0].source, "settings-changes-monitoring"); - assert_eq!(events[0].status, RemoteSettingsSyncStatus::Success); - assert_eq!(events[1].source, format!("main/{collection}")); - assert_eq!(events[1].status, RemoteSettingsSyncStatus::NetworkError); assert_eq!( - events[1].extras.errorName, - Some("ResponseError".to_string()) + events[0].source, + Some("settings-changes-monitoring".to_string()) ); + assert_eq!(events[0].value, Some("success".to_string())); + assert_eq!(events[1].source, Some(format!("main/{collection}"))); + assert_eq!(events[1].value, Some("network_error".to_string())); + assert_eq!(events[1].errorName, Some("ResponseError".to_string())); } #[cfg(feature = "signatures")] @@ -452,11 +447,14 @@ mod test { let events = telemetry.events.lock().unwrap(); assert_eq!(events.len(), 2); - assert_eq!(events[0].source, "settings-changes-monitoring"); - assert_eq!(events[1].source, format!("main/{collection}")); - assert_eq!(events[1].status, RemoteSettingsSyncStatus::SignatureError); assert_eq!( - events[1].extras.errorName, + events[0].source, + Some("settings-changes-monitoring".to_string()) + ); + assert_eq!(events[1].source, Some(format!("main/{collection}"))); + assert_eq!(events[1].value, Some("signature_error".to_string())); + assert_eq!( + events[1].errorName, Some("IncompleteSignatureDataError".to_string()) ); } diff --git a/components/remote_settings/src/telemetry.rs b/components/remote_settings/src/telemetry.rs index 19bf309488..822dc5a19b 100644 --- a/components/remote_settings/src/telemetry.rs +++ b/components/remote_settings/src/telemetry.rs @@ -2,13 +2,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::sync::Arc; +use std::{fmt, sync::Arc}; use crate::error::Error; /// Remote Settings sync status. #[derive(Debug, PartialEq, uniffi::Enum)] -pub enum RemoteSettingsSyncStatus { +pub enum SyncStatus { /// Sync completed and new data was stored. Success, /// Local data is already up to date, no new data was stored. @@ -25,11 +25,36 @@ pub enum RemoteSettingsSyncStatus { UnknownError, } +impl fmt::Display for SyncStatus { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + SyncStatus::Success => "success", + SyncStatus::UpToDate => "up_to_date", + SyncStatus::NetworkError => "network_error", + SyncStatus::BackoffError => "backoff_error", + SyncStatus::SignatureError => "signature_error", + SyncStatus::ServerError => "server_error", + SyncStatus::UnknownError => "unknown_error", + }; + f.write_str(s) + } +} + #[derive(Debug, PartialEq, uniffi::Record, Default)] #[allow(non_snake_case)] -pub struct SyncStatusExtras { +pub struct UptakeEventExtras { + /// Main sync status. + pub value: Option, + /// Source of the sync (eg. "settings-changes-monitoring", "main/{collection}", ...) + pub source: Option, + /// Age of the data in milliseconds, if available. + pub age: Option, + /// Trigger that caused the sync (eg. "manual", "startup", "scheduled", ...) if available. + pub trigger: Option, + /// Timestamp received from the server, if available. + pub timestamp: Option, /// Duration of the sync operation in milliseconds, if available. - pub duration: Option, + pub duration: Option, /// The name of the error that occurred, if available. pub errorName: Option, } @@ -41,35 +66,30 @@ pub struct SyncStatusExtras { /// /// Consumers implement the trait like this (Kotlin example): /// ```kotlin +/// /* Import the UniFFI-generated bindings */ /// import mozilla.appservices.remote_settings.RemoteSettingsTelemetry -/// import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettingsClient +/// import mozilla.appservices.remote_settings.UptakeEventExtras +/// /* Import the Glean-generated bindings */ +/// import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettings as RSMetrics /// -/// class RSTelemetry : RemoteSettingsTelemetry { -/// override fun report(source: String, value: RemoteSettingsSyncStatus, extras: SyncStatusExtras) { -/// RemoteSettingsClient.syncStatus.record( -/// RemoteSettingsClient.SyncStatusExtra( -/// value = value, -/// source = source, -/// duration = extras.duration, -/// errorName = extras.errorName -/// ) -/// ) +/// class GleanTelemetry : RemoteSettingsTelemetry { +/// override fun report_uptake(eventExtras: UptakeEventExtras) { +/// RSMetrics.uptakeRemotesettings.record(eventExtras) /// } /// } /// -/// service.setTelemetry(RSTelemetry()) +/// service.setTelemetry(GleanTelemetry()) /// ``` #[uniffi::export(with_foreign)] pub trait RemoteSettingsTelemetry: Send + Sync { /// Report uptake event. - fn report(&self, source: String, value: RemoteSettingsSyncStatus, extras: SyncStatusExtras); + fn report_uptake(&self, extras: UptakeEventExtras); } struct NoopRemoteSettingsTelemetry; impl RemoteSettingsTelemetry for NoopRemoteSettingsTelemetry { - fn report(&self, _source: String, _value: RemoteSettingsSyncStatus, _extras: SyncStatusExtras) { - } + fn report_uptake(&self, _extras: UptakeEventExtras) {} } /// Wrapper around [RemoteSettingsTelemetry] used internally. @@ -89,29 +109,31 @@ impl RemoteSettingsTelemetryWrapper { } } - pub fn report_success(&self, source: &str, duration: Option) { - self.inner.report( - source.to_string(), - RemoteSettingsSyncStatus::Success, - SyncStatusExtras { - duration, - errorName: None, - }, - ); + pub fn report_uptake_success(&self, source: &str, duration: Option) { + self.inner.report_uptake(UptakeEventExtras { + value: Some(SyncStatus::Success.to_string()), + source: Some(source.to_string()), + age: None, + trigger: None, + timestamp: None, + duration: duration.map(|d| d.to_string()), + errorName: None, + }); } - pub fn report_up_to_date(&self, source: &str, duration: Option) { - self.inner.report( - source.to_string(), - RemoteSettingsSyncStatus::UpToDate, - SyncStatusExtras { - duration, - errorName: None, - }, - ); + pub fn report_uptake_up_to_date(&self, source: &str, duration: Option) { + self.inner.report_uptake(UptakeEventExtras { + value: Some(SyncStatus::UpToDate.to_string()), + source: Some(source.to_string()), + age: None, + trigger: None, + timestamp: None, + duration: duration.map(|d| d.to_string()), + errorName: None, + }); } - pub fn report_sync_error(&self, error: &Error, source: &str) { + pub fn report_uptake_error(&self, error: &Error, source: &str) { // This is a bit hacky and naive, but it allows us to get the original // error type without needing to add too much machinery to our error types. // This mimics what we do in the desktop client: @@ -122,26 +144,27 @@ impl RemoteSettingsTelemetryWrapper { .unwrap_or("") .trim() .to_string(); - self.inner.report( - source.to_string(), - error_to_status(error), - SyncStatusExtras { - duration: None, - errorName: Some(error_name), - }, - ); + self.inner.report_uptake(UptakeEventExtras { + value: Some(error_to_status(error).to_string()), + source: Some(source.to_string()), + age: None, + trigger: None, + timestamp: None, + duration: None, + errorName: Some(error_name), + }); } } -fn error_to_status(error: &Error) -> RemoteSettingsSyncStatus { +fn error_to_status(error: &Error) -> SyncStatus { match error { Error::RequestError(viaduct::ViaductError::NetworkError(_)) - | Error::ResponseError { .. } => RemoteSettingsSyncStatus::NetworkError, - Error::BackoffError(_) => RemoteSettingsSyncStatus::BackoffError, + | Error::ResponseError { .. } => SyncStatus::NetworkError, + Error::BackoffError(_) => SyncStatus::BackoffError, #[cfg(feature = "signatures")] - Error::IncompleteSignatureDataError(_) => RemoteSettingsSyncStatus::SignatureError, + Error::IncompleteSignatureDataError(_) => SyncStatus::SignatureError, #[cfg(feature = "signatures")] - Error::SignatureError(_) => RemoteSettingsSyncStatus::SignatureError, - _ => RemoteSettingsSyncStatus::UnknownError, + Error::SignatureError(_) => SyncStatus::SignatureError, + _ => SyncStatus::UnknownError, } } From 6cd7cf477af1badd79c48b234150949b4f3c4eaf Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 1 Apr 2026 18:16:14 +0200 Subject: [PATCH 12/20] Publish the Kotlin wrapper for Telemetry --- .../remote_settings/android/build.gradle | 34 +++++++++++++++++++ .../remotesettings/RemoteSettingsTelemetry.kt | 18 ++++++++++ components/remote_settings/metrics.yaml | 1 + 3 files changed, 53 insertions(+) create mode 100644 components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt diff --git a/components/remote_settings/android/build.gradle b/components/remote_settings/android/build.gradle index cd79ece806..f3977e1175 100644 --- a/components/remote_settings/android/build.gradle +++ b/components/remote_settings/android/build.gradle @@ -1,10 +1,44 @@ +buildscript { + if (gradle.hasProperty("mozconfig")) { + repositories { + gradle.mozconfig.substs.GRADLE_MAVEN_REPOSITORIES.each { repository -> + maven { + url = repository + if (gradle.mozconfig.substs.ALLOW_INSECURE_GRADLE_REPOSITORIES) { + allowInsecureProtocol = true + } + } + } + } + + dependencies { + classpath libs.mozilla.glean.gradle.plugin + } + } +} + apply from: "$appServicesRootDir/build-scripts/component-common.gradle" apply from: "$appServicesRootDir/publish.gradle" +ext { + gleanNamespace = "mozilla.telemetry.glean" + gleanYamlFiles = ["${project.projectDir}/../metrics.yaml"] + if (gradle.hasProperty("mozconfig")) { + gleanPythonEnvDir = gradle.mozconfig.substs.GRADLE_GLEAN_PARSER_VENV + } +} +apply plugin: "org.mozilla.telemetry.glean-gradle-plugin" + android { namespace 'org.mozilla.appservices.remotesettings' } +dependencies { + implementation libs.mozilla.glean + + testImplementation libs.mozilla.glean.forUnitTests +} + ext.configureUniFFIBindgen("remote_settings") ext.dependsOnTheMegazord() ext.configurePublish() diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt new file mode 100644 index 0000000000..4fe2418c6e --- /dev/null +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt @@ -0,0 +1,18 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.appservices.remotesettings + +/* UniFFI-generated bindings */ +import mozilla.appservices.remote_settings.RemoteSettingsTelemetry +import mozilla.appservices.remote_settings.UptakeEventExtras +/* Glean-generated bindings */ +import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettings as RSMetrics + + +class GleanTelemetry : RemoteSettingsTelemetry { + override fun report_uptake(extras: SyncStatusExtras) { + RSMetrics.uptakeRemotesettings.record(extras) + } +} diff --git a/components/remote_settings/metrics.yaml b/components/remote_settings/metrics.yaml index 6fabb6e72f..4875be4233 100644 --- a/components/remote_settings/metrics.yaml +++ b/components/remote_settings/metrics.yaml @@ -17,6 +17,7 @@ $schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 remote_settings: uptake_remotesettings: type: event + disabled: true # To be controlled by server knobs due to expected high volume description: > Was the remote content successfully pulled? This uptake telemetry allows to monitor the behaviour of our clients when it comes to From 28f2fcb878bac5b864ec8656b61aeee2910d72c5 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 11:20:22 +0200 Subject: [PATCH 13/20] Try add envs plugin --- components/remote_settings/android/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/remote_settings/android/build.gradle b/components/remote_settings/android/build.gradle index f3977e1175..3a7dc8ff2f 100644 --- a/components/remote_settings/android/build.gradle +++ b/components/remote_settings/android/build.gradle @@ -17,6 +17,10 @@ buildscript { } } +plugins { + alias libs.plugins.python.envs.plugin +} + apply from: "$appServicesRootDir/build-scripts/component-common.gradle" apply from: "$appServicesRootDir/publish.gradle" From b249cecd94673e949581ac8ddfb63c2058b629fb Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 11:22:58 +0200 Subject: [PATCH 14/20] Revert "Remove logins.db fixture file" This reverts commit 0b2bb387e16535d507a044cd89d4ab3f9d706c6e. --- .../rc_crypto/nss/fixtures/profile/logins.db | Bin 0 -> 45056 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 components/support/rc_crypto/nss/fixtures/profile/logins.db diff --git a/components/support/rc_crypto/nss/fixtures/profile/logins.db b/components/support/rc_crypto/nss/fixtures/profile/logins.db new file mode 100644 index 0000000000000000000000000000000000000000..9253f9b3b7d66435ed5e050539c91b1bcaa20ae0 GIT binary patch literal 45056 zcmeI*+in_17zgm(*qE5ap-QE4VMUpXMm4fc>@;m!tV+drli)636R=EfR?F@fuf6P& z-33f~(Q;E&Ri*Z}57WE8K;Iz`kVtue&g=nL9}-{F|H=pUFf-r$X6EB$k{A23;ZbsA zI~~;{UkaZHq9}Y%h#&~_?E4n`j<(zE20s>UBD;}@cCPq6FXVo^yOekL%DvLaWK3G z{toNIh3%T1vK@nk886MdL@HJG-H_#=@9XPCI#V^z91`22#6BX&p4Y8W)$9=4@$cgb znrGNnHK@-$wtJ>dT9mld^JAP`KWU6N6x7e}Bx4n@iza7#E_E!mL!TOyRT^xQv)yng zEWEdlv^eHcq5bDeX6;R8Uz|2OGDtVVVZ3+_rR=tf<5t zTH5AU`TPs&^r=(lJf7O=l1|U`jIL?009(V@9AlMiZftB2)zXRQ^em0TWMh}G0b-RY z9a}e!cnz0#F^bA?6`Nh|7`jgR+^d%=O;%z&=gyBuRE`D0YByOwY>OC{W_Jv$O&r!Y zhaDSGqTALN9yzt_0a06a&+|ie?4G52j?vAgtrM!T zk}fMu#_OH9tOlzcuY#RbdKPcaQo_K3F&j6uM)+LfGi2M(O$C!W-j3mR43|&IkSY61 zP4v^i?>|nZ0XfJL@{LPUhdhn>`Zj}(@Iyq=%=t}Zri?3#;e zYjGh{&5D9y>GaHfWwIw@irVvRe_sg}*9+EonR=?S8Q)Kh#GA=XHY<+4n_7|4)$wZ= zFIk__l~c>~ln(v7wCRT#lVsiJ$!DAZ{jO$5LNb|owk(cT{g4g)ER0%KCe})@bi-V6 zGmNEF*^v&(2TS;ER4XJbNmeFx^7+9Z7GzVS#f4<1%tCl_E`%}o-0gfE)!N&n>YBgkt(Lfqh|v00bZa0SG_<0uX=z1Rwwb2>j0kMs4w~uqABW zyC)_RiBac4LKFr5=75`j_;4^72UYB z^-MFW#_p+lSTf||uG#98`TlnKr17Y>QK^)yja*K7d{TU@Y`$!t9>}!#`ejb3tUoWP zMW@?8*>6(GYnJ5gr;b@`J{fk`zqWJFkNdqrrTO}#bNHzC+}>>**aKzY?)2*ied(LN z_xjr)4&g?MjjWh{E3hwY5P$##AOHafKmY;|fB*y_009Whh(LPo!Q%Am0KxnJ?}haH z8R;Op0Rad=00Izz00bZa0SG_<0uX?JFOU-FmKVcMRPg8j>_h+9AOHafKmY;|fB*y_ z009U<00I!0VFCX4|9JjC!vl=2K>z{}fB*y_009U<00Izz00fu-p8sPSKmY;|fB*y_ z009U<00Izz00d@VAo%zH-U#V8v)@ Date: Thu, 2 Apr 2026 12:22:26 +0200 Subject: [PATCH 15/20] ktlint format --- .../{RemoteSettingsTelemetry.kt => GleanTelemetry.kt} | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) rename components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/{RemoteSettingsTelemetry.kt => GleanTelemetry.kt} (85%) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt similarity index 85% rename from components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt rename to components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index 4fe2418c6e..812482be15 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/RemoteSettingsTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -4,13 +4,15 @@ package mozilla.appservices.remotesettings -/* UniFFI-generated bindings */ import mozilla.appservices.remote_settings.RemoteSettingsTelemetry import mozilla.appservices.remote_settings.UptakeEventExtras -/* Glean-generated bindings */ import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettings as RSMetrics +/** + * GleanTelemetry is a thin wrapper used to expose + * callbacks used to emit telemetry events to Glean. + */ class GleanTelemetry : RemoteSettingsTelemetry { override fun report_uptake(extras: SyncStatusExtras) { RSMetrics.uptakeRemotesettings.record(extras) From 8a18d0b35a41194c3e65ea689e19c88c2a7e67d4 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 12:31:00 +0200 Subject: [PATCH 16/20] More klint formatting --- .../java/mozilla/appservices/remotesettings/GleanTelemetry.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index 812482be15..f4681f3864 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -8,7 +8,6 @@ import mozilla.appservices.remote_settings.RemoteSettingsTelemetry import mozilla.appservices.remote_settings.UptakeEventExtras import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettings as RSMetrics - /** * GleanTelemetry is a thin wrapper used to expose * callbacks used to emit telemetry events to Glean. From d39f463d5efa3120cf100451fcb1f55e1822c275 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 13:00:01 +0200 Subject: [PATCH 17/20] Guess glean generated code --- .../java/mozilla/appservices/remotesettings/GleanTelemetry.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index f4681f3864..bd62a3f469 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -6,14 +6,14 @@ package mozilla.appservices.remotesettings import mozilla.appservices.remote_settings.RemoteSettingsTelemetry import mozilla.appservices.remote_settings.UptakeEventExtras -import org.mozilla.appservices.remote_settings.GleanMetrics.RemoteSettings as RSMetrics +import org.mozilla.appservices.remotesettings.GleanMetrics.RemoteSettings as RSMetrics /** * GleanTelemetry is a thin wrapper used to expose * callbacks used to emit telemetry events to Glean. */ class GleanTelemetry : RemoteSettingsTelemetry { - override fun report_uptake(extras: SyncStatusExtras) { + override fun reportUptake(extras: UptakeEventExtras) { RSMetrics.uptakeRemotesettings.record(extras) } } From 28dc0f94a1395f327c4f6c87e51c9a1c67e0cc96 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 13:17:18 +0200 Subject: [PATCH 18/20] Adjust interface types and imports --- .../appservices/remotesettings/GleanTelemetry.kt | 16 +++++++++++++--- components/remote_settings/metrics.yaml | 1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index bd62a3f469..27d3e760bb 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -4,8 +4,8 @@ package mozilla.appservices.remotesettings -import mozilla.appservices.remote_settings.RemoteSettingsTelemetry -import mozilla.appservices.remote_settings.UptakeEventExtras +import mozilla.appservices.remotesettings.RemoteSettingsTelemetry +import mozilla.appservices.remotesettings.UptakeEventExtras import org.mozilla.appservices.remotesettings.GleanMetrics.RemoteSettings as RSMetrics /** @@ -14,6 +14,16 @@ import org.mozilla.appservices.remotesettings.GleanMetrics.RemoteSettings as RSM */ class GleanTelemetry : RemoteSettingsTelemetry { override fun reportUptake(extras: UptakeEventExtras) { - RSMetrics.uptakeRemotesettings.record(extras) + RSMetrics.uptakeRemotesettings.record( + RSMetrics.UptakeRemotesettingsExtra( + value = extras.value, + source = extras.source, + age = extras.age, + trigger = extras.trigger, + timestamp = extras.timestamp, + duration = extras.duration, + errorName = extras.errorName, + ), + ) } } diff --git a/components/remote_settings/metrics.yaml b/components/remote_settings/metrics.yaml index 4875be4233..35e2644c01 100644 --- a/components/remote_settings/metrics.yaml +++ b/components/remote_settings/metrics.yaml @@ -36,6 +36,7 @@ remote_settings: type: string errorName: description: An optional string with the error name attribute in case of failure. + type: string trigger: description: > A label to distinguish what triggered the polling/fetching of remote content (eg. "broadcast", "timer", "forced", "manual") From f49369fa83e9b55260bcda555246324a9a7ac537 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 2 Apr 2026 16:53:17 +0200 Subject: [PATCH 19/20] glean does some casing magics --- .../java/mozilla/appservices/remotesettings/GleanTelemetry.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index 27d3e760bb..ad45840ed8 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -22,7 +22,7 @@ class GleanTelemetry : RemoteSettingsTelemetry { trigger = extras.trigger, timestamp = extras.timestamp, duration = extras.duration, - errorName = extras.errorName, + errorname = extras.errorName, ), ) } From b1242e75ae4e8dba3806a88ee25accaee54e919d Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 9 Apr 2026 16:06:20 +0200 Subject: [PATCH 20/20] Add debug logs for uptake --- .../appservices/remotesettings/GleanTelemetry.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt index ad45840ed8..c7437a4482 100644 --- a/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt +++ b/components/remote_settings/android/src/main/java/mozilla/appservices/remotesettings/GleanTelemetry.kt @@ -4,6 +4,7 @@ package mozilla.appservices.remotesettings +import android.util.Log import mozilla.appservices.remotesettings.RemoteSettingsTelemetry import mozilla.appservices.remotesettings.UptakeEventExtras import org.mozilla.appservices.remotesettings.GleanMetrics.RemoteSettings as RSMetrics @@ -14,6 +15,17 @@ import org.mozilla.appservices.remotesettings.GleanMetrics.RemoteSettings as RSM */ class GleanTelemetry : RemoteSettingsTelemetry { override fun reportUptake(extras: UptakeEventExtras) { + Log.d( + GleanTelemetry::javaClass.name, + "Remote Settings Telemetry Uptake called with: " + + "value=${extras.value}, " + + "source=${extras.source}, " + + "age=${extras.age}, " + + "trigger=${extras.trigger}, " + + "timestamp=${extras.timestamp}, " + + "duration=${extras.duration}, " + + "errorName=${extras.errorName}", + ) RSMetrics.uptakeRemotesettings.record( RSMetrics.UptakeRemotesettingsExtra( value = extras.value,