Skip to content

Commit

Permalink
futures_ext: remove slog dependency
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markbt authored and facebook-github-bot committed Jan 30, 2025
1 parent 9e894f3 commit 3365d0c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
5 changes: 4 additions & 1 deletion eden/mononoke/mononoke_api/src/changeset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1354,7 +1355,9 @@ impl<R: MononokeRepo> ChangesetContext<R> {
.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)),
Expand Down
5 changes: 4 additions & 1 deletion eden/mononoke/mononoke_api/src/sparse_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -266,7 +267,9 @@ async fn create_matchers<R: MononokeRepo>(
) -> Result<HashMap<String, Arc<dyn Matcher + Send + Sync>>> {
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();
Expand Down
5 changes: 4 additions & 1 deletion eden/mononoke/scs/scs_methods/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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?;
Expand Down

0 comments on commit 3365d0c

Please sign in to comment.