From 70e961c15e7f71e80d275b97768eae0ecd97e109 Mon Sep 17 00:00:00 2001 From: Kaveh Ahmadi Date: Tue, 10 Dec 2024 15:24:32 -0800 Subject: [PATCH] Send scmstore Sapling counters through OBC API Summary: ## Background: Currently we are using FB303 API to send stats to ODS. We are migrating a few stats to OBC API, because it makes it possible for us to collect some ODS stats from sandcastle. This diff D65554643 describe the pros and cons of OBC API over FB303 API, and how we can have same performance by migrating to OBC API. The `scmstore` counters are collected in Sapling. However, we are reporting the counters on Eden. Moving the `scmstore` counters to OBC API not only enable them on sandcastle, but also gives us a histogram of `.sum.60` instead of accumulated values. The next diff send the `scmstore` counters through OBC API. In this diff, I want to add a new Sapling config. Then we can control roll out of this new counter types ## This diff: Update `ods::increment()` function to bump OBC API counters in addition to incrementing the ODS counters. Therefore, we can have the histogram value in 60 sec in addition to the accumulated values. It bumps ODS directly from Sapling, and don't transfer counters to eden for collecting ODS. ## Note: The changes are behind a config which is false now, then it doesn't have any performance regression. I will open it gradually. Also, bumping ODS doesn't break any thing. Reviewed By: quark-zju Differential Revision: D66969706 fbshipit-source-id: 1a8ee52f00e330f85718fd10ea4b9025cbb74305 --- eden/scm/lib/metrics/Cargo.toml | 2 ++ eden/scm/lib/metrics/src/dummy_ods.rs | 6 ++++- eden/scm/lib/metrics/src/lib.rs | 6 ++--- eden/scm/lib/metrics/src/ods.rs | 39 ++++++++++++++++++++++++++- eden/scm/lib/repo/src/repo.rs | 8 ++++++ 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/eden/scm/lib/metrics/Cargo.toml b/eden/scm/lib/metrics/Cargo.toml index 2340b08eca480..83fe29e5f5186 100644 --- a/eden/scm/lib/metrics/Cargo.toml +++ b/eden/scm/lib/metrics/Cargo.toml @@ -13,10 +13,12 @@ license = "MIT" name = "metrics" [dependencies] +anyhow = "1.0.86" fbinit = { version = "0.2.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main", optional = true } futures = { version = "0.3.30", features = ["async-await", "compat"] } once_cell = "1.12" parking_lot = { version = "0.12.1", features = ["send_guard"] } +sapling-util = { version = "0.1.0", path = "../util" } stats = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main", optional = true } stats_traits = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main", optional = true } diff --git a/eden/scm/lib/metrics/src/dummy_ods.rs b/eden/scm/lib/metrics/src/dummy_ods.rs index 04de6359b5ee2..7ea211aef9e19 100644 --- a/eden/scm/lib/metrics/src/dummy_ods.rs +++ b/eden/scm/lib/metrics/src/dummy_ods.rs @@ -11,4 +11,8 @@ pub(crate) fn new_counter(_name: &'static str) -> Counter { () } -pub(crate) fn increment(_counter: &Counter, _value: i64) {} +pub fn initialize_obc_client() -> anyhow::Result<()> { + Ok(()) +} + +pub(crate) fn increment(_counter: &Counter, _name: &str, _value: i64) {} diff --git a/eden/scm/lib/metrics/src/lib.rs b/eden/scm/lib/metrics/src/lib.rs index 7882203c4a911..d192f4558e48d 100644 --- a/eden/scm/lib/metrics/src/lib.rs +++ b/eden/scm/lib/metrics/src/lib.rs @@ -15,7 +15,7 @@ use once_cell::sync::OnceCell; use parking_lot::RwLock; #[cfg_attr(not(feature = "ods"), path = "dummy_ods.rs")] -mod ods; +pub mod ods; pub struct Counter { name: &'static str, @@ -48,13 +48,13 @@ impl Counter { pub fn add(&'static self, val: usize) { let (counter, ods) = self.counter(); counter.fetch_add(val, Ordering::Relaxed); - ods::increment(ods, val as i64); + ods::increment(ods, self.name, val as i64); } pub fn sub(&'static self, val: usize) { let (counter, ods) = self.counter(); counter.fetch_sub(val, Ordering::Relaxed); - ods::increment(ods, -(val as i64)); + ods::increment(ods, self.name, -(val as i64)); } pub fn value(&'static self) -> usize { diff --git a/eden/scm/lib/metrics/src/ods.rs b/eden/scm/lib/metrics/src/ods.rs index 3fd225044893b..7c7e7748dae4d 100644 --- a/eden/scm/lib/metrics/src/ods.rs +++ b/eden/scm/lib/metrics/src/ods.rs @@ -5,16 +5,53 @@ * LICENSE file in the root directory of this source tree. */ +use std::sync::Arc; + +use obc_lib::obc_client::OBCClient; +use obc_lib::obc_client::OBCClientOptionsBuilder; +use obc_lib::AggValue; +use obc_lib::OBCBumper as _; +use once_cell::sync::OnceCell; + +static OBC_CLIENT: OnceCell> = OnceCell::new(); + pub(crate) type Counter = stats_traits::stat_types::BoxSingletonCounter; pub(crate) fn new_counter(name: &'static str) -> Counter { stats::create_singleton_counter(name.to_string()) } -pub(crate) fn increment(counter: &Counter, value: i64) { +pub fn initialize_obc_client() -> anyhow::Result<()> { + if !fbinit::was_performed() { + return Err(anyhow::anyhow!( + "Failed to initilize obc client for logging to ods" + )); + } + + if OBC_CLIENT.get().is_some() { + return Ok(()); + } + + let opts = OBCClientOptionsBuilder::default() + .ods_category("eden".to_string()) + .append_agg_type_suffix(false) + .build() + .map_err(anyhow::Error::msg)?; + + OBC_CLIENT + .set(Arc::new(OBCClient::new(fbinit::expect_init(), opts)?)) + .map_err(|_| anyhow::anyhow!("Failed to initilize obc client for logging to ods")) +} + +pub(crate) fn increment(counter: &Counter, name: &str, value: i64) { if !fbinit::was_performed() { return; } + if let Some(client) = OBC_CLIENT.get() { + let obc_entity = util::sys::hostname().to_string(); + let _ = client.bump_entity_key_agg(&obc_entity, name, AggValue::Sum(value as f64), None); + } + counter.increment_value(fbinit::expect_init(), value); } diff --git a/eden/scm/lib/repo/src/repo.rs b/eden/scm/lib/repo/src/repo.rs index 2b73c4d7f29ca..8f29d96157f31 100644 --- a/eden/scm/lib/repo/src/repo.rs +++ b/eden/scm/lib/repo/src/repo.rs @@ -27,6 +27,7 @@ use edenapi::SaplingRemoteApi; use edenapi::SaplingRemoteApiError; use manifest_tree::ReadTreeManifest; use metalog::MetaLog; +use metrics::ods; use once_cell::sync::OnceCell; use parking_lot::RwLock; use repo_minimal_info::constants::SUPPORTED_DEFAULT_REQUIREMENTS; @@ -172,6 +173,13 @@ impl Repo { let locker = Arc::new(RepoLocker::new(&config, store_path.clone())?); + let is_obc_enabled = config.get_or::("scmstore", "enable-obc", || false)?; + if is_obc_enabled { + if let Err(err) = ods::initialize_obc_client() { + tracing::warn!(?err, "error creating OBC client"); + } + } + Ok(Repo { path, ident,