diff options
Diffstat (limited to 'third_party/rust/glean-core/src/metrics')
| -rw-r--r-- | third_party/rust/glean-core/src/metrics/mod.rs | 21 | ||||
| -rw-r--r-- | third_party/rust/glean-core/src/metrics/object.rs | 29 | ||||
| -rw-r--r-- | third_party/rust/glean-core/src/metrics/ping.rs | 74 | ||||
| -rw-r--r-- | third_party/rust/glean-core/src/metrics/remote_settings_config.rs (renamed from third_party/rust/glean-core/src/metrics/metrics_enabled_config.rs) | 16 |
4 files changed, 111 insertions, 29 deletions
diff --git a/third_party/rust/glean-core/src/metrics/mod.rs b/third_party/rust/glean-core/src/metrics/mod.rs index 92001efd2a..9234fff2d1 100644 --- a/third_party/rust/glean-core/src/metrics/mod.rs +++ b/third_party/rust/glean-core/src/metrics/mod.rs @@ -22,13 +22,13 @@ mod experiment; pub(crate) mod labeled; mod memory_distribution; mod memory_unit; -mod metrics_enabled_config; mod numerator; mod object; mod ping; mod quantity; mod rate; mod recorded_experiment; +mod remote_settings_config; mod string; mod string_list; mod text; @@ -72,7 +72,7 @@ pub use self::uuid::UuidMetric; pub use crate::histogram::HistogramType; pub use recorded_experiment::RecordedExperiment; -pub use self::metrics_enabled_config::MetricsEnabledConfig; +pub use self::remote_settings_config::RemoteSettingsConfig; /// A snapshot of all buckets and the accumulated sum of a distribution. // @@ -180,7 +180,7 @@ pub trait MetricType { // Technically nothing prevents multiple calls to should_record() to run in parallel, // meaning both are reading self.meta().disabled and later writing it. In between it can - // also read remote_settings_metrics_config, which also could be modified in between those 2 reads. + // also read remote_settings_config, which also could be modified in between those 2 reads. // This means we could write the wrong remote_settings_epoch | current_disabled value. All in all // at worst we would see that metric enabled/disabled wrongly once. // But since everything is tunneled through the dispatcher, this should never ever happen. @@ -200,11 +200,7 @@ pub trait MetricType { } // The epoch's didn't match so we need to look up the disabled flag // by the base_identifier from the in-memory HashMap - let metrics_enabled = &glean - .remote_settings_metrics_config - .lock() - .unwrap() - .metrics_enabled; + let remote_settings_config = &glean.remote_settings_config.lock().unwrap(); // Get the value from the remote configuration if it is there, otherwise return the default value. let current_disabled = { let base_id = self.meta().base_identifier(); @@ -215,8 +211,13 @@ pub trait MetricType { // NOTE: The `!` preceding the `*is_enabled` is important for inverting the logic since the // underlying property in the metrics.yaml is `disabled` and the outward API is treating it as // if it were `enabled` to make it easier to understand. - if let Some(is_enabled) = metrics_enabled.get(identifier) { - u8::from(!*is_enabled) + + if !remote_settings_config.metrics_enabled.is_empty() { + if let Some(is_enabled) = remote_settings_config.metrics_enabled.get(identifier) { + u8::from(!*is_enabled) + } else { + u8::from(self.meta().inner.disabled) + } } else { u8::from(self.meta().inner.disabled) } diff --git a/third_party/rust/glean-core/src/metrics/object.rs b/third_party/rust/glean-core/src/metrics/object.rs index 6071e2b33a..58c8a04c73 100644 --- a/third_party/rust/glean-core/src/metrics/object.rs +++ b/third_party/rust/glean-core/src/metrics/object.rs @@ -48,6 +48,10 @@ impl ObjectMetric { /// * `value` - the value to set. #[doc(hidden)] pub fn set_sync(&self, glean: &Glean, value: JsonValue) { + if !self.should_record(glean) { + return; + } + let value = Metric::Object(serde_json::to_string(&value).unwrap()); glean.storage().record(glean, &self.meta, &value) } @@ -65,6 +69,31 @@ impl ObjectMetric { crate::launch_with_glean(move |glean| metric.set_sync(glean, value)) } + /// Sets to the specified structure. + /// + /// Parses the passed JSON string. + /// If it can't be parsed into a valid object it records an invalid value error. + /// + /// Note: This does not check the structure. This needs to be done by the wrapper. + /// + /// # Arguments + /// + /// * `object` - JSON representation of the object to set. + pub fn set_string(&self, object: String) { + let metric = self.clone(); + crate::launch_with_glean(move |glean| { + let object = match serde_json::from_str(&object) { + Ok(object) => object, + Err(_) => { + let msg = "Value did not match predefined schema"; + record_error(glean, &metric.meta, ErrorType::InvalidValue, msg, None); + return; + } + }; + metric.set_sync(glean, object) + }) + } + /// Record an `InvalidValue` error for this metric. /// /// Only to be used by the RLB. diff --git a/third_party/rust/glean-core/src/metrics/ping.rs b/third_party/rust/glean-core/src/metrics/ping.rs index 5defab7a71..1c5b93c165 100644 --- a/third_party/rust/glean-core/src/metrics/ping.rs +++ b/third_party/rust/glean-core/src/metrics/ping.rs @@ -29,13 +29,12 @@ struct InnerPing { pub precise_timestamps: bool, /// Whether to include the {client|ping}_info sections on assembly. pub include_info_sections: bool, + /// Whether this ping is enabled. + pub enabled: bool, + /// Other pings that should be scheduled when this ping is sent. + pub schedules_pings: Vec<String>, /// The "reason" codes that this ping can send pub reason_codes: Vec<String>, - - /// Whether this ping is enabled. - /// Note: Data for disabled pings is still recorded. - /// It will not be cleared out on submit. - enabled: bool, } impl fmt::Debug for PingType { @@ -46,6 +45,8 @@ impl fmt::Debug for PingType { .field("send_if_empty", &self.0.send_if_empty) .field("precise_timestamps", &self.0.precise_timestamps) .field("include_info_sections", &self.0.include_info_sections) + .field("enabled", &self.0.enabled) + .field("schedules_pings", &self.0.schedules_pings) .field("reason_codes", &self.0.reason_codes) .finish() } @@ -64,13 +65,20 @@ impl PingType { /// * `name` - The name of the ping. /// * `include_client_id` - Whether to include the client ID in the assembled ping when submitting. /// * `send_if_empty` - Whether the ping should be sent empty or not. + /// * `precise_timestamps` - Whether the ping should use precise timestamps for the start and end time. + /// * `include_info_sections` - Whether the ping should include the client/ping_info sections. + /// * `enabled` - Whether or not this ping is enabled. Note: Data that would be sent on a disabled + /// ping will still be collected but is discarded rather than being submitted. /// * `reason_codes` - The valid reason codes for this ping. + #[allow(clippy::too_many_arguments)] pub fn new<A: Into<String>>( name: A, include_client_id: bool, send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: Vec<String>, reason_codes: Vec<String>, ) -> Self { Self::new_internal( @@ -79,19 +87,22 @@ impl PingType { send_if_empty, precise_timestamps, include_info_sections, + enabled, + schedules_pings, reason_codes, - true, ) } + #[allow(clippy::too_many_arguments)] pub(crate) fn new_internal<A: Into<String>>( name: A, include_client_id: bool, send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, - reason_codes: Vec<String>, enabled: bool, + schedules_pings: Vec<String>, + reason_codes: Vec<String>, ) -> Self { let this = Self(Arc::new(InnerPing { name: name.into(), @@ -99,8 +110,9 @@ impl PingType { send_if_empty, precise_timestamps, include_info_sections, - reason_codes, enabled, + schedules_pings, + reason_codes, })); // Register this ping. @@ -130,6 +142,22 @@ impl PingType { self.0.include_info_sections } + pub(crate) fn enabled(&self, glean: &Glean) -> bool { + let remote_settings_config = &glean.remote_settings_config.lock().unwrap(); + + if !remote_settings_config.pings_enabled.is_empty() { + if let Some(remote_enabled) = remote_settings_config.pings_enabled.get(self.name()) { + return *remote_enabled; + } + } + + self.0.enabled + } + + pub(crate) fn schedules_pings(&self) -> &[String] { + &self.0.schedules_pings + } + /// Submits the ping for eventual uploading. /// /// The ping content is assembled as soon as possible, but upload is not @@ -166,11 +194,6 @@ impl PingType { /// Whether the ping was succesfully assembled and queued. #[doc(hidden)] pub fn submit_sync(&self, glean: &Glean, reason: Option<&str>) -> bool { - if !self.0.enabled { - log::info!("Ping disabled: not submitting '{}' ping.", self.0.name); - return false; - } - if !glean.is_upload_enabled() { log::info!("Glean disabled: not submitting any pings."); return false; @@ -208,6 +231,15 @@ impl PingType { false } Some(ping) => { + if !self.enabled(glean) { + log::info!( + "The ping '{}' is disabled and will be discarded and not submitted", + ping.name + ); + + return false; + } + // 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 @@ -219,7 +251,10 @@ impl PingType { .add_sync(glean, 1); if let Err(e) = ping_maker.store_ping(glean.get_data_path(), &ping) { - log::warn!("IO error while writing ping to file: {}. Enqueuing upload of what we have in memory.", e); + log::warn!( + "IO error while writing ping to file: {}. Enqueuing upload of what we have in memory.", + e + ); glean.additional_metrics.io_errors.add_sync(glean, 1); // `serde_json::to_string` only fails if serialization of the content // fails or it contains maps with non-string keys. @@ -248,6 +283,17 @@ impl PingType { ping.name ); + if !ping.schedules_pings.is_empty() { + log::info!( + "The ping '{}' is being used to schedule other pings: {:?}", + ping.name, + ping.schedules_pings + ); + for scheduled_ping_name in &ping.schedules_pings { + glean.submit_ping_by_name(scheduled_ping_name, reason); + } + } + true } } diff --git a/third_party/rust/glean-core/src/metrics/metrics_enabled_config.rs b/third_party/rust/glean-core/src/metrics/remote_settings_config.rs index b36cbc150a..e7a560e81c 100644 --- a/third_party/rust/glean-core/src/metrics/metrics_enabled_config.rs +++ b/third_party/rust/glean-core/src/metrics/remote_settings_config.rs @@ -16,25 +16,31 @@ use serde::{Deserialize, Serialize}; /// } /// ``` #[derive(Serialize, Deserialize, Debug, Clone, Default)] -pub struct MetricsEnabledConfig { +pub struct RemoteSettingsConfig { /// This is a `HashMap` consisting of base_identifiers as keys /// and bool values representing an override for the `disabled` /// property of the metric, only inverted to reduce confusion. /// 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(flatten)] + #[serde(default)] pub metrics_enabled: HashMap<String, bool>, + + /// 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)] + pub pings_enabled: HashMap<String, bool>, } -impl MetricsEnabledConfig { - /// Creates a new MetricsEnabledConfig +impl RemoteSettingsConfig { + /// Creates a new RemoteSettingsConfig pub fn new() -> Self { Default::default() } } -impl TryFrom<String> for MetricsEnabledConfig { +impl TryFrom<String> for RemoteSettingsConfig { type Error = crate::ErrorKind; fn try_from(json: String) -> Result<Self, Self::Error> { |