Implement layer::Filter for Option<Filter>
#2402
Conversation
Pass all the Filter methods through to the underlying Box/Arc Filter rather than relying on default implementations.
Where None means "no filtering".
jsgf
force-pushed
the
option-layer-filter-0.1
branch
from
fc5d0d4
to
d2c7a43
Compare
jsgf
force-pushed
the
option-layer-filter-0.1
branch
from
02f1f13
to
0ea1a27
Compare
jsgf
changed the title
jsgf
changed the title
layer::Filter for Option<Filter>
There was a problem hiding this comment.
I am broadly in favor of this change (as a concrete example, Option<EnvFilter> will now be filter, where Option::None means that there is no filter enabled), but I think we should add documentation to https://docs.rs/tracing-subscriber/latest/tracing_subscriber/layer/index.html#per-layer-filtering that notes the implications of an Option-enabled filter. I can write those docs in a followup/on this branch.
While we're here, would you be able to add tests for a None filter? it's a bit silly, but ensuring that it's equivalent to not calling with_filter at all would help me get confidence in this, especially since layers/filters are kinda hairy!
| @@ -134,7 +134,7 @@ fn gen_block<B: ToTokens>( | |||
| .into_iter() | |||
| .flat_map(|param| match param { | |||
| FnArg::Typed(PatType { pat, ty, .. }) => { | |||
| param_names(*pat, RecordType::parse_from_ty(&*ty)) | |||
There was a problem hiding this comment.
thanks for addressing the clippy lints.
Make sure None lets everything through.
|
Closing in favour of #2407 which is based on master |
Summary: Import tokio-rs/tracing#2402 until it lands. Reviewed By: zertosh Differential Revision: D41689583 fbshipit-source-id: f1923244fcc652ba4dfcd6064aa37926a2b79bc7
Summary: Import tokio-rs/tracing#2402 until it lands. Reviewed By: zertosh Differential Revision: D41689583 fbshipit-source-id: f1923244fcc652ba4dfcd6064aa37926a2b79bc7
Summary: Import tokio-rs/tracing#2402 until it lands. Reviewed By: zertosh Differential Revision: D41689583 fbshipit-source-id: f1923244fcc652ba4dfcd6064aa37926a2b79bc7
Motivation
It's currently awkward to have an optional layer filter.
Solution
Implement
Filter<S>forOption<F> where F: Filter<S>, following the example ofLayer. ANonelayer filter passes everything through.Also, it looks like Filter for Arc/Box doesn't pass through all the methods, so extend the
filter_impl_bodymacro to include them.