Skip to content

Commit

Permalink
Send scmstore Sapling counters through OBC API
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kavehahmadi60 authored and facebook-github-bot committed Dec 10, 2024
1 parent 995d831 commit 70e961c
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 5 deletions.
2 changes: 2 additions & 0 deletions eden/scm/lib/metrics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
6 changes: 5 additions & 1 deletion eden/scm/lib/metrics/src/dummy_ods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
6 changes: 3 additions & 3 deletions eden/scm/lib/metrics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
39 changes: 38 additions & 1 deletion eden/scm/lib/metrics/src/ods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<OBCClient>> = 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);
}
8 changes: 8 additions & 0 deletions eden/scm/lib/repo/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -172,6 +173,13 @@ impl Repo {

let locker = Arc::new(RepoLocker::new(&config, store_path.clone())?);

let is_obc_enabled = config.get_or::<bool>("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,
Expand Down

0 comments on commit 70e961c

Please sign in to comment.