From 3365d0c9b2662d21276084c0f4eb1362950dcbef Mon Sep 17 00:00:00 2001 From: Mark Juggurnauth-Thomas Date: Thu, 30 Jan 2025 15:26:28 -0800 Subject: [PATCH] futures_ext: remove slog dependency Summary: We are migrating from `slog` to `tracing`. The `YieldPeriodically` stream adapter has the ability to log to a `slog::Logger`. This is difficult to migrate to `tracing`, as it allows capturing of code locations which is not possible in tracing, as tracing uses static locations generated by its macros. Instead, we remove the logging dependency and abstract it out to a callback that is called when polling exceeds the budged by a significant amount. It is then up to the caller to log appropriately. Since the logging happens in the caller, the log context, file name and line number are all correct. Reviewed By: andreacampi Differential Revision: D68837434 fbshipit-source-id: 6348754ec8cb2a45bec24b3022e4d715c4f6b561 --- eden/mononoke/mononoke_api/src/changeset.rs | 5 ++++- eden/mononoke/mononoke_api/src/sparse_profile.rs | 5 ++++- eden/mononoke/scs/scs_methods/src/history.rs | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/eden/mononoke/mononoke_api/src/changeset.rs b/eden/mononoke/mononoke_api/src/changeset.rs index cbb95ff5f68c8..41077ff4080f0 100644 --- a/eden/mononoke/mononoke_api/src/changeset.rs +++ b/eden/mononoke/mononoke_api/src/changeset.rs @@ -68,6 +68,7 @@ use repo_derived_data::RepoDerivedDataArc; use repo_derived_data::RepoDerivedDataRef; use skeleton_manifest::RootSkeletonManifestId; use skeleton_manifest_v2::RootSkeletonManifestV2Id; +use slog::warn; use smallvec::SmallVec; use sorted_vector_map::SortedVectorMap; use unodes::RootUnodeManifestId; @@ -1354,7 +1355,9 @@ impl ChangesetContext { .watched(self.ctx().logger()) .await? .yield_periodically() - .with_logger(self.ctx().logger()) + .on_large_overshoot(|budget, elapsed| { + warn!(self.ctx().logger(), "yield_periodically(): budget overshot: current_budget={budget:?}, elapsed={elapsed:?}"); + }) .try_filter_map(|(path, entry)| async move { match (path.into_optional_non_root_path(), entry) { (Some(mpath), ManifestEntry::Leaf(_)) if diff_files => Ok(Some(mpath)), diff --git a/eden/mononoke/mononoke_api/src/sparse_profile.rs b/eden/mononoke/mononoke_api/src/sparse_profile.rs index 5f601b484a830..39aa72eac473e 100644 --- a/eden/mononoke/mononoke_api/src/sparse_profile.rs +++ b/eden/mononoke/mononoke_api/src/sparse_profile.rs @@ -33,6 +33,7 @@ use pathmatcher::Matcher; use repo_blobstore::RepoBlobstoreRef; use repo_sparse_profiles::RepoSparseProfiles; use slog::debug; +use slog::warn; use types::RepoPath; use crate::errors::MononokeError; @@ -266,7 +267,9 @@ async fn create_matchers( ) -> Result>> { stream::iter(paths) .yield_periodically() - .with_logger(ctx.logger()) + .on_large_overshoot(|budget, elapsed| { + warn!(ctx.logger(), "yield_periodically(): budget overshot: current_budget={budget:?}, elapsed={elapsed:?}"); + }) .map(|path| async move { let content = format!("%include {path}"); let dummy_source = "repo_root".to_string(); diff --git a/eden/mononoke/scs/scs_methods/src/history.rs b/eden/mononoke/scs/scs_methods/src/history.rs index 954c41f0bab1b..99d25765b103d 100644 --- a/eden/mononoke/scs/scs_methods/src/history.rs +++ b/eden/mononoke/scs/scs_methods/src/history.rs @@ -19,6 +19,7 @@ use mononoke_api::ChangesetContext; use mononoke_api::CoreContext; use mononoke_api::MononokeError; use mononoke_api::Repo; +use slog::warn; use source_control as thrift; use crate::into_response::AsyncIntoResponseWith; @@ -85,7 +86,9 @@ pub(crate) async fn collect_history( }) .buffered(10) .yield_periodically() - .with_logger(ctx.logger()) + .on_large_overshoot(|budget, elapsed| { + warn!(ctx.logger(), "yield_periodically(): budget overshot: current_budget={budget:?}, elapsed={elapsed:?}"); + }) .try_collect() .watched(ctx.logger()) .await?;