From 50c4dbc3e8d2663a5ccc92652de9b599b3434bb7 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Tue, 31 Mar 2020 14:48:34 +0200 Subject: [PATCH] Bug 1625916 - Detect the operating system at compile time and set the metric --- .../java/mozilla/telemetry/glean/Glean.kt | 1 - glean-core/ios/Glean/Glean.swift | 1 - glean-core/ios/Glean/Utils/Sysctl.swift | 3 - glean-core/metrics.yaml | 3 + glean-core/preview/src/core_metrics.rs | 9 --- glean-core/preview/src/lib.rs | 1 - glean-core/preview/src/system.rs | 66 +++++++-------- glean-core/preview/src/test.rs | 6 +- glean-core/python/glean/glean.py | 1 - glean-core/src/internal_metrics.rs | 10 +++ glean-core/src/lib.rs | 11 +++ glean-core/src/system.rs | 81 +++++++++++++++++++ 12 files changed, 136 insertions(+), 57 deletions(-) create mode 100644 glean-core/src/system.rs diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt index ee4f54d21..7e6c32b0a 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt @@ -433,7 +433,6 @@ open class GleanInternalAPI internal constructor () { // that they are guaranteed to be available with the first ping that is // generated. We use an internal only API to do that. GleanBaseline.locale.setSync(getLocaleTag()) - GleanInternalMetrics.os.setSync("Android") // https://developer.android.com/reference/android/os/Build.VERSION GleanInternalMetrics.androidSdkVersion.setSync(Build.VERSION.SDK_INT.toString()) GleanInternalMetrics.osVersion.setSync(Build.VERSION.RELEASE) diff --git a/glean-core/ios/Glean/Glean.swift b/glean-core/ios/Glean/Glean.swift index daf7fa3d3..b96690506 100644 --- a/glean-core/ios/Glean/Glean.swift +++ b/glean-core/ios/Glean/Glean.swift @@ -167,7 +167,6 @@ public class Glean { // generated. We use an internal only API to do that. GleanBaseline.locale.setSync(getLocaleTag()) - GleanInternalMetrics.os.setSync(Sysctl.os) GleanInternalMetrics.osVersion.setSync(UIDevice.current.systemVersion) GleanInternalMetrics.deviceManufacturer.setSync(Sysctl.manufacturer) GleanInternalMetrics.deviceModel.setSync(Sysctl.model) diff --git a/glean-core/ios/Glean/Utils/Sysctl.swift b/glean-core/ios/Glean/Utils/Sysctl.swift index 0f1272e63..19df3f422 100644 --- a/glean-core/ios/Glean/Utils/Sysctl.swift +++ b/glean-core/ios/Glean/Utils/Sysctl.swift @@ -144,9 +144,6 @@ struct Sysctl { return try string(for: keys(for: name)) } - /// Always the same on iOS - public static var os: String = "iOS" - /// Always the same on Apple hardware public static var manufacturer: String = "Apple" diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index 470516f23..20b34ec71 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -51,6 +51,9 @@ glean.internal.metrics: - glean_client_info description: | The name of the operating system. + Possible values: + Android, iOS, Linux, Darwin, Windows, + FreeBSD, NetBSD, OpenBSD, Solaris, unknown bugs: - https://bugzilla.mozilla.org/1497894 data_reviews: diff --git a/glean-core/preview/src/core_metrics.rs b/glean-core/preview/src/core_metrics.rs index 2215e87e1..973780c0f 100644 --- a/glean-core/preview/src/core_metrics.rs +++ b/glean-core/preview/src/core_metrics.rs @@ -28,7 +28,6 @@ pub struct InternalMetrics { pub app_build: StringMetric, pub app_display_version: StringMetric, pub app_channel: StringMetric, - pub os: StringMetric, pub os_version: StringMetric, pub architecture: StringMetric, pub device_manufacturer: StringMetric, @@ -62,14 +61,6 @@ impl InternalMetrics { disabled: false, dynamic_label: None, }), - os: StringMetric::new(CommonMetricData { - name: "os".into(), - category: "".into(), - send_in_pings: vec!["glean_client_info".into()], - lifetime: Lifetime::Application, - disabled: false, - dynamic_label: None, - }), os_version: StringMetric::new(CommonMetricData { name: "os_version".into(), category: "".into(), diff --git a/glean-core/preview/src/lib.rs b/glean-core/preview/src/lib.rs index 07e27075a..ac9120659 100644 --- a/glean-core/preview/src/lib.rs +++ b/glean-core/preview/src/lib.rs @@ -140,7 +140,6 @@ fn initialize_core_metrics( if let Some(app_channel) = channel { core_metrics.app_channel.set(glean, app_channel); } - core_metrics.os.set(glean, system::OS.to_string()); core_metrics.os_version.set(glean, "unknown".to_string()); core_metrics .architecture diff --git a/glean-core/preview/src/system.rs b/glean-core/preview/src/system.rs index 468cc9887..5bb7d3c34 100644 --- a/glean-core/preview/src/system.rs +++ b/glean-core/preview/src/system.rs @@ -1,43 +1,33 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -// Detect and expose `target_os` as a constant. -// Whether this is a good idea is somewhat debatable. +// Copyright (c) 2017 The Rust Project Developers +// Licensed under the MIT License. +// Original license: +// https://github.com/RustSec/platforms-crate/blob/ebbd3403243067ba3096f31684557285e352b639/LICENSE-MIT // -// Code adopted from the "platforms" crate: . +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. -#[cfg(target_os = "android")] -/// `target_os` when building this crate: `android` -pub const OS: &str = "Android"; - -#[cfg(target_os = "ios")] -/// `target_os` when building this crate: `ios` -pub const OS: &str = "iOS"; - -#[cfg(target_os = "linux")] -/// `target_os` when building this crate: `linux` -pub const OS: &str = "Linux"; - -#[cfg(target_os = "macos")] -/// `target_os` when building this crate: `macos` -pub const OS: &str = "MacOS"; - -#[cfg(target_os = "windows")] -/// `target_os` when building this crate: `windows` -pub const OS: &str = "Windows"; - -#[cfg(not(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos", - target_os = "windows", -)))] -pub const OS: &str = "unknown"; - -// Detect and expose `target_arch` as a constant -// Whether this is a good idea is somewhat debatable +//! Detect and expose `target_arch` as a constant #[cfg(target_arch = "aarch64")] /// `target_arch` when building this crate: `aarch64` diff --git a/glean-core/preview/src/test.rs b/glean-core/preview/src/test.rs index bf1095021..2ef3dc4ab 100644 --- a/glean-core/preview/src/test.rs +++ b/glean-core/preview/src/test.rs @@ -66,7 +66,7 @@ fn client_info_reset_after_toggle() { // At start we should have a value. with_glean(|glean| { assert!(core_metrics - .os + .architecture .test_get_value(glean, "glean_client_info") .is_some()); }); @@ -75,7 +75,7 @@ fn client_info_reset_after_toggle() { crate::set_upload_enabled(false); with_glean(|glean| { assert!(!core_metrics - .os + .architecture .test_get_value(glean, "glean_client_info") .is_some()); }); @@ -84,7 +84,7 @@ fn client_info_reset_after_toggle() { crate::set_upload_enabled(true); with_glean(|glean| { assert!(core_metrics - .os + .architecture .test_get_value(glean, "glean_client_info") .is_some()); }); diff --git a/glean-core/python/glean/glean.py b/glean-core/python/glean/glean.py index 4efb14003..083fc0fb9 100644 --- a/glean-core/python/glean/glean.py +++ b/glean-core/python/glean/glean.py @@ -353,7 +353,6 @@ class Glean: from ._builtins import metrics metrics.glean.baseline.locale.set(_util.get_locale_tag()) - metrics.glean.internal.metrics.os.set(platform.system()) metrics.glean.internal.metrics.os_version.set(platform.release()) metrics.glean.internal.metrics.architecture.set(platform.machine()) metrics.glean.internal.metrics.locale.set(_util.get_locale_tag()) diff --git a/glean-core/src/internal_metrics.rs b/glean-core/src/internal_metrics.rs index db712a76d..aaf6386dd 100644 --- a/glean-core/src/internal_metrics.rs +++ b/glean-core/src/internal_metrics.rs @@ -8,6 +8,7 @@ use super::{metrics::*, CommonMetricData, Lifetime}; pub struct CoreMetrics { pub client_id: UuidMetric, pub first_run_date: DatetimeMetric, + pub os: StringMetric, } impl CoreMetrics { @@ -33,6 +34,15 @@ impl CoreMetrics { }, TimeUnit::Day, ), + + os: StringMetric::new(CommonMetricData { + name: "os".into(), + category: "".into(), + send_in_pings: vec!["glean_client_info".into()], + lifetime: Lifetime::Application, + disabled: false, + dynamic_label: None, + }), } } } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index 1049b3c4a..f661f3880 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -36,6 +36,7 @@ mod internal_pings; pub mod metrics; pub mod ping; pub mod storage; +mod system; #[cfg(feature = "upload")] mod upload; mod util; @@ -249,6 +250,8 @@ impl Glean { // time it is set, that's indeed our "first run". self.is_first_run = true; } + + self.set_application_lifetime_core_metrics(); } /// Called when Glean is initialized to the point where it can correctly @@ -611,6 +614,11 @@ impl Glean { Ok(()) } + /// Set internally-handled application lifetime metrics. + fn set_application_lifetime_core_metrics(&self) { + self.core_metrics.os.set(self, system::OS); + } + /// ** This is not meant to be used directly.** /// /// Clear all the metrics that have `Lifetime::Application`. @@ -619,6 +627,9 @@ impl Glean { if let Some(data) = self.data_store.as_ref() { data.clear_lifetime(Lifetime::Application); } + + // Set internally handled app lifetime metrics again. + self.set_application_lifetime_core_metrics(); } /// Return whether or not this is the first run on this profile. diff --git a/glean-core/src/system.rs b/glean-core/src/system.rs new file mode 100644 index 000000000..1a8c10a4c --- /dev/null +++ b/glean-core/src/system.rs @@ -0,0 +1,81 @@ +// Copyright (c) 2017 The Rust Project Developers +// Licensed under the MIT License. +// Original license: +// https://github.com/RustSec/platforms-crate/blob/ebbd3403243067ba3096f31684557285e352b639/LICENSE-MIT +// +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +//! Detect and expose `target_os` as a constant. +//! +//! Code adopted from the "platforms" crate: . + +#[cfg(target_os = "android")] +/// `target_os` when building this crate: `android` +pub const OS: &str = "Android"; + +#[cfg(target_os = "ios")] +/// `target_os` when building this crate: `ios` +pub const OS: &str = "iOS"; + +#[cfg(target_os = "linux")] +/// `target_os` when building this crate: `linux` +pub const OS: &str = "Linux"; + +#[cfg(target_os = "macos")] +/// `target_os` when building this crate: `macos` +pub const OS: &str = "Darwin"; + +#[cfg(target_os = "windows")] +/// `target_os` when building this crate: `windows` +pub const OS: &str = "Windows"; + +#[cfg(target_os = "freebsd")] +/// `target_os` when building this crate: `freebsd` +pub const OS: &str = "FreeBSD"; + +#[cfg(target_os = "netbsd")] +/// `target_os` when building this crate: `netbsd` +pub const OS: &str = "NetBSD"; + +#[cfg(target_os = "openbsd")] +/// `target_os` when building this crate: `openbsd` +pub const OS: &str = "OpenBSD"; + +#[cfg(target_os = "solaris")] +/// `target_os` when building this crate: `solaris` +pub const OS: &str = "Solaris"; + +#[cfg(not(any( + target_os = "android", + target_os = "ios", + target_os = "linux", + target_os = "macos", + target_os = "windows", + target_os = "freebsd", + target_os = "netbsd", + target_os = "openbsd", + target_os = "solaris", +)))] +pub const OS: &str = "unknown";