Consider bringing back `kv` `Value` downcasting #641

New issue

Open

opened 2024-10-29 08:29:39 -05:00 by AlexTMjugador · 2 comments

AlexTMjugador commented

2024-10-29 08:29:39 -05:00

(Migrated from github.com)

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:

github.com/rust-lang/log@96dbf58ec0/src/kv/value.rs (L1100-L1107)

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.

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: https://github.com/rust-lang/log/blob/96dbf58ec05e844ec55aa2282f32f612372fbced/src/kv/value.rs#L1100-L1107 In my view, there are compelling use cases for downcasting log record `Value`s: that enables different software components to process them in a loosely coupled way, effectively repurposing log `Record`s as a versatile, typed message-passing mechanism. For instance, imagine integrating an existing codebase that logs `anyhow::Error`s with [`sentry`](https://docs.rs/sentry/latest/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: ```rust 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`](https://docs.rs/sentry-log/0.34.0/src/sentry_log/logger.rs.html#130-139) that retrieves the `error` value from the record, downcasts it to an `anyhow::Error`, calls the [`backtrace()`](https://docs.rs/anyhow/latest/anyhow/struct.Error.html#method.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](https://github.com/ComunidadAylas/PackSquash/blob/1b46499d1524ab34d1f3af2c97c9c92754d0a069/packages/packsquash_cli/src/bin/packsquash/main.rs#L332-L339). 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::Error`s implement the `Error` trait... But that's still not helpful for the different codebase example, and not applicable when introspecting `Value`s other than `Error`s is desired. Edit 2: nevermind the edit above, `anyhow::Error` does not implement `std::error::Error` and thus cannot be downcasted like that.

KodrAus commented

2024-11-05 16:44:45 -06:00

(Migrated from github.com)

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.

Thanks for the note @AlexTMjugador :+1: 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.

❤️ 1

AlexTMjugador commented

2024-11-06 07:52:42 -06:00

(Migrated from github.com)

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?

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?