diff --git a/CHANGELOG.md b/CHANGELOG.md index 4888c0b..0747b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Allow to `LocalSpan::add_property()` when the local parent is a `Span`. + ## v0.7.1 - Lower MSRV to 1.75. diff --git a/fastrace/src/collector/global_collector.rs b/fastrace/src/collector/global_collector.rs index a28941e..18862e8 100644 --- a/fastrace/src/collector/global_collector.rs +++ b/fastrace/src/collector/global_collector.rs @@ -1,5 +1,6 @@ // Copyright 2020 TiKV Project Authors. Licensed under Apache-2.0. +use std::borrow::Cow; use std::cell::UnsafeCell; use std::collections::HashMap; use std::sync::atomic::AtomicBool; @@ -24,6 +25,7 @@ use crate::collector::SpanRecord; use crate::collector::SpanSet; use crate::collector::TraceId; use crate::local::local_collector::LocalSpansInner; +use crate::local::raw_span::RawKind; use crate::local::raw_span::RawSpan; use crate::util::object_pool; use crate::util::spsc::Receiver; @@ -191,7 +193,7 @@ enum SpanCollection { struct ActiveCollector { span_collections: Vec, span_count: usize, - dangling_events: HashMap>, + danglings: HashMap>, } pub(crate) struct GlobalCollector { @@ -354,7 +356,7 @@ impl GlobalCollector { active_collector.span_collections, &anchor, &mut committed_records, - &mut active_collector.dangling_events, + &mut active_collector.danglings, ); } } @@ -365,7 +367,7 @@ impl GlobalCollector { active_collector.span_collections.drain(..), &anchor, &mut committed_records, - &mut active_collector.dangling_events, + &mut active_collector.danglings, ); } } @@ -377,26 +379,31 @@ impl GlobalCollector { impl LocalSpansInner { pub fn to_span_records(&self, parent: SpanContext) -> Vec { let anchor: Anchor = Anchor::new(); - let mut dangling_events = HashMap::new(); + let mut danglings = HashMap::new(); let mut records = Vec::new(); amend_local_span( self, parent.trace_id, parent.span_id, &mut records, - &mut dangling_events, + &mut danglings, &anchor, ); - mount_events(&mut records, &mut dangling_events); + mount_danglings(&mut records, &mut danglings); records } } +enum DanglingItem { + Event(EventRecord), + Properties(Vec<(Cow<'static, str>, Cow<'static, str>)>), +} + fn postprocess_span_collection( span_collections: impl IntoIterator, anchor: &Anchor, committed_records: &mut Vec, - dangling_events: &mut HashMap>, + danglings: &mut HashMap>, ) { let committed_len = committed_records.len(); @@ -412,7 +419,7 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), SpanSet::LocalSpansInner(local_spans) => amend_local_span( @@ -420,7 +427,7 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), SpanSet::SharedLocalSpans(local_spans) => amend_local_span( @@ -428,7 +435,7 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), }, @@ -442,7 +449,7 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), SpanSet::LocalSpansInner(local_spans) => amend_local_span( @@ -450,7 +457,7 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), SpanSet::SharedLocalSpans(local_spans) => amend_local_span( @@ -458,14 +465,14 @@ fn postprocess_span_collection( trace_id, parent_id, committed_records, - dangling_events, + danglings, anchor, ), }, } } - mount_events(&mut committed_records[committed_len..], dangling_events); + mount_danglings(&mut committed_records[committed_len..], danglings); } fn amend_local_span( @@ -473,92 +480,117 @@ fn amend_local_span( trace_id: TraceId, parent_id: SpanId, spans: &mut Vec, - events: &mut HashMap>, + dangling: &mut HashMap>, anchor: &Anchor, ) { for span in local_spans.spans.iter() { - let begin_time_unix_ns = span.begin_instant.as_unix_nanos(anchor); let parent_id = if span.parent_id == SpanId::default() { parent_id } else { span.parent_id }; - if span.is_event { - let event = EventRecord { - name: span.name.clone(), - timestamp_unix_ns: begin_time_unix_ns, - properties: span.properties.clone(), - }; - events.entry(parent_id).or_default().push(event); - continue; + match span.raw_kind { + RawKind::Span => { + let begin_time_unix_ns = span.begin_instant.as_unix_nanos(anchor); + let end_time_unix_ns = if span.end_instant == Instant::ZERO { + local_spans.end_time.as_unix_nanos(anchor) + } else { + span.end_instant.as_unix_nanos(anchor) + }; + spans.push(SpanRecord { + trace_id, + span_id: span.id, + parent_id, + begin_time_unix_ns, + duration_ns: end_time_unix_ns.saturating_sub(begin_time_unix_ns), + name: span.name.clone(), + properties: span.properties.clone(), + events: vec![], + }); + } + RawKind::Event => { + let begin_time_unix_ns = span.begin_instant.as_unix_nanos(anchor); + let event = EventRecord { + name: span.name.clone(), + timestamp_unix_ns: begin_time_unix_ns, + properties: span.properties.clone(), + }; + dangling + .entry(parent_id) + .or_default() + .push(DanglingItem::Event(event)); + } + RawKind::Properties => { + dangling + .entry(parent_id) + .or_default() + .push(DanglingItem::Properties(span.properties.clone())); + } } - - let end_time_unix_ns = if span.end_instant == Instant::ZERO { - local_spans.end_time.as_unix_nanos(anchor) - } else { - span.end_instant.as_unix_nanos(anchor) - }; - spans.push(SpanRecord { - trace_id, - span_id: span.id, - parent_id, - begin_time_unix_ns, - duration_ns: end_time_unix_ns.saturating_sub(begin_time_unix_ns), - name: span.name.clone(), - properties: span.properties.clone(), - events: vec![], - }); } } fn amend_span( - raw_span: &RawSpan, + span: &RawSpan, trace_id: TraceId, parent_id: SpanId, spans: &mut Vec, - events: &mut HashMap>, + dangling: &mut HashMap>, anchor: &Anchor, ) { - let begin_time_unix_ns = raw_span.begin_instant.as_unix_nanos(anchor); - - if raw_span.is_event { - let event = EventRecord { - name: raw_span.name.clone(), - timestamp_unix_ns: begin_time_unix_ns, - properties: raw_span.properties.clone(), - }; - events.entry(parent_id).or_default().push(event); - return; + match span.raw_kind { + RawKind::Span => { + let begin_time_unix_ns = span.begin_instant.as_unix_nanos(anchor); + let end_time_unix_ns = span.end_instant.as_unix_nanos(anchor); + spans.push(SpanRecord { + trace_id, + span_id: span.id, + parent_id, + begin_time_unix_ns, + duration_ns: end_time_unix_ns.saturating_sub(begin_time_unix_ns), + name: span.name.clone(), + properties: span.properties.clone(), + events: vec![], + }); + } + RawKind::Event => { + let begin_time_unix_ns = span.begin_instant.as_unix_nanos(anchor); + let event = EventRecord { + name: span.name.clone(), + timestamp_unix_ns: begin_time_unix_ns, + properties: span.properties.clone(), + }; + dangling + .entry(parent_id) + .or_default() + .push(DanglingItem::Event(event)); + } + RawKind::Properties => { + dangling + .entry(parent_id) + .or_default() + .push(DanglingItem::Properties(span.properties.clone())); + } } - - let end_time_unix_ns = raw_span.end_instant.as_unix_nanos(anchor); - spans.push(SpanRecord { - trace_id, - span_id: raw_span.id, - parent_id, - begin_time_unix_ns, - duration_ns: end_time_unix_ns.saturating_sub(begin_time_unix_ns), - name: raw_span.name.clone(), - properties: raw_span.properties.clone(), - events: vec![], - }); } -fn mount_events( - records: &mut [SpanRecord], - dangling_events: &mut HashMap>, -) { +fn mount_danglings(records: &mut [SpanRecord], danglings: &mut HashMap>) { for record in records.iter_mut() { - if dangling_events.is_empty() { + if danglings.is_empty() { return; } - if let Some(event) = dangling_events.remove(&record.span_id) { - if record.events.is_empty() { - record.events = event; - } else { - record.events.extend(event); + if let Some(danglings) = danglings.remove(&record.span_id) { + for dangling in danglings { + match dangling { + DanglingItem::Event(event) => { + record.events.push(event); + } + DanglingItem::Properties(properties) => { + record.properties.extend(properties); + } + } } } } diff --git a/fastrace/src/event.rs b/fastrace/src/event.rs index b82ba6e..7ec2623 100644 --- a/fastrace/src/event.rs +++ b/fastrace/src/event.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use crate::local::local_span_stack::LOCAL_SPAN_STACK; +use crate::local::raw_span::RawKind; use crate::Span; /// An event that represents a single point in time during the execution of a span. @@ -29,7 +30,7 @@ impl Event { { let mut span = Span::enter_with_parent(name, parent).with_properties(properties); if let Some(mut inner) = span.inner.take() { - inner.raw_span.is_event = true; + inner.raw_span.raw_kind = RawKind::Event; inner.submit_spans(); } } diff --git a/fastrace/src/local/local_span.rs b/fastrace/src/local/local_span.rs index 25a6d2c..9672e32 100644 --- a/fastrace/src/local/local_span.rs +++ b/fastrace/src/local/local_span.rs @@ -55,7 +55,7 @@ impl LocalSpan { } /// Add a single property to the current local parent. If the local parent is a [`Span`], - /// the property will not be added to the `Span`. + /// the property will be added to the `Span`. /// /// A property is an arbitrary key-value pair associated with a span. /// @@ -79,7 +79,7 @@ impl LocalSpan { } /// Add multiple properties to the current local parent. If the local parent is a [`Span`], - /// the properties will not be added to the `Span`. + /// the properties will be added to the `Span`. /// /// # Examples /// @@ -103,9 +103,7 @@ impl LocalSpan { LOCAL_SPAN_STACK .try_with(|s| { let span_stack = &mut *s.borrow_mut(); - let span_line = span_stack.current_span_line()?; - let parent_handle = span_line.current_parent_handle()?; - span_line.add_properties(&parent_handle, properties); + span_stack.add_properties(properties); Some(()) }) .ok(); @@ -155,7 +153,7 @@ impl LocalSpan { #[cfg(feature = "enable")] if let Some(LocalSpanInner { stack, span_handle }) = &self.inner { let span_stack = &mut *stack.borrow_mut(); - span_stack.add_properties(span_handle, properties); + span_stack.with_properties(span_handle, properties); } self diff --git a/fastrace/src/local/local_span_line.rs b/fastrace/src/local/local_span_line.rs index 03a05b7..9bcc725 100644 --- a/fastrace/src/local/local_span_line.rs +++ b/fastrace/src/local/local_span_line.rs @@ -57,7 +57,18 @@ impl SpanLine { } #[inline] - pub fn add_properties(&mut self, handle: &LocalSpanHandle, properties: F) + pub fn add_properties(&mut self, properties: F) + where + K: Into>, + V: Into>, + I: IntoIterator, + F: FnOnce() -> I, + { + self.span_queue.add_properties(properties()); + } + + #[inline] + pub fn with_properties(&mut self, handle: &LocalSpanHandle, properties: F) where K: Into>, V: Into>, @@ -66,7 +77,7 @@ impl SpanLine { { if self.epoch == handle.span_line_epoch { self.span_queue - .add_properties(&handle.span_handle, properties()); + .with_properties(&handle.span_handle, properties()); } } @@ -89,16 +100,6 @@ impl SpanLine { }) } - #[inline] - pub fn current_parent_handle(&self) -> Option { - let span_handle = self.span_queue.current_parent_handle()?; - let span_line_epoch = self.epoch; - Some(LocalSpanHandle { - span_handle, - span_line_epoch, - }) - } - #[inline] pub fn collect(self, span_line_epoch: usize) -> Option<(RawSpans, Option)> { (self.epoch == span_line_epoch) @@ -127,7 +128,7 @@ mod tests { let span2 = span_line.start_span("span2").unwrap(); { let span3 = span_line.start_span("span3").unwrap(); - span_line.add_properties(&span3, || [("k1", "v1")]); + span_line.with_properties(&span3, || [("k1", "v1")]); span_line.finish_span(span3); } span_line.finish_span(span2); @@ -210,7 +211,7 @@ span [] assert_eq!(span_line2.span_line_epoch(), 2); let span = span_line1.start_span("span").unwrap(); - span_line2.add_properties(&span, || [("k1", "v1")]); + span_line2.with_properties(&span, || [("k1", "v1")]); span_line1.finish_span(span); let raw_spans = span_line1.collect(1).unwrap().0.into_inner(); diff --git a/fastrace/src/local/local_span_stack.rs b/fastrace/src/local/local_span_stack.rs index 23ba1dd..f3382cc 100644 --- a/fastrace/src/local/local_span_stack.rs +++ b/fastrace/src/local/local_span_stack.rs @@ -98,12 +98,28 @@ impl LocalSpanStack { } #[inline] - pub fn add_properties(&mut self, local_span_handle: &LocalSpanHandle, properties: F) + pub fn add_properties(&mut self, properties: F) where K: Into>, V: Into>, I: IntoIterator, F: FnOnce() -> I, + { + if let Some(span_line) = self.current_span_line() { + span_line.add_properties(properties); + } + } + + #[inline] + pub fn with_properties( + &mut self, + local_span_handle: &LocalSpanHandle, + properties: F, + ) where + K: Into>, + V: Into>, + I: IntoIterator, + F: FnOnce() -> I, { debug_assert!(self.current_span_line().is_some()); if let Some(span_line) = self.current_span_line() { @@ -111,7 +127,7 @@ impl LocalSpanStack { span_line.span_line_epoch(), local_span_handle.span_line_epoch ); - span_line.add_properties(local_span_handle, properties); + span_line.with_properties(local_span_handle, properties); } } @@ -367,7 +383,7 @@ span1 [] .into(), )) .unwrap(); - span_stack.add_properties(&span1, || [("k1", "v1")]); + span_stack.with_properties(&span1, || [("k1", "v1")]); let _ = span_stack.unregister_and_collect(span_line2).unwrap(); } span_stack.exit_span(span1); diff --git a/fastrace/src/local/raw_span.rs b/fastrace/src/local/raw_span.rs index 4f57ecd..a25d0a8 100644 --- a/fastrace/src/local/raw_span.rs +++ b/fastrace/src/local/raw_span.rs @@ -7,6 +7,13 @@ use minstant::Instant; use crate::collector::SpanId; use crate::util::Properties; +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum RawKind { + Span, + Event, + Properties, +} + #[derive(Debug)] pub struct RawSpan { pub id: SpanId, @@ -14,7 +21,7 @@ pub struct RawSpan { pub begin_instant: Instant, pub name: Cow<'static, str>, pub properties: Properties, - pub is_event: bool, + pub raw_kind: RawKind, // Will write this field at post processing pub end_instant: Instant, @@ -27,7 +34,7 @@ impl RawSpan { parent_id: SpanId, begin_instant: Instant, name: impl Into>, - is_event: bool, + raw_kind: RawKind, ) -> Self { RawSpan { id, @@ -35,7 +42,7 @@ impl RawSpan { begin_instant, name: name.into(), properties: Properties::default(), - is_event, + raw_kind, end_instant: Instant::ZERO, } } @@ -57,7 +64,7 @@ impl Clone for RawSpan { begin_instant: self.begin_instant, name: self.name.clone(), properties, - is_event: self.is_event, + raw_kind: self.raw_kind, end_instant: self.end_instant, } } diff --git a/fastrace/src/local/span_queue.rs b/fastrace/src/local/span_queue.rs index b475418..5104f8b 100644 --- a/fastrace/src/local/span_queue.rs +++ b/fastrace/src/local/span_queue.rs @@ -4,6 +4,7 @@ use std::borrow::Cow; use minstant::Instant; +use super::raw_span::RawKind; use crate::collector::SpanId; use crate::local::raw_span::RawSpan; use crate::util::RawSpans; @@ -38,7 +39,7 @@ impl SpanQueue { self.next_parent_id.unwrap_or_default(), Instant::now(), name, - false, + RawKind::Span, ); self.next_parent_id = Some(span.id); @@ -77,7 +78,7 @@ impl SpanQueue { self.next_parent_id.unwrap_or_default(), Instant::now(), name, - true, + RawKind::Event, ); span.properties.extend(properties()); @@ -85,7 +86,31 @@ impl SpanQueue { } #[inline] - pub fn add_properties(&mut self, span_handle: &SpanHandle, properties: I) + pub fn add_properties(&mut self, properties: I) + where + K: Into>, + V: Into>, + I: IntoIterator, + { + if self.span_queue.len() >= self.capacity { + return; + } + + let mut span = RawSpan::begin_with( + SpanId::next_id(), + self.next_parent_id.unwrap_or_default(), + Instant::ZERO, + Cow::Borrowed(""), + RawKind::Properties, + ); + span.properties + .extend(properties.into_iter().map(|(k, v)| (k.into(), v.into()))); + + self.span_queue.push(span); + } + + #[inline] + pub fn with_properties(&mut self, span_handle: &SpanHandle, properties: I) where K: Into>, V: Into>, @@ -108,17 +133,6 @@ impl SpanQueue { self.next_parent_id } - #[inline] - pub fn current_parent_handle(&self) -> Option { - self.next_parent_id.and_then(|id| { - let index = self.span_queue.iter().position(|span| span.id == id); - - debug_assert!(index.is_some()); - - Some(SpanHandle { index: index? }) - }) - } - #[cfg(test)] pub fn get_raw_span(&self, handle: &SpanHandle) -> &RawSpan { &self.span_queue[handle.index] @@ -160,10 +174,10 @@ span1 [] let mut queue = SpanQueue::with_capacity(16); { let span1 = queue.start_span("span1").unwrap(); - queue.add_properties(&span1, [("k1", "v1"), ("k2", "v2")]); + queue.with_properties(&span1, [("k1", "v1"), ("k2", "v2")]); { let span2 = queue.start_span("span2").unwrap(); - queue.add_properties(&span2, [("k1", "v1")]); + queue.with_properties(&span2, [("k1", "v1")]); queue.finish_span(span2); } queue.finish_span(span1); diff --git a/fastrace/src/span.rs b/fastrace/src/span.rs index 2afb544..69fea9b 100644 --- a/fastrace/src/span.rs +++ b/fastrace/src/span.rs @@ -17,6 +17,7 @@ use crate::collector::SpanSet; use crate::local::local_collector::LocalSpansInner; use crate::local::local_span_stack::LocalSpanStack; use crate::local::local_span_stack::LOCAL_SPAN_STACK; +use crate::local::raw_span::RawKind; use crate::local::raw_span::RawSpan; use crate::local::LocalCollector; use crate::local::LocalSpans; @@ -382,7 +383,13 @@ impl Span { ) -> Self { let span_id = SpanId::next_id(); let begin_instant = Instant::now(); - let raw_span = RawSpan::begin_with(span_id, SpanId::default(), begin_instant, name, false); + let raw_span = RawSpan::begin_with( + span_id, + SpanId::default(), + begin_instant, + name, + RawKind::Span, + ); let collect = current_collect(); Self { diff --git a/fastrace/tests/lib.rs b/fastrace/tests/lib.rs index 153b6be..a19ba89 100644 --- a/fastrace/tests/lib.rs +++ b/fastrace/tests/lib.rs @@ -676,18 +676,18 @@ fn test_add_property() { { let root = Span::root("root", SpanContext::random()); let _g = root.set_local_parent(); - LocalSpan::add_property(|| ("noop", "noop")); - LocalSpan::add_properties(|| [("noop", "noop")]); - let _span = LocalSpan::enter_with_local_parent("span"); LocalSpan::add_property(|| ("k1", "v1")); - LocalSpan::add_properties(|| [("k2", "v2"), ("k3", "v3")]); + LocalSpan::add_properties(|| [("k2", "v2")]); + let _span = LocalSpan::enter_with_local_parent("span"); + LocalSpan::add_property(|| ("k3", "v3")); + LocalSpan::add_properties(|| [("k4", "v4"), ("k5", "v5")]); } fastrace::flush(); let expected_graph = r#" -root [] - span [("k1", "v1"), ("k2", "v2"), ("k3", "v3")] +root [("k1", "v1"), ("k2", "v2")] + span [("k3", "v3"), ("k4", "v4"), ("k5", "v5")] "#; assert_eq!( tree_str_from_span_records(collected_spans.lock().clone()),