Skip to content
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

Impl filter for EnvFilter #1983

Merged
merged 20 commits into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
27722bc
Impl Filter for EnvFilter
tfreiberg-fastly Mar 10, 2022
76c2a3d
Update tracing-subscriber/src/filter/env/mod.rs
tfreiberg-fastly Mar 10, 2022
abc3b76
Port EnvFilter Layer tests to test EnvFilter's Filter impl
tfreiberg-fastly Mar 14, 2022
d7a9a86
Remove unused MockSpans
tfreiberg-fastly Mar 14, 2022
6b821c5
Make per_layer_filter::span_name_filter_is_dynamic pass
tfreiberg-fastly Mar 14, 2022
6f273ca
Merge remote-tracking branch 'upstream/v0.1.x' into impl-filter-for-e…
tfreiberg-fastly Mar 22, 2022
dd7f95f
Add span::named(...) function
tfreiberg-fastly Mar 22, 2022
463b0cd
Add failing test for EnvFilter's Filter impl
tfreiberg-fastly Mar 22, 2022
384601d
Merge remote-tracking branch 'upstream/v0.1.x' into impl-filter-for-e…
tfreiberg-fastly Mar 23, 2022
9bb9d1a
Minimize failing EnvFilter filter test, add succeeding global test
tfreiberg-fastly Mar 23, 2022
2e23ff5
Fix Filtered's on_exit impl delegating to the filter's on_exit
tfreiberg-fastly Mar 23, 2022
1f40475
Fix level_filter_event_with_target_and_span
tfreiberg-fastly Mar 23, 2022
e411c7e
add test for multiple dynamic per-layer EnvFilters
hawkw Mar 25, 2022
63a53fe
add test for multiple dynamic per-layer EnvFilters
hawkw Mar 25, 2022
fba65a5
use `ThreadLocal` struct in `EnvFilter`
hawkw Mar 25, 2022
ffd5900
Apply suggestions from code review
hawkw Mar 25, 2022
b769851
split per-layer filter tests into their own file
hawkw Mar 25, 2022
07bb1e0
add note on `Filter` and `Layer` impls to docs
hawkw Mar 25, 2022
5939234
Merge branch 'v0.1.x' into impl-filter-for-envfilter
hawkw Mar 25, 2022
802313c
fix unnecessary clone
hawkw Mar 25, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tracing-subscriber/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ rust-version = "1.49.0"
default = ["smallvec", "fmt", "ansi", "tracing-log", "std"]
alloc = []
std = ["alloc", "tracing-core/std"]
env-filter = ["matchers", "regex", "lazy_static", "tracing", "std"]
env-filter = ["matchers", "regex", "lazy_static", "tracing", "std", "thread_local"]
fmt = ["registry", "std"]
ansi = ["fmt", "ansi_term"]
registry = ["sharded-slab", "thread_local", "std"]
Expand Down
151 changes: 115 additions & 36 deletions tracing-subscriber/src/filter/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ mod field;

use crate::{
filter::LevelFilter,
layer::{Context, Layer},
layer::{self, Context, Layer},
sync::RwLock,
};
use directive::ParseError;
use std::{cell::RefCell, collections::HashMap, env, error::Error, fmt, str::FromStr};
use thread_local::ThreadLocal;
use tracing_core::{
callsite,
field::Field,
Expand Down Expand Up @@ -109,10 +110,7 @@ pub struct EnvFilter {
has_dynamics: bool,
by_id: RwLock<HashMap<span::Id, directive::SpanMatcher>>,
by_cs: RwLock<HashMap<callsite::Identifier, directive::CallsiteMatcher>>,
}

thread_local! {
static SCOPE: RefCell<Vec<LevelFilter>> = RefCell::new(Vec::new());
scope: ThreadLocal<RefCell<Vec<LevelFilter>>>,
}

type FieldMap<T> = HashMap<Field, T>;
Expand Down Expand Up @@ -350,6 +348,10 @@ impl EnvFilter {
has_dynamics,
by_id: RwLock::new(HashMap::new()),
by_cs: RwLock::new(HashMap::new()),
// TODO(eliza): maybe worth allocating capacity for `num_cpus`
// threads or something (assuming we're running in Tokio)? or
// `num_cpus * 2` or something?
scope: ThreadLocal::new(),
}
}

Expand All @@ -365,9 +367,7 @@ impl EnvFilter {
Interest::never()
}
}
}

impl<S: Subscriber> Layer<S> for EnvFilter {
fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest {
if self.has_dynamics && metadata.is_span() {
// If this metadata describes a span, first, check if there is a
Expand All @@ -388,20 +388,7 @@ impl<S: Subscriber> Layer<S> for EnvFilter {
}
}

fn max_level_hint(&self) -> Option<LevelFilter> {
if self.dynamics.has_value_filters() {
// If we perform any filtering on span field *values*, we will
// enable *all* spans, because their field values are not known
// until recording.
return Some(LevelFilter::TRACE);
}
std::cmp::max(
self.statics.max_level.into(),
self.dynamics.max_level.into(),
)
}

fn enabled(&self, metadata: &Metadata<'_>, _: Context<'_, S>) -> bool {
fn enabled(&self, metadata: &Metadata<'_>) -> bool {
let level = metadata.level();

// is it possible for a dynamic filter directive to enable this event?
Expand All @@ -421,14 +408,15 @@ impl<S: Subscriber> Layer<S> for EnvFilter {
}
}

let enabled_by_scope = SCOPE.with(|scope| {
for filter in scope.borrow().iter() {
let enabled_by_scope = {
let scope = self.scope.get_or_default().borrow();
for filter in &*scope {
if filter >= level {
return true;
}
}
false
});
};
if enabled_by_scope {
return true;
}
Expand All @@ -444,36 +432,43 @@ impl<S: Subscriber> Layer<S> for EnvFilter {
false
}

fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, _: Context<'_, S>) {
fn max_level_hint(&self) -> Option<LevelFilter> {
if self.dynamics.has_value_filters() {
// If we perform any filtering on span field *values*, we will
// enable *all* spans, because their field values are not known
// until recording.
return Some(LevelFilter::TRACE);
}
std::cmp::max(
self.statics.max_level.into(),
self.dynamics.max_level.into(),
)
}

fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id) {
let by_cs = try_lock!(self.by_cs.read());
if let Some(cs) = by_cs.get(&attrs.metadata().callsite()) {
let span = cs.to_span_match(attrs);
try_lock!(self.by_id.write()).insert(id.clone(), span);
}
}

fn on_record(&self, id: &span::Id, values: &span::Record<'_>, _: Context<'_, S>) {
if let Some(span) = try_lock!(self.by_id.read()).get(id) {
span.record_update(values);
}
}

fn on_enter(&self, id: &span::Id, _: Context<'_, S>) {
fn on_enter(&self, id: &span::Id) {
// XXX: This is where _we_ could push IDs to the stack instead, and use
// that to allow changing the filter while a span is already entered.
// But that might be much less efficient...
if let Some(span) = try_lock!(self.by_id.read()).get(id) {
SCOPE.with(|scope| scope.borrow_mut().push(span.level()));
self.scope.get_or_default().borrow_mut().push(span.level());
}
}

fn on_exit(&self, id: &span::Id, _: Context<'_, S>) {
fn on_exit(&self, id: &span::Id) {
if self.cares_about_span(id) {
SCOPE.with(|scope| scope.borrow_mut().pop());
self.scope.get_or_default().borrow_mut().pop();
}
}

fn on_close(&self, id: span::Id, _: Context<'_, S>) {
fn on_close(&self, id: span::Id) {
// If we don't need to acquire a write lock, avoid doing so.
if !self.cares_about_span(&id) {
return;
Expand All @@ -484,6 +479,90 @@ impl<S: Subscriber> Layer<S> for EnvFilter {
}
}

impl<S: Subscriber> Layer<S> for EnvFilter {
#[inline]
fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest {
EnvFilter::register_callsite(self, metadata)
}

#[inline]
fn max_level_hint(&self) -> Option<LevelFilter> {
EnvFilter::max_level_hint(self)
}

#[inline]
fn enabled(&self, metadata: &Metadata<'_>, _: Context<'_, S>) -> bool {
self.enabled(metadata)
}

#[inline]
fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, _: Context<'_, S>) {
self.on_new_span(attrs, id)
}

fn on_record(&self, id: &span::Id, values: &span::Record<'_>, _: Context<'_, S>) {
if let Some(span) = try_lock!(self.by_id.read()).get(id) {
span.record_update(values);
}
}

#[inline]
fn on_enter(&self, id: &span::Id, _: Context<'_, S>) {
self.on_enter(id);
}

#[inline]
fn on_exit(&self, id: &span::Id, _: Context<'_, S>) {
self.on_exit(id);
}

#[inline]
fn on_close(&self, id: span::Id, _: Context<'_, S>) {
self.on_close(id);
}
}

feature! {
#![all(feature = "registry", feature = "std")]

impl<S> layer::Filter<S> for EnvFilter {
#[inline]
fn enabled(&self, meta: &Metadata<'_>, _: &Context<'_, S>) -> bool {
self.enabled(meta)
}

#[inline]
fn callsite_enabled(&self, meta: &'static Metadata<'static>) -> Interest {
self.register_callsite(meta)
}

#[inline]
fn max_level_hint(&self) -> Option<LevelFilter> {
EnvFilter::max_level_hint(self)
}

#[inline]
fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, _: Context<'_, S>) {
self.on_new_span(attrs, id)
}

#[inline]
fn on_enter(&self, id: &span::Id, _: Context<'_, S>) {
self.on_enter(id);
}

#[inline]
fn on_exit(&self, id: &span::Id, _: Context<'_, S>) {
self.on_exit(id);
}

#[inline]
fn on_close(&self, id: span::Id, _: Context<'_, S>) {
self.on_close(id);
}
}
}

impl FromStr for EnvFilter {
type Err = directive::ParseError;

Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/filter/layer_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ where

fn on_exit(&self, id: &span::Id, cx: Context<'_, S>) {
if let Some(cx) = cx.if_enabled_for(id, self.id()) {
self.filter.on_enter(id, cx.clone());
self.filter.on_exit(id, cx.clone());
self.layer.on_exit(id, cx);
}
}
Expand Down
Loading