Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider bringing back kv Value downcasting #641

Open
AlexTMjugador opened this issue Oct 29, 2024 · 2 comments
Open

Consider bringing back kv Value downcasting #641

AlexTMjugador opened this issue Oct 29, 2024 · 2 comments

Comments

@AlexTMjugador
Copy link

AlexTMjugador commented Oct 29, 2024

While working on a Rust application, I became interested in downcasting a Value associated with a log record when using the kv feature of the log crate. To my surprise, I discovered that this capability was removed when the kv feature was stabilized:

log/src/kv/value.rs

Lines 1100 to 1107 in 96dbf58

/// Check whether this value can be downcast to `T`.
#[cfg(feature = "kv_unstable")]
#[deprecated(
note = "downcasting has been removed; log an issue at https://github.com/rust-lang/log/issues if this is something you rely on"
)]
pub fn is<T: 'static>(&self) -> bool {
false
}

In my view, there are compelling use cases for downcasting log record Values: that enables different software components to process them in a loosely coupled way, effectively repurposing log Records as a versatile, typed message-passing mechanism.

For instance, imagine integrating an existing codebase that logs anyhow::Errors with sentry reporting, with the requirement that generated Sentry events should include the error backtrace. With Value downcasting, this could be achieved by doing this at the logging site:

log::error!(error = anyhow_error; "Something went wrong");

And then, to emit Sentry events from these log records with the error backtrace, one could simply pass a custom mapper function to a SentryLogger that retrieves the error value from the record, downcasts it to an anyhow::Error, calls the backtrace() method on it, and attaches that backtrace to the Sentry event.

Without record Value downcasting, there isn’t an elegant solution for the scenario above. Approaches like using custom serde serializers or establishing an additional communication channel between log sites and logger implementations may work in certain cases, but they are far from ideal.

On a different codebase, but this time proof of concept and open-source, I've also found downcasting useful for passing along typed status update messages that get formatted in the way most appropriate for the target environment by just switching between custom logger implementations.

I've scoured the repository for any issues or commits explaining the rationale for removing Value downcasting but couldn’t find anything beyond a suggestion to file an issue if this is a feature "you rely on". Therefore, I’m opening this issue to understand the context of its removal and to inquire if adding it back is feasible. Are there any drawbacks or implementation constraints I might be overlooking?

Edit: after giving it more thought, the Sentry integration example above perhaps could be solved by using the downcast method available on dyn Error, as anyhow::Errors implement the Error trait... But that's still not helpful for the different codebase example, and not applicable when introspecting Values other than Errors is desired.

Edit 2: nevermind the edit above, anyhow::Error does not implement std::error::Error and thus cannot be downcasted like that.

@KodrAus
Copy link
Contributor

KodrAus commented Nov 5, 2024

Thanks for the note @AlexTMjugador 👍 We did a lot of discussion back and forth in PRs while pruning and stabilizing these APIs, so it probably has all ended up buried.

Downcasting support was soft-deprecated in log because it can be brittle. It is a useful tool to apply opportunistically, for example, to downcast a value to an IpAddr before attempting to parse it as one. The anyhow::Error case is also a reasonable one, although the ideal long-term solution would be to use some stable backtrace API and capture errors as dyn Error + 'static.

I'd be happy to revisit stabilizing downcasting.

@AlexTMjugador
Copy link
Author

Thanks for the reply @KodrAus, it's very appreciated!

I can understand how downcasting can indeed be brittle when considering the broader crate ecosystem, as Rust’s usual type-safety expectations could be undermined if duck typing in log record values gets overused across crates maintained by different people. If this sort of brittleness was the main concern for not stabilizing this feature, perhaps a clear warning in the docs advising crate authors to limit it to applications that control both logging and logger components or opportunistic use cases would be sufficient to mitigate any wider issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants