diff --git a/CHANGELOG.md b/CHANGELOG.md index 507e39b201..8f07f3f509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * General * Updated to `glean_parser` v19.0.0 ([#3421](https://github.com/mozilla/glean/issues/3421)) + * Add server knobs config to `ping_info` ([#3396](https://github.com/mozilla/glean/pull/3396)) # v67.1.0 (2026-03-12) diff --git a/docs/user/_includes/server-knobs-config-in-pings.md b/docs/user/_includes/server-knobs-config-in-pings.md new file mode 100644 index 0000000000..cb3230cc06 --- /dev/null +++ b/docs/user/_includes/server-knobs-config-in-pings.md @@ -0,0 +1,33 @@ +## Server Knobs Configuration in Pings + +When Server Knobs configuration is applied through `applyServerKnobsConfig`, the entire configuration is automatically recorded as an `ObjectMetric` and included in the `ping_info` section of all pings except for those with `metadata.include_info_sections: false`. + +This makes it easier to identify which metrics are being controlled by Server Knobs and to calculate effective sampling rates in analysis. + +The configuration is stored using a standard `ObjectMetric` (at `glean.internal.metrics.server_knobs_config`), which provides schema definition support for downstream tooling and requires minimal changes to ingestion pipeline schemas. + +### How It Appears in Pings + +The complete Server Knobs configuration is included in `ping_info.server_knobs_config`: + +```json +{ + "ping_info": { + "seq": 123, + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T01:00:00Z", + "server_knobs_config": { + "metrics_enabled": { + "urlbar.engagement": true, + "urlbar.impression": true + } + } + }, + "metrics": { + "counter": { + "urlbar.engagement": 5, + "urlbar.impression": 2 + } + } +} +``` diff --git a/docs/user/user/server-knobs/metrics/experimenter-configuration.md b/docs/user/user/server-knobs/metrics/experimenter-configuration.md index 798db46ec7..b6451054d2 100644 --- a/docs/user/user/server-knobs/metrics/experimenter-configuration.md +++ b/docs/user/user/server-knobs/metrics/experimenter-configuration.md @@ -21,3 +21,5 @@ This configuration would be what is entered into the branch configuration setup } } ``` + +{{#include ../../../_includes/server-knobs-config-in-pings.md}} diff --git a/docs/user/user/server-knobs/other/max-events.md b/docs/user/user/server-knobs/other/max-events.md index 9d4b043ed3..7dff480571 100644 --- a/docs/user/user/server-knobs/other/max-events.md +++ b/docs/user/user/server-knobs/other/max-events.md @@ -14,3 +14,5 @@ For instance, if you wanted to disable batching in order to transmit an events p } } ``` + +{{#include ../../../_includes/server-knobs-config-in-pings.md}} diff --git a/docs/user/user/server-knobs/pings/experimenter-configuration.md b/docs/user/user/server-knobs/pings/experimenter-configuration.md index 71591cdd9a..cae178667d 100644 --- a/docs/user/user/server-knobs/pings/experimenter-configuration.md +++ b/docs/user/user/server-knobs/pings/experimenter-configuration.md @@ -19,3 +19,5 @@ This configuration would be what is entered into the branch configuration setup } } ``` + +{{#include ../../../_includes/server-knobs-config-in-pings.md}} diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index 69785b2e5d..0f4b8aac5a 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -398,6 +398,54 @@ glean.internal.metrics: - glean-team@mozilla.com expires: never + server_knobs_config: + type: object + lifetime: application + send_in_pings: + - glean_internal_info + description: | + The Server Knobs configuration applied via applyServerKnobsConfig. + This contains the complete configuration including metrics_enabled, + pings_enabled, and event_threshold settings that control metric + recording behavior through remote configuration. + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2007406 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2007406 + data_sensitivity: + - technical + notification_emails: + - glean-team@mozilla.com + expires: never + structure: + type: object + properties: + metrics_enabled: + type: object + description: | + Map of metric identifiers (category.name) to boolean values + indicating whether the metric is enabled. + properties: + key: + type: string + value: + type: boolean + pings_enabled: + type: object + description: | + Map of ping names to boolean values indicating whether + the ping is enabled. + properties: + key: + type: string + value: + type: boolean + event_threshold: + type: number + description: | + Optional threshold for event buffering before an events + ping is collected and submitted. Can be null if not set. + glean.internal.metrics.attribution: source: diff --git a/glean-core/src/core/mod.rs b/glean-core/src/core/mod.rs index 1f50f50e48..e38798ffb4 100644 --- a/glean-core/src/core/mod.rs +++ b/glean-core/src/core/mod.rs @@ -1120,21 +1120,32 @@ impl Glean { /// /// * `cfg` - The stringified JSON representation of a `RemoteSettingsConfig` object pub fn apply_server_knobs_config(&self, cfg: RemoteSettingsConfig) { - // Set the current RemoteSettingsConfig, keeping the lock until the epoch is - // updated to prevent against reading a "new" config but an "old" epoch - let mut remote_settings_config = self.remote_settings_config.lock().unwrap(); - - // Merge the exising metrics configuration with the supplied one - remote_settings_config - .metrics_enabled - .extend(cfg.metrics_enabled); - - // Merge the exising ping configuration with the supplied one - remote_settings_config - .pings_enabled - .extend(cfg.pings_enabled); + let config_value = { + // Hold the lock while merging config and serializing, then release + // before performing IO in set_sync. + let mut remote_settings_config = self.remote_settings_config.lock().unwrap(); + + // Merge the exising metrics configuration with the supplied one + remote_settings_config + .metrics_enabled + .extend(cfg.metrics_enabled); + + // Merge the exising ping configuration with the supplied one + remote_settings_config + .pings_enabled + .extend(cfg.pings_enabled); + + remote_settings_config.event_threshold = cfg.event_threshold; + + // Store the Server Knobs configuration as an ObjectMetric + // Since RemoteSettingsConfig only contains maps with string keys and primitives, + // serialization via the derived Serialize impl cannot fail so it is safe to unwrap. + serde_json::to_value(&*remote_settings_config).unwrap() + }; - remote_settings_config.event_threshold = cfg.event_threshold; + self.additional_metrics + .server_knobs_config + .set_sync(self, config_value); // Update remote_settings epoch self.remote_settings_epoch.fetch_add(1, Ordering::SeqCst); diff --git a/glean-core/src/internal_metrics.rs b/glean-core/src/internal_metrics.rs index eef6476acf..fd46c642b1 100644 --- a/glean-core/src/internal_metrics.rs +++ b/glean-core/src/internal_metrics.rs @@ -44,6 +44,9 @@ pub struct AdditionalMetrics { /// The number of times we had to clamp an event timestamp /// for exceeding the range of a signed 64-bit integer (9223372036854775807). pub event_timestamp_clamped: CounterMetric, + + /// Server knobs configuration received from remote settings. + pub server_knobs_config: ObjectMetric, } impl CoreMetrics { @@ -211,6 +214,15 @@ impl AdditionalMetrics { disabled: false, dynamic_label: None, }), + + server_knobs_config: ObjectMetric::new(CommonMetricData { + name: "server_knobs_config".into(), + category: "glean.internal.metrics".into(), + send_in_pings: vec!["glean_internal_info".into()], + lifetime: Lifetime::Application, + disabled: false, + dynamic_label: None, + }), } } } diff --git a/glean-core/src/metrics/remote_settings_config.rs b/glean-core/src/metrics/remote_settings_config.rs index ae0fbca8cc..1c7c4c5c09 100644 --- a/glean-core/src/metrics/remote_settings_config.rs +++ b/glean-core/src/metrics/remote_settings_config.rs @@ -26,19 +26,19 @@ pub struct RemoteSettingsConfig { /// If a particular metric has a value of `true` here, it means /// the default of the metric will be overriden and set to the /// enabled state. - #[serde(default)] + #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub metrics_enabled: HashMap, /// This is a `HashMap` consisting of ping names as keys and /// boolean values representing on override for the default /// enabled state of the ping of the same name. - #[serde(default)] + #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub pings_enabled: HashMap, /// The threshold of events that will be buffered before an events ping is /// collected and submitted. /// It overrides the value configured at initialization time. - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub event_threshold: Option, } diff --git a/glean-core/src/ping/mod.rs b/glean-core/src/ping/mod.rs index f5d3725476..ce21b26e0d 100644 --- a/glean-core/src/ping/mod.rs +++ b/glean-core/src/ping/mod.rs @@ -162,6 +162,20 @@ impl PingMaker { .insert("experiments".to_string(), experiment_data); }; + // Get the Server Knobs configuration, if available. + if let Some(config_json) = glean + .additional_metrics + .server_knobs_config + .get_value(glean, INTERNAL_STORAGE) + { + // Object metrics always hold a string produced by serde_json::to_string, + // so deserializing it back into a JsonValue cannot fail. + let server_knobs_config = serde_json::from_str(&config_json).unwrap(); + map.as_object_mut() + .unwrap() // safe unwrap, we created the object above + .insert("server_knobs_config".to_string(), server_knobs_config); + } + map } @@ -533,4 +547,104 @@ mod test { assert_eq!(0, ping_maker.get_ping_seq(&glean, "store1")); assert_eq!(1, ping_maker.get_ping_seq(&glean, "store1")); } + + #[test] + fn test_server_knobs_config_appears_in_ping_info() { + use crate::metrics::RemoteSettingsConfig; + use std::collections::HashMap; + + let (glean, _t) = new_glean(None); + + // Apply complete Server Knobs config with all three fields + let mut metrics_enabled = HashMap::new(); + metrics_enabled.insert("test.counter".to_string(), true); + + let mut pings_enabled = HashMap::new(); + pings_enabled.insert("custom".to_string(), false); + + let config = RemoteSettingsConfig { + metrics_enabled, + pings_enabled, + event_threshold: Some(41), + }; + glean.apply_server_knobs_config(config); + + // Verify complete config structure appears in ping_info + let ping_maker = PingMaker::new(); + let ping_info = ping_maker.get_ping_info(&glean, "store1", None, TimeUnit::Minute); + + let server_knobs = &ping_info["server_knobs_config"]; + assert_eq!(server_knobs["metrics_enabled"]["test.counter"], true); + assert_eq!(server_knobs["pings_enabled"]["custom"], false); + assert_eq!(server_knobs["event_threshold"], 41); + } + + #[test] + fn test_server_knobs_not_included_when_no_config() { + let (glean, _t) = new_glean(None); + + let ping_maker = PingMaker::new(); + let ping_info = ping_maker.get_ping_info(&glean, "store1", None, TimeUnit::Minute); + + assert!(ping_info.get("server_knobs_config").is_none()); + } + + #[test] + fn test_server_knobs_appears_in_all_pings() { + use crate::metrics::RemoteSettingsConfig; + use std::collections::HashMap; + + let (glean, _t) = new_glean(None); + + let mut metrics_enabled = HashMap::new(); + metrics_enabled.insert("test.counter".to_string(), true); + + let config = RemoteSettingsConfig { + metrics_enabled, + ..Default::default() + }; + glean.apply_server_knobs_config(config); + + // Verify config appears in multiple different pings + let ping_maker = PingMaker::new(); + let ping_info1 = ping_maker.get_ping_info(&glean, "store1", None, TimeUnit::Minute); + let ping_info2 = ping_maker.get_ping_info(&glean, "store2", None, TimeUnit::Minute); + + assert_eq!( + ping_info1["server_knobs_config"]["metrics_enabled"]["test.counter"], + true + ); + assert_eq!( + ping_info2["server_knobs_config"]["metrics_enabled"]["test.counter"], + true + ); + } + + #[test] + fn test_server_knobs_config_omits_empty_fields() { + use crate::metrics::RemoteSettingsConfig; + use std::collections::HashMap; + + let (glean, _t) = new_glean(None); + + // Apply config with only metrics_enabled set; pings_enabled and event_threshold are empty/None + let mut metrics_enabled = HashMap::new(); + metrics_enabled.insert("test.counter".to_string(), true); + + let config = RemoteSettingsConfig { + metrics_enabled, + ..Default::default() + }; + glean.apply_server_knobs_config(config); + + let ping_maker = PingMaker::new(); + let ping_info = ping_maker.get_ping_info(&glean, "store1", None, TimeUnit::Minute); + + let server_knobs = &ping_info["server_knobs_config"]; + // metrics_enabled should be present + assert_eq!(server_knobs["metrics_enabled"]["test.counter"], true); + // pings_enabled and event_threshold should be absent (not empty object / null) + assert!(server_knobs.get("pings_enabled").is_none()); + assert!(server_knobs.get("event_threshold").is_none()); + } }