Do you use LevelFilter with `<` or `<=`?

[CAD97]

Bug Report

Version

│   └── tracing v0.1.32
│       ├── tracing-attributes v0.1.20 (proc-macro)
│       └── tracing-core v0.1.23
├── tracing v0.1.32 (*)
├── tracing-appender v0.2.1
│   └── tracing-subscriber v0.3.9
│       ├── tracing-core v0.1.23 (*)
│       └── tracing-log v0.1.2
│           └── tracing-core v0.1.23 (*)
└── tracing-subscriber v0.3.9 (*)

Platform

Microsoft Windows Version 21H2 (OS Build 19044.1586)

Crates

tracing-core

Description

When checking if a level is (statically) enabled, is it Level::TRACE < STATIC_MAX_LEVEL or Level::TRACE <= STATIC_MAX_LEVEL?

LevelFilter docs:

If a Level is considered less than a LevelFilter, it should be considered enabled; if greater than or equal to the LevelFilter, that level is disabled. See LevelFilter::current for more details.

LevelFilter::current docs:

If a given span or event has a level higher than the returned LevelFilter, it will not be enabled. However, if the level is less than or equal to this value, the span or event is not guaranteed to be enabled; the subscriber will still filter each callsite individually.

The type docs seem to say that is_enabled = level < filter and is_disabled = level >= filter.

The method docs seem to say that is_disabled = level > filter and is_enabled = level <= filter.

Which one is it?

[j4nk3e]

After reading the docs again, the only thing that makes sense is the 2nd one.
Otherwise, the LevelFilter::OFF would be useless since it would have the same impact as LevelFilter::Error and it would be impossible to have a filter that let's everything through:
https://github.com/tokio-rs/tracing/blob/68f830d392b3b2fc07704d107d223414a1291da4/tracing-core/src/metadata.rs#L231-#L233

[djc]

FWIW, I also got mighty confused with the documentation for LevelFilter. Would be nice if that was fixed.

[maddiemort]

This inconsistency confused me today too. It seems like the docs for LevelFilter were incorrectly updated in response to #1669. To clarify:

#1669 pointed out that the docs said:

• level < filter -> disabled
• level >= filter -> enabled

The docs were updated to say:

• level < filter -> enabled
• level >= filter -> disabled

...but I think they should have been updated to say:

• level <= filter -> enabled
• level > filter -> disabled

[loganmzz]

Just few notes:

• level word is little bit confusing, but still catchable, here. May be effective, actual, current or recorded would have been better. And I also have changed from filter to filtering :)

• Having recorded <= filtering = enabled seems too much close to internal representation (I suppose, not checked). In common opinion, levels are sorted from "criticity": TRACE (smallest) to ERROR (highest). ALL and OFF are just special meanings. It could be interpreted as negative and positive infinities (aka unreachable, aka uninstantiable).

[loganmzz]

Ok, I haven't checked PartialOrd impl. I agree with @nerosnm !