Remove low-value per-repo counters

Summary: Remove dynamic per-repo counters that are unused, misleading, or duplicated by better request-scoped signals. Keep the file-history perf counter and MononokeApiStats missing-bookmark events.

Reviewed By: lmvasquezg

Differential Revision: D115719733

fbshipit-source-id: adf01b1a6ed8fd7200dc6c35695db6ed45997246

Author: Clara Rull

eden/mononoke/blobrepo/BUCK

@@ -109,7 +109,6 @@ rust_library(
         "fbsource//third-party/rust:thiserror",
         "fbsource//third-party/rust:tracing",
         "//common/rust/shed/cloned:cloned",
-        "//common/rust/shed/stats:stats",
         "//eden/mononoke/blobstore:blobstore",
         "//eden/mononoke/manifest:manifest",
         "//eden/mononoke/mercurial:mercurial_types",

eden/mononoke/blobrepo/common/Cargo.toml

@@ -22,7 +22,6 @@ mercurial_types = { version = "0.1.0", path = "../../mercurial/types" }
 mononoke_types = { version = "0.1.0", path = "../../mononoke_types" }
 repo_blobstore = { version = "0.1.0", path = "../../repo_attributes/repo_blobstore" }
 repo_identity = { version = "0.1.0", path = "../../repo_attributes/repo_identity" }
-stats = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }
 thiserror = "2.0.20"
 tracing = { version = "0.1.41", features = ["attributes", "valuable"] }
 

eden/mononoke/blobrepo/common/src/file_history.rs

@@ -43,7 +43,6 @@ use mercurial_types::RepoPath;
 use mononoke_types::ChangesetId;
 use repo_blobstore::RepoBlobstoreRef;
 use repo_identity::RepoIdentityRef;
-use stats::prelude::*;
 use thiserror::Error;
 use tracing::debug;
 
@@ -65,11 +64,6 @@ pub enum FilenodesRelatedResult {
     SecondAncestorOfFirst,
 }
 
-define_stats! {
-    prefix = "mononoke.file_history";
-    too_big: dynamic_timeseries("{}.too_big", (repo: String); Rate, Sum),
-}
-
 pub trait Repo = RepoIdentityRef
     + RepoBlobstoreRef
     + FilenodesRef
@@ -260,7 +254,6 @@ pub async fn get_file_history(
         FilenodeResult::Present(FilenodeRange::TooBig) => {
             ctx.perf_counters()
                 .increment_counter(PerfCounterType::FilenodesTooBigHistory);
-            STATS::too_big.add_value(1, (repo.repo_identity().name().to_string(),));
             let history = get_file_history_using_prefetched(
                 ctx,
                 repo,

eden/mononoke/mononoke_api/src/repo.rs

@@ -207,14 +207,6 @@ define_stats! {
         "staleness.secs.{}.{}",
         (repoid: ::mononoke_types::RepositoryId, bookmark: String)
     ),
-    missing_from_cache: dynamic_singleton_counter(
-        "missing_from_cache.{}.{}",
-        (repoid: ::mononoke_types::RepositoryId, bookmark: String)
-    ),
-    missing_from_repo: dynamic_singleton_counter(
-        "missing_from_repo.{}.{}",
-        (repoid: ::mononoke_types::RepositoryId, bookmark: String)
-    ),
 }
 
 #[facet::container]
@@ -554,23 +546,13 @@ pub async fn report_monitoring_stats(
     Ok(())
 }
 
-fn report_bookmark_missing_from_cache(
-    ctx: &CoreContext,
-    repo: &impl RepoIdentityRef,
-    bookmark: &BookmarkKey,
-) {
+fn report_bookmark_missing_from_cache(repo: &impl RepoIdentityRef, bookmark: &BookmarkKey) {
     error!(
         "Monitored bookmark does not exist in the cache: {}, repo: {}",
         bookmark,
         repo.repo_identity().name()
     );
 
-    STATS::missing_from_cache.set_value(
-        ctx.fb,
-        1,
-        (repo.repo_identity().id(), bookmark.to_string()),
-    );
-
     #[cfg(fbcode_build)]
     API_STATS_INSTRUMENT.observe(MononokeApiStats {
         repo: Some(repo.repo_identity().name().to_string()),
@@ -582,22 +564,12 @@ fn report_bookmark_missing_from_cache(
     });
 }
 
-fn report_bookmark_missing_from_repo(
-    ctx: &CoreContext,
-    repo: &impl RepoIdentityRef,
-    bookmark: &BookmarkKey,
-) {
+fn report_bookmark_missing_from_repo(repo: &impl RepoIdentityRef, bookmark: &BookmarkKey) {
     error!(
         "Monitored bookmark does not exist in the repo: {}",
         bookmark
     );
 
-    STATS::missing_from_repo.set_value(
-        ctx.fb,
-        1,
-        (repo.repo_identity().id(), bookmark.to_string()),
-    );
-
     #[cfg(fbcode_build)]
     API_STATS_INSTRUMENT.observe(MononokeApiStats {
         repo: Some(repo.repo_identity().name().to_string()),
@@ -653,11 +625,11 @@ async fn report_bookmark_age_difference(
         .await?;
 
     if maybe_bcs_id_from_blobrepo.is_none() {
-        report_bookmark_missing_from_repo(ctx, repo, bookmark);
+        report_bookmark_missing_from_repo(repo, bookmark);
     }
 
     if maybe_bcs_id_from_service.is_none() {
-        report_bookmark_missing_from_cache(ctx, repo, bookmark);
+        report_bookmark_missing_from_cache(repo, bookmark);
     }
 
     if let (Some(service_bcs_id), Some(blobrepo_bcs_id)) =

eden/mononoke/repo_attributes/bookmarks/src/cache.rs

@@ -31,23 +31,15 @@ use futures::stream::BoxStream;
 use futures::stream::StreamExt;
 use futures::stream::TryStreamExt;
 use mononoke_types::ChangesetId;
-use mononoke_types::RepositoryId;
 use shared_error::anyhow::IntoSharedError;
 use shared_error::anyhow::SharedError;
-use stats::prelude::*;
 
 use crate::Bookmarks;
 use crate::log::BookmarkUpdateReason;
 use crate::subscription::BookmarksSubscription;
 use crate::transaction::BookmarkTransaction;
 use crate::transaction::BookmarkTransactionHook;
 
-define_stats! {
-    prefix = "mononoke.bookmarks.cache";
-    cached_bookmarks_hits: dynamic_timeseries("{}.hit", (repo: String); Rate, Sum),
-    cached_bookmarks_misses: dynamic_timeseries("{}.miss", (repo: String); Rate, Sum),
-}
-
 type CacheData = BTreeMap<BookmarkKey, (BookmarkKind, ChangesetId)>;
 
 #[derive(Clone)]
@@ -110,7 +102,6 @@ impl Cache {
 
 #[derive(Clone)]
 pub struct CachedBookmarks {
-    repo_id: RepositoryId,
     cache: Arc<Mutex<Option<Cache>>>,
     bookmarks: Arc<dyn Bookmarks>,
 }
@@ -126,9 +117,8 @@ fn ttl() -> Option<Duration> {
 }
 
 impl CachedBookmarks {
-    pub fn new(bookmarks: Arc<dyn Bookmarks>, repo_id: RepositoryId) -> Self {
+    pub fn new(bookmarks: Arc<dyn Bookmarks>) -> Self {
         Self {
-            repo_id,
             bookmarks,
             cache: Arc::new(Mutex::new(None)),
         }
@@ -142,9 +132,7 @@ impl CachedBookmarks {
             Some(ref mut cache) => {
                 // create new cache if the old one has either expired or failed
                 let cache_failed = cache.is_failed();
-                let mut cache_hit = true;
                 if cache.expires <= now || cache_failed {
-                    cache_hit = false;
                     *cache = Cache::new(
                         ctx,
                         self.bookmarks.clone(),
@@ -165,12 +153,6 @@ impl CachedBookmarks {
                     );
                 }
 
-                if cache_hit {
-                    STATS::cached_bookmarks_hits.add_value(1, (self.repo_id.id().to_string(),))
-                } else {
-                    STATS::cached_bookmarks_misses.add_value(1, (self.repo_id.id().to_string(),))
-                }
-
                 cache.clone()
             }
             None => {
@@ -781,12 +763,10 @@ mod tests {
             .build()
             .unwrap();
         let ctx = CoreContext::test_mock(fb);
-        let repo_id = RepositoryId::new(0);
-
         let (mock, requests) = MockBookmarks::create();
         let requests = requests.into_future();
 
-        let bookmarks = CachedBookmarks::new(Arc::new(mock), repo_id);
+        let bookmarks = CachedBookmarks::new(Arc::new(mock));
 
         let spawn_query = |prefix: &'static str, ttl: Option<i64>, rt: &Runtime| {
             let (sender, receiver) = oneshot::channel();
@@ -983,12 +963,10 @@ mod tests {
             .build()
             .unwrap();
         let ctx = CoreContext::test_mock(fb);
-        let repo_id = RepositoryId::new(0);
-
         let (mock, requests) = MockBookmarks::create();
         let requests = requests.into_future();
 
-        let store = CachedBookmarks::new(Arc::new(mock), repo_id);
+        let store = CachedBookmarks::new(Arc::new(mock));
 
         let (sender, receiver) = oneshot::channel();
 

eden/mononoke/repo_client/src/client.rs

@@ -102,7 +102,6 @@ define_stats! {
     push_success: dynamic_timeseries("push_success.{}", (reponame: String); Rate, Sum),
     push_hook_failure: dynamic_timeseries("push_hook_failure.{}.{}", (reponame: String, hook_failure: String); Rate, Sum),
     push_conflicts: dynamic_timeseries("push_conflicts.{}", (reponame: String); Rate, Sum),
-    rate_limits_exceeded: dynamic_timeseries("rate_limits_exceeded.{}", (reponame: String); Rate, Sum),
     push_error: dynamic_timeseries("push_error.{}", (reponame: String); Rate, Sum),
 }
 

eden/mononoke/repo_factory/src/lib.rs

@@ -901,15 +901,8 @@ impl RepoFactory {
         Ok(Arc::new(sql_bookmarks))
     }
 
-    pub fn bookmarks(
-        &self,
-        sql_bookmarks: &ArcSqlBookmarks,
-        repo_identity: &ArcRepoIdentity,
-    ) -> ArcBookmarks {
-        Arc::new(CachedBookmarks::new(
-            sql_bookmarks.clone(),
-            repo_identity.id(),
-        ))
+    pub fn bookmarks(&self, sql_bookmarks: &ArcSqlBookmarks) -> ArcBookmarks {
+        Arc::new(CachedBookmarks::new(sql_bookmarks.clone()))
     }
 
     pub fn bookmark_update_log(&self, sql_bookmarks: &ArcSqlBookmarks) -> ArcBookmarkUpdateLog {