From a21444be9dc8b8da874b98e5c858b08b650eb332 Mon Sep 17 00:00:00 2001 From: David Stevens Date: Thu, 28 Mar 2024 11:24:09 +0900 Subject: [PATCH] metrics: Switch metrics from Tube to SendTube Switch the metrics client API from Tube to SendTube for more specific typing. We're already making downstream breaking vendor changes, so it's a good time to do this cleanup. BUG=b:332466813 TEST=./tools/dev_container ./tools/presubmit all Change-Id: Id870da0298291955496e0fa339142ac379a49d37 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5404371 Reviewed-by: Noah Gold Commit-Queue: David Stevens --- broker_ipc/src/lib.rs | 6 +++--- src/crosvm/sys/windows/broker.rs | 13 +++++++------ vendor/generic/metrics/src/client.rs | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/broker_ipc/src/lib.rs b/broker_ipc/src/lib.rs index 4f396615a3..71a7313ee8 100644 --- a/broker_ipc/src/lib.rs +++ b/broker_ipc/src/lib.rs @@ -17,7 +17,7 @@ use base::EnabledHighResTimer; use base::FromRawDescriptor; use base::IntoRawDescriptor; use base::SafeDescriptor; -use base::Tube; +use base::SendTube; #[cfg(feature = "process-invariants")] pub use broker_ipc_product::init_broker_process_invariants; use broker_ipc_product::init_child_crash_reporting; @@ -33,7 +33,7 @@ use serde::Serialize; pub struct CommonChildStartupArgs { log_args: LogArgs, syslog_file: Option, - metrics_tube: Option, + metrics_tube: Option, product_attrs: ProductAttributes, } @@ -44,7 +44,7 @@ impl CommonChildStartupArgs { syslog_path: Option, #[cfg(feature = "crash-report")] _crash_attrs: crash_report::CrashReportAttributes, #[cfg(feature = "process-invariants")] _process_invariants: EmulatorProcessInvariants, - metrics_tube: Option, + metrics_tube: Option, ) -> anyhow::Result { Ok(Self { log_args: log_args.clone(), diff --git a/src/crosvm/sys/windows/broker.rs b/src/crosvm/sys/windows/broker.rs index 31a1e3764f..c5ba51821d 100644 --- a/src/crosvm/sys/windows/broker.rs +++ b/src/crosvm/sys/windows/broker.rs @@ -462,10 +462,11 @@ fn get_log_path(cfg: &Config, file_name: &str) -> Option { /// IMPORTANT NOTE: The metrics process must receive the client (second) end /// of the Tube pair in order to allow the connection to be properly shut /// down without data loss. -fn metrics_tube_pair(metric_tubes: &mut Vec) -> Result { +fn metrics_tube_pair(metric_tubes: &mut Vec) -> Result { // TODO(nkgold): as written, this Tube pair won't handle ancillary data properly because the // PIDs are not set properly at each end; however, we don't plan to send ancillary data. - let (t1, t2) = Tube::pair().exit_context(Exit::CreateTube, "failed to create tube")?; + let (t1, t2) = + Tube::directional_pair().exit_context(Exit::CreateTube, "failed to create tube")?; metric_tubes.push(t2); Ok(t1) } @@ -1189,7 +1190,7 @@ fn start_up_block_backends( exit_events: &mut Vec, wait_ctx: &mut WaitContext, main_child: &mut ChildProcess, - metric_tubes: &mut Vec, + metric_tubes: &mut Vec, #[cfg(feature = "process-invariants")] process_invariants: &EmulatorProcessInvariants, ) -> Result> { let mut block_children = Vec::new(); @@ -1417,7 +1418,7 @@ fn start_up_net_backend( wait_ctx: &mut WaitContext, cfg: &mut Config, log_args: &LogArgs, - metric_tubes: &mut Vec, + metric_tubes: &mut Vec, #[cfg(feature = "process-invariants")] process_invariants: &EmulatorProcessInvariants, ) -> Result<(ChildProcess, ChildProcess)> { let (host_pipe, guest_pipe) = named_pipes::pair_with_buffer_size( @@ -1605,7 +1606,7 @@ fn start_up_snd( main_child: &mut ChildProcess, children: &mut HashMap, wait_ctx: &mut WaitContext, - metric_tubes: &mut Vec, + metric_tubes: &mut Vec, #[cfg(feature = "process-invariants")] process_invariants: &EmulatorProcessInvariants, ) -> Result { // Extract the backend config from the sound config, so it can run elsewhere. @@ -1798,7 +1799,7 @@ fn start_up_gpu( main_child: &mut ChildProcess, children: &mut HashMap, wait_ctx: &mut WaitContext, - metric_tubes: &mut Vec, + metric_tubes: &mut Vec, wndproc_thread_builder: WindowProcedureThreadBuilder, #[cfg(feature = "process-invariants")] process_invariants: &EmulatorProcessInvariants, ) -> Result { diff --git a/vendor/generic/metrics/src/client.rs b/vendor/generic/metrics/src/client.rs index a27d1bdb42..f86095497f 100644 --- a/vendor/generic/metrics/src/client.rs +++ b/vendor/generic/metrics/src/client.rs @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use base::Tube; +use base::SendTube; use metrics_events::MetricEventType; use metrics_events::RecordDetails; @@ -10,9 +10,9 @@ use crate::MetricsClientDestructor; /// This interface exists to be used and re-implemented by downstream forks. Updates shouldn't be /// done without ensuring they won't cause breakages in dependent codebases. -pub fn initialize(_: Tube) {} +pub fn initialize(_: SendTube) {} #[cfg(test)] -pub fn force_initialize(_: Tube) {} +pub fn force_initialize(_: SendTube) {} pub fn get_destructor() -> MetricsClientDestructor { MetricsClientDestructor::new(|| {}) }