-
Notifications
You must be signed in to change notification settings - Fork 707
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
implement enabled!
#1821
implement enabled!
#1821
Changes from 33 commits
0b4c3ac
12474e1
5ccc947
289bfed
04b7fcc
7d5831d
b3d042d
ac87e42
9b09230
7f9aee9
dee8846
14d8137
9a1942f
147d793
48240a1
133154b
27c3633
d88df7c
a0c69eb
55ee5c2
dd563c3
61dbf07
ad24e27
0b26585
18ae036
b7f9ada
8e68af0
0bee395
f68cc1f
93657b7
405e61c
b620da7
9395dc1
50a2c87
9cb95d5
f993288
fbe5f84
3cf628a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -783,6 +783,123 @@ macro_rules! event { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Checks whether a span or event is enabled depending on the provided metadata. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// This macro is a specialized tool: it is intended to be used prior | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// to an expensive computation required *just* for that event, but | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// *cannot* be done as part of an argument to that event, such as | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// when multiple events are emitted (e.g., iterating over a collection | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// and emitting an event for each item). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// ## Usage | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// It is possible for `enabled!` to return a false positive or false negative. This might | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// occur when a subscriber is using a _more specific_ filter than what was (or could be) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// provided to `enabled!`. Below are several examples where this might occur: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// - If a subscriber is using a filter which may enable a span or event based | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// on field names, but `enabled!` is invoked without listing field names, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// `enabled!` may return a false negative if a specific field name would | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// cause the subscriber to enable something that would otherwise be disabled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// - If a subscriber is using a filter which enables or disables specific events by | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// file path and line number, a particular event may be enabled/disabled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// even if an `enabled!` invocation with the same level, target, and fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// indicated otherwise. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// - The subscriber can choose to enable _only_ spans or _only_ events, which `enabled` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// will not reflect. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// ## Examples | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// `enabled!()` requires a level argument, an optional `target:` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, take it or leave it: i might consider adding a link
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// argument, and an optional set of fields. If the fields are not provided, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: might be more explicit to say field names here, so that we don't incorrectly suggest you could pass field values
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// they are considered to be unknown. `enabled!` attempts to match the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// syntax of `event!()` as closely as possible, which can be seen in the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// examples below. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// ```rust | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// use tracing::{enabled, Level}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// # fn main() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // If the underlying collector is interested in recording | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // DEBUG-level spans and events, this will evaluate to true. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// if enabled!(Level::DEBUG) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // some expensive work... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // If the underlying collector is interested in recording spans and events | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // with the target "my_crate" at the level DEBUG, this will evaluate to true. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// if enabled!(target: "my_crate", Level::DEBUG) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // some expensive work... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // If the underlying collector is interested in recording spans and events | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // with the target "my_crate", at the level DEBUG, and the field name "hello", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // this will evaluate to true. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: i don't love "underlying" here. how about "current" or "default":
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// if enabled!(target: "my_crate", Level::DEBUG, hello) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// // some expensive work... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// # } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// [`log::log_enabled`]: https://docs.rs/log/0.4.14/log/macro.log_enabled.html | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// [`Targets`]: https://docs.rs/tracing-subscriber/0.3.5/tracing_subscriber/filter/targets/struct.Targets.html | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// [`filter_fn`]: https://docs.rs/tracing-subscriber/0.3.5/tracing_subscriber/filter/fn.filter_fn.html | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// [`EnvFilter`]: https://docs.rs/tracing-subscriber/0.3.5/tracing_subscriber/struct.EnvFilter.html | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#[macro_export] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
macro_rules! enabled { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(target: $target:expr, $lvl:expr, { $($fields:tt)* } )=> ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if $crate::level_enabled!($lvl) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use $crate::__macro_support::Callsite as _; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: concat!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"enabled ", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
file!(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
":", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
line!() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+869
to
+874
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, i wonder if we should also add the option to pass a |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kind: $crate::metadata::Kind::EVENT, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
hawkw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: $target, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
level: $lvl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fields: $($fields)* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let interest = CALLSITE.interest(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if !interest.is_never() && CALLSITE.is_enabled(interest) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let meta = CALLSITE.metadata(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$crate::dispatch::get_default(|current| current.enabled(meta)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Just target and level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(target: $target:expr, $lvl:expr ) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$crate::enabled!(target: $target, $lvl, { }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// These two cases handle fields with no values | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(target: $target:expr, $lvl:expr, $($field:tt)*) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$crate::enabled!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: $target, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$lvl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ $($field)*} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
($lvl:expr, $($field:tt)*) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$crate::enabled!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: module_path!(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$lvl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ $($field)*} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Simplest `enabled!` case | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
( $lvl:expr ) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$crate::enabled!(target: module_path!(), $lvl, { }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Constructs an event at the trace level. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// This functions similarly to the [`event!`] macro. See [the top-level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// liballoc is required because the test subscriber cannot be constructed | ||
// statically | ||
#![cfg(feature = "alloc")] | ||
|
||
mod support; | ||
|
||
use self::support::*; | ||
use tracing::Level; | ||
|
||
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] | ||
#[test] | ||
fn level_and_target() { | ||
let (collector, handle) = collector::mock() | ||
.with_filter(|meta| { | ||
if meta.target() == "debug_module" { | ||
meta.level() <= &Level::DEBUG | ||
} else { | ||
meta.level() <= &Level::INFO | ||
} | ||
}) | ||
.done() | ||
.run_with_handle(); | ||
|
||
tracing::collect::set_global_default(collector).unwrap(); | ||
|
||
assert!(tracing::enabled!(target: "debug_module", Level::DEBUG)); | ||
assert!(tracing::enabled!(Level::ERROR)); | ||
assert!(!tracing::enabled!(Level::DEBUG)); | ||
handle.assert_finished(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thought I had: a
Subscriber
may have a filter that chooses to enable only spans with a particular metadata, or only events with particular metadata. Having aHINT
callsite limits specificity with theenabled!
macro, as we don't get a way to ask "would you enable a span in this module with the fieldsfoo
andbar
?" or "would you enable an event with those fields?"I wonder if we want to change the internal representation of callsite kinds to permit a callsite to be a hint and a span or an event. We could do that by changing the hint state to be a separate flag, like:
Alternatively, if we wanted to avoid making the struct a bit larger, we could implement the same thing by changing the internal representation to bit flags:
This might be a bit less clear, though.
In either case, we can address this in a follow-up branch, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite interesting! I agree follow-up branch is better for that kind of change!