Obtain an owned Record?

[tisonkun]

When I'm trying to implement fast/logforth#145 I noticed that Record<'a> has a lifetime argument and thus can be hardly sent to another thread.

In spdlog-rs @SpriteOvO defineds its own Record and RecordOwned to support AsyncSink. spdlog-rs also supports a bridge to this log crate, but it comes with some data loss (https://github.com/SpriteOvO/spdlog-rs/blob/654b440d/spdlog/src/record.rs#L115-L148), mainly the args get resolved and we can't revert it to fmt::Arguments<'a>.

Is there any possible way we get an owned Record? Otherwise it's impossible to implmement:

impl Append for AsyncAppender {
  fn append(&self, record: &Record, ...) -> .. { ... }
}

impl Append for RollingFileAppender {
  fn append(&self, record: &Record, ...) -> .. { ... }
}

... and combine them together.

cc @KodrAus @Thomasdezeeuw

[tisonkun]

Other fields seems easy to get an owned value, the hard part includes:

• args: fmt::Arguments<'a>
• key_values: KeyValues<'a>

For args, I may call args.to_string() and reconstruct it with format_args!.

For key_values, I have no idea yet.

And I wonder if this is reasonable, it's possible to provide a Record::to_owned at upstream so that we maintain the compatibility.

[SpriteOvO]

For key_values, I have no idea yet.

FYI, spdlog-rs has implemented kv feature in dev branch (not released yet), and the solution for this is... also an owned version of key-value struct.

https://github.com/SpriteOvO/spdlog-rs/blob/6b3cb89bdc5202cbcb8c5da59a8ff02e27f7a132/spdlog/src/record.rs#L143

[tisonkun]

Thanks a lot @SpriteOvO!

https://github.com/SpriteOvO/spdlog-rs/blob/6b3cb89bdc5202cbcb8c5da59a8ff02e27f7a132/spdlog/src/kv.rs would be valuable as a reference and fortunately we can start with value_bag's abstraction. Let me see if I can make a RecordOwned in logforth's scope.

[tisonkun]

So far, I'm considering wrapping my own Record and RecordOwned type over log::Record. Besides an owned Record, since an Async logger combinator passes the record to another thread, and the final logging time would be biased, it would be better to add a time field.

Not sure how long we can get these proposals processed in this crate.

[Thomasdezeeuw]

I've been thinking about this for a bit and wanted to bring up some points of discussion.

First, I don't think it's possible to always provide some kind of ToOwned implementation. Take file descriptors for example, should we be calling dup(2) on those to provide an OwnedRecord? That seems like a mistake to me. Though I fully acknowledge this a contrived example (though I have logged file details like the file descriptor (number) in the past).

Second, as you already pointed out, you could format the record inline of the Log::log call and send the buffer across the thread or write it later as necessary.

Finally, and this is off topic, I recommend you do not use asynchronous logging. For the simple reason that when you want your logs the most is when something went badly wrong, but you don't know what because the relevant logs haven't been written yet 🤷

[tisonkun]

Finally, and this is off topic, I recommend you do not use asynchronous logging. For the simple reason that when you want your logs the most is when something went badly wrong, but you don't know what because the relevant logs haven't been written yet

Reasonable. Let me try to drop the async/nonblocking impls and see the results.

Typically, it's added when the log appender sinks to a file or socket that if we use a mutex globally, it may impact the app performance significantly.

[KodrAus]

I think we're unlikely to support an owned record type in log directly. log's API is centred around its macros, and its responsibility largely ends at the point a Log is handed a Record.

One of the issues with owned values is it's harder to abstract the kind of storage than it is with references, which is something you likely do want some control over. In value-bag for instance, it encapsulates the choice of Box and Arc, but you could also reasonably want Rc for thread-local storage or something else entirely. value-bag is an internally pretty complex library, because it tries to bridge multiple strategies together into a single API. Here's what the internals of an owned value look like for instance.

If your framework settles on a single internal value representation, like Arc<dyn Valuable> with primitives stored inline then it can be simpler than value-bag. sval is a library I wrote around the same time as valuable that tries to solve a similar set of tasks, but with a focus on being buffer-friendly. serde, unfortunately, is not really buffer-friendly, because it lets you choose between text and binary formats, but you don't know what you'll be asked to produce at the point you need to buffer. I think valuable types can be reasonably buffered too.

So all that being said, even though I think owned records are a feature you'll likely end up needing if you're building a logging framework, I don't think it's one that log can provide a good solution for itself without changing its scope.

[tisonkun]

Thanks for your input.

According to the investigation in fast/logforth#166, I realized that the key point is how to store and convey Values in kv for a thirdparty log implementation.

So far, I'd enable log's value-bag and sval feature flags so that I can store the value "naturally" with value_bag::Value::from_sval2(log_record_kv_value). Not sure if the log crate can use that as the default representation. Or otherwise downstream impls must choose between:

1. Depend on kv_sval feature flag and its impls
2. Introduce extra burden to convert the value to their own value; even more complex because you'd consider log's user may enable kv_serde feature flag so that impl like spdlog's this snippet can lose info, resulting in native support for log's kv impossible.

IMO the log crate should determinate its key-values repr so that downstream impls can depend on. At least how downstream impls can store and convey easily.

[tisonkun]

Looks like turning on kv_sval is the way to go. However, I wonder if sval is already a lightweight dependency, can we use it as the KV repr so simplify the feature matrix and code (we now have a handmade Value impl if sval is opt-out)?