From e8aae48787f5b68b3324bbb96bfbe6393981c612 Mon Sep 17 00:00:00 2001 From: Alex Franchuk Date: Tue, 16 Jul 2024 14:08:52 +0000 Subject: [PATCH] Bug 1821091 - Send Glean crash pings from the crashreporter r=gsvelto,glandium Differential Revision: https://phabricator.services.mozilla.com/D214442 --- Cargo.lock | 1 + Makefile.in | 5 + build/rust/mozbuild/generate_buildconfig.py | 15 ++ config/faster/rules.mk | 3 + toolkit/crashreporter/client/app/Cargo.toml | 1 + toolkit/crashreporter/client/app/build.rs | 25 ++ .../client/app/generate_glean.py | 34 +++ toolkit/crashreporter/client/app/moz.build | 12 +- toolkit/crashreporter/client/app/src/glean.rs | 145 ++++++++++++ .../crashreporter/client/app/src/lang/mod.rs | 5 + .../crashreporter/client/app/src/logging.rs | 1 - toolkit/crashreporter/client/app/src/logic.rs | 68 ++---- toolkit/crashreporter/client/app/src/main.rs | 10 + .../crashreporter/client/app/src/net/http.rs | 22 ++ .../crashreporter/client/app/src/net/mod.rs | 2 +- .../client/app/src/net/ping/glean.rs | 214 ++++++++++++++++++ .../src/net/{ => ping}/legacy_telemetry.rs | 21 +- .../client/app/src/net/ping/mod.rs | 97 ++++++++ .../crashreporter/client/app/src/std/mock.rs | 25 ++ .../client/app/src/std/mock_stub.rs | 6 + .../crashreporter/client/app/src/std/path.rs | 8 + toolkit/crashreporter/client/app/src/test.rs | 92 +++++--- 22 files changed, 711 insertions(+), 101 deletions(-) create mode 100644 toolkit/crashreporter/client/app/generate_glean.py create mode 100644 toolkit/crashreporter/client/app/src/glean.rs create mode 100644 toolkit/crashreporter/client/app/src/net/ping/glean.rs rename toolkit/crashreporter/client/app/src/net/{ => ping}/legacy_telemetry.rs (95%) create mode 100644 toolkit/crashreporter/client/app/src/net/ping/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 112295a2f4fb..50284a95103c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1028,6 +1028,7 @@ dependencies = [ "env_logger", "flate2", "fluent", + "glean", "gtkbind", "intl-memoizer", "libloading", diff --git a/Makefile.in b/Makefile.in index 2658629f9183..c2d53c0e326b 100644 --- a/Makefile.in +++ b/Makefile.in @@ -36,6 +36,11 @@ source-repo.h: $(MDDEPDIR)/source-repo.h.stub buildid.h: $(MDDEPDIR)/buildid.h.stub # Add explicit dependencies that moz.build can't declare yet. build/$(MDDEPDIR)/application.ini.stub: source-repo.h buildid.h + # The mozbuild crate includes the buildid (via `variables.py:get_buildid()`), + # so it can only be generated after the buildid file is generated. +ifeq ($(and $(JS_STANDALONE),$(MOZ_BUILD_APP)),) +build/rust/mozbuild/$(MDDEPDIR)/buildconfig.rs.stub: buildid.h +endif BUILD_BACKEND_FILES := $(addprefix backend.,$(addsuffix Backend,$(BUILD_BACKENDS))) diff --git a/build/rust/mozbuild/generate_buildconfig.py b/build/rust/mozbuild/generate_buildconfig.py index ace7db891a9c..955d852f91ac 100644 --- a/build/rust/mozbuild/generate_buildconfig.py +++ b/build/rust/mozbuild/generate_buildconfig.py @@ -6,6 +6,7 @@ import string import textwrap import buildconfig +from variables import get_buildid def generate_bool(name): @@ -79,6 +80,20 @@ def generate(output): ) ) + # buildid.h is only available in these conditions (see the top-level moz.build) + if not buildconfig.substs.get("JS_STANDALONE") or not buildconfig.substs.get( + "MOZ_BUILD_APP" + ): + output.write( + textwrap.dedent( + f""" + /// The build id of the current build. + pub const MOZ_BUILDID: &str = {escape_rust_string(get_buildid())}; + + """ + ) + ) + windows_rs_dir = buildconfig.substs.get("MOZ_WINDOWS_RS_DIR") if windows_rs_dir: output.write( diff --git a/config/faster/rules.mk b/config/faster/rules.mk index 79833a10348e..b09413f62d4c 100644 --- a/config/faster/rules.mk +++ b/config/faster/rules.mk @@ -93,3 +93,6 @@ $(addprefix install-,$(INSTALL_MANIFESTS)): install-%: $(addprefix $(TOPOBJDIR)/ # that are not supported by data in moz.build. $(TOPOBJDIR)/build/.deps/application.ini.stub: $(TOPOBJDIR)/buildid.h $(TOPOBJDIR)/source-repo.h +ifeq ($(and $(JS_STANDALONE),$(MOZ_BUILD_APP)),) +$(TOPOBJDIR)/build/rust/mozbuild/.deps/buildconfig.rs.stub: buildid.h +endif diff --git a/toolkit/crashreporter/client/app/Cargo.toml b/toolkit/crashreporter/client/app/Cargo.toml index 713c425404e9..1d99ff7940ba 100644 --- a/toolkit/crashreporter/client/app/Cargo.toml +++ b/toolkit/crashreporter/client/app/Cargo.toml @@ -12,6 +12,7 @@ cfg-if = "1.0" env_logger = { version = "0.10", default-features = false } flate2 = "1" fluent = "0.16.0" +glean = { workspace = true } intl-memoizer = "0.5" libloading = "0.8" log = "0.4.17" diff --git a/toolkit/crashreporter/client/app/build.rs b/toolkit/crashreporter/client/app/build.rs index d0d4874be38e..9d15f1262a56 100644 --- a/toolkit/crashreporter/client/app/build.rs +++ b/toolkit/crashreporter/client/app/build.rs @@ -8,6 +8,7 @@ fn main() { windows_manifest(); crash_ping_annotations(); set_mock_cfg(); + set_glean_metrics_file(); } fn windows_manifest() { @@ -88,3 +89,27 @@ fn set_mock_cfg() { println!("cargo:rustc-cfg=mock"); } } + +/// Set the GLEAN_METRICS_FILE environment variable to the location of the generated Glean metrics. +/// +/// We do this here to avoid a hardcoded path in the source (just in case this crate's src dir +/// moves, not that it's likely). +fn set_glean_metrics_file() { + let full_path = Path::new(env!("CARGO_MANIFEST_DIR")); + let relative_path = full_path + .strip_prefix(mozbuild::TOPSRCDIR) + .expect("CARGO_MANIFEST_DIR not a child of TOPSRCDIR"); + let glean_metrics_path = { + let mut p = mozbuild::TOPOBJDIR.join(relative_path); + // Generated by generate_glean.py + p.push("glean_metrics.rs"); + p + }; + // We don't really need anything like `rerun-if-env-changed=CARGO_MANIFEST_DIR` because that + // will inevitably mean the entire crate has moved or the srcdir has moved, which would result + // in a rebuild anyway. + println!( + "cargo:rustc-env=GLEAN_METRICS_FILE={}", + glean_metrics_path.display() + ); +} diff --git a/toolkit/crashreporter/client/app/generate_glean.py b/toolkit/crashreporter/client/app/generate_glean.py new file mode 100644 index 000000000000..5da687441fb6 --- /dev/null +++ b/toolkit/crashreporter/client/app/generate_glean.py @@ -0,0 +1,34 @@ +# 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/. + +import shutil +import sys +from contextlib import redirect_stderr, redirect_stdout +from io import StringIO +from pathlib import Path +from tempfile import TemporaryDirectory + + +def main(output, *paths): + # There's no way to just get the output as a string nor to write to our + # `output`, so we have to make a temporary directory for glean_parser to + # write to (which is ironic as glean_parser makes a temporary directory + # itself). + with TemporaryDirectory() as outdir: + outdir_path = Path(outdir) + # Capture translate output to only display on error + translate_output = StringIO() + with redirect_stdout(translate_output), redirect_stderr(translate_output): + # This is a bit tricky: sys.stderr is bound as a default argument + # in some functions of glean_parser, so we must redirect stderr + # _before_ importing the module. + from glean_parser.translate import translate + + result = translate([Path(p) for p in paths], "rust", outdir_path) + if result != 0: + print(translate_output.getvalue()) + sys.exit(result) + glean_metrics_file = outdir_path / "glean_metrics.rs" + with glean_metrics_file.open() as glean_metrics: + shutil.copyfileobj(glean_metrics, output) diff --git a/toolkit/crashreporter/client/app/moz.build b/toolkit/crashreporter/client/app/moz.build index 6e5c19173fdc..e1dcb0946ad2 100644 --- a/toolkit/crashreporter/client/app/moz.build +++ b/toolkit/crashreporter/client/app/moz.build @@ -4,4 +4,14 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -RUST_PROGRAMS = ["crashreporter"] +GeneratedFile( + "glean_metrics.rs", + script="generate_glean.py", + inputs=[ + "/toolkit/components/crashes/metrics.yaml", + "/toolkit/components/crashes/pings.yaml", + "/toolkit/components/glean/tags.yaml", + ], +) + +RustProgram("crashreporter") diff --git a/toolkit/crashreporter/client/app/src/glean.rs b/toolkit/crashreporter/client/app/src/glean.rs new file mode 100644 index 000000000000..32b75c626875 --- /dev/null +++ b/toolkit/crashreporter/client/app/src/glean.rs @@ -0,0 +1,145 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Glean telemetry integration. + +use crate::config::Config; +use glean::{ClientInfoMetrics, Configuration, ConfigurationBuilder}; + +const APP_ID: &str = if cfg!(mock) { + "firefox.crashreporter.mock" +} else { + "firefox.crashreporter" +}; +const TELEMETRY_SERVER: &str = if cfg!(mock) { + "https://incoming.glean.example.com" +} else { + "https://incoming.telemetry.mozilla.org" +}; + +/// Initialize glean based on the given configuration. +/// +/// When mocking, this should be called on a thread where the mock data is present. +pub fn init(cfg: &Config) { + glean::initialize(config(cfg), client_info_metrics(cfg)); +} + +fn config(cfg: &Config) -> Configuration { + ConfigurationBuilder::new(true, glean_data_dir(cfg), APP_ID) + .with_server_endpoint(TELEMETRY_SERVER) + .with_use_core_mps(false) + .with_internal_pings(false) + .with_uploader(uploader::Uploader::new()) + .build() +} + +#[cfg(not(mock))] +fn glean_data_dir(cfg: &Config) -> ::std::path::PathBuf { + cfg.data_dir().join("glean") +} + +#[cfg(mock)] +fn glean_data_dir(_cfg: &Config) -> ::std::path::PathBuf { + // Use a (non-mocked) temp directory since glean won't access our mocked API. + ::std::env::temp_dir().join("crashreporter-mock/glean") +} + +fn client_info_metrics(cfg: &Config) -> ClientInfoMetrics { + glean::ClientInfoMetrics { + app_build: mozbuild::config::MOZ_BUILDID.into(), + app_display_version: env!("CARGO_PKG_VERSION").into(), + channel: None, + locale: cfg.strings.as_ref().map(|s| s.locale()), + } +} + +mod uploader { + use crate::net::http; + use glean::net::{PingUploadRequest, PingUploader, UploadResult}; + + #[derive(Debug)] + pub struct Uploader { + #[cfg(mock)] + mock_data: crate::std::mock::SharedMockData, + } + + impl Uploader { + pub fn new() -> Self { + Uploader { + #[cfg(mock)] + mock_data: crate::std::mock::SharedMockData::new(), + } + } + } + + impl PingUploader for Uploader { + fn upload(&self, upload_request: PingUploadRequest) -> UploadResult { + let request_builder = http::RequestBuilder::Post { + body: upload_request.body.as_slice(), + headers: upload_request.headers.as_slice(), + }; + + let do_send = move || match request_builder.build(upload_request.url.as_ref()) { + Err(e) => { + log::error!("failed to build request for glean ping: {e}"); + UploadResult::unrecoverable_failure() + } + Ok(request) => match request.send() { + Err(e) => { + log::error!("failed to send glean ping: {e}"); + UploadResult::recoverable_failure() + } + Ok(_) => UploadResult::http_status(200), + }, + }; + + #[cfg(mock)] + return self.mock_data.call(do_send); + #[cfg(not(mock))] + return do_send(); + } + } +} + +#[cfg(test)] +mod test { + use super::*; + use once_cell::sync::Lazy; + use std::sync::{Mutex, MutexGuard}; + + pub fn test_init(cfg: &Config) -> GleanTest { + GleanTest::new(cfg) + } + + pub struct GleanTest { + _guard: MutexGuard<'static, ()>, + } + + impl GleanTest { + fn new(cfg: &Config) -> Self { + // Tests using glean can only run serially as glean is initialized as a global static. + static GLOBAL_LOCK: Lazy> = Lazy::new(|| Mutex::new(())); + + let lock = GLOBAL_LOCK.lock().unwrap(); + glean::test_reset_glean(config(cfg), client_info_metrics(cfg), true); + GleanTest { _guard: lock } + } + } + + impl Drop for GleanTest { + fn drop(&mut self) { + glean::test_reset_glean( + ConfigurationBuilder::new(false, ::std::env::temp_dir(), "none.none").build(), + glean::ClientInfoMetrics::unknown(), + true, + ); + } + } +} + +#[cfg(test)] +pub use test::test_init; + +// Env variable set to the file generated by generate_glean.py (by build.rs). +include!(env!("GLEAN_METRICS_FILE")); diff --git a/toolkit/crashreporter/client/app/src/lang/mod.rs b/toolkit/crashreporter/client/app/src/lang/mod.rs index cbde7c9265a5..fba6364431c8 100644 --- a/toolkit/crashreporter/client/app/src/lang/mod.rs +++ b/toolkit/crashreporter/client/app/src/lang/mod.rs @@ -35,6 +35,11 @@ impl LangStrings { LangStrings { bundle, rtl } } + /// Return the language identifier string for the primary locale. + pub fn locale(&self) -> String { + self.bundle.locales.first().unwrap().to_string() + } + /// Return whether the localized language has right-to-left text flow. pub fn is_rtl(&self) -> bool { self.rtl diff --git a/toolkit/crashreporter/client/app/src/logging.rs b/toolkit/crashreporter/client/app/src/logging.rs index 102a536c067b..9580dad37618 100644 --- a/toolkit/crashreporter/client/app/src/logging.rs +++ b/toolkit/crashreporter/client/app/src/logging.rs @@ -12,7 +12,6 @@ use crate::std::{ /// Initialize logging and return a log target which can be used to change the destination of log /// statements. -#[cfg_attr(mock, allow(unused))] pub fn init() -> LogTarget { let log_target_inner = LogTargetInner::default(); diff --git a/toolkit/crashreporter/client/app/src/logic.rs b/toolkit/crashreporter/client/app/src/logic.rs index 2872293a5ab3..8836e21ebbfc 100644 --- a/toolkit/crashreporter/client/app/src/logic.rs +++ b/toolkit/crashreporter/client/app/src/logic.rs @@ -61,15 +61,12 @@ impl ReportCrash { pub fn run(mut self) -> anyhow::Result { self.set_log_file(); let hash = self.compute_minidump_hash().map(Some).unwrap_or_else(|e| { - log::warn!("failed to compute minidump hash: {e}"); - None - }); - let ping_uuid = self.send_crash_ping(hash.as_deref()).unwrap_or_else(|e| { - log::warn!("failed to send crash ping: {e}"); + log::warn!("failed to compute minidump hash: {e:#}"); None }); + let ping_uuid = self.send_crash_ping(hash.as_deref()); if let Err(e) = self.update_events_file(hash.as_deref(), ping_uuid) { - log::warn!("failed to update events file: {e}"); + log::warn!("failed to update events file: {e:#}"); } self.sanitize_extra(); self.check_eol_version()?; @@ -111,57 +108,18 @@ impl ReportCrash { Ok(s) } - /// Send a crash ping to telemetry. + /// Send crash pings to legacy telemetry and Glean. /// - /// Returns the crash ping uuid. - fn send_crash_ping(&self, minidump_hash: Option<&str>) -> anyhow::Result> { - if self.config.ping_dir.is_none() { - log::warn!("not sending crash ping because no ping directory configured"); - return Ok(None); + /// Returns the crash ping uuid used in legacy telemetry. + fn send_crash_ping(&self, minidump_hash: Option<&str>) -> Option { + net::ping::CrashPing { + crash_id: self.config.local_dump_id().as_ref(), + extra: &self.extra, + ping_dir: self.config.ping_dir.as_deref(), + minidump_hash, + pingsender_path: self.config.sibling_program_path("pingsender").as_ref(), } - - //TODO support glean crash pings (or change pingsender to do so) - - let dump_id = self.config.local_dump_id(); - let ping = net::legacy_telemetry::Ping::crash(&self.extra, dump_id.as_ref(), minidump_hash) - .context("failed to create telemetry crash ping")?; - - let submission_url = ping - .submission_url(&self.extra) - .context("failed to generate ping submission URL")?; - - let target_file = self - .config - .ping_dir - .as_ref() - .unwrap() - .join(format!("{}.json", ping.id())); - - let file = std::fs::File::create(&target_file).with_context(|| { - format!( - "failed to open ping file {} for writing", - target_file.display() - ) - })?; - - serde_json::to_writer(file, &ping).context("failed to serialize telemetry crash ping")?; - - let pingsender_path = self.config.sibling_program_path("pingsender"); - - crate::process::background_command(&pingsender_path) - .arg(submission_url) - .arg(target_file) - .spawn() - .with_context(|| { - format!( - "failed to launch pingsender process at {}", - pingsender_path.display() - ) - })?; - - // TODO asynchronously get pingsender result and log it? - - Ok(Some(ping.id().clone())) + .send() } /// Remove unneeded entries from the extra file, and add some that indicate from where the data diff --git a/toolkit/crashreporter/client/app/src/main.rs b/toolkit/crashreporter/client/app/src/main.rs index 2606226d96c8..4d445868a7c3 100644 --- a/toolkit/crashreporter/client/app/src/main.rs +++ b/toolkit/crashreporter/client/app/src/main.rs @@ -61,6 +61,7 @@ macro_rules! ekey { mod async_task; mod config; mod data; +mod glean; mod lang; mod logging; mod logic; @@ -82,6 +83,8 @@ fn main() { let config_result = config.read_from_environment(); config.log_target = Some(log_target); + glean::init(&config); + let mut config = Arc::new(config); let result = config_result.and_then(|()| { @@ -136,6 +139,10 @@ fn main() { const MOCK_PING_UUID: uuid::Uuid = uuid::Uuid::nil(); const MOCK_REMOTE_CRASH_ID: &str = "8cbb847c-def2-4f68-be9e-000000000000"; + // Initialize logging but don't set it in the configuration, so that it won't be redirected to + // a file (only shown on stderr). + logging::init(); + // Create a default set of files which allow successful operation. let mock_files = MockFiles::new(); mock_files @@ -186,6 +193,9 @@ fn main() { cfg.dump_file = Some("minidump.dmp".into()); cfg.restart_command = Some("mockfox".into()); cfg.strings = Some(lang::load().unwrap()); + + glean::init(&cfg); + let mut cfg = Arc::new(cfg); try_run(&mut cfg) }); diff --git a/toolkit/crashreporter/client/app/src/net/http.rs b/toolkit/crashreporter/client/app/src/net/http.rs index 05e74c7d4361..01e1a8a96429 100644 --- a/toolkit/crashreporter/client/app/src/net/http.rs +++ b/toolkit/crashreporter/client/app/src/net/http.rs @@ -43,6 +43,11 @@ pub enum RequestBuilder<'a> { /// Gzip and POST a file's contents. #[allow(unused)] GzipAndPostFile { file: &'a Path }, + /// Send a POST. + Post { + body: &'a [u8], + headers: &'a [(String, String)], + }, } /// A single mime part to send. @@ -161,6 +166,14 @@ impl RequestBuilder<'_> { let encoder = flate2::read::GzEncoder::new(File::open(file)?, Default::default()); stdin = Some(Box::new(encoder)); } + Self::Post { body, headers } => { + for (k, v) in headers.iter() { + cmd.args(["--header", &format!("{k}: {v}")]); + } + + cmd.args(["--data-binary", "@-"]); + stdin = Some(Box::new(std::io::Cursor::new(body.to_vec()))); + } } cmd.arg(url); @@ -206,6 +219,15 @@ impl RequestBuilder<'_> { encoder.read_to_end(&mut data)?; easy.set_postfields(data)?; } + Self::Post { body, headers } => { + let mut header_list = easy.slist(); + for (k, v) in headers.iter() { + header_list.append(&format!("{k}: {v}"))?; + } + easy.set_headers(header_list)?; + + easy.set_postfields(*body)?; + } } Ok(Request::LibCurl { easy }) diff --git a/toolkit/crashreporter/client/app/src/net/mod.rs b/toolkit/crashreporter/client/app/src/net/mod.rs index ac607eb92331..38b842bc6e79 100644 --- a/toolkit/crashreporter/client/app/src/net/mod.rs +++ b/toolkit/crashreporter/client/app/src/net/mod.rs @@ -3,8 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ pub mod http; -pub mod legacy_telemetry; mod libcurl; +pub mod ping; pub mod report; #[cfg(test)] diff --git a/toolkit/crashreporter/client/app/src/net/ping/glean.rs b/toolkit/crashreporter/client/app/src/net/ping/glean.rs new file mode 100644 index 000000000000..9286e80ecebc --- /dev/null +++ b/toolkit/crashreporter/client/app/src/net/ping/glean.rs @@ -0,0 +1,214 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Glean crash ping support. This mainly sets glean metrics which will be sent later. + +use crate::glean; +use anyhow::Context; + +/// Set glean metrics to be sent in the crash ping. +pub fn set_crash_ping_metrics( + extra: &serde_json::Value, + minidump_hash: Option<&str>, +) -> anyhow::Result<()> { + let now: time::OffsetDateTime = crate::std::time::SystemTime::now().into(); + glean::crash::process_type.set("main".into()); + glean::crash::time.set(Some(glean_datetime(now))); + if let Some(hash) = minidump_hash { + glean::crash::minidump_sha256_hash.set(hash.into()); + } + + macro_rules! set_metrics_from_extra { + ( ) => {}; + ( $category:ident { $($inner:tt)+ } $($rest:tt)* ) => { + set_metrics_from_extra!(@metrics $category, $($inner)+); + set_metrics_from_extra!($($rest)*); + }; + ( @metrics $category:ident, $metric:ident : $type:tt = $key:literal $($next:tt $($rest:tt)*)? ) => { + if let Some(value) = extra.get($key) { + (|| -> anyhow::Result<()> { + set_metrics_from_extra!(@set glean::$category::$metric , $type , value); + Ok(()) + })().context(concat!("while trying to set glean::", stringify!($category), "::", stringify!($metric), " from extra data key ", $key))?; + } + $(set_metrics_from_extra!(@metrics $category, $next $($rest)*);)? + }; + ( @set $metric:expr , bool , $val:expr ) => { + $metric.set( + $val.as_str() + .map(|s| s == "1") + .context("expected a string")? + ); + }; + ( @set $metric:expr , str , $val:expr ) => { + $metric.set( + $val.as_str() + .context("expected a string")? + .to_owned() + ); + }; + ( @set $metric:expr , quantity , $val:expr ) => { + $metric.set( + $val.as_i64() + .context("expected a number")? + ); + }; + ( @set $metric:expr , seconds , $val:expr ) => { + $metric.set_raw( + crate::std::time::Duration::from_secs_f32( + $val.as_str().context("expected a string")? + .parse().context("couldn't parse floating point value")? + ) + ); + }; + ( @set $metric:expr , (object $f:ident) , $val:expr ) => { + $metric.set_string( + serde_json::to_string(&$f($val)?).context("failed to serialize data")? + ); + }; + ( @set $metric:expr , (string_list $separator:literal) , $val:expr ) => { + $metric.set( + $val.as_str().context("expected a string")? + .split($separator) + .filter(|s| !s.is_empty()) + .map(|s| s.to_owned()) + .collect() + ); + }; + } + + set_metrics_from_extra! { + crash { + app_channel: str = "ReleaseChannel" + app_display_version: str = "Version" + app_build: str = "BuildID" + async_shutdown_timeout: (object convert_async_shutdown_timeout) = "AsyncShutdownTimeout" + background_task_name: str = "BackgroundTaskName" + event_loop_nesting_level: quantity = "EventLoopNestingLevel" + font_name: str = "FontName" + gpu_process_launch: quantity = "GPUProcessLaunchCount" + ipc_channel_error: str = "ipc_channel_error" + is_garbage_collecting: bool = "IsGarbageCollecting" + main_thread_runnable_name: str = "MainThreadRunnableName" + moz_crash_reason: str = "MozCrashReason" + profiler_child_shutdown_phase: str = "ProfilerChildShutdownPhase" + quota_manager_shutdown_timeout: (object convert_quota_manager_shutdown_timeout) = "QuotaManagerShutdownTimeout" + remote_type: str = "RemoteType" + shutdown_progress: str = "ShutdownProgress" + stack_traces: (object convert_stack_traces) = "StackTraces" + startup: bool = "StartupCrash" + } + crash_windows { + error_reporting: bool = "WindowsErrorReporting" + file_dialog_error_code: str = "WindowsFileDialogErrorCode" + } + dll_blocklist { + list: (string_list ';') = "BlockedDllList" + init_failed: bool = "BlocklistInitFailed" + user32_loaded_before: bool = "User32BeforeBlocklist" + } + environment { + experimental_features: (string_list ',') = "ExperimentalFeatures" + headless_mode: bool = "HeadlessMode" + uptime: seconds = "UptimeTS" + } + memory { + available_commit: quantity = "AvailablePageFile" + available_physical: quantity = "AvailablePhysicalMemory" + available_swap: quantity = "AvailableSwapMemory" + available_virtual: quantity = "AvailableVirtualMemory" + low_physical: quantity = "LowPhysicalMemoryEvents" + oom_allocation_size: quantity = "OOMAllocationSize" + purgeable_physical: quantity = "PurgeablePhysicalMemory" + system_use_percentage: quantity = "SystemMemoryUsePercentage" + texture: quantity = "TextureUsage" + total_page_file: quantity = "TotalPageFile" + total_physical: quantity = "TotalPhysicalMemory" + total_virtual: quantity = "TotalVirtualMemory" + } + windows { + package_family_name: str = "WindowsPackageFamilyName" + } + } + + Ok(()) +} + +fn glean_datetime(datetime: time::OffsetDateTime) -> ::glean::Datetime { + ::glean::Datetime { + year: datetime.year(), + month: datetime.month() as _, + day: datetime.day() as _, + hour: datetime.hour() as _, + minute: datetime.minute() as _, + second: datetime.second() as _, + nanosecond: datetime.nanosecond(), + offset_seconds: datetime.offset().whole_seconds(), + } +} + +fn convert_async_shutdown_timeout(value: &serde_json::Value) -> anyhow::Result { + let mut ret = value.as_object().context("expected object")?.clone(); + if let Some(conditions) = ret.get_mut("conditions") { + if !conditions.is_string() { + *conditions = serde_json::to_string(conditions) + .context("failed to serialize conditions")? + .into(); + } + } + if let Some(blockers) = ret.remove("brokenAddBlockers") { + ret.insert("broken_add_blockers".into(), blockers); + } + Ok(ret.into()) +} + +fn convert_quota_manager_shutdown_timeout( + value: &serde_json::Value, +) -> anyhow::Result { + // The Glean metric is an array of the lines. + Ok(value + .as_str() + .context("expected string")? + .lines() + .collect::>() + .into()) +} + +fn convert_stack_traces(value: &serde_json::Value) -> anyhow::Result { + // glean stack_traces has a slightly different layout + let mut st = value.as_object().context("expected object")?.clone(); + if let Some(v) = st.remove("status") { + if v != "OK" { + st.insert("error".into(), v.into()); + } + } + + if let Some(mut v) = st.remove("crash_info").and_then(|v| match v { + serde_json::Value::Object(m) => Some(m), + _ => None, + }) { + if let Some(t) = v.remove("type") { + st.insert("crash_type".into(), t); + } + if let Some(a) = v.remove("address") { + st.insert("crash_address".into(), a); + } + if let Some(ct) = v.remove("crashing_thread") { + st.insert("crash_thread".into(), ct); + } + } + + if let Some(modules) = st.get_mut("modules").and_then(|v| v.as_array_mut()) { + for m in modules.iter_mut().filter_map(|m| m.as_object_mut()) { + if let Some(v) = m.remove("base_addr") { + m.insert("base_address".into(), v); + } + if let Some(v) = m.remove("end_addr") { + m.insert("end_address".into(), v); + } + } + } + + Ok(st.into()) +} diff --git a/toolkit/crashreporter/client/app/src/net/legacy_telemetry.rs b/toolkit/crashreporter/client/app/src/net/ping/legacy_telemetry.rs similarity index 95% rename from toolkit/crashreporter/client/app/src/net/legacy_telemetry.rs rename to toolkit/crashreporter/client/app/src/net/ping/legacy_telemetry.rs index 7a8816a8e718..d65a3feeac47 100644 --- a/toolkit/crashreporter/client/app/src/net/legacy_telemetry.rs +++ b/toolkit/crashreporter/client/app/src/net/ping/legacy_telemetry.rs @@ -2,9 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -//! Support for legacy telemetry ping creation. The ping support serialization which should be used -//! when submitting. +//! Support for legacy telemetry ping creation. The ping supports serialization which should be +//! used when submitting. +use crate::std; use anyhow::Context; use serde::Serialize; use std::collections::BTreeMap; @@ -37,7 +38,7 @@ time::serde::format_description!(time_format, OffsetDateTime, TIME_FORMAT); )] pub enum Ping<'a> { Crash { - id: Uuid, + id: &'a Uuid, version: u64, #[serde(with = "time_format")] creation_date: time::OffsetDateTime, @@ -87,6 +88,7 @@ pub struct Application<'a> { impl<'a> Ping<'a> { pub fn crash( + ping_id: &'a Uuid, extra: &'a serde_json::Value, crash_id: &'a str, minidump_sha256_hash: Option<&'a str>, @@ -132,7 +134,7 @@ impl<'a> Ping<'a> { .map(ToOwned::to_owned); Ok(Ping::Crash { - id: crate::std::mock::hook(Uuid::new_v4(), "ping_uuid"), + id: ping_id, version: TELEMETRY_VERSION, creation_date: now, client_id: extra["TelemetryClientId"] @@ -172,7 +174,9 @@ impl<'a> Ping<'a> { let url = extra["TelemetryServerURL"] .as_str() .context("missing TelemetryServerURL")?; - let id = self.id(); + let id = match self { + Self::Crash { id, .. } => id, + }; let name = extra["ProductName"] .as_str() .context("missing ProductName")?; @@ -183,11 +187,4 @@ impl<'a> Ping<'a> { let buildid = extra["BuildID"].as_str().context("missing BuildID")?; Ok(format!("{url}/submit/telemetry/{id}/crash/{name}/{version}/{channel}/{buildid}?v={TELEMETRY_VERSION}")) } - - /// Get the ping identifier. - pub fn id(&self) -> &Uuid { - match self { - Ping::Crash { id, .. } => id, - } - } } diff --git a/toolkit/crashreporter/client/app/src/net/ping/mod.rs b/toolkit/crashreporter/client/app/src/net/ping/mod.rs new file mode 100644 index 000000000000..7c020df40e2f --- /dev/null +++ b/toolkit/crashreporter/client/app/src/net/ping/mod.rs @@ -0,0 +1,97 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Crash pings. + +use crate::std; +use crate::std::path::Path; +use anyhow::Context; +use uuid::Uuid; + +mod glean; +mod legacy_telemetry; + +pub struct CrashPing<'a> { + pub crash_id: &'a str, + pub extra: &'a serde_json::Value, + pub ping_dir: Option<&'a Path>, + pub minidump_hash: Option<&'a str>, + pub pingsender_path: &'a Path, +} + +impl CrashPing<'_> { + /// Send the crash ping. + /// + /// Returns the crash ping id if the ping could be sent. Any errors are logged. + pub fn send(&self) -> Option { + let id = new_id(); + + // Glean ping tests have to be run serially (because the glean interface is a global), but + // we can run tests that are uninterested in glean pings in parallel by disabling the pings + // here. + if std::mock::hook(true, "enable_glean_pings") { + if let Err(e) = self.send_glean() { + log::error!("failed to send glean ping: {e:#}"); + } + } + + match self.send_legacy(&id) { + Err(e) => { + log::error!("failed to send legacy ping: {e:#}"); + None + } + Ok(sent) => sent.then_some(id), + } + } + + fn send_glean(&self) -> anyhow::Result<()> { + glean::set_crash_ping_metrics(self.extra, self.minidump_hash)?; + crate::glean::crash.submit(Some("crash")); + Ok(()) + } + + fn send_legacy(&self, id: &Uuid) -> anyhow::Result { + let Some(ping_dir) = self.ping_dir else { + log::warn!("not sending legacy crash ping because no ping directory configured"); + return Ok(false); + }; + + let ping = legacy_telemetry::Ping::crash(id, self.extra, self.crash_id, self.minidump_hash) + .context("failed to create telemetry crash ping")?; + + let submission_url = ping + .submission_url(self.extra) + .context("failed to generate ping submission URL")?; + + let target_file = ping_dir.join(format!("{}.json", id)); + + let file = std::fs::File::create(&target_file).with_context(|| { + format!( + "failed to open ping file {} for writing", + target_file.display() + ) + })?; + + serde_json::to_writer(file, &ping).context("failed to serialize telemetry crash ping")?; + + crate::process::background_command(self.pingsender_path) + .arg(submission_url) + .arg(target_file) + .spawn() + .with_context(|| { + format!( + "failed to launch pingsender process at {}", + self.pingsender_path.display() + ) + })?; + + // TODO asynchronously get pingsender result and log it? + + Ok(true) + } +} + +fn new_id() -> Uuid { + crate::std::mock::hook(Uuid::new_v4(), "ping_uuid") +} diff --git a/toolkit/crashreporter/client/app/src/std/mock.rs b/toolkit/crashreporter/client/app/src/std/mock.rs index d958423b3986..c225c913c9a8 100644 --- a/toolkit/crashreporter/client/app/src/std/mock.rs +++ b/toolkit/crashreporter/client/app/src/std/mock.rs @@ -103,6 +103,12 @@ pub trait MockKey: MockKeyStored + Sized { /// Mock data which can be shared amongst threads. pub struct SharedMockData(AtomicPtr); +impl std::fmt::Debug for SharedMockData { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.debug_struct("SharedMockData").finish_non_exhaustive() + } +} + impl Clone for SharedMockData { fn clone(&self) -> Self { SharedMockData(AtomicPtr::new(self.0.load(Relaxed))) @@ -122,6 +128,14 @@ impl SharedMockData { pub unsafe fn set(self) { MOCK_DATA.with(|ptr| ptr.store(self.0.into_inner(), Relaxed)); } + + /// Call the given function with this mock data set. + pub fn call(&self, f: impl FnOnce() -> R) -> R { + let prev = MOCK_DATA.with(|ptr| ptr.swap(self.0.load(Relaxed), Relaxed)); + let ret = f(); + MOCK_DATA.with(|ptr| ptr.store(prev, Relaxed)); + ret + } } /// Create a mock builder, which allows adding mock data and running functions under that mock @@ -252,3 +266,14 @@ macro_rules! mock_key { } pub(crate) use mock_key; + +/// A trait for unwrapping mocked types for use with external APIs. +pub trait MockUnwrap { + type Inner; + fn unwrap(self) -> Self::Inner; +} + +/// Unwrap a mocked type. +pub fn unwrap(value: T) -> T::Inner { + value.unwrap() +} diff --git a/toolkit/crashreporter/client/app/src/std/mock_stub.rs b/toolkit/crashreporter/client/app/src/std/mock_stub.rs index a02ac3dea142..eec72d31fb61 100644 --- a/toolkit/crashreporter/client/app/src/std/mock_stub.rs +++ b/toolkit/crashreporter/client/app/src/std/mock_stub.rs @@ -18,3 +18,9 @@ pub fn hook(normally: T, _name: &'static pub fn try_hook(fallback: T, _name: &'static str) -> T { fallback } + +/// Unwrap a mocked type. +#[inline(always)] +pub fn unwrap(value: T) -> T { + value +} diff --git a/toolkit/crashreporter/client/app/src/std/path.rs b/toolkit/crashreporter/client/app/src/std/path.rs index 61983c414f99..a4a91a19ba2d 100644 --- a/toolkit/crashreporter/client/app/src/std/path.rs +++ b/toolkit/crashreporter/client/app/src/std/path.rs @@ -174,3 +174,11 @@ impl From<&str> for PathBuf { PathBuf(s.into()) } } + +impl super::mock::MockUnwrap for PathBuf { + type Inner = std::path::PathBuf; + + fn unwrap(self) -> Self::Inner { + self.0 + } +} diff --git a/toolkit/crashreporter/client/app/src/test.rs b/toolkit/crashreporter/client/app/src/test.rs index 92aef9796f03..0a46ea1d2d44 100644 --- a/toolkit/crashreporter/client/app/src/test.rs +++ b/toolkit/crashreporter/client/app/src/test.rs @@ -140,6 +140,17 @@ fn test_config() -> Config { cfg } +fn init_test_logger() { + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + env_logger::builder() + .target(env_logger::Target::Stderr) + .filter(Some("crashreporter"), log::LevelFilter::Debug) + .is_test(true) + .init(); + }) +} + /// A test fixture to make configuration, mocking, and assertions easier. struct GuiTest { /// The configuration used in the test. Initialized to [`test_config`]. @@ -149,11 +160,15 @@ struct GuiTest { pub mock: mock::Builder, /// The mocked filesystem, which can be used for mock setup and assertions after completion. pub files: MockFiles, + /// Whether glean should be initialized. + enable_glean: bool, } impl GuiTest { /// Create a new GuiTest with enough configured for the application to run pub fn new() -> Self { + init_test_logger(); + // Create a default set of files which allow successful operation. let mock_files = MockFiles::new(); mock_files @@ -192,15 +207,23 @@ impl GuiTest { "work_dir/crashreporter".into(), ) .set(crate::std::time::MockCurrentTime, current_system_time()) + .set(mock::MockHook::new("enable_glean_pings"), false) .set(mock::MockHook::new("ping_uuid"), MOCK_PING_UUID); GuiTest { config: test_config(), mock, files: mock_files, + enable_glean: false, } } + pub fn enable_glean_pings(&mut self) { + self.enable_glean = true; + self.mock + .set(mock::MockHook::new("enable_glean_pings"), true); + } + /// Run the test as configured, using the given function to interact with the GUI. /// /// Returns the final result of the application logic. @@ -211,12 +234,20 @@ impl GuiTest { let GuiTest { ref mut config, ref mut mock, + ref enable_glean, .. } = self; let mut config = Arc::new(std::mem::take(config)); // Run the mock environment. - mock.run(move || gui_interact(move || try_run(&mut config), interact)) + mock.run(move || { + let _glean = if *enable_glean { + Some(glean::test_init(&config)) + } else { + None + }; + gui_interact(move || try_run(&mut config), interact) + }) } /// Run the test as configured, using the given function to interact with the GUI. @@ -270,12 +301,6 @@ impl AssertFiles { self } - /// Ignore the generated log file. - pub fn ignore_log(&mut self) -> &mut Self { - self.inner.ignore(self.data("submit.log")); - self - } - /// Assert that the crash report was submitted according to the filesystem. pub fn submitted(&mut self) -> &mut Self { self.inner.check( @@ -455,7 +480,7 @@ fn auto_submit() { test.mock.run(|| { assert!(try_run(&mut Arc::new(std::mem::take(&mut test.config))).is_ok()); }); - test.assert_files().ignore_log().submitted().pending(); + test.assert_files().submitted().pending(); } #[test] @@ -477,7 +502,6 @@ fn restart() { interact.element("restart", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending(); @@ -505,7 +529,7 @@ fn no_restart_with_windows_error_reporting() { "TelemetrySessionId": "telemetry_session", "SomeNestedJson": { "foo": "bar" }, "URL": "https://url.example.com", - "WindowsErrorReporting": 1 + "WindowsErrorReporting": "1" }"#; test.files = { let mock_files = MockFiles::new(); @@ -542,10 +566,7 @@ fn no_restart_with_windows_error_reporting() { }); }); let mut assert_files = test.assert_files(); - assert_files - .ignore_log() - .saved_settings(Settings::default()) - .submitted(); + assert_files.saved_settings(Settings::default()).submitted(); { let dmp = assert_files.data("pending/minidump.dmp"); let extra = assert_files.data("pending/minidump.extra"); @@ -564,7 +585,6 @@ fn quit() { interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending(); @@ -578,7 +598,6 @@ fn delete_dump() { interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted(); } @@ -620,7 +639,6 @@ fn no_submit() { interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings { submit_report: false, include_url: false, @@ -645,7 +663,6 @@ fn ping_and_event_files() { interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending() @@ -689,7 +706,6 @@ fn pingsender_failure() { interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending() @@ -712,6 +728,31 @@ fn pingsender_failure() { ); } +#[test] +fn glean_ping() { + let mut test = GuiTest::new(); + test.enable_glean_pings(); + let received_glean_ping = Counter::new(); + test.mock.set( + net::http::MockHttp, + Box::new(cc! { (received_glean_ping) + move | _request, url | { + if url.starts_with("https://incoming.glean.example.com") + { + received_glean_ping.inc(); + Ok(Ok(vec![])) + } else { + net::http::MockHttp::try_others() + } + } + }), + ); + test.run(|interact| { + interact.element("quit", |_style, b: &model::Button| b.click.fire(&())); + }); + received_glean_ping.assert_one(); +} + #[test] fn eol_version() { let mut test = GuiTest::new(); @@ -725,7 +766,6 @@ fn eol_version() { "Version end of life: crash reports are no longer accepted." ); test.assert_files() - .ignore_log() .pending() .ignore("data_dir/EndOfLife100.0"); } @@ -781,7 +821,6 @@ fn data_dir_default() { }); test.assert_files() .set_data_dir("data_dir/FooCorp/Bar/Crash Reports") - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending(); @@ -932,7 +971,6 @@ fn report_not_sent() { }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submission_event(false) .pending(); @@ -951,7 +989,6 @@ fn report_response_failed() { }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submission_event(false) .pending(); @@ -993,10 +1030,7 @@ fn response_indicates_discarded() { }); let mut assert_files = test.assert_files(); - assert_files - .ignore_log() - .saved_settings(Settings::default()) - .pending(); + assert_files.saved_settings(Settings::default()).pending(); for i in SHOULD_BE_PRUNED..MINIDUMP_PRUNE_SAVE_COUNT + SHOULD_BE_PRUNED - 1 { assert_files.check_exists(format!("data_dir/pending/minidump{i}.dmp")); if i % 2 == 0 { @@ -1025,7 +1059,6 @@ fn response_view_url() { }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .pending() .check( @@ -1057,7 +1090,6 @@ fn response_stop_sending_reports() { }); test.assert_files() - .ignore_log() .saved_settings(Settings::default()) .submitted() .pending() @@ -1218,7 +1250,6 @@ fn real_curl_binary() { test.assert_files() .set_data_dir(data_dir.display()) - .ignore_log() .saved_settings(Settings::default()) .submitted(); } @@ -1259,7 +1290,6 @@ fn real_curl_library() { test.assert_files() .set_data_dir(data_dir.display()) - .ignore_log() .saved_settings(Settings::default()) .submitted(); }