From 6ec40405c924ddef46250203923887acb4d6a288 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Sun, 25 Jun 2023 22:14:04 +0200 Subject: [PATCH 1/4] fix(subscriber): do not register poll on parent spans --- console-subscriber/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/console-subscriber/src/lib.rs b/console-subscriber/src/lib.rs index 29b85bd75..091f4949a 100644 --- a/console-subscriber/src/lib.rs +++ b/console-subscriber/src/lib.rs @@ -816,9 +816,6 @@ where 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() @@ -860,9 +857,6 @@ where 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.record(|| record::Event::Exit { From bf5d57d68300399938900806039f99dfebd0fdc0 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Sun, 25 Jun 2023 22:24:50 +0200 Subject: [PATCH 2/4] style(subscriber): inline update function --- console-subscriber/src/lib.rs | 56 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/console-subscriber/src/lib.rs b/console-subscriber/src/lib.rs index 091f4949a..a11099e44 100644 --- a/console-subscriber/src/lib.rs +++ b/console-subscriber/src/lib.rs @@ -789,33 +789,27 @@ where } fn on_enter(&self, id: &span::Id, cx: Context<'_, S>) { - fn update LookupSpan<'a>>( - span: &SpanRef, - at: Option, - ) -> Option { + 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::>() { - let at = at.unwrap_or_else(Instant::now); - stats.start_poll(at); - Some(at) + let is_relevant = if let Some(stats) = exts.get::>() { + stats.start_poll(now); + true } else if let Some(stats) = exts.get::>() { - let at = at.unwrap_or_else(Instant::now); - stats.start_poll(at); - Some(at) + stats.start_poll(now); + true // 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::>().is_some() { - Some(at.unwrap_or_else(Instant::now)) + true } else { - None - } - } + false + }; - if let Some(span) = cx.span(id) { - if let Some(now) = update(&span, None) { + if is_relevant { self.current_spans .get_or_default() .borrow_mut() @@ -830,33 +824,27 @@ where } fn on_exit(&self, id: &span::Id, cx: Context<'_, S>) { - fn update LookupSpan<'a>>( - span: &SpanRef, - at: Option, - ) -> Option { + 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::>() { - let at = at.unwrap_or_else(Instant::now); - stats.end_poll(at); - Some(at) + let is_relevant = if let Some(stats) = exts.get::>() { + stats.end_poll(now); + true } else if let Some(stats) = exts.get::>() { - let at = at.unwrap_or_else(Instant::now); - stats.end_poll(at); - Some(at) + stats.end_poll(now); + true // 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::>().is_some() { - Some(at.unwrap_or_else(Instant::now)) + true } else { - None - } - } + false + }; - if let Some(span) = cx.span(id) { - if let Some(now) = update(&span, None) { + if is_relevant { self.current_spans.get_or_default().borrow_mut().pop(id); self.record(|| record::Event::Exit { From 56f26d4efcdea478dd277a922186021144a7eda3 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Thu, 29 Jun 2023 18:23:56 +0200 Subject: [PATCH 3/4] style(subscriber): simplify with early return --- console-subscriber/src/lib.rs | 52 ++++++++++++++--------------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/console-subscriber/src/lib.rs b/console-subscriber/src/lib.rs index a11099e44..6838e1950 100644 --- a/console-subscriber/src/lib.rs +++ b/console-subscriber/src/lib.rs @@ -25,7 +25,7 @@ use tracing_core::{ }; use tracing_subscriber::{ layer::Context, - registry::{Extensions, LookupSpan, SpanRef}, + registry::{Extensions, LookupSpan}, Layer, }; @@ -794,32 +794,27 @@ where let exts = span.extensions(); // if the span we are entering is a task or async op, record the // poll stats. - let is_relevant = if let Some(stats) = exts.get::>() { + if let Some(stats) = exts.get::>() { stats.start_poll(now); - true } else if let Some(stats) = exts.get::>() { stats.start_poll(now); - true // 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::>().is_some() { - true } else { - false + return; }; - if is_relevant { - self.current_spans - .get_or_default() - .borrow_mut() - .push(id.clone()); + 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.record(|| record::Event::Enter { + id: id.into_u64(), + at: self.base_time.to_system_time(now), + }); } } @@ -829,29 +824,24 @@ where let now = Instant::now(); // if the span we are entering is a task or async op, record the // poll stats. - let is_relevant = if let Some(stats) = exts.get::>() { + if let Some(stats) = exts.get::>() { stats.end_poll(now); - true } else if let Some(stats) = exts.get::>() { stats.end_poll(now); - true - // otherwise, is the span a resource? in that case, we also want - // to enter it, although we don't care about recording poll - // stats. + // 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::>().is_some() { - true } else { - false + return; }; - if is_relevant { - 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), + }); } } From 9dbb6cd13f396031a0e46efa2773bb4081605af3 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Fri, 30 Jun 2023 14:52:59 +0200 Subject: [PATCH 4/4] style(subscriber): move comment --- console-subscriber/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/console-subscriber/src/lib.rs b/console-subscriber/src/lib.rs index 6838e1950..6b0c1a75e 100644 --- a/console-subscriber/src/lib.rs +++ b/console-subscriber/src/lib.rs @@ -798,10 +798,10 @@ where stats.start_poll(now); } else if let Some(stats) = exts.get::>() { stats.start_poll(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 if exts.get::>().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 { return; }; @@ -828,10 +828,10 @@ where stats.end_poll(now); } else if let Some(stats) = exts.get::>() { stats.end_poll(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 if exts.get::>().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 { return; };