Bug 1790949 - Bump URL metric limit to 8k

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v51.2.0...main)
 
+* General
+  * Update URL metric character limit to 8k to support longer URLs. URLs that are too long now are truncated to `MAX_URL_LENGTH` and still recorded along with an Overflow error. ([#2199](https://github.com/mozilla/glean/pull/2199))
 * Kotlin
   * Gradle plugin: Fix quoting issue in Python wrapper code ([#2193](https://github.com/mozilla/glean/pull/2193))
   * Bumped the required Android NDK to version 25.1.8937393 ([#2195](https://github.com/mozilla/glean/pull/2195))

docs/user/reference/metrics/url.md

@@ -132,12 +132,12 @@ search.template.setUrl(new URL("https://mysearchengine.com/"));
 * [`invalid_value`](../../user/metrics/error-reporting.md):
   * If the URL passed does not start with a [scheme](https://url.spec.whatwg.org/#url-representation) followed by a `:` character.
   * If the URL passed uses the `data:` protocol.
-* [`invalid_overflow`](../../user/metrics/error-reporting.md): if the URL passed is longer than 2048 characters (before encoding).
+* [`invalid_overflow`](../../user/metrics/error-reporting.md): if the URL passed is longer than 8192 characters (before encoding).
 * [`invalid_type`](../../user/metrics/error-reporting.md): if a non-string value is given.
 
 #### Limits
 
-* Fixed maximum URL length: 2048. Longer URLs are dropped.
+* Fixed maximum URL length: 8192. Longer URLs are truncated and recorded along with an `invalid_overflow` error.
 
 ## Testing API
 

glean-core/android/src/test/java/mozilla/telemetry/glean/private/UrlMetricTypeTest.kt

@@ -117,8 +117,25 @@ class UrlMetricTypeTest {
             )
         )
 
-        urlMetric.set("glean://" + "testing".repeat(2000))
+        // Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the
+        // MAX_URL_LENGTH.
+        //
+        // This 8-character string was chosen so we could have an even number that is
+        // a divisor of our MAX_URL_LENGTH.
+        val longPathBase = "abcdefgh"
 
+        val testUrl = "glean://" + longPathBase.repeat(2000)
+
+        // Using 2000 creates a string > 16000 characters, well over MAX_URL_LENGTH.
+        urlMetric.set(testUrl)
+
+        // "glean://" is 8 characters
+        // "abcdefgh" (longPathBase) is 8 characters
+        // `longPathBase` is repeated 1023 times (8184)
+        // 8 + 8184 = 8192 (MAX_URL_LENGTH)
+        val expected = "glean://" + longPathBase.repeat(1023)
+
+        assertEquals(urlMetric.testGetValue("store1"), expected)
         assertEquals(1, urlMetric.testGetNumRecordedErrors(ErrorType.INVALID_OVERFLOW))
     }
 }

glean-core/ios/GleanTests/Metrics/UrlMetricTests.swift

@@ -91,9 +91,24 @@ class UrlMetricTypeTests: XCTestCase {
             disabled: false
         ))
 
-        let host = String(repeating: "testing", count: 2000)
-        urlMetric.set("glean://" + host)
-
+        // Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the
+        // MAX_URL_LENGTH.
+        //
+        // This 8-character string was chosen so we could have an even number that is
+        // a divisor of our MAX_URL_LENGTH.
+        let longPathBase = "abcdefgh"
+
+        // Using 2000 creates a string > 16000 characters, well over MAX_URL_LENGTH.
+        let testUrl = "glean://" + String(repeating: longPathBase, count: 2000)
+        urlMetric.set(testUrl)
+
+        // "glean://" is 8 characters
+        // "abcdefgh" (longPathBase) is 8 characters
+        // `longPathBase` is repeated 1023 times (8184)
+        // 8 + 8184 = 8192 (MAX_URL_LENGTH)
+        let expected = "glean://" + String(repeating: longPathBase, count: 1023)
+
+        XCTAssertEqual(expected, urlMetric.testGetValue())
         XCTAssertEqual(1, urlMetric.testGetNumRecordedErrors(.invalidOverflow))
     }
 

glean-core/python/tests/metrics/test_url.py

@@ -79,8 +79,24 @@ def test_setting_a_long_url_records_an_error():
         )
     )
 
-    url_metric.set("glean://" + "testing" * 2000)
-
+    # Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the
+    # MAX_URL_LENGTH.
+    #
+    # This 8-character string was chosen so we could have an even number that is
+    # a divisor of our MAX_URL_LENGTH.
+    long_path_base = "abcdefgh"
+
+    # Using 2000 creates a string > 16000 characters, well over MAX_URL_LENGTH.
+    test_url = "glean://" + (long_path_base * 2000)
+    url_metric.set(test_url)
+
+    # "glean://" is 8 characters
+    # "abcdefgh" (long_path_base) is 8 characters
+    # `long_path_base` is repeated 1023 times (8184)
+    # 8 + 8184 = 8192 (MAX_URL_LENGTH)
+    expected = "glean://" + (long_path_base * 1023)
+
+    assert expected == url_metric.test_get_value()
     assert 1 == url_metric.test_get_num_recorded_errors(
         testing.ErrorType.INVALID_OVERFLOW
     )

glean-core/src/metrics/url.rs

@@ -8,11 +8,12 @@ use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorTy
 use crate::metrics::Metric;
 use crate::metrics::MetricType;
 use crate::storage::StorageManager;
+use crate::util::truncate_string_at_boundary_with_error;
 use crate::CommonMetricData;
 use crate::Glean;
 
 // The maximum number of characters a URL Metric may have, before encoding.
-const MAX_URL_LENGTH: usize = 2048;
+const MAX_URL_LENGTH: usize = 8192;
 
 /// A URL metric.
 ///
@@ -80,16 +81,7 @@ impl UrlMetric {
             return;
         }
 
-        let s = value.into();
-        if s.len() > MAX_URL_LENGTH {
-            let msg = format!(
-                "Value length {} exceeds maximum of {}",
-                s.len(),
-                MAX_URL_LENGTH
-            );
-            record_error(glean, &self.meta, ErrorType::InvalidOverflow, msg, None);
-            return;
-        }
+        let s = truncate_string_at_boundary_with_error(glean, &self.meta, value, MAX_URL_LENGTH);
 
         if s.starts_with("data:") {
             record_error(
@@ -204,12 +196,24 @@ mod test {
             dynamic_label: None,
         });
 
-        let long_path = "testing".repeat(2000);
-        let test_url = format!("glean://{}", long_path);
+        // Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the
+        // MAX_URL_LENGTH.
+        //
+        // This 8-character string was chosen so we could have an even number that is
+        // a divisor of our MAX_URL_LENGTH.
+        let long_path_base = "abcdefgh";
+
+        // Using 2000 creates a string > 16000 characters, well over MAX_URL_LENGTH.
+        let test_url = format!("glean://{}", long_path_base.repeat(2000));
         metric.set_sync(&glean, test_url);
 
-        assert!(metric.get_value(&glean, "store1").is_none());
+        // "glean://" is 8 characters
+        // "abcdefgh" (long_path_base) is 8 characters
+        // `long_path_base` is repeated 1023 times (8184)
+        // 8 + 8184 = 8192 (MAX_URL_LENGTH)
+        let expected = format!("glean://{}", long_path_base.repeat(1023));
 
+        assert_eq!(metric.get_value(&glean, "store1").unwrap(), expected);
         assert_eq!(
             1,
             test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidOverflow)