From 44452591d3451ce878e349e242f6748207b3138a Mon Sep 17 00:00:00 2001
From: Jan-Erik Rediger <jrediger@mozilla.com>
Date: Mon, 25 Nov 2024 12:36:33 +0100
Subject: [PATCH] Don't record `pings_submitted` for custom pings

---
 glean-core/metrics.yaml        |  5 ++++-
 glean-core/src/metrics/ping.rs | 19 ++++++++++++++-----
 glean-core/tests/ping.rs       | 27 +++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml
index e654b80bc2..008eb5d5a4 100644
--- a/glean-core/metrics.yaml
+++ b/glean-core/metrics.yaml
@@ -756,7 +756,7 @@ glean.validation:
   pings_submitted:
     type: labeled_counter
     description: |
-      A count of the pings submitted, by ping type.
+      A count of the built-in pings submitted, by ping type.
 
       This metric appears in both the metrics and baseline pings.
 
@@ -764,6 +764,9 @@ glean.validation:
         the last metrics ping (including the last metrics ping)
       - On the baseline ping, the counts include the number of pings send since
         the last baseline ping (including the last baseline ping)
+
+      Note: Previously this also recorded the number of submitted custom pings.
+      Now it only records counts for the Glean built-in pings.
     send_in_pings:
       - metrics
       - baseline
diff --git a/glean-core/src/metrics/ping.rs b/glean-core/src/metrics/ping.rs
index b4d1e95a31..55b5da35d5 100644
--- a/glean-core/src/metrics/ping.rs
+++ b/glean-core/src/metrics/ping.rs
@@ -240,15 +240,24 @@ impl PingType {
                     return false;
                 }
 
+                const BUILTIN_PINGS: [&str; 4] = [
+                    "baseline",
+                    "metrics",
+                    "events",
+                    "deletion-request"
+                ];
+
                 // This metric is recorded *after* the ping is collected (since
                 // that is the only way to know *if* it will be submitted). The
                 // implication of this is that the count for a metrics ping will
                 // be included in the *next* metrics ping.
-                glean
-                    .additional_metrics
-                    .pings_submitted
-                    .get(ping.name)
-                    .add_sync(glean, 1);
+                if BUILTIN_PINGS.contains(&ping.name) {
+                    glean
+                        .additional_metrics
+                        .pings_submitted
+                        .get(ping.name)
+                        .add_sync(glean, 1);
+                }
 
                 if let Err(e) = ping_maker.store_ping(glean.get_data_path(), &ping) {
                     log::warn!(
diff --git a/glean-core/tests/ping.rs b/glean-core/tests/ping.rs
index 467f2ba294..db0e782b1b 100644
--- a/glean-core/tests/ping.rs
+++ b/glean-core/tests/ping.rs
@@ -157,6 +157,9 @@ fn test_pings_submitted_metric() {
     let baseline_ping = PingType::new("baseline", true, false, true, true, true, vec![], vec![]);
     glean.register_ping_type(&baseline_ping);
 
+    let custom_ping = PingType::new("custom", true, true, true, true, true, vec![], vec![]);
+    glean.register_ping_type(&custom_ping);
+
     // We need to store a metric as an empty ping is not stored.
     let counter = CounterMetric::new(CommonMetricData {
         name: "counter".into(),
@@ -167,6 +170,8 @@ fn test_pings_submitted_metric() {
     counter.add_sync(&glean, 1);
 
     assert!(metrics_ping.submit_sync(&glean, None));
+    // A custom ping is just never recorded.
+    assert!(custom_ping.submit_sync(&glean, None));
 
     // Check recording in the metrics ping
     assert_eq!(
@@ -177,6 +182,10 @@ fn test_pings_submitted_metric() {
         None,
         pings_submitted.get("baseline").get_value(&glean, "metrics")
     );
+    assert_eq!(
+        None,
+        pings_submitted.get("custom").get_value(&glean, "metrics")
+    );
 
     // Check recording in the baseline ping
     assert_eq!(
@@ -189,6 +198,10 @@ fn test_pings_submitted_metric() {
             .get("baseline")
             .get_value(&glean, "baseline")
     );
+    assert_eq!(
+        None,
+        pings_submitted.get("custom").get_value(&glean, "baseline")
+    );
 
     // Trigger 2 baseline pings.
     // This should record a count of 2 baseline pings in the metrics ping, but
@@ -196,6 +209,8 @@ fn test_pings_submitted_metric() {
     // baseline ping recorded in the baseline ping itsef.
     assert!(baseline_ping.submit_sync(&glean, None));
     assert!(baseline_ping.submit_sync(&glean, None));
+    // A custom ping is just never recorded.
+    assert!(custom_ping.submit_sync(&glean, None));
 
     // Check recording in the metrics ping
     assert_eq!(
@@ -210,6 +225,12 @@ fn test_pings_submitted_metric() {
             .get("baseline")
             .get_value(&glean, Some("metrics"))
     );
+    assert_eq!(
+        None,
+        pings_submitted
+            .get("custom")
+            .get_value(&glean, Some("metrics"))
+    );
 
     // Check recording in the baseline ping
     assert_eq!(
@@ -224,6 +245,12 @@ fn test_pings_submitted_metric() {
             .get("baseline")
             .get_value(&glean, Some("baseline"))
     );
+    assert_eq!(
+        None,
+        pings_submitted
+            .get("custom")
+            .get_value(&glean, Some("baseline"))
+    );
 }
 
 #[test]