feat: allow LocalSpan::add_property() when the local parent is a `S…

…pan` (#33)

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.

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<SpanCollection>,
     span_count: usize,
-    dangling_events: HashMap<SpanId, Vec<EventRecord>>,
+    danglings: HashMap<SpanId, Vec<DanglingItem>>,
 }
 
 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<SpanRecord> {
         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<Item = SpanCollection>,
     anchor: &Anchor,
     committed_records: &mut Vec<SpanRecord>,
-    dangling_events: &mut HashMap<SpanId, Vec<EventRecord>>,
+    danglings: &mut HashMap<SpanId, Vec<DanglingItem>>,
 ) {
     let committed_len = committed_records.len();
 
@@ -412,23 +419,23 @@ fn postprocess_span_collection(
                     trace_id,
                     parent_id,
                     committed_records,
-                    dangling_events,
+                    danglings,
                     anchor,
                 ),
                 SpanSet::LocalSpansInner(local_spans) => amend_local_span(
                     &local_spans,
                     trace_id,
                     parent_id,
                     committed_records,
-                    dangling_events,
+                    danglings,
                     anchor,
                 ),
                 SpanSet::SharedLocalSpans(local_spans) => amend_local_span(
                     &local_spans,
                     trace_id,
                     parent_id,
                     committed_records,
-                    dangling_events,
+                    danglings,
                     anchor,
                 ),
             },
@@ -442,123 +449,148 @@ fn postprocess_span_collection(
                     trace_id,
                     parent_id,
                     committed_records,
-                    dangling_events,
+                    danglings,
                     anchor,
                 ),
                 SpanSet::LocalSpansInner(local_spans) => amend_local_span(
                     local_spans,
                     trace_id,
                     parent_id,
                     committed_records,
-                    dangling_events,
+                    danglings,
                     anchor,
                 ),
                 SpanSet::SharedLocalSpans(local_spans) => amend_local_span(
                     local_spans,
                     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(
     local_spans: &LocalSpansInner,
     trace_id: TraceId,
     parent_id: SpanId,
     spans: &mut Vec<SpanRecord>,
-    events: &mut HashMap<SpanId, Vec<EventRecord>>,
+    dangling: &mut HashMap<SpanId, Vec<DanglingItem>>,
     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<SpanRecord>,
-    events: &mut HashMap<SpanId, Vec<EventRecord>>,
+    dangling: &mut HashMap<SpanId, Vec<DanglingItem>>,
     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<SpanId, Vec<EventRecord>>,
-) {
+fn mount_danglings(records: &mut [SpanRecord], danglings: &mut HashMap<SpanId, Vec<DanglingItem>>) {
     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);
+                    }
+                }
             }
         }
     }

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();
             }
         }

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