Bug 1597761 - Don't create PingRequest if body exceeds 1MB

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v31.6.0...main)
 
+* General
+  * Limit ping request body size to 1MB. ([#1098](https://github.com/mozilla/glean/pull/1098))
+* iOS
+  * Implement ping tagging (i.e. the `X-Source-Tags` header) through custom URL ([#1100](https://github.com/mozilla/glean/pull/1100)).
 * C#
   * Add support for Labeled Strings and Labeled Booleans.
   * Add support for the Counter metric type and Labeled Counter.

docs/user/collected-metrics/metrics.md

@@ -22,7 +22,7 @@ The following metrics are added to the ping:
 | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
 | --- | --- | --- | --- | --- | --- | --- |
 | glean.error.invalid_label |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set with an invalid label. The labels are the `category.name` identifier of the metric.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1499761#c5)||never |1 |
-| glean.error.invalid_overflow |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set a value that overflew. The labels are the `category.name` identifier of the metric.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1591912#c3)||never |1 |
+| glean.error.invalid_overflow |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set a value that overflowed. The labels are the `category.name` identifier of the metric.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1591912#c3)||never |1 |
 | glean.error.invalid_state |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a timing metric was used incorrectly. The labels are the `category.name` identifier of the metric.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1499761#c5)||never |1 |
 | glean.error.invalid_value |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set to an invalid value. The labels are the `category.name` identifier of the metric.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1499761#c5)||never |1 |
 
@@ -129,6 +129,7 @@ The following metrics are added to the ping:
 | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
 | --- | --- | --- | --- | --- | --- | --- |
 | glean.error.preinit_tasks_overflow |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |The number of tasks queued in the pre-initialization buffer. Only sent if the buffer overflows.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1609482#c3)||never |1 |
+| glean.upload.discarded_exceeding_pings_size |[memory_distribution](https://mozilla.github.io/glean/book/user/metrics/memory_distribution.html) |The size of pings that exceeded the maximum ping size allowed for upload.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1597761#c10)||never |1 |
 | glean.upload.ping_upload_failure |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of ping upload failures, by type of failure. This includes failures for all ping types, though the counts appear in the next successfully sent `metrics` ping.  |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1589124#c1)|<ul><li>status_code_4xx</li><li>status_code_5xx</li><li>status_code_unknown</li><li>unrecoverable</li><li>recoverable</li></ul>|never |1 |
 
 

docs/user/pings/index.md

@@ -80,8 +80,6 @@ See [Using the Experiments API](../experiments-api.md) on how to record experime
 
 The pings that the Glean SDK generates are submitted to the Mozilla servers at specific paths, in order to provide additional metadata without the need to unpack the ping payload.
 
-> **Note**: To keep resource usage in check, the Glean SDK allows only up to 10 ping submissions every 60 seconds. There are no exposed methods to change these rate limiting defaults yet, follow [Bug 1647630](https://bugzilla.mozilla.org/show_bug.cgi?id=1647630) for updates.
-
 A typical submission URL looks like
 
   `"<server-address>/submit/<application-id>/<doc-type>/<glean-schema-version>/<document-id>"`
@@ -94,6 +92,13 @@ where:
 - `<glean-schema-version>`: the version of the Glean ping schema;
 - `<document-id>`: a unique identifier for this ping.
 
+### Limitations
+
+To keep resource usage in check, the Glean SDK enforces some limitations on the pings it will upload and in the amount of uploads allowed per a given time interval.
+
+- **Rate limiting**: only up to 10 ping submissions every 60 seconds are allowed. There are no exposed methods to change these rate limiting defaults yet, follow [Bug 1647630](https://bugzilla.mozilla.org/show_bug.cgi?id=1647630) for updates.
+- **Request body size limiting**: the body of a ping request may have up to 1MB. Pings that exceed this size are discarded and don't get uploaded. Size and number of discarded pings are recorded on the internal Glean metric [`glean.upload.discarded_exceeding_pings_size`](../collected-metrics/metrics.md#metrics-1).
+
 ### Submitted headers
 A pre-defined set of headers is additionally sent along with the submitted ping:
 

glean-core/metrics.yaml

@@ -374,3 +374,20 @@ glean.upload:
     expires: never
     no_lint:
       - COMMON_PREFIX
+
+  discarded_exceeding_pings_size:
+    type: memory_distribution
+    description: >
+      The size of pings that exceeded the maximum ping size allowed for upload.
+    memory_unit: kilobyte
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1597761
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1597761#c10
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    no_lint:
+      - COMMON_PREFIX

glean-core/src/error.rs

@@ -60,6 +60,9 @@ pub enum ErrorKind {
 
     /// Glean not initialized
     NotInitialized,
+
+    /// Ping request body size overflowed
+    PingBodyOverflow(usize),
 }
 
 /// A specialized [`Error`] type for this crate's operations.
@@ -86,14 +89,19 @@ impl Error {
             kind: ErrorKind::NotInitialized,
         }
     }
+
+    /// Returns the kind of the current error instance.
+    pub fn kind(&self) -> &ErrorKind {
+        &self.kind
+    }
 }
 
 impl std::error::Error for Error {}
 
 impl Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use ErrorKind::*;
-        match &self.kind {
+        match self.kind() {
             Lifetime(l) => write!(f, "Lifetime conversion from {} failed", l),
             Handle(e) => write!(f, "Invalid handle: {}", e),
             IoError(e) => write!(f, "An I/O error occurred: {}", e),
@@ -106,6 +114,11 @@ impl Display for Error {
             Utf8Error => write!(f, "Invalid UTF-8 byte sequence in string"),
             InvalidConfig => write!(f, "Invalid Glean configuration provided"),
             NotInitialized => write!(f, "Global Glean object missing"),
+            PingBodyOverflow(s) => write!(
+                f,
+                "Ping request body size exceeded maximum size allowed: {}kB.",
+                s / 1024
+            ),
         }
     }
 }

glean-core/src/internal_metrics.rs

@@ -9,7 +9,6 @@ pub struct CoreMetrics {
     pub client_id: UuidMetric,
     pub first_run_date: DatetimeMetric,
     pub os: StringMetric,
-    pub ping_upload_failure: LabeledMetric<CounterMetric>,
 }
 
 impl CoreMetrics {
@@ -44,7 +43,19 @@ impl CoreMetrics {
                 disabled: false,
                 dynamic_label: None,
             }),
+        }
+    }
+}
 
+#[derive(Debug)]
+pub struct UploadMetrics {
+    pub ping_upload_failure: LabeledMetric<CounterMetric>,
+    pub discarded_exceeding_pings_size: MemoryDistributionMetric,
+}
+
+impl UploadMetrics {
+    pub fn new() -> UploadMetrics {
+        UploadMetrics {
             ping_upload_failure: LabeledMetric::new(
                 CounterMetric::new(CommonMetricData {
                     name: "ping_upload_failure".into(),
@@ -62,6 +73,18 @@ impl CoreMetrics {
                     "recoverable".into(),
                 ]),
             ),
+
+            discarded_exceeding_pings_size: MemoryDistributionMetric::new(
+                CommonMetricData {
+                    name: "discarded_exceeding_ping_size".into(),
+                    category: "glean.upload".into(),
+                    send_in_pings: vec!["metrics".into()],
+                    lifetime: Lifetime::Ping,
+                    disabled: false,
+                    dynamic_label: None,
+                },
+                MemoryUnit::Kilobyte,
+            ),
         }
     }
 }

glean-core/src/lib.rs

@@ -496,7 +496,7 @@ impl Glean {
     ///
     /// `PingUploadTask` - an enum representing the possible tasks.
     pub fn get_upload_task(&self) -> PingUploadTask {
-        self.upload_manager.get_upload_task(self.log_pings())
+        self.upload_manager.get_upload_task(self, self.log_pings())
     }
 
     /// Processes the response from an attempt to upload a ping.
@@ -506,13 +506,8 @@ impl Glean {
     /// * `uuid` - The UUID of the ping in question.
     /// * `status` - The upload result.
     pub fn process_ping_upload_response(&self, uuid: &str, status: UploadResult) {
-        if let Some(label) = status.get_label() {
-            let metric = self.core_metrics.ping_upload_failure.get(label);
-            metric.add(self, 1);
-        }
-
         self.upload_manager
-            .process_ping_upload_response(uuid, status);
+            .process_ping_upload_response(self, uuid, status);
     }
 
     /// Take a snapshot for the given store and optionally clear it.
@@ -586,7 +581,7 @@ impl Glean {
                     return Err(e.into());
                 }
 
-                self.upload_manager.enqueue_ping_from_file(&doc_id);
+                self.upload_manager.enqueue_ping_from_file(self, &doc_id);
 
                 log::info!(
                     "The ping '{}' was submitted and will be sent as soon as possible",

glean-core/src/upload/directory.rs

@@ -17,7 +17,7 @@ use crate::{DELETION_REQUEST_PINGS_DIRECTORY, PENDING_PINGS_DIRECTORY};
 
 /// A representation of the data extracted from a ping file,
 /// this will contain the document_id, path, JSON encoded body of a ping and the persisted headers.
-type PingPayload = (String, String, String, Option<HeaderMap>);
+pub type PingPayload = (String, String, String, Option<HeaderMap>);
 
 /// Get the file name from a path as a &str.
 ///