[WIP] Metadata name is Cow<'static, str>

NOTE: requires changes in opentelemetry-rust as well

Cargo.toml

@@ -17,3 +17,6 @@ members = [
     "tracing-journald",
     "examples"
 ]
+
+[patch.crates-io]
+opentelemetry = { path = "../opentelemetry-rust" }

tracing-core/src/metadata.rs

@@ -1,6 +1,7 @@
 //! Metadata describing trace data.
 use super::{callsite, field};
 use crate::stdlib::{
+    borrow::Cow,
     cmp, fmt,
     str::FromStr,
     sync::atomic::{AtomicUsize, Ordering},
@@ -60,7 +61,7 @@ use crate::stdlib::{
 /// [callsite identifier]: ../callsite/struct.Identifier.html
 pub struct Metadata<'a> {
     /// The name of the span described by this metadata.
-    name: &'static str,
+    name: Cow<'static, str>,
 
     /// The part of the system that the span that this metadata describes
     /// occurred in.
@@ -132,7 +133,7 @@ impl<'a> Metadata<'a> {
         kind: Kind,
     ) -> Self {
         Metadata {
-            name,
+            name: Cow::Borrowed(name),
             target,
             level,
             module_path,
@@ -154,8 +155,9 @@ impl<'a> Metadata<'a> {
     }
 
     /// Returns the name of the span.
-    pub fn name(&self) -> &'static str {
-        self.name
+    pub fn name(&self) -> Cow<'static, str> {
+        // TODO: do better than this.
+        self.name.clone()
     }
 
     /// Returns a string describing the part of the system where the span or

tracing-log/src/trace_logger.rs

@@ -19,6 +19,7 @@
 )]
 use crate::AsLog;
 use std::{
+    borrow::Cow,
     cell::RefCell,
     collections::HashMap,
     fmt::{self, Write},
@@ -190,7 +191,7 @@ struct SpanLineBuilder {
     module_path: Option<String>,
     target: String,
     level: log::Level,
-    name: &'static str,
+    name: Cow<'static, str>,
 }
 
 impl SpanLineBuilder {
@@ -373,7 +374,7 @@ impl Subscriber for TraceLogger {
                 .map(|span| {
                     let fields = span.fields.as_ref();
                     let parent = if self.settings.log_parent {
-                        Some(span.name)
+                        Some(span.name.as_ref())
                     } else {
                         None
                     };

tracing-opentelemetry/src/layer.rs

@@ -489,6 +489,7 @@ mod tests {
     use super::*;
     use opentelemetry::api;
     use opentelemetry::api::TraceContextExt;
+    use std::borrow::Cow;
     use std::sync::{Arc, Mutex};
     use std::time::SystemTime;
     use tracing_subscriber::prelude::*;
@@ -500,11 +501,17 @@ mod tests {
         fn invalid(&self) -> Self::Span {
             api::NoopSpan::new()
         }
-        fn start_from_context(&self, _name: &str, _context: &api::Context) -> Self::Span {
+        fn start_from_context<'a, S>(&self, _name: S, _context: &api::Context) -> Self::Span
+        where
+            S: Into<Cow<'a, str>>
+        {
             self.invalid()
         }
-        fn span_builder(&self, name: &str) -> api::SpanBuilder {
-            api::SpanBuilder::from_name(name.to_string())
+        fn span_builder<'a, S>(&self, name: S) -> api::SpanBuilder
+        where
+            S: Into<Cow<'a, str>>
+        {
+            api::SpanBuilder::from_name(name.into().into_owned())
         }
         fn build_with_context(&self, builder: api::SpanBuilder, _cx: &api::Context) -> Self::Span {
             *self.0.lock().unwrap() = Some(builder);

tracing-serde/src/lib.rs

@@ -244,7 +244,7 @@ impl<'a> Serialize for SerializeMetadata<'a> {
         S: Serializer,
     {
         let mut state = serializer.serialize_struct("Metadata", 9)?;
-        state.serialize_field("name", self.0.name())?;
+        state.serialize_field("name", &self.0.name())?;
         state.serialize_field("target", self.0.target())?;
         state.serialize_field("level", &SerializeLevel(self.0.level()))?;
         state.serialize_field("module_path", &self.0.module_path())?;

tracing-subscriber/src/filter/env/directive.rs

@@ -151,7 +151,7 @@ impl Match for Directive {
         // Do we have a name filter, and does it match the metadata's name?
         // TODO(eliza): put name globbing here?
         if let Some(ref name) = self.in_span {
-            if name != meta.name() {
+            if name != meta.name().as_ref() {
                 return false;
             }
         }

tracing-subscriber/src/fmt/format/json.rs

@@ -170,7 +170,7 @@ where
             // that the fields are not supposed to be missing.
             Err(e) => serializer.serialize_entry("field_error", &format!("{}", e))?,
         };
-        serializer.serialize_entry("name", self.0.metadata().name())?;
+        serializer.serialize_entry("name", &self.0.metadata().name())?;
         serializer.end()
     }
 }

tracing-subscriber/src/registry/mod.rs

@@ -63,6 +63,7 @@
 //! [`LookupSpan`]: trait.LookupSpan.html
 //! [`SpanData`]: trait.SpanData.html
 use tracing_core::{field::FieldSet, span::Id, Metadata};
+use std::borrow::Cow;
 
 /// A module containing a type map of span extensions.
 mod extensions;
@@ -213,8 +214,8 @@ where
     }
 
     /// Returns the span's name,
-    pub fn name(&self) -> &'static str {
-        self.data.metadata().name()
+    pub fn name(&self) -> Cow<'static, str> {
+        self.metadata().name()
     }
 
     /// Returns a list of [fields] defined by the span.

tracing-subscriber/src/registry/sharded.rs

@@ -369,6 +369,7 @@ mod tests {
     use super::Registry;
     use crate::{layer::Context, registry::LookupSpan, Layer};
     use std::{
+        borrow::Cow,
         collections::HashMap,
         sync::{Arc, Mutex, Weak},
     };
@@ -422,8 +423,8 @@ mod tests {
 
     #[derive(Default)]
     struct CloseState {
-        open: HashMap<&'static str, Weak<()>>,
-        closed: Vec<(&'static str, Weak<()>)>,
+        open: HashMap<Cow<'static, str>, Weak<()>>,
+        closed: Vec<(Cow<'static, str>, Weak<()>)>,
     }
 
     struct SetRemoved(Arc<()>);
@@ -459,7 +460,7 @@ mod tests {
             let name = span.name();
             println!("close {} ({:?})", name, id);
             if let Ok(mut lock) = self.inner.lock() {
-                if let Some(is_removed) = lock.open.remove(name) {
+                if let Some(is_removed) = lock.open.remove(name.as_ref()) {
                     assert!(is_removed.upgrade().is_some());
                     lock.closed.push((name, is_removed));
                 }
@@ -554,24 +555,26 @@ mod tests {
         }
 
         #[allow(unused)] // may want this for future tests
+        // TODO: take an `Option<Cow<'a, str>>` instead?
         fn assert_last_closed(&self, span: Option<&str>) {
             let lock = self.state.lock().unwrap();
-            let last = lock.closed.last().map(|(span, _)| span);
+            let last = lock.closed.last().map(|(span, _)| span.as_ref());
             assert_eq!(
                 last,
-                span.as_ref(),
+                span,
                 "expected {:?} to have closed last",
                 span
             );
         }
 
+        // TODO: take an `AsRef[Cow<'a, str>]` instead?
         fn assert_closed_in_order(&self, order: impl AsRef<[&'static str]>) {
             let lock = self.state.lock().unwrap();
             let order = order.as_ref();
             for (i, name) in order.iter().enumerate() {
                 assert_eq!(
-                    lock.closed.get(i).map(|(span, _)| span),
-                    Some(name),
+                    lock.closed.get(i).map(|(span, _)| span.as_ref()),
+                    Some(*name),
                     "expected close order: {:?}, actual: {:?}",
                     order,
                     lock.closed.iter().map(|(name, _)| name).collect::<Vec<_>>()

tracing/tests/filter_caching_is_lexically_scoped.rs

@@ -34,7 +34,7 @@ fn filter_caching_is_lexically_scoped() {
     let count2 = count.clone();
 
     let subscriber = subscriber::mock()
-        .with_filter(move |meta| match meta.name() {
+        .with_filter(move |meta| match meta.name().as_ref() {
             "emily" | "frank" => {
                 count2.fetch_add(1, Ordering::Relaxed);
                 true

tracing/tests/filters_are_not_reevaluated_for_the_same_span.rs

@@ -29,7 +29,7 @@ fn filters_are_not_reevaluated_for_the_same_span() {
     let bob_count2 = bob_count.clone();
 
     let (subscriber, handle) = subscriber::mock()
-        .with_filter(move |meta| match meta.name() {
+        .with_filter(move |meta| match meta.name().as_ref() {
             "alice" => {
                 alice_count2.fetch_add(1, Ordering::Relaxed);
                 false

tracing/tests/filters_are_reevaluated_for_different_call_sites.rs

@@ -31,7 +31,7 @@ fn filters_are_reevaluated_for_different_call_sites() {
     let subscriber = subscriber::mock()
         .with_filter(move |meta| {
             println!("Filter: {:?}", meta.name());
-            match meta.name() {
+            match meta.name().as_ref() {
                 "charlie" => {
                     charlie_count2.fetch_add(1, Ordering::Relaxed);
                     false

tracing/tests/support/event.rs

@@ -69,7 +69,7 @@ impl MockEvent {
 
     pub fn with_explicit_parent(self, parent: Option<&str>) -> MockEvent {
         let parent = match parent {
-            Some(name) => Parent::Explicit(name.into()),
+            Some(name) => Parent::Explicit(name.to_string()),
             None => Parent::ExplicitRoot,
         };
         Self {

tracing/tests/support/metadata.rs

@@ -13,7 +13,7 @@ impl Expect {
         if let Some(ref expected_name) = self.name {
             let name = actual.name();
             assert!(
-                expected_name == name,
+                *expected_name == name,
                 "expected {} to be named `{}`, but got one named `{}`",
                 ctx,
                 expected_name,

tracing/tests/support/span.rs

@@ -1,5 +1,6 @@
 #![allow(missing_docs)]
 use super::{field, metadata, Parent};
+use std::borrow::Cow;
 use std::fmt;
 
 /// A mock span.
@@ -60,7 +61,7 @@ impl MockSpan {
 
     pub fn with_explicit_parent(self, parent: Option<&str>) -> NewSpan {
         let parent = match parent {
-            Some(name) => Parent::Explicit(name.into()),
+            Some(name) => Parent::Explicit(name.to_string()),
             None => Parent::ExplicitRoot,
         };
         NewSpan {
@@ -72,7 +73,7 @@ impl MockSpan {
 
     pub fn with_contextual_parent(self, parent: Option<&str>) -> NewSpan {
         let parent = match parent {
-            Some(name) => Parent::Contextual(name.into()),
+            Some(name) => Parent::Contextual(name.to_string()),
             None => Parent::ContextualRoot,
         };
         NewSpan {
@@ -82,8 +83,10 @@ impl MockSpan {
         }
     }
 
-    pub fn name(&self) -> Option<&str> {
-        self.metadata.name.as_ref().map(String::as_ref)
+    pub fn name(&self) -> Option<Cow<'static, str>> {
+        // TODO: can do better than this I think, I hope.
+        // Was: self.metadata.name.as_ref().map(String::as_ref)
+        self.metadata.name.as_ref().map(|s| s.clone().into())
     }
 
     pub fn with_field<I>(self, fields: I) -> NewSpan
@@ -125,7 +128,7 @@ impl Into<NewSpan> for MockSpan {
 impl NewSpan {
     pub fn with_explicit_parent(self, parent: Option<&str>) -> NewSpan {
         let parent = match parent {
-            Some(name) => Parent::Explicit(name.into()),
+            Some(name) => Parent::Explicit(name.to_string()),
             None => Parent::ExplicitRoot,
         };
         NewSpan {
@@ -136,7 +139,7 @@ impl NewSpan {
 
     pub fn with_contextual_parent(self, parent: Option<&str>) -> NewSpan {
         let parent = match parent {
-            Some(name) => Parent::Contextual(name.into()),
+            Some(name) => Parent::Contextual(name.to_string()),
             None => Parent::ContextualRoot,
         };
         NewSpan {

tracing/tests/support/subscriber.rs

@@ -1,11 +1,13 @@
 #![allow(missing_docs)]
+
 use super::{
     event::MockEvent,
     field as mock_field,
     span::{MockSpan, NewSpan},
     Parent,
 };
 use std::{
+    borrow::Cow,
     collections::{HashMap, VecDeque},
     fmt,
     sync::{
@@ -34,7 +36,8 @@ enum Expect {
 }
 
 struct SpanState {
-    name: &'static str,
+    name: Cow<'static, str>,
+    // name: &'static str,
     refs: usize,
 }
 
@@ -250,9 +253,9 @@ where
                     }
                     Some(Parent::Explicit(expected_parent)) => {
                         let actual_parent =
-                            event.parent().and_then(|id| spans.get(id)).map(|s| s.name);
+                            event.parent().and_then(|id| spans.get(id)).map(|s| s.name.to_string());
                         assert_eq!(
-                            Some(expected_parent.as_ref()),
+                            Some(expected_parent.clone()),
                             actual_parent,
                             "[{}] expected {:?} to have explicit parent {:?}",
                             self.name,
@@ -283,9 +286,9 @@ where
                         );
                         let stack = self.current.lock().unwrap();
                         let actual_parent =
-                            stack.last().and_then(|id| spans.get(id)).map(|s| s.name);
+                            stack.last().and_then(|id| spans.get(id)).map(|s| s.name.to_string());
                         assert_eq!(
-                            Some(expected_parent.as_ref()),
+                            Some(expected_parent.clone()),
                             actual_parent,
                             "[{}] expected {:?} to have contextual parent {:?}",
                             self.name,
@@ -345,9 +348,9 @@ where
                     }
                     Some(Parent::Explicit(expected_parent)) => {
                         let actual_parent =
-                            span.parent().and_then(|id| spans.get(id)).map(|s| s.name);
+                            span.parent().and_then(|id| spans.get(id)).map(|s| s.name.to_string());
                         assert_eq!(
-                            Some(expected_parent.as_ref()),
+                            Some(expected_parent.clone()),
                             actual_parent,
                             "[{}] expected {:?} to have explicit parent {:?}",
                             self.name,
@@ -378,9 +381,11 @@ where
                         );
                         let stack = self.current.lock().unwrap();
                         let actual_parent =
-                            stack.last().and_then(|id| spans.get(id)).map(|s| s.name);
+                            stack.last()
+                                .and_then(|id| spans.get(id))
+                                .map(|s| s.name.to_string());
                         assert_eq!(
-                            Some(expected_parent.as_ref()),
+                            Some(expected_parent.clone()),
                             actual_parent,
                             "[{}] expected {:?} to have contextual parent {:?}",
                             self.name,
@@ -444,7 +449,7 @@ where
                     "[{}] exited span {:?}, but the current span was {:?}",
                     self.name,
                     span.name,
-                    curr.as_ref().and_then(|id| spans.get(id)).map(|s| s.name)
+                    curr.as_ref().and_then(|id| spans.get(id)).map(|s| &s.name)
                 );
             }
             Some(ex) => ex.bad(&self.name, format_args!("exited span {:?}", span.name)),
@@ -453,7 +458,9 @@ where
 
     fn clone_span(&self, id: &Id) -> Id {
         let name = self.spans.lock().unwrap().get_mut(id).map(|span| {
-            let name = span.name;
+            // let name = span.name.into_owned();
+            // let name = Cow::Owned(span.name.into_owned());
+            let name = span.name.to_string();
             println!(
                 "[{}] clone_span: {}; id={:?}; refs={:?};",
                 self.name, name, id, span.refs
@@ -468,7 +475,7 @@ where
         let was_expected = if let Some(Expect::CloneSpan(ref span)) = expected.front() {
             assert_eq!(
                 name,
-                span.name(),
+                span.name().map(|s| s.to_string()),
                 "[{}] expected to clone a span named {:?}",
                 self.name,
                 span.name()
@@ -487,7 +494,7 @@ where
         let mut is_event = false;
         let name = if let Ok(mut spans) = self.spans.try_lock() {
             spans.get_mut(&id).map(|span| {
-                let name = span.name;
+                let name = span.name.to_string();
                 if name.contains("event") {
                     is_event = true;
                 }
@@ -510,7 +517,7 @@ where
                     // Don't assert if this function was called while panicking,
                     // as failing the assertion can cause a double panic.
                     if !::std::thread::panicking() {
-                        assert_eq!(name, span.name());
+                        assert_eq!(name, span.name().map(|s| s.to_string() ));
                     }
                     true
                 }