Bug 1650781: Use overflow rather than invalid for long strings (#1079)

* Bug 1650781: Use invalid_overflow rather than invalid_value for long strings

* Update tests

* Fix iOS test

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * General
   * Implement ping tagging (i.e. the `X-Source-Tags` header) ([#1074](https://github.com/mozilla/glean/pull/1074)). Note that this is not yet implemented for iOS.
+  * String values that are too long now record `invalid_overflow` rather than `invalid_value` through the Glean error reporting mechanism. This affects the string, event and string list metrics.
 * Python
   * The Python unit tests no longer send telemetry to the production telemetry endpoint.
 * Android

docs/user/metrics/event.md

@@ -137,7 +137,7 @@ TODO. To be implemented in [bug 1648422](https://bugzilla.mozilla.org/show_bug.c
 
 ## Recorded errors
 
-* `invalid_value`: if any of the values in the `extras` object are greater than 50 bytes in length.
+* `invalid_overflow`: if any of the values in the `extras` object are greater than 50 bytes in length.  (Prior to Glean TODO, this recorded an `invalid_value`).
  
 ## Reference
 

docs/user/metrics/string.md

@@ -183,7 +183,7 @@ Assert.Equal(
 
 ## Recorded errors
 
-* `invalid_value`: if the string is too long
+* `invalid_overflow`: if the string is too long. (Prior to Glean TODO, this recorded an `invalid_value`).
 
 ## Reference
 

docs/user/metrics/string_list.md

@@ -135,7 +135,7 @@ TODO. To be implemented in [bug 1648441](https://bugzilla.mozilla.org/show_bug.c
 
 ## Recorded errors
 
-* `invalid_value`: if the string is too long
+* `invalid_overflow`: if the string is too long. (Prior to Glean TODO, this recorded an `invalid_value`).
 
 * `invalid_value`: if the list is too long
 

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

@@ -396,6 +396,6 @@ class EventMetricTypeTest {
 
         click.record(extra = mapOf(clickKeys.objectId to longString))
 
-        assertEquals(1, click.testGetNumRecordedErrors(ErrorType.InvalidValue))
+        assertEquals(1, click.testGetNumRecordedErrors(ErrorType.InvalidOverflow))
     }
 }

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

@@ -120,6 +120,6 @@ class StringMetricTypeTest {
 
         stringMetric.set("0123456789".repeat(11))
 
-        assertEquals(1, stringMetric.testGetNumRecordedErrors(ErrorType.InvalidValue))
+        assertEquals(1, stringMetric.testGetNumRecordedErrors(ErrorType.InvalidOverflow))
     }
 }

glean-core/csharp/GleanTests/Metrics/StringMetricTypeTest.cs

@@ -120,7 +120,7 @@ namespace Mozilla.Glean.Tests.Metrics
 
             stringMetric.Set(new string('3', 110));
 
-            Assert.Equal(1, stringMetric.TestGetNumRecordedErrors(ErrorType.InvalidValue));
+            Assert.Equal(1, stringMetric.TestGetNumRecordedErrors(ErrorType.InvalidOverflow));
         }
     }
 }

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

@@ -302,6 +302,6 @@ class EventMetricTypeTests: XCTestCase {
         )
 
         metric.record(extra: [.testName: String(repeating: "0123456789", count: 11)])
-        XCTAssertEqual(1, metric.testGetNumRecordedErrors(.invalidValue))
+        XCTAssertEqual(1, metric.testGetNumRecordedErrors(.invalidOverflow))
     }
 }

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

@@ -94,6 +94,6 @@ class StringMetricTests: XCTestCase {
 
         stringMetric.set(String(repeating: "0123456789", count: 11))
 
-        XCTAssertEqual(1, stringMetric.testGetNumRecordedErrors(.invalidValue))
+        XCTAssertEqual(1, stringMetric.testGetNumRecordedErrors(.invalidOverflow))
     }
 }

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

@@ -297,7 +297,7 @@ def test_long_extra_values_record_an_error():
 
     click.record(extra={ClickKeys.OBJECT_ID: long_string})
 
-    assert 1 == click.test_get_num_recorded_errors(testing.ErrorType.INVALID_VALUE)
+    assert 1 == click.test_get_num_recorded_errors(testing.ErrorType.INVALID_OVERFLOW)
 
 
 def test_event_enum_is_generated_correctly():

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

@@ -74,7 +74,7 @@ def test_setting_a_long_string_records_an_error():
     string_metric.set("0123456789" * 11)
 
     assert 1 == string_metric.test_get_num_recorded_errors(
-        testing.ErrorType.INVALID_VALUE
+        testing.ErrorType.INVALID_OVERFLOW
     )
 
 

glean-core/src/util.rs

@@ -104,7 +104,7 @@ pub(crate) fn truncate_string_at_boundary_with_error<S: Into<String>>(
     let s = value.into();
     if s.len() > length {
         let msg = format!("Value length {} exceeds maximum of {}", s.len(), length);
-        record_error(glean, meta, ErrorType::InvalidValue, msg, None);
+        record_error(glean, meta, ErrorType::InvalidOverflow, msg, None);
         truncate_string_at_boundary(s, length)
     } else {
         s

glean-core/tests/string.rs

@@ -116,6 +116,6 @@ fn long_string_values_are_truncated() {
     // Make sure that the errors have been recorded
     assert_eq!(
         Ok(1),
-        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None)
+        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidOverflow, None)
     );
 }

glean-core/tests/string_list.rs

@@ -128,7 +128,7 @@ fn long_string_values_are_truncated() {
     // Ensure the error has been recorded.
     assert_eq!(
         Ok(1),
-        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None)
+        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidOverflow, None)
     );
 
     metric.set(&glean, vec![test_string.clone()]);
@@ -142,7 +142,7 @@ fn long_string_values_are_truncated() {
     // Ensure the error has been recorded.
     assert_eq!(
         Ok(2),
-        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None)
+        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidOverflow, None)
     );
 }