docs: add suggested documentation #1
Conversation
`enabled!` is similar to `event!`, but never dispatches an event. It also allows a simplistic version of calling it, filling in a fake message in the metadata.
There was a problem hiding this comment.
some notes:
-
there's a weird paragraph about
enabled!being an alternative to using filtering, which doesn't make any sense to me?enabled!is used with filtering, it only returns whatever the filter says. -
can we actually discuss the behavior of the enabled macro with and without fields in the documenation, as well as in examples?
tracing::enabled!()would require a level argument, an optionaltarget:argument, and then an optional set of fields. If the fields are not provided, they are considered unknown. In the interests of matching theevent!()macro as closely as possible, this might look liketracing::enabled!(target: "foo", Level::INFO, message, request)if we know the fields, ortracing::enabled!(target: "foo", Level::INFO)if we don't. -
the documentation needs to state clearly and explicitly that a
Subscribermay be using a filter which may enable a span or event based on field names, so ifenabled!is invoked without listing field names, it may return a false negative if a specific field name would cause the subscriber to enable something that would otherwise be disabled. -
the documentation needs to state clearly and explicitly that a
Subscribermay be using a filter which enables or disables specific events by file path and line number, in which case, a particular event may be enabled/disabled even if anenabled!invocation with the same level, target, and fields indicated otherwise. -
the documentation should state that a subscriber may choose to enable only spans or only events, which
enabled!will not reflect (because the metadata it tests will have theHINTcallsite kind, an implementation detail the docs don't need to mention) -
We should also note that
enabled!()should be called from within the same file and module as theevent!()call(s) it guards, as that info is in the metadata and therefore available to filters.(specifically, this notes that file and module path are provided separately from the target, and the subscriber might care about them)
we'll also want to improve the documentation for Subscriber implementations on how the HINT callsite kind should be handled, as discussed in tokio-rs#1668:
The
metadata::Kindfield would have a new variantHINTthat indicates this is anenabled!()call. Additionally, thelinefield would have the valueNone(as the enabled call is unlikely to be the same line as the event). Subscribers must be aware ofHINTif they care about fields or line, such that they treatline: Noneas representing any potential line, and they treat an emptyFieldSetas meaning the fields are unknown.register_callsitecan also useHINTto determine that no events will be logged against this callsite and thus avoid doing things like preallocating storage for it; the call here is only to get anInterest, not to prepare for emitting events.
but, that part ca be done in a separate PR.
tracing/src/macros.rs
Outdated
| /// it is recommended that it be used sparingly. In most cases, a centralized | ||
| /// filtering approach such as [`Targets`], [`filter_fn`], or [`EnvFilter`] will provide | ||
| /// similar performance and greater ease-of-use. Similarly, while `enabled!` supports |
There was a problem hiding this comment.
i don't understand this? enabled! asks the filter (which could be an EnvFilter, FilterFn, etc) if it would enable a span/event with particular metadata. it's not a replacement for filtering?
`enabled!` is similar to `event!`, but never dispatches an event. It also allows a simplistic version of calling it, filling in a fake message in the metadata.
Set the `SYSLOG_IDENTIFIER` field on all events. I noticed that the subscriber didn't do this so far. ## Motivation The identifier is used with `journalctl -t`, and while it's normally better to filter by unit with `journalctl -u` the identifier is still nice for processes that are not started through systemd units. Upstream does this as well, see [here]:  `program_invocation_short_name` is a glibc variable which holds the file name of the current executable; I tried to replicate this behaviour in Rust. ## Solution Add a syslog identifier field to the subscriber, defaulting to the filename of the current executable, and write this value as `SYSLOG_IDENTIFIER` with every event. It's not written for spans, because it's a global value and inevitably the same for each span. [here]: https://github.com/systemd/systemd/blob/81218ac1e14b4b50b4337938bcf55cacc76f0728/src/libsystemd/sd-journal/journal-send.c#L270
Currently, the documentation states in a few places that the `tracing-futures` crate is required for asynchronous code to use `tracing`. This is no longer the case, as `tracing` provides the `Instrument` combinator for futures; `tracing-futures` is only needed for things defined in the `futures` crate, such as `Stream` and `Sink`. This branch updates the documentation so that it doesn't incorrectly state that `tracing-futures` is required.
## Motivation PR tokio-rs#1661 introduced a regression with the `Pretty` formatter: the `PrettyVisitor` type was [accidentally changed][1] from being constructed with `is_empty: true` to being constructed with `is_empty: false` This means that when visiting a set of span fields, we emit a leading `, ` _before_ the first field, which looks quite bad. ## Solution This branch changes it back, and now the output looks nice again. :) ### Before ``` 2022-01-13T17:09:04.772411Z TRACE fmt_pretty::yak_shave: hello! I'm gonna shave a yak, excitement: "yay!" at examples/examples/fmt/yak_shave.rs:16 on main in fmt_pretty::yak_shave::shave with , yak: 2 in fmt_pretty::yak_shave::shaving_yaks with , yaks: 3 ``` ### After ``` 2022-01-13T17:10:28.472525Z TRACE fmt_pretty::yak_shave: hello! I'm gonna shave a yak, excitement: "yay!" at examples/examples/fmt/yak_shave.rs:16 on main in fmt_pretty::yak_shave::shave with yak: 1 in fmt_pretty::yak_shave::shaving_yaks with yaks: 3, ``` Fixes: tokio-rs#1832 [1]: tokio-rs@937c5d7#diff-a27a4c3564a0c2f1b7af32be0f9eec25ddfbe8b4c2be8d74e84d874b919b393bR227
…1773) ## Motivation Logging line numbers and file names can be useful for debugging. This feature was suggested by tokio-rs#1326 ## Solution As per @hawkw's suggestions, fields were added on `Format`, along with builder methods. Filename and line number information was gathered from the `meta` variable. The `Pretty` formatter already supports printing source locations, but this is configured separately on the `Pretty` formatter rather than on the `Format` type. This branch also changes `Pretty` to honor the `Format`-level configurations and deprecates the `Pretty`-specific method. Fixes tokio-rs#1326 Closes tokio-rs#1804 Co-authored-by: David Barsky <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
The `time` crate bumped its MSRV to 1.53.0 in v0.3.6: time-rs/time@2d37c01 Since `tracing-appender` has a non-optional dependency on `time`, it's necessary to increase its MSRV to track this.
Fixes https://rustsec.org/advisories/RUSTSEC-2022-0006. ## Motivation `cargo audit` prevents us using `tracing-subscriber` at the moment. Upgrading `thread_local` to 1.1.4 would fix that. ## Solution Upgrade `thread_local`.
# Conflicts: # tracing/src/macros.rs
|
Ill rebase my branch onto tracing master and merge this in! |
## Motivation In my library I define a `macro_rules! concat` macro, i.e. [`callbag::concat`](https://docs.rs/callbag/latest/callbag/macro.concat.html). When I try to call `tracing::info!(...)`, I get error output such as this: <details> <summary>error output</summary> <!-- leave a blank line above --> ``` > RUSTFLAGS='-Z macro-backtrace' cargo +nightly clippy --features trace Checking callbag v0.14.0 (/home/teohhanhui/projects/teohhanhui/callbag-rs) error[E0308]: mismatched types --> src/concat.rs:89:9 | 89 | info!("from sink: {message:?}"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `u32` error[E0277]: the trait bound `std::sync::Arc<core::Callbag<never::Never, _>>: std::convert::From<&str>` is not satisfied --> src/concat.rs:58:9 | 56 | / macro_rules! concat { 57 | | ($($s:expr),* $(,)?) => { 58 | | $crate::concat(::std::vec![$($s),*].into_boxed_slice()) | | ^^^^^^^^^^^^^^ the trait `std::convert::From<&str>` is not implemented for `std::sync::Arc<core::Callbag<never::Never, _>>` 59 | | }; 60 | | } | |_- in this expansion of `concat!` (#5) ... 89 | info!("from sink: {message:?}"); | ------------------------------- in this macro invocation (#1) | ::: src/utils/tracing.rs:47:1 | 47 | / macro_rules! info { 48 | | ($($arg:tt)+) => { 49 | | ::cfg_if::cfg_if! { 50 | | if #[cfg(feature = "trace")] { 51 | | ::tracing::info!($($arg)+) | | -------------------------- in this macro invocation (#2) ... | 54 | | }; 55 | | } | |_- in this expansion of `info!` (#1) | ::: /home/teohhanhui/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1 | 586 | macro_rules! event { | _- | |_| | | 587 | | (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> ( 588 | | $crate::__tracing_log!( 589 | | target: $target, ... | 644 | name: concat!( | _______________________- 645 | "event ", 646 | file!(), 647 | ":", 648 | line!() 649 | | ), | |_________________- in this macro invocation (#5) ... 667 | / $crate::event!( 668 | target: $target, 669 | $lvl, 670 | { message = format_args!($($arg)+), $($fields)* } 671 | | ) | |_________- in this macro invocation (#4) ... 791 | | ); 792 | | } | | - | |_| | |_in this expansion of `$crate::event!` (#3) | in this expansion of `$crate::event!` (#4) ... 1229 | / macro_rules! info { 1230 | (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => ( 1231 | $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*) 1232 | ); ... 1398 | / $crate::event!( 1399 | | target: module_path!(), 1400 | | $crate::Level::INFO, 1401 | | {}, 1402 | | $($arg)+ 1403 | | ) | |_________- in this macro invocation (#3) 1404 | ); 1405 | | } | |_- in this expansion of `::tracing::info!` (#2) | = help: the following implementations were found: <std::sync::Arc<B> as std::convert::From<std::borrow::Cow<'a, B>>> <std::sync::Arc<T> as std::convert::From<T>> <std::sync::Arc<T> as std::convert::From<std::boxed::Box<T>>> <std::sync::Arc<[T]> as std::convert::From<&[T]>> and 9 others = note: required because of the requirements on the impl of `std::convert::Into<std::sync::Arc<core::Callbag<never::Never, _>>>` for `&str` note: required by a bound in `concat::concat` --> src/concat.rs:81:8 | 72 | pub fn concat< | ------ required by a bound in this ... 81 | S: Into<Arc<Source<T>>> + Send + Sync, | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `concat::concat` error[E0308]: mismatched types --> src/concat.rs:58:9 | 56 | / macro_rules! concat { 57 | | ($($s:expr),* $(,)?) => { 58 | | $crate::concat(::std::vec![$($s),*].into_boxed_slice()) | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found struct `core::Callbag` 59 | | }; 60 | | } | |_- in this expansion of `concat!` (#5) ... 89 | info!("from sink: {message:?}"); | ------------------------------- in this macro invocation (#1) | ::: src/utils/tracing.rs:47:1 | 47 | / macro_rules! info { 48 | | ($($arg:tt)+) => { 49 | | ::cfg_if::cfg_if! { 50 | | if #[cfg(feature = "trace")] { 51 | | ::tracing::info!($($arg)+) | | -------------------------- in this macro invocation (#2) ... | 54 | | }; 55 | | } | |_- in this expansion of `info!` (#1) | ::: /home/teohhanhui/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1 | 586 | macro_rules! event { | _- | |_| | | 587 | | (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> ( 588 | | $crate::__tracing_log!( 589 | | target: $target, ... | 644 | name: concat!( | _______________________- 645 | "event ", 646 | file!(), 647 | ":", 648 | line!() 649 | | ), | |_________________- in this macro invocation (#5) ... 667 | / $crate::event!( 668 | target: $target, 669 | $lvl, 670 | { message = format_args!($($arg)+), $($fields)* } 671 | | ) | |_________- in this macro invocation (#4) ... 791 | | ); 792 | | } | | - | |_| | |_in this expansion of `$crate::event!` (#3) | in this expansion of `$crate::event!` (#4) ... 1229 | / macro_rules! info { 1230 | (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => ( 1231 | $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*) 1232 | ); ... 1398 | / $crate::event!( 1399 | | target: module_path!(), 1400 | | $crate::Level::INFO, 1401 | | {}, 1402 | | $($arg)+ 1403 | | ) | |_________- in this macro invocation (#3) 1404 | ); 1405 | | } | |_- in this expansion of `::tracing::info!` (#2) | = note: expected reference `&'static str` found struct `core::Callbag<never::Never, _>` Some errors have detailed explanations: E0277, E0308. For more information about an error, try `rustc --explain E0277`. error: could not compile `callbag` due to 3 previous errors ``` </details> This is because of my `concat` macro being in scope. ## Solution This branch adds a re-export of `core::concat!` in the `__macro_support` module, and changes all the `tracing` macros to use that, rather than using an un-namespaced `concat!`. The re-export ensures that everything still works even in a crate that redefines the `core` name to something else. Co-authored-by: Eliza Weisman <[email protected]>
As mentioned over Discord, this style of documentation is closer to what tracing requires.