tracing icon indicating copy to clipboard operation
tracing copied to clipboard

Improve access to the Value of an Event from within a Subscriber

Open dannypike opened this issue 4 years ago • 4 comments

Feature Request

Crates

tracing-core v0.1.10

Motivation

The Event structure has a method called fields():

    /// Returns an iterator over the set of values on this `Event`.
    pub fn fields(&self) -> field::Iter {
        self.fields.field_set().iter()
    }

but no simple way to get access to the value for each field, when iterating them.

(Aside: please note the apparent typo in the comment - it returns an iterator over the fields, not the values!)

Proposal

To add a method that takes a Field value or, preferably, a name: &str and returns the Option<dyn Value> for the field with that name.

Alternatives

There is a visitor mechanism for the ValueSet but implementing all of the logic for using that feels like a lot of extra work (both in extra source code to write and test, and in extra bytes/CPU at runtime). Especially, if I only want the value of one field and I already know its name.

dannypike avatar Apr 19 '20 13:04 dannypike

To add a method that takes a Field value or, preferably, a name: &str and returns the Option<dyn Value> for the field with that name.

We could certainly add this. However, note that actually recording the field would still require a Visit implementation, as the Visit/Value interaction is responsible for determining how a dynamically typed value is recorded.

The main advantage of allowing field lookup like this is that, if used with a &Field rather than a &str, we could look up a single field without having to iterate over the entire value set. Looking up a field value with a &str key would first require translating that string to a field index, which would still require iterating over the FieldSet until a match is found (although if a field with that name exists, it would not necessarily need to iterate over all fields in the fieldset). See the implementation of FieldSet::field: https://github.com/tokio-rs/tracing/blob/0c01c0d3f527d131dce1c0f39f69bc71df34ea37/tracing-core/src/field.rs#L555-L567

Note, however, that because ValueSets are capped at 32 fields (and are typically much smaller than that), the performance difference between O(n) linear search and O(1) lookups is not going to be that significant. In particular, linear search may often be faster than using a hash map — despite hash map lookup being O(1), the constant factors are high enough that a linear search over a small array will still perform better.

The main advantages of having such an API will probably be ergonomics for Layers or Subscribers that only care about specific fields. Since record_debug is used as a catch-all for all visitors, we could probably add a way to record dyn Values with fmt::Debug without having to write a Visit impl.

hawkw avatar Apr 27 '20 19:04 hawkw

Thanks for your reply. I don't really understand the details of your reasoning, but that sort of demonstrates the point that I was trying to make in my original question. After being very encouraged by the excellent introduction to Rust ("The Book"), I hit a brick wall when trying to implement a real application. For me,a new application generally means that I start with getting debug logging and that's how I got to your library. After a considerable amount of time struggling with trying to understand why it was so difficult to customize, I am sorry to say that I had to give up. I don't think that my problem is restricted to tokio; it "feels" like a problem with Rust, itself. I have seen several people describing similar issues with how hard it is to do anything useful, once you have finished reading The Book.

When I am designing a library, I try to ensure it makes simple tasks easy to understand and straightforward to use. I only ask my users to get into more complex mechanisms when they want to do something that is more complex and slightly off-piste. That tokio expects me to implement a visitor pattern when I only want to get/set a property-value on an entity (object or struct or whatever) just "feels" wrong to me. Maybe it's because I am too set in my ways - I accept that there are many languages that I have not have the time to learn over the years - but, if I don't like the taste of something, I won't eat it. And, as I am the one who decides what language we will use, I am the one who has to feel comfortable.

I'm sorry to say that I'm returning to the more mature world of C/C++/C#, which has some significant risks and complexities, but those are limited to well-known areas. Those languages don't make me struggle to do something that I consider to be simple.

It's a pity, and I am sorry to be joining the growing ranks of those who have given up with this potentially great new language. I may well be back, if Rust works out how it can become more accessible to those like me.

On Mon, 27 Apr 2020 at 20:04, Eliza Weisman [email protected] wrote:

To add a method that takes a Field value or, preferably, a name: &str and returns the Option<dyn Value> for the field with that name.

We could certainly add this. However, note that actually recording the field would still require a Visit implementation, as the Visit/Value interaction is responsible for determining how a dynamically typed value is recorded.

The main advantage of allowing field lookup like this is that, if used with a &Field rather than a &str, we could look up a single field without having to iterate over the entire value set. Looking up a field value with a &str key would first require translating that string to a field index, which would still require iterating over the FieldSet until a match is found (although if a field with that name exists, it would not necessarily need to iterate over all fields in the fieldset). See the implementation of FieldSet::field:

https://github.com/tokio-rs/tracing/blob/0c01c0d3f527d131dce1c0f39f69bc71df34ea37/tracing-core/src/field.rs#L555-L567

Note, however, that because ValueSets are capped at 32 fields (and are typically much smaller than that), the performance difference between O( n) linear search and O(1) lookups is not going to be that significant. In particular, linear search may often be faster than using a hash map — despite hash map lookup being O(1), the constant factors are high enough that a linear search over a small array will still perform better.

The main advantages of having such an API will probably be ergonomics for Layers or Subscribers that only care about specific fields. Since record_debug is used as a catch-all for all visitors, we could probably add a way to record dyn Values with fmt::Debug without having to write a Visit impl.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tokio-rs/tracing/issues/680#issuecomment-620173781, or unsubscribe https://github.com/notifications/unsubscribe-auth/AELWEUIQ22TKM2VNDQ5IB2TROXJKBANCNFSM4MLZLDRA .

dannypike avatar Apr 27 '20 20:04 dannypike

To add a method that takes a Field value or, preferably, a name: &str and returns the Option<dyn Value>

So while getting value by field name doesn't make a lot of sense over iterating values, getting value by &Field makes ergonomics for subscribers/layers that only care about specific fields so much better... But I guess there wasn't much interest in this?

theli-ua avatar Aug 04 '22 22:08 theli-ua

So while getting value by field name doesn't make a lot of sense over iterating values, getting value by &Field makes ergonomics for subscribers/layers that only care about specific fields so much better... But I guess there wasn't much interest in this?

Long time no see, Anton :). There has been interest, but most of the work has gone towards rethinking how values are recorded via Valuable. If you're willing to submit a PR to the master branch that implements this feature, I'd be happy to review it.

davidbarsky avatar Aug 06 '22 16:08 davidbarsky