From 97159bd88d6294128479cd1ee8b74de3fdad93d8 Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Thu, 27 Jun 2024 17:03:47 -0700
Subject: [PATCH] Associate logs from log_err with the calling crate (#13617)

Now, when you selectively enable logs from particular crates with
`RUST_LOG=call,worktree`, logs created via `log_err` calls in those
crates get correctly enabled. Previously, they were all attributed to
the `util` crate, because they used the normal logging macros, which
implicitly insert the current crate name.

This relies on the regularity of our directory naming. Rust's
`track_caller` feature allows you to obtain the file system path of the
caller, but not its rust module path, so I'm inferring the crate name
from the file system path (which I believe is always valid, in our
codebase).

Release Notes:

- N/A
---
 crates/util/src/util.rs | 42 ++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs
index 688f17e43f0ee1a4cdf7f6f5cf4189c693a63d0e..566ad336986d2bfbdddae76fe2045ddc2fd2cc6e 100644
--- a/crates/util/src/util.rs
+++ b/crates/util/src/util.rs
@@ -335,7 +335,6 @@ pub trait ResultExt<E> {
     /// Assert that this result should never be an error in development or tests.
     fn debug_assert_ok(self, reason: &str) -> Self;
     fn warn_on_err(self) -> Option<Self::Ok>;
-    fn inspect_error(self, func: impl FnOnce(&E)) -> Self;
 }
 
 impl<T, E> ResultExt<E> for Result<T, E>
@@ -349,8 +348,7 @@ where
         match self {
             Ok(value) => Some(value),
             Err(error) => {
-                let caller = Location::caller();
-                log::error!("{}:{}: {:?}", caller.file(), caller.line(), error);
+                log_error_with_caller(*Location::caller(), error, log::Level::Error);
                 None
             }
         }
@@ -364,24 +362,36 @@ where
         self
     }
 
+    #[track_caller]
     fn warn_on_err(self) -> Option<T> {
         match self {
             Ok(value) => Some(value),
             Err(error) => {
-                log::warn!("{:?}", error);
+                log_error_with_caller(*Location::caller(), error, log::Level::Warn);
                 None
             }
         }
     }
+}
 
-    /// https://doc.rust-lang.org/std/result/enum.Result.html#method.inspect_err
-    fn inspect_error(self, func: impl FnOnce(&E)) -> Self {
-        if let Err(err) = &self {
-            func(err);
-        }
-
-        self
-    }
+fn log_error_with_caller<E>(caller: core::panic::Location<'_>, error: E, level: log::Level)
+where
+    E: std::fmt::Debug,
+{
+    // In this codebase, the first segment of the file path is
+    // the 'crates' folder, followed by the crate name.
+    let target = caller.file().split('/').nth(1);
+
+    log::logger().log(
+        &log::Record::builder()
+            .target(target.unwrap_or(""))
+            .module_path(target)
+            .args(format_args!("{:?}", error))
+            .file(Some(caller.file()))
+            .line(Some(caller.line()))
+            .level(level)
+            .build(),
+    );
 }
 
 pub trait TryFutureExt {
@@ -457,13 +467,7 @@ where
             Poll::Ready(output) => Poll::Ready(match output {
                 Ok(output) => Some(output),
                 Err(error) => {
-                    log::log!(
-                        level,
-                        "{}:{}: {:?}",
-                        location.file(),
-                        location.line(),
-                        error
-                    );
+                    log_error_with_caller(location, error, level);
                     None
                 }
             }),