-
Notifications
You must be signed in to change notification settings - Fork 734
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
Merged
Merged
implement enabled!
#1821
Changes from 36 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
0b4c3ac
implement `enabled!`
guswynn 12474e1
simplify the macro
guswynn 5ccc947
remove new event methods
guswynn 289bfed
fix Kind::HINT's doc's
guswynn 04b7fcc
Merge branch 'master' into enabled
hawkw 7d5831d
implement `enabled!`
guswynn b3d042d
simplify the macro
guswynn ac87e42
remove new event methods
guswynn 9b09230
fix Kind::HINT's doc's
guswynn 7f9aee9
simplify dot cases
guswynn dee8846
Merge branch 'master' into enabled
davidbarsky 14d8137
journald: Set syslog identifier (#1822)
swsnr 9a1942f
docs: don't state that `tracing-futures` is required (#1827)
hawkw 147d793
simplify dot cases
guswynn 48240a1
subscriber: fix leading comma with `Pretty` formatter (#1833)
hawkw 133154b
subscriber: add `Format::with_file` and `with_line_number` (#1773)
renecouto 27c3633
appender: bump MSRV to 1.53.0 (#1851)
hawkw d88df7c
subscriber: update thread_local to 1.1.4 (#1858)
matze a0c69eb
WIP docs
davidbarsky 55ee5c2
add examples
davidbarsky dd563c3
wip, sorry
davidbarsky 61dbf07
okay, it covers stuff now
davidbarsky ad24e27
whoops, typo.
davidbarsky 0b26585
rebase, i think?
davidbarsky 18ae036
fix phrasing fuckups
davidbarsky b7f9ada
Merge branch 'enabled' into david/edit-documentation-for-enabled
davidbarsky 8e68af0
implement `enabled!`
guswynn 0bee395
simplify the macro
guswynn f68cc1f
remove new event methods
guswynn 93657b7
fix Kind::HINT's doc's
guswynn 405e61c
simplify dot cases
guswynn b620da7
Merge branch 'enabled' into david/edit-documentation-for-enabled
guswynn 9395dc1
Merge pull request #1 from davidbarsky/david/edit-documentation-for-e…
guswynn 50a2c87
Apply docs suggestions from code review
hawkw 9cb95d5
fix wrong callsite kind for `enabled!`
hawkw f993288
Merge branch 'master' into enabled
hawkw fbe5f84
Update tracing/src/macros.rs
hawkw 3cf628a
bleh rustfmt
hawkw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -783,6 +783,139 @@ macro_rules! event { | |
); | ||
} | ||
|
||
/// Checks whether a span or event is [enabled] based on the provided [metadata]. | ||
/// | ||
/// [enabled]: crate::Collect::enabled | ||
/// [metadata]: crate::Metadata | ||
/// | ||
/// 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 | ||
/// | ||
/// [Collectors] can make filtering decisions based all the data included in a | ||
/// span or event's [`Metadata`]. This means that it is possible for `enabled!` | ||
/// to return a _false positive_ (indicating that something would be enabled | ||
/// when it actually would not be) or a _false negative_ (indicating that | ||
/// something would be disabled when it would actually be enabled). | ||
/// | ||
/// [Collectors]: crate::collect::Collect | ||
/// [`Metadata`]: crate::metadata::Metadata | ||
/// | ||
/// This occurs when a subscriber is using a _more specific_ filter than the | ||
/// metadata provided to the `enabled!` macro. Some situations that can result | ||
/// in false positives or false negatives include: | ||
/// | ||
/// - If a collector 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 collector to enable something that would otherwise be disabled. | ||
/// - If a collector 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 collector can choose to enable _only_ spans or _only_ events, which `enabled` | ||
/// will not reflect. | ||
/// | ||
/// `enabled!()` requires a [level](crate::Level) argument, an optional `target:` | ||
/// argument, and an optional set of field names. If the fields are not provided, | ||
/// 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. | ||
/// | ||
/// # Examples | ||
/// | ||
/// If the current collector is interested in recording `DEBUG`-level spans and | ||
/// events in the current file and module path, this will evaluate to true: | ||
/// ```rust | ||
/// use tracing::{enabled, Level}; | ||
/// | ||
/// if enabled!(Level::DEBUG) { | ||
/// // some expensive work... | ||
/// } | ||
/// ``` | ||
/// | ||
/// If the current collector is interested in recording spans and events | ||
/// in the current file and module path, with the target "my_crate", and at the | ||
/// level `DEBUG`, this will evaluate to true: | ||
/// ```rust | ||
/// # use tracing::{enabled, Level}; | ||
/// if enabled!(target: "my_crate", Level::DEBUG) { | ||
/// // some expensive work... | ||
/// } | ||
/// ``` | ||
/// | ||
/// If the current collector is interested in recording spans and events | ||
/// in the current file and module path, with the target "my_crate", at | ||
/// the level `DEBUG`, and with a field named "hello", this will evaluate | ||
/// to true: | ||
/// | ||
/// ```rust | ||
/// # use tracing::{enabled, Level}; | ||
/// if enabled!(target: "my_crate", Level::DEBUG, hello) { | ||
/// // some expensive work... | ||
/// } | ||
/// # } | ||
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::HINT, | ||
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 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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!