tracing::instrument decorator should require a level argument.

[Hoverbear]

Feature Request

Crates

tracing::instrument

Motivation

The tracing::instrument proc macro is incredibly useful, and the docs encourage users to use it, as well as set a level. Not all users read documentation, so many use defaults. (Like me)

By default, they seem to decorate at all logging levels, which can impact performance for users in unexpected ways.

We met this problem in Vector recently, we merged vectordotdev/vector#4485 which introduced tracing::instrument on some hot paths, and, since we didn't pick a level, this lead to a 5x slowdown of the overall code along many of our topologies. We only caught it after during testing with bench users. (Our recent CI migration had benching out of commission, we're feeling that bad right now!)

Proposal

I propose #[tracing::instrument(level = "something")] be the required minimal syntax. This forces users to understand the performance characteristics of the tool and make a deliberate choice.

Alternatives

We could choose a default level, but someone will always complain.

We could not change. There is no harm in this.

We could update the docs to better highlight this issue. (This should probably be done regardless)

[hawkw]

IIRC, when the macro was initially prototyped, it defaulted to the TRACE level, and then folks asked to change it to INFO. I think that having a default level was probably a mistake --- no matter what the default is, I think it's going to surprise someone. I'm +1 on removing the default in tracing-attributes 0.2.

I think it would be nice to be able to use the attributes without required arguments, though, so maybe we should consider introducing leveled shorthand attributes (similar to the trace_span!, debug_span!, info_span!, warn_span!, and error_span! function-like macros). The hardest problem there is figuring out what their names should be --- instrument_debug and similar feels like a lot to type, enough that i'm not sure if it's any better than just taking a level argument. Ideally, we would be able to use the same names as the function-like macros (e.g. #[tracing::debug_span] and so on) but it turns out that attribute- and function-like macros share one namespace, so we can't really do that.

Of course, we can add leveled shorthand macros separately from removing the default level, so figuring out the right naming isn't a blocker.

[hawkw]

As a side note, removing the default level should be pretty simple, so if anyone is interested in helping contribute to 0.2, I'd be happy to provide guidance.

[Hoverbear]

I found myself kind of wishing I could specify level on a per-field basis in the tracing::instrument proc macro. But the syntax sounds not awesome.

[psinghal20]

Hey @hawkw, I would love to take up this issue. I have added a compile error in case of no level specified in the instrument attribute. Let me know if I should take up shorthands alongside this change. Also, Can you direct me to all the places where I need to update documentation and examples?

[Stargateur]

IIRC, when the macro was initially prototyped, it defaulted to the TRACE level, and then folks asked to change it to INFO. I think that having a default level was probably a mistake --- no matter what the default is, I think it's going to surprise someone. I'm +1 on removing the default in tracing-attributes 0.2.

I really don't understand how something that literally trace a function call on a crate named tracing default to info level.

I agree with remove the default. And I agree about having alias.

instrument is indeed quite long.

We could rename to tracing, having a macro as the same name that a crate name is common and for me it's express very well the purpose of the macro so I propose:

• tracing
• trace_tracing <= meh
• debug_tracing
• info_tracing
• warn_tracing <= is this really make sense ? maybe not necessary
• error_tracing <= same ^

or short instrument

• instrument
• trace_instru
• debug_instru
• info_instru
• warn_instru
• error_instru

for completeness:

• trace_instr
• debug_inst
• info_ins
• warn_in
• error_i