tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.43k stars 714 forks source link

Confusion when reading doc examples for MakeWriterExt in tracing-subscriber #2819

Open yorkz1994 opened 10 months ago

yorkz1994 commented 10 months ago

Bug Report

Version

tracing-subscriber v0.3.18

Platform

any

Crates

tracing-subscriber in doc.rs

Description

Enter your issue details below this comment.

One way to structure the description:

I read this code: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/writer/trait.MakeWriterExt.html#examples and https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/writer/trait.MakeWriterExt.html#examples-1 In the comment, it said for example the first one about `with_max_level`: ```// Construct a writer that outputs events to `stderr` only if the span or // event's level is **>=** WARN (WARN and ERROR).``` I assume here **>=** `WARN` should be **<=**, because `WARN` and `ERROR` should be less or equal than `WARN`? The same applies to the second example where **<=** should be **>=**?
kaffarell commented 10 months ago

No, the docs are correct. If you look at the code, you can find this enum:

#[repr(usize)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
enum LevelInner {
    /// The "trace" level.
    ///
    /// Designates very low priority, often extremely verbose, information.
    Trace = 0,
    /// The "debug" level.
    ///
    /// Designates lower priority information.
    Debug = 1,
    /// The "info" level.
    ///
    /// Designates useful information.
    Info = 2,
    /// The "warn" level.
    ///
    /// Designates hazardous situations.
    Warn = 3,
    /// The "error" level.
    ///
    /// Designates very serious errors.
    Error = 4,
}

Level also implements PartialEq, and casts LevelInner as usize, thus comparing the above values.

yorkz1994 commented 10 months ago

No, PartialEq is not the same as PartialOrd. Here: https://docs.rs/tracing-core/0.1.32/tracing_core/metadata/struct.Level.html It clearly said:

use tracing_core::Level;

assert!(Level::TRACE > Level::DEBUG);
assert!(Level::ERROR < Level::WARN);
assert!(Level::INFO <= Level::DEBUG);
assert_eq!(Level::TRACE, Level::TRACE);

Also you can see the Ord impl for Level is inversed compare, i.e. other compare self instead of self compare other:

impl PartialOrd<LevelFilter> for Level {
    #[inline(always)]
    fn partial_cmp(&self, other: &LevelFilter) -> Option<cmp::Ordering> {
        Some(filter_as_usize(&other.0).cmp(&(self.0 as usize)))
    }
...
kaffarell commented 10 months ago

Oops, overlooked that, sorry :) You are right the <= and >= is technically reversed. It's kinda counterintuitive, but the 'accepted' levels in the brackets should make it clear I think.

yorkz1994 commented 10 months ago

Since this is a document bug, could you please correct it. That is the reason why I opened this issue.