filteredfs: switch to simpler metrics

Summary: See D66848951 for more details. Basically, use static counters from the "metrics" crate (which handle ods export internally).

Reviewed By: MichaelCuevas

Differential Revision: D67718795

fbshipit-source-id: 2fad3adaa359410491d9a3211f839ca0e95f1f5b

Author: muirdm

eden/integration/hg/filteredhg_test.py

@@ -14,7 +14,6 @@
     filteredhg_test,
     FilteredHgTestCase,
 )
-
 from eden.integration.lib import hgrepo
 
 
@@ -342,10 +341,7 @@ def test_ods_counters_exist(self) -> None:
         self.set_active_filter("top_level_filter")
         counters = self.get_counters()
         expected_counters = [
-            "edenffi.ffs.invalid_repo",
-            "edenffi.ffs.lookup_failures",
             "edenffi.ffs.lookups",
-            "edenffi.ffs.repo_cache_hits",
             "edenffi.ffs.repo_cache_misses",
         ]
         for ec in expected_counters:
@@ -354,7 +350,7 @@ def test_ods_counters_exist(self) -> None:
     def test_lookup_failure_counter(self) -> None:
         self.set_active_filter("top_level_filter")
         counters = self.get_counters()
-        self.assertEqual(counters["edenffi.ffs.lookup_failures"], 0)
+        self.assertNotIn("edenffi.ffs.lookup_failures", counters)
         self.set_active_filter("does_not_exist")
         counters = self.get_counters()
         self.assertGreaterEqual(counters["edenffi.ffs.lookup_failures"], 1)

eden/scm/lib/edenfs_ffi/Cargo.toml

@@ -16,21 +16,21 @@ crate-type = ["lib", "staticlib"]
 [dependencies]
 anyhow = "1.0.95"
 cxx = "1.0.119"
-fbinit = { version = "0.2.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main", optional = true }
 once_cell = "1.12"
 parking_lot = { version = "0.12.1", features = ["send_guard"] }
 sapling-async-runtime = { version = "0.1.0", path = "../async-runtime" }
 sapling-identity = { version = "0.1.0", path = "../identity" }
 sapling-manifest = { version = "0.1.0", path = "../manifest" }
+sapling-metrics = { version = "0.1.0", path = "../metrics" }
 sapling-pathmatcher = { version = "0.1.0", path = "../pathmatcher" }
 sapling-repo = { version = "0.1.0", path = "../repo" }
 sapling-sparse = { version = "0.1.0", path = "../sparse" }
 sapling-types = { version = "0.1.0", path = "../types" }
-stats = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main", optional = true }
 tracing = { version = "0.1.41", features = ["attributes", "valuable"] }
 
 [build-dependencies]
 cxx-build = "1.0.119"
 
 [features]
-ods = ["fbinit", "stats"]
+default = []
+fb = []

eden/scm/lib/edenfs_ffi/src/lib.rs

@@ -17,6 +17,7 @@ use cxx::UniquePtr;
 use manifest::FileMetadata;
 use manifest::FsNodeMetadata;
 use manifest::Manifest;
+use metrics::Counter;
 use once_cell::sync::Lazy;
 use parking_lot::Mutex;
 use pathmatcher::DirectoryMatch;
@@ -29,18 +30,21 @@ use types::HgId;
 use types::RepoPath;
 use types::RepoPathBuf;
 
-mod metrics;
-
 use crate::ffi::set_matcher_error;
 use crate::ffi::set_matcher_promise_error;
 use crate::ffi::set_matcher_promise_result;
 use crate::ffi::set_matcher_result;
 use crate::ffi::MatcherPromise;
 use crate::ffi::MatcherWrapper;
-use crate::metrics::FilteredFSMetrics;
 
 static REPO_HASHMAP: Lazy<Mutex<HashMap<PathBuf, Repo>>> = Lazy::new(|| Mutex::new(HashMap::new()));
 
+static LOOKUPS: Counter = Counter::new_counter("edenffi.ffs.lookups");
+static LOOKUP_FAILURES: Counter = Counter::new_counter("edenffi.ffs.lookup_failures");
+static INVALID_REPO: Counter = Counter::new_counter("edenffi.ffs.invalid_repo");
+static REPO_CACHE_MISSES: Counter = Counter::new_counter("edenffi.ffs.repo_cache_misses");
+static REPO_CACHE_HITS: Counter = Counter::new_counter("edenffi.ffs.repo_cache_hits");
+
 // A helper class to parse/validate FilterIDs that are passed to Mercurial
 struct FilterId {
     pub repo_path: RepoPathBuf,
@@ -207,9 +211,8 @@ fn profile_contents_from_repo(
     id: FilterId,
     abs_repo_path: PathBuf,
     promise: UniquePtr<MatcherPromise>,
-    metrics: &mut FilteredFSMetrics,
 ) {
-    match _profile_contents_from_repo(id, abs_repo_path, metrics) {
+    match _profile_contents_from_repo(id, abs_repo_path) {
         Ok(res) => {
             set_matcher_promise_result(promise, res);
         }
@@ -223,18 +226,17 @@ fn profile_contents_from_repo(
 fn _profile_contents_from_repo(
     id: FilterId,
     abs_repo_path: PathBuf,
-    metrics: &mut FilteredFSMetrics,
 ) -> Result<Box<MercurialMatcher>, anyhow::Error> {
     let mut repo_map = REPO_HASHMAP.lock();
     if !repo_map.contains_key(&abs_repo_path) {
         // Load the repo and store it for later use
-        metrics.repo_miss();
+        REPO_CACHE_MISSES.increment();
         let repo = Repo::load(&abs_repo_path, &[]).with_context(|| {
             anyhow!("failed to load Repo object for {}", abs_repo_path.display())
         })?;
         repo_map.insert(abs_repo_path.clone(), repo);
     } else {
-        metrics.repo_hit();
+        REPO_CACHE_HITS.increment();
     }
     let repo = repo_map.get_mut(&abs_repo_path).context("loading repo")?;
 
@@ -291,7 +293,7 @@ fn _profile_contents_from_repo(
     // invalid. Return an always matcher instead of erroring out.
     let sparse_matcher = matcher.unwrap_or_else(|e| {
         tracing::warn!("Failed to get sparse matcher for active filter: {:?}", e);
-        metrics.failure();
+        LOOKUP_FAILURES.increment();
         Matcher::new(
             vec![TreeMatcher::always()],
             vec![vec!["always_matcher".to_string()]],
@@ -310,8 +312,7 @@ pub fn profile_from_filter_id(
     checkout_path: &str,
     promise: UniquePtr<MatcherPromise>,
 ) -> Result<(), anyhow::Error> {
-    let mut metrics = FilteredFSMetrics::default();
-    metrics.lookup();
+    LOOKUPS.increment();
 
     // Parse the FilterID
     let filter_id = FilterId::from_str(id)?;
@@ -320,7 +321,7 @@ pub fn profile_from_filter_id(
     // should correspond to a valid hg/sl repo that Mercurial is aware of.
     let abs_repo_path = PathBuf::from(checkout_path);
     if identity::sniff_dir(&abs_repo_path).is_err() {
-        metrics.invalid_repo();
+        INVALID_REPO.increment();
         return Err(anyhow!(
             "{} is not a valid hg repo",
             abs_repo_path.display()
@@ -330,12 +331,7 @@ pub fn profile_from_filter_id(
     // If we've already loaded a filter from this repo before, we can skip Repo
     // object creation. Otherwise, we need to pay the 1 time cost of creating
     // the Repo object.
-    profile_contents_from_repo(filter_id, abs_repo_path, promise, &mut metrics);
-
-    // Update ODS counters with information about the filter we just loaded
-    if let Err(err) = metrics.update_ods() {
-        tracing::error!(?err, "error updating edenffi ods counters");
-    }
+    profile_contents_from_repo(filter_id, abs_repo_path, promise);
 
     Ok(())
 }