Skip to content

Commit

Permalink
Don't record pings_submitted for custom pings
Browse files Browse the repository at this point in the history
  • Loading branch information
badboy committed Nov 25, 2024
1 parent ecbb871 commit 4445259
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
5 changes: 4 additions & 1 deletion glean-core/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -756,14 +756,17 @@ 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.
- On the metrics ping, the counts include the number of pings sent since
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
Expand Down
19 changes: 14 additions & 5 deletions glean-core/src/metrics/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
27 changes: 27 additions & 0 deletions glean-core/tests/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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!(
Expand All @@ -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!(
Expand All @@ -189,13 +198,19 @@ 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
// it resets each time on the baseline ping, so we should only ever get 1
// 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!(
Expand All @@ -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!(
Expand All @@ -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]
Expand Down

0 comments on commit 4445259

Please sign in to comment.