From 43201a5d20c17d4793588b83d836b01d42fe9cfd Mon Sep 17 00:00:00 2001 From: ahanot Date: Wed, 21 Oct 2026 21:59:28 -0400 Subject: [PATCH 1/2] Bug 2047640 - Cut ads-client telemetry strings to Glean's 255-byte limit --- CHANGELOG.md | 1 + .../ads-client/src/telemetry/backend.rs | 33 ++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97ffb2bc2f..c7641124d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ ### Ads-Client - Telemetry is now a process-wide singleton internally; the binding API is unchanged, but the `MozAdsTelemetry` callback now receives metrics from every client in the process. +- Error strings passed to `MozAdsTelemetry` are cut to 255 bytes, Glean's string limit ([bug 2047640](https://bugzilla.mozilla.org/show_bug.cgi?id=2047640)). - `request_spoc_or_image_ads` is added to return ads that may ambiguously be spocs or images. - Added support for relevance scores ([AC-205](https://mozilla-hub.atlassian.net/browse/AC-205)) ([AC-206](https://mozilla-hub.atlassian.net/browse/AC-206)) Accepts a HashMap. diff --git a/components/ads-client/src/telemetry/backend.rs b/components/ads-client/src/telemetry/backend.rs index 655bfed660..1345f1a1a4 100644 --- a/components/ads-client/src/telemetry/backend.rs +++ b/components/ads-client/src/telemetry/backend.rs @@ -55,12 +55,28 @@ fn with_callback(f: impl FnOnce(&dyn Telemetry)) { f(callback.as_ref()); } +/// Glean drops the end of longer string values and records an +/// `invalid_overflow` error (bug 2047640). +const MAX_STRING_BYTES: usize = 255; + +/// Cuts `value` to fit in a Glean string, on a char boundary. +fn fit(mut value: String) -> String { + if value.len() > MAX_STRING_BYTES { + let mut end = MAX_STRING_BYTES; + while !value.is_char_boundary(end) { + end -= 1; + } + value.truncate(end); + } + value +} + pub(super) fn build_cache_error(label: &str, value: String) { - with_callback(|cb| cb.record_build_cache_error(label, value)); + with_callback(|cb| cb.record_build_cache_error(label, fit(value))); } pub(super) fn client_error(label: &str, value: String) { - with_callback(|cb| cb.record_client_error(label, value)); + with_callback(|cb| cb.record_client_error(label, fit(value))); } pub(super) fn client_operation_total(label: &str) { @@ -68,11 +84,11 @@ pub(super) fn client_operation_total(label: &str) { } pub(super) fn deserialization_error(label: &str, value: String) { - with_callback(|cb| cb.record_deserialization_error(label, value)); + with_callback(|cb| cb.record_deserialization_error(label, fit(value))); } pub(super) fn http_cache_outcome(label: &str, value: String) { - with_callback(|cb| cb.record_http_cache_outcome(label, value)); + with_callback(|cb| cb.record_http_cache_outcome(label, fit(value))); } /// Serializes tests that install a callback, since they all share the one @@ -102,6 +118,15 @@ mod tests { fn record_http_cache_outcome(&self, _label: &str, _value: String) {} } + #[test] + fn test_fit_cuts_on_a_char_boundary() { + assert_eq!(fit("short".into()), "short"); + assert_eq!(fit("a".repeat(300)).len(), MAX_STRING_BYTES); + let cut = fit("é".repeat(200)); + assert_eq!(cut.len(), 254); + assert!(cut.chars().all(|c| c == 'é')); + } + #[test] fn test_records_to_installed_callback() { let _lock = test_lock(); From 60e2d4ec43dfa33d6f56ab72312653d808bef054 Mon Sep 17 00:00:00 2001 From: ahanot Date: Wed, 21 Oct 2026 22:12:32 -0400 Subject: [PATCH 2/2] Use ByteSize for the Glean string limit --- components/ads-client/src/telemetry/backend.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/components/ads-client/src/telemetry/backend.rs b/components/ads-client/src/telemetry/backend.rs index 1345f1a1a4..388131489f 100644 --- a/components/ads-client/src/telemetry/backend.rs +++ b/components/ads-client/src/telemetry/backend.rs @@ -18,6 +18,8 @@ use std::sync::Arc; use parking_lot::RwLock; +use crate::bytesize::ByteSize; + /// Receives the ads client's metrics. pub trait Telemetry: Send + Sync { fn record_build_cache_error(&self, label: &str, value: String); @@ -57,12 +59,15 @@ fn with_callback(f: impl FnOnce(&dyn Telemetry)) { /// Glean drops the end of longer string values and records an /// `invalid_overflow` error (bug 2047640). -const MAX_STRING_BYTES: usize = 255; +const MAX_STRING_SIZE: ByteSize = ByteSize::b(255); /// Cuts `value` to fit in a Glean string, on a char boundary. fn fit(mut value: String) -> String { - if value.len() > MAX_STRING_BYTES { - let mut end = MAX_STRING_BYTES; + let max = MAX_STRING_SIZE.as_u64() as usize; + if value.len() > max { + // `str::floor_char_boundary` does this, but needs Rust 1.91 and + // mozilla-central still builds with 1.90. + let mut end = max; while !value.is_char_boundary(end) { end -= 1; } @@ -121,7 +126,7 @@ mod tests { #[test] fn test_fit_cuts_on_a_char_boundary() { assert_eq!(fit("short".into()), "short"); - assert_eq!(fit("a".repeat(300)).len(), MAX_STRING_BYTES); + assert_eq!(fit("a".repeat(300)).len() as u64, MAX_STRING_SIZE.as_u64()); let cut = fit("é".repeat(200)); assert_eq!(cut.len(), 254); assert!(cut.chars().all(|c| c == 'é'));