Skip to content

Commit

Permalink
fix(subscriber): do not report excessive polling (#378) (#440)
Browse files Browse the repository at this point in the history
When entering and exiting a span the old code was also updating the parent
stats. This was causing excessive polling being reported for the parent tasks.
See issue #378 for more details. The regression was introduced by the refactor
in #238.
This fixes the issue by limiting updates to the current span.

Closes #378

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
3 people committed Sep 29, 2023
1 parent 90e5918 commit 8b483bf
Showing 1 changed file with 31 additions and 59 deletions.
90 changes: 31 additions & 59 deletions console-subscriber/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use tracing_core::{
};
use tracing_subscriber::{
layer::Context,
registry::{Extensions, LookupSpan, SpanRef},
registry::{Extensions, LookupSpan},
Layer,
};

Expand Down Expand Up @@ -789,87 +789,59 @@ where
}

fn on_enter(&self, id: &span::Id, cx: Context<'_, S>) {
fn update<S: Subscriber + for<'a> LookupSpan<'a>>(
span: &SpanRef<S>,
at: Option<Instant>,
) -> Option<Instant> {
if let Some(span) = cx.span(id) {
let now = Instant::now();
let exts = span.extensions();
// if the span we are entering is a task or async op, record the
// poll stats.
if let Some(stats) = exts.get::<Arc<stats::TaskStats>>() {
let at = at.unwrap_or_else(Instant::now);
stats.start_poll(at);
Some(at)
stats.start_poll(now);
} else if let Some(stats) = exts.get::<Arc<stats::AsyncOpStats>>() {
let at = at.unwrap_or_else(Instant::now);
stats.start_poll(at);
Some(at)
// otherwise, is the span a resource? in that case, we also want
// to enter it, although we don't care about recording poll
// stats.
stats.start_poll(now);
} else if exts.get::<Arc<stats::ResourceStats>>().is_some() {
Some(at.unwrap_or_else(Instant::now))
// otherwise, is the span a resource? in that case, we also want
// to enter it, although we don't care about recording poll
// stats.
} else {
None
}
}
return;
};

if let Some(span) = cx.span(id) {
if let Some(now) = update(&span, None) {
if let Some(parent) = span.parent() {
update(&parent, Some(now));
}
self.current_spans
.get_or_default()
.borrow_mut()
.push(id.clone());

self.record(|| record::Event::Enter {
id: id.into_u64(),
at: self.base_time.to_system_time(now),
});
}
self.current_spans
.get_or_default()
.borrow_mut()
.push(id.clone());

self.record(|| record::Event::Enter {
id: id.into_u64(),
at: self.base_time.to_system_time(now),
});
}
}

fn on_exit(&self, id: &span::Id, cx: Context<'_, S>) {
fn update<S: Subscriber + for<'a> LookupSpan<'a>>(
span: &SpanRef<S>,
at: Option<Instant>,
) -> Option<Instant> {
if let Some(span) = cx.span(id) {
let exts = span.extensions();
let now = Instant::now();
// if the span we are entering is a task or async op, record the
// poll stats.
if let Some(stats) = exts.get::<Arc<stats::TaskStats>>() {
let at = at.unwrap_or_else(Instant::now);
stats.end_poll(at);
Some(at)
stats.end_poll(now);
} else if let Some(stats) = exts.get::<Arc<stats::AsyncOpStats>>() {
let at = at.unwrap_or_else(Instant::now);
stats.end_poll(at);
Some(at)
stats.end_poll(now);
} else if exts.get::<Arc<stats::ResourceStats>>().is_some() {
// otherwise, is the span a resource? in that case, we also want
// to enter it, although we don't care about recording poll
// stats.
} else if exts.get::<Arc<stats::ResourceStats>>().is_some() {
Some(at.unwrap_or_else(Instant::now))
} else {
None
}
}
return;
};

if let Some(span) = cx.span(id) {
if let Some(now) = update(&span, None) {
if let Some(parent) = span.parent() {
update(&parent, Some(now));
}
self.current_spans.get_or_default().borrow_mut().pop(id);
self.current_spans.get_or_default().borrow_mut().pop(id);

self.record(|| record::Event::Exit {
id: id.into_u64(),
at: self.base_time.to_system_time(now),
});
}
self.record(|| record::Event::Exit {
id: id.into_u64(),
at: self.base_time.to_system_time(now),
});
}
}

Expand Down

0 comments on commit 8b483bf

Please sign in to comment.