Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] Add support for additional log levels #3072

Open
0x221A opened this issue Sep 5, 2024 · 2 comments
Open

[Draft] Add support for additional log levels #3072

0x221A opened this issue Sep 5, 2024 · 2 comments

Comments

@0x221A
Copy link

0x221A commented Sep 5, 2024

Feature Request

Crates

  • tracing
  • tracing-core

Motivation

Currently, tracing supports five log levels: trace, debug, info, warn and error. While these levels cover most
use cases, there are scenarios where more nuanced control over log severity is needed. For example, the info level may
generate excessive noise for certain events, while warn may be too severe. Similarly, error may not prompt immediate
user action for critical events that require urgent attention.

Proposal

The additional log level will be inspired by syslog and opentelemetry because most of the applications use these
as a standard. This proposal is not going to support every log level that both projects have but going to support only "
needed" and "nice to have" levels.

Syslog supports 8 log levels from debug to emergency and OpenTelemetry supports 6 log levels from trace
to fatal4 as shown in the table below:

tracing syslog opentelemtry
emergency fatal2 - fatal4
alert
critical fatal
error error error
warn warn warn
notice info2 - info4
info info info
debug debug debug
trace trace

As shown in the table, most of the log levels between syslog and opentelemetry overlap. The only difference
is syslog has no support for trace and opentelemetry has no direct support for emergency, alert, notice but
it already provided a room for them.

So which log levels should be added? For me the most important log level that needs to be added as of now is critical
and
it is nice to have a notice level. The level above critical seems unnecessary because it's rarely used and the level
below notice is already supported by opentelemetry.

Why notice and critical?

  • notice: can be used for events that are significant but do not require immediate attention. This would fall
    between info and warn, highlighting noteworthy occurrences without generating excessive noise like info.
  • critical: can be used for events that require immediate user action. This would fall above error, indicating a
    more urgent situation where a failure or issue could have severe consequences if not addressed quickly.

How to implement

As I discussed with @hds in the tracing-dev discord channel, the implementation of this feature is likely to be messy
because the Level definition is used in many places in the codebase and third-party crates. This change will surely
make the codebase that relies on the Level definition such as the project that uses match expression to match the log
level to be broken. So, the best way to implement this feature as of now is to hide it under a cfg flag. For the crate
authors that want to support this feature, the crate author needs to hide this feature under the same cfg flag.

We will discuss the use of cfg flags and what needs to be changed later in the comments.

@0x221A 0x221A changed the title Add support for additional log levels [Draft] Add support for additional log levels Sep 5, 2024
@mladedav
Copy link
Contributor

mladedav commented Sep 5, 2024

I think this relates to this issue and comment.

Personally, I'm not convinced the additional levels are necessary, but focusing purely on the technical side:

I think the implementation with cfg would be too messy. I'm not sure if you mean cargo features or rustc flags, but:

  • if cargo features, then you need to enable the feature on each crate that would use the additional fields. There's no way I know of to turn the feature on in tracing and then have it turned on on all dependents (like tracing-tree/tracing-opentelemetry, any other subscriber). And if one crate doesn't have the feature enabled (or implemented), it would not compile.
  • if rustc flags, then (I think) you can turn it on globally, but still every single library that matches over Level needs to be updated and be ready for this and the whole ecosystem must use the same flag. That would be a lot of effort for two more levels. And again, if a crate uses Level and doesn't implement this, it would not compile.

An alternative would be to add this in a new breaking version. There's been talks of a breaking version for a few years and I wouldn't expect it anytime soon so if you want this early, that might not be viable. But I think that's the most likely way to get this into tracing itself.

What I think you could do right now is have a separate field like info!(levelOverride = "notice", "this is a notice log"); that subscribers can expect and then treat the log differently based on that. If a subscriber doesn't know about this (or it doesn't make sense to support it there), it will simply use the actual level. All tracing internal handling for fast filters would still work the same way. And you could add notice!(...) and critical!(...) macros that just add the custom field. As this whole thing would be basically an extension, I don't think it would live in tracing itself so that would hinder ecosystem-wide adoption, but it's just an idea.

@0x221A
Copy link
Author

0x221A commented Sep 5, 2024

I think the implementation with cfg would be too messy. I'm not sure if you mean cargo features or rustc flags, but:

In this case, I’m talking about rustc flags. I get that every library using Level would need to be updated, but adding this under an unstable flag could be a good way to go. This means not every crate has to support it right away, but it leaves room for future updates. It gives people time to adopt it without rushing full support across the board from day one.

An alternative would be to add this in a new breaking version.

I don’t think that will happen anytime soon, so I won't wait for that to happen.

What I think you could do right now is have a separate field like info!(levelOverride = "notice", "this is a notice log");

The problem with that approach is how filtering would work. The current filtering system relies on the Level enum, so if you filter out the info level, this notice log could also get filtered out unintentionally. This makes it hard to manage log levels properly, so I don’t think that solution would work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants