From 5868a6d4e65a3d4d730d707f99057236f6ebc814 Mon Sep 17 00:00:00 2001 From: David Palm Date: Mon, 28 Sep 2020 17:13:42 +0200 Subject: [PATCH 01/20] [WIP] Metadata name is Cow<'static, str> NOTE: requires changes in opentelemetry-rust as well --- Cargo.toml | 3 ++ tracing-core/src/metadata.rs | 10 +++--- tracing-log/src/trace_logger.rs | 5 +-- tracing-opentelemetry/src/layer.rs | 13 +++++-- tracing-serde/src/lib.rs | 2 +- .../src/filter/env/directive.rs | 2 +- tracing-subscriber/src/fmt/format/json.rs | 2 +- tracing-subscriber/src/registry/mod.rs | 5 +-- tracing-subscriber/src/registry/sharded.rs | 17 +++++---- .../filter_caching_is_lexically_scoped.rs | 2 +- ...s_are_not_reevaluated_for_the_same_span.rs | 2 +- ...re_reevaluated_for_different_call_sites.rs | 2 +- tracing/tests/support/event.rs | 2 +- tracing/tests/support/metadata.rs | 2 +- tracing/tests/support/span.rs | 15 ++++---- tracing/tests/support/subscriber.rs | 35 +++++++++++-------- 16 files changed, 73 insertions(+), 46 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 79b600dbfb..e68d0f4b2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,3 +17,6 @@ members = [ "tracing-journald", "examples" ] + +[patch.crates-io] +opentelemetry = { path = "../opentelemetry-rust" } diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index c575a55b45..e3f0271ef0 100644 --- a/tracing-core/src/metadata.rs +++ b/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 diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index a2c931c68f..b2b4176f1d 100644 --- a/tracing-log/src/trace_logger.rs +++ b/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, 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 }; diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 02b8689013..3f0b6f3b0b 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/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> + { 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> + { + 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); diff --git a/tracing-serde/src/lib.rs b/tracing-serde/src/lib.rs index d98e28b459..568c4a76b2 100644 --- a/tracing-serde/src/lib.rs +++ b/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())?; diff --git a/tracing-subscriber/src/filter/env/directive.rs b/tracing-subscriber/src/filter/env/directive.rs index de8019437b..f9f71297c7 100644 --- a/tracing-subscriber/src/filter/env/directive.rs +++ b/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; } } diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs index d26c822df0..9a5761e59c 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/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() } } diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index cc84e5fb5c..98ff256b55 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/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. diff --git a/tracing-subscriber/src/registry/sharded.rs b/tracing-subscriber/src/registry/sharded.rs index c55b8081ce..5de6f40259 100644 --- a/tracing-subscriber/src/registry/sharded.rs +++ b/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, 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>` 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::>() diff --git a/tracing/tests/filter_caching_is_lexically_scoped.rs b/tracing/tests/filter_caching_is_lexically_scoped.rs index 9b997a0c6b..4931d0906c 100644 --- a/tracing/tests/filter_caching_is_lexically_scoped.rs +++ b/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 diff --git a/tracing/tests/filters_are_not_reevaluated_for_the_same_span.rs b/tracing/tests/filters_are_not_reevaluated_for_the_same_span.rs index 53e11ef448..e5d6d19c8c 100644 --- a/tracing/tests/filters_are_not_reevaluated_for_the_same_span.rs +++ b/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 diff --git a/tracing/tests/filters_are_reevaluated_for_different_call_sites.rs b/tracing/tests/filters_are_reevaluated_for_different_call_sites.rs index 5675e4e678..0362c50013 100644 --- a/tracing/tests/filters_are_reevaluated_for_different_call_sites.rs +++ b/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 diff --git a/tracing/tests/support/event.rs b/tracing/tests/support/event.rs index 7033d8a134..11ef3553a7 100644 --- a/tracing/tests/support/event.rs +++ b/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 { diff --git a/tracing/tests/support/metadata.rs b/tracing/tests/support/metadata.rs index 2c3606b05e..144c738768 100644 --- a/tracing/tests/support/metadata.rs +++ b/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, diff --git a/tracing/tests/support/span.rs b/tracing/tests/support/span.rs index 023e5b7079..a4819ea544 100644 --- a/tracing/tests/support/span.rs +++ b/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> { + // 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(self, fields: I) -> NewSpan @@ -125,7 +128,7 @@ impl Into 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 { diff --git a/tracing/tests/support/subscriber.rs b/tracing/tests/support/subscriber.rs index 3f90e8daf9..055d701c43 100644 --- a/tracing/tests/support/subscriber.rs +++ b/tracing/tests/support/subscriber.rs @@ -1,4 +1,5 @@ #![allow(missing_docs)] + use super::{ event::MockEvent, field as mock_field, @@ -6,6 +7,7 @@ use super::{ 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 } From f40de7b36a3789c4e88a84db1ef1b3ac8810f2d5 Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 29 Sep 2020 09:37:58 +0200 Subject: [PATCH 02/20] Fetch patch from git --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e68d0f4b2f..eabdc77b06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,4 +19,4 @@ members = [ ] [patch.crates-io] -opentelemetry = { path = "../opentelemetry-rust" } +opentelemetry = { git = "https://github.com/dvdplm/opentelemetry-rust", branch = "dp-use-cow" } From ed04e30392fd96940c28361204d0734d08d7809e Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 29 Sep 2020 10:16:21 +0200 Subject: [PATCH 03/20] Clean up MockSpan --- tracing/tests/support/span.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tracing/tests/support/span.rs b/tracing/tests/support/span.rs index a4819ea544..4da0494221 100644 --- a/tracing/tests/support/span.rs +++ b/tracing/tests/support/span.rs @@ -84,9 +84,7 @@ impl MockSpan { } pub fn name(&self) -> Option> { - // 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()) + self.metadata.name.as_ref().map(|s| Cow::Owned(s.clone())) } pub fn with_field(self, fields: I) -> NewSpan @@ -100,7 +98,7 @@ impl MockSpan { } } - pub(in crate::support) fn check_metadata(&self, actual: &tracing::Metadata<'_>) { + pub(in crate::support) fn check_metadata(&self, actual: &'static tracing::Metadata<'_>) { self.metadata.check(actual, format_args!("span {}", self)); assert!(actual.is_span(), "expected a span but got {:?}", actual); } From 645c133952da920cf19cccf76b1fa530077b2422 Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 30 Sep 2020 11:20:02 +0200 Subject: [PATCH 04/20] Don't clone the name in Metadata::name Subscriber is static --- tracing-core/src/metadata.rs | 5 ++--- tracing-log/src/trace_logger.rs | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index e3f0271ef0..b9d6f68bfd 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -155,9 +155,8 @@ impl<'a> Metadata<'a> { } /// Returns the name of the span. - pub fn name(&self) -> Cow<'static, str> { - // TODO: do better than this. - self.name.clone() + pub fn name(&'a self) -> Cow<'a, str> { + self.name.as_ref().into() } /// Returns a string describing the part of the system where the span or diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index b2b4176f1d..3759787c2a 100644 --- a/tracing-log/src/trace_logger.rs +++ b/tracing-log/src/trace_logger.rs @@ -43,9 +43,9 @@ use tracing_core::{ /// /// [`Subscriber`]: https://docs.rs/tracing/0.1.7/tracing/subscriber/trait.Subscriber.html /// [flags]: https://docs.rs/tracing/latest/tracing/#crate-feature-flags -pub struct TraceLogger { +pub struct TraceLogger<'a> { settings: Builder, - spans: Mutex>, + spans: Mutex>>, next_id: AtomicUsize, } @@ -67,7 +67,7 @@ pub struct Builder { // ===== impl TraceLogger ===== -impl TraceLogger { +impl TraceLogger<'static> { /// Returns a new `TraceLogger` with the default configuration. pub fn new() -> Self { Self::builder().finish() @@ -153,7 +153,7 @@ impl Builder { /// Complete the builder, returning a configured [`TraceLogger`]. /// /// [`TraceLogger`]: struct.TraceLogger.html - pub fn finish(self) -> TraceLogger { + pub fn finish(self) -> TraceLogger<'static> { TraceLogger::from_builder(self) } } @@ -171,7 +171,7 @@ impl Default for Builder { } } -impl Default for TraceLogger { +impl Default for TraceLogger<'_> { fn default() -> Self { TraceLogger { settings: Default::default(), @@ -182,7 +182,7 @@ impl Default for TraceLogger { } #[derive(Debug)] -struct SpanLineBuilder { +struct SpanLineBuilder<'a> { parent: Option, ref_count: usize, fields: String, @@ -191,11 +191,11 @@ struct SpanLineBuilder { module_path: Option, target: String, level: log::Level, - name: Cow<'static, str>, + name: Cow<'a, str>, } -impl SpanLineBuilder { - fn new(parent: Option, meta: &Metadata<'_>, fields: String) -> Self { +impl<'a> SpanLineBuilder<'a> { + fn new(parent: Option, meta: &'a Metadata<'_>, fields: String) -> Self { Self { parent, ref_count: 1, @@ -234,14 +234,14 @@ impl SpanLineBuilder { } } -impl field::Visit for SpanLineBuilder { +impl field::Visit for SpanLineBuilder<'_> { fn record_debug(&mut self, field: &field::Field, value: &dyn fmt::Debug) { write!(self.fields, " {}={:?};", field.name(), value) .expect("write to string should never fail") } } -impl Subscriber for TraceLogger { +impl Subscriber for TraceLogger<'static> { fn enabled(&self, metadata: &Metadata<'_>) -> bool { log::logger().enabled(&metadata.as_log()) } @@ -425,7 +425,7 @@ impl Subscriber for TraceLogger { } } -impl TraceLogger { +impl TraceLogger<'static> { #[inline] fn current_id(&self) -> Option { CURRENT @@ -456,7 +456,7 @@ impl<'a> fmt::Display for LogEvent<'a> { } } -impl fmt::Debug for TraceLogger { +impl fmt::Debug for TraceLogger<'static> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("TraceLogger") .field("settings", &self.settings) From 5a69f070d3e180b2bebd863c925787ef4d2726e3 Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 30 Sep 2020 14:03:44 +0200 Subject: [PATCH 05/20] Use Cow in assert_last_closed --- tracing-subscriber/src/registry/sharded.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/registry/sharded.rs b/tracing-subscriber/src/registry/sharded.rs index 5de6f40259..4bb78fa5e1 100644 --- a/tracing-subscriber/src/registry/sharded.rs +++ b/tracing-subscriber/src/registry/sharded.rs @@ -555,19 +555,17 @@ mod tests { } #[allow(unused)] // may want this for future tests - // TODO: take an `Option>` instead? - fn assert_last_closed(&self, span: Option<&str>) { + fn assert_last_closed(&self, span: Option>) { let lock = self.state.lock().unwrap(); - let last = lock.closed.last().map(|(span, _)| span.as_ref()); + let last = lock.closed.last().map(|(span, _)| span); assert_eq!( last, - span, + span.as_ref(), "expected {:?} to have closed last", - span + span.as_ref() ); } - // 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(); From f922cc138cb02ac313002a7b468d4bbd19392bef Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 30 Sep 2020 14:16:31 +0200 Subject: [PATCH 06/20] If Metadata is always used with a 'static lifetime it might make sense to just have `name` be a `Cow<'a, str>` instead for clarity. --- tracing-core/src/metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index b9d6f68bfd..0a6bcfcde0 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -61,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: Cow<'static, str>, + name: Cow<'a, str>, /// The part of the system that the span that this metadata describes /// occurred in. @@ -156,7 +156,7 @@ impl<'a> Metadata<'a> { /// Returns the name of the span. pub fn name(&'a self) -> Cow<'a, str> { - self.name.as_ref().into() + Cow::Borrowed(&self.name) } /// Returns a string describing the part of the system where the span or From fa4a808c5f69aedb1ea8f673a0a7d77bed3426e8 Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 30 Sep 2020 15:18:27 +0200 Subject: [PATCH 07/20] cleanup --- tracing/tests/support/subscriber.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/tracing/tests/support/subscriber.rs b/tracing/tests/support/subscriber.rs index 055d701c43..cfacf208b3 100644 --- a/tracing/tests/support/subscriber.rs +++ b/tracing/tests/support/subscriber.rs @@ -37,7 +37,6 @@ enum Expect { struct SpanState { name: Cow<'static, str>, - // name: &'static str, refs: usize, } @@ -458,8 +457,6 @@ where fn clone_span(&self, id: &Id) -> Id { let name = self.spans.lock().unwrap().get_mut(id).map(|span| { - // 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={:?};", From 97e5b318826195e3d229566ff38afb3274b96f6f Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 17:29:36 +0200 Subject: [PATCH 08/20] =?UTF-8?q?Metadata.target=20is=20Cow=20=E2=80=93=20?= =?UTF-8?q?breaks=20AsLog=20impl?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tracing-core/src/metadata.rs | 9 +++++---- tracing-log/src/lib.rs | 15 ++++++++------- tracing-log/src/trace_logger.rs | 2 +- tracing-serde/src/lib.rs | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 0a6bcfcde0..683a6a9573 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -65,7 +65,8 @@ pub struct Metadata<'a> { /// The part of the system that the span that this metadata describes /// occurred in. - target: &'a str, + // target: &'a str, + target: Cow<'a, str>, /// The level of verbosity of the described span. level: Level, @@ -134,7 +135,7 @@ impl<'a> Metadata<'a> { ) -> Self { Metadata { name: Cow::Borrowed(name), - target, + target: Cow::Borrowed(target), level, module_path, file, @@ -164,8 +165,8 @@ impl<'a> Metadata<'a> { /// /// Typically, this is the module path, but alternate targets may be set /// when spans or events are constructed. - pub fn target(&self) -> &'a str { - self.target + pub fn target(&'a self) -> Cow<'a, str> { + Cow::Borrowed(&self.target) } /// Returns the path to the Rust module where the span occurred, or diff --git a/tracing-log/src/lib.rs b/tracing-log/src/lib.rs index c4cf239dcc..43fa53ab5c 100644 --- a/tracing-log/src/lib.rs +++ b/tracing-log/src/lib.rs @@ -208,11 +208,11 @@ pub fn format_trace(record: &log::Record<'_>) -> io::Result<()> { /// Trait implemented for `tracing` types that can be converted to a `log` /// equivalent. -pub trait AsLog: crate::sealed::Sealed { +pub trait AsLog<'a>: crate::sealed::Sealed { /// The `log` type that this type can be converted into. type Log; /// Returns the `log` equivalent of `self`. - fn as_log(&self) -> Self::Log; + fn as_log<'b: 'a>(&'b self) -> Self::Log; } /// Trait implemented for `log` types that can be converted to a `tracing` @@ -226,12 +226,13 @@ pub trait AsTrace: crate::sealed::Sealed { impl<'a> crate::sealed::Sealed for Metadata<'a> {} -impl<'a> AsLog for Metadata<'a> { +use std::borrow::Cow; +impl<'a> AsLog<'a> for Metadata<'a> { type Log = log::Metadata<'a>; - fn as_log(&self) -> Self::Log { + fn as_log<'b: 'a>(&'b self) -> Self::Log { log::Metadata::builder() .level(self.level().as_log()) - .target(self.target()) + .target(&self.target()) .build() } } @@ -351,9 +352,9 @@ impl<'a> AsTrace for log::Record<'a> { impl crate::sealed::Sealed for tracing_core::Level {} -impl AsLog for tracing_core::Level { +impl<'a> AsLog<'a> for tracing_core::Level { type Log = log::Level; - fn as_log(&self) -> log::Level { + fn as_log<'b: 'a>(&'b self) -> log::Level { match *self { tracing_core::Level::ERROR => log::Level::Error, tracing_core::Level::WARN => log::Level::Warn, diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index 3759787c2a..62f1730bbe 100644 --- a/tracing-log/src/trace_logger.rs +++ b/tracing-log/src/trace_logger.rs @@ -384,7 +384,7 @@ impl Subscriber for TraceLogger<'static> { logger.log( &log::Record::builder() .metadata(log_meta) - .target(meta.target()) + .target(&meta.target()) .module_path(meta.module_path().as_ref().cloned()) .file(meta.file().as_ref().cloned()) .line(meta.line()) diff --git a/tracing-serde/src/lib.rs b/tracing-serde/src/lib.rs index 568c4a76b2..b2ef94dc29 100644 --- a/tracing-serde/src/lib.rs +++ b/tracing-serde/src/lib.rs @@ -245,7 +245,7 @@ impl<'a> Serialize for SerializeMetadata<'a> { { let mut state = serializer.serialize_struct("Metadata", 9)?; state.serialize_field("name", &self.0.name())?; - state.serialize_field("target", self.0.target())?; + state.serialize_field("target", &self.0.target())?; state.serialize_field("level", &SerializeLevel(self.0.level()))?; state.serialize_field("module_path", &self.0.module_path())?; state.serialize_field("file", &self.0.file())?; From 028ae659e272301dbcc871b0753b58194c1ec266 Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 20:11:53 +0200 Subject: [PATCH 09/20] Make it build (ty Ben) Store Cows for name and target but return &str --- tracing-core/src/metadata.rs | 10 +++++----- tracing-log/src/lib.rs | 1 - tracing-log/src/trace_logger.rs | 2 +- tracing-subscriber/src/filter/env/directive.rs | 2 +- tracing-subscriber/src/registry/mod.rs | 3 +-- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 683a6a9573..61ce1570c7 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -124,7 +124,7 @@ impl<'a> Metadata<'a> { /// Construct new metadata for a span or event, with a name, target, level, field /// names, and optional source code location. pub const fn new( - name: &'static str, + name: &'a str, target: &'a str, level: Level, file: Option<&'a str>, @@ -156,8 +156,8 @@ impl<'a> Metadata<'a> { } /// Returns the name of the span. - pub fn name(&'a self) -> Cow<'a, str> { - Cow::Borrowed(&self.name) + pub fn name(&self) -> &str { + &self.name } /// Returns a string describing the part of the system where the span or @@ -165,8 +165,8 @@ impl<'a> Metadata<'a> { /// /// Typically, this is the module path, but alternate targets may be set /// when spans or events are constructed. - pub fn target(&'a self) -> Cow<'a, str> { - Cow::Borrowed(&self.target) + pub fn target(&self) -> &str { + &self.target } /// Returns the path to the Rust module where the span occurred, or diff --git a/tracing-log/src/lib.rs b/tracing-log/src/lib.rs index 43fa53ab5c..ab285bceb7 100644 --- a/tracing-log/src/lib.rs +++ b/tracing-log/src/lib.rs @@ -226,7 +226,6 @@ pub trait AsTrace: crate::sealed::Sealed { impl<'a> crate::sealed::Sealed for Metadata<'a> {} -use std::borrow::Cow; impl<'a> AsLog<'a> for Metadata<'a> { type Log = log::Metadata<'a>; fn as_log<'b: 'a>(&'b self) -> Self::Log { diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index 62f1730bbe..f978f8b5c3 100644 --- a/tracing-log/src/trace_logger.rs +++ b/tracing-log/src/trace_logger.rs @@ -205,7 +205,7 @@ impl<'a> SpanLineBuilder<'a> { module_path: meta.module_path().map(String::from), target: String::from(meta.target()), level: meta.level().as_log(), - name: meta.name(), + name: Cow::Borrowed(meta.name()), // TODO (dp): can do better than this? } } diff --git a/tracing-subscriber/src/filter/env/directive.rs b/tracing-subscriber/src/filter/env/directive.rs index f9f71297c7..de8019437b 100644 --- a/tracing-subscriber/src/filter/env/directive.rs +++ b/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().as_ref() { + if name != meta.name() { return false; } } diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 98ff256b55..f1fe72a434 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -63,7 +63,6 @@ //! [`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; @@ -214,7 +213,7 @@ where } /// Returns the span's name, - pub fn name(&self) -> Cow<'static, str> { + pub fn name(&self) -> &str { self.metadata().name() } From a3661f846e52e62bafbeaf4191e8dafd5fffaf3c Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 20:45:23 +0200 Subject: [PATCH 10/20] Make tests pass Change test-only `CloseState` to use `String` Undo a few changes that are no longer necessary --- tracing-subscriber/src/registry/sharded.rs | 13 ++++++------- tracing/tests/support/subscriber.rs | 3 +-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tracing-subscriber/src/registry/sharded.rs b/tracing-subscriber/src/registry/sharded.rs index 4bb78fa5e1..fa342e2536 100644 --- a/tracing-subscriber/src/registry/sharded.rs +++ b/tracing-subscriber/src/registry/sharded.rs @@ -369,7 +369,6 @@ mod tests { use super::Registry; use crate::{layer::Context, registry::LookupSpan, Layer}; use std::{ - borrow::Cow, collections::HashMap, sync::{Arc, Mutex, Weak}, }; @@ -423,8 +422,8 @@ mod tests { #[derive(Default)] struct CloseState { - open: HashMap, Weak<()>>, - closed: Vec<(Cow<'static, str>, Weak<()>)>, + open: HashMap>, + closed: Vec<(String, Weak<()>)>, } struct SetRemoved(Arc<()>); @@ -439,7 +438,7 @@ mod tests { let is_removed = Arc::new(()); assert!( lock.open - .insert(span.name(), Arc::downgrade(&is_removed)) + .insert(span.name().to_string(), Arc::downgrade(&is_removed)) .is_none(), "test layer saw multiple spans with the same name, the test is probably messed up" ); @@ -460,9 +459,9 @@ 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.as_ref()) { + if let Some(is_removed) = lock.open.remove(name) { assert!(is_removed.upgrade().is_some()); - lock.closed.push((name, is_removed)); + lock.closed.push((name.to_string(), is_removed)); } } } @@ -555,7 +554,7 @@ mod tests { } #[allow(unused)] // may want this for future tests - fn assert_last_closed(&self, span: Option>) { + fn assert_last_closed(&self, span: Option) { let lock = self.state.lock().unwrap(); let last = lock.closed.last().map(|(span, _)| span); assert_eq!( diff --git a/tracing/tests/support/subscriber.rs b/tracing/tests/support/subscriber.rs index cfacf208b3..710cd60313 100644 --- a/tracing/tests/support/subscriber.rs +++ b/tracing/tests/support/subscriber.rs @@ -7,7 +7,6 @@ use super::{ Parent, }; use std::{ - borrow::Cow, collections::{HashMap, VecDeque}, fmt, sync::{ @@ -36,7 +35,7 @@ enum Expect { } struct SpanState { - name: Cow<'static, str>, + name: &'static str, refs: usize, } From f0a644352b32e2dc8f3440b6fe5264f3efa16f46 Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 20:56:33 +0200 Subject: [PATCH 11/20] Undo changes to TestTracer and patched opentelemetry, not needed --- Cargo.toml | 4 ++-- tracing-opentelemetry/src/layer.rs | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index eabdc77b06..a370b82a3c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,5 +18,5 @@ members = [ "examples" ] -[patch.crates-io] -opentelemetry = { git = "https://github.com/dvdplm/opentelemetry-rust", branch = "dp-use-cow" } +# [patch.crates-io] +# opentelemetry = { git = "https://github.com/dvdplm/opentelemetry-rust", branch = "dp-use-cow" } diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 3f0b6f3b0b..02b8689013 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -489,7 +489,6 @@ 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::*; @@ -501,17 +500,11 @@ mod tests { fn invalid(&self) -> Self::Span { api::NoopSpan::new() } - fn start_from_context<'a, S>(&self, _name: S, _context: &api::Context) -> Self::Span - where - S: Into> - { + fn start_from_context(&self, _name: &str, _context: &api::Context) -> Self::Span { self.invalid() } - fn span_builder<'a, S>(&self, name: S) -> api::SpanBuilder - where - S: Into> - { - api::SpanBuilder::from_name(name.into().into_owned()) + fn span_builder(&self, name: &str) -> api::SpanBuilder { + api::SpanBuilder::from_name(name.to_string()) } fn build_with_context(&self, builder: api::SpanBuilder, _cx: &api::Context) -> Self::Span { *self.0.lock().unwrap() = Some(builder); From a9a05686d1e314683beae1871c7edd807e59062d Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 20:58:10 +0200 Subject: [PATCH 12/20] Remove patch --- Cargo.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a370b82a3c..79b600dbfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,3 @@ members = [ "tracing-journald", "examples" ] - -# [patch.crates-io] -# opentelemetry = { git = "https://github.com/dvdplm/opentelemetry-rust", branch = "dp-use-cow" } From 2ee5da9814e2ec46b2abff7be8a629289cf2717f Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 21:48:07 +0200 Subject: [PATCH 13/20] cleanup --- tracing-log/src/trace_logger.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index f978f8b5c3..6ac9a51304 100644 --- a/tracing-log/src/trace_logger.rs +++ b/tracing-log/src/trace_logger.rs @@ -205,7 +205,7 @@ impl<'a> SpanLineBuilder<'a> { module_path: meta.module_path().map(String::from), target: String::from(meta.target()), level: meta.level().as_log(), - name: Cow::Borrowed(meta.name()), // TODO (dp): can do better than this? + name: meta.name().into(), } } From d8afcbe34781dcf0065c274437d1639dc9f4bdc1 Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 6 Oct 2020 22:04:04 +0200 Subject: [PATCH 14/20] cleanup --- tracing-core/src/metadata.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 61ce1570c7..4de8056cc8 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -65,7 +65,6 @@ pub struct Metadata<'a> { /// The part of the system that the span that this metadata describes /// occurred in. - // target: &'a str, target: Cow<'a, str>, /// The level of verbosity of the described span. From 891375979b542e9810a5640efe23fbb1cb11b417 Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 7 Oct 2020 12:47:53 +0200 Subject: [PATCH 15/20] Store file as Cow --- tracing-core/src/metadata.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 4b3d79abac..ef1f79683f 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -76,7 +76,7 @@ pub struct Metadata<'a> { /// The name of the source code file where the span occurred, or `None` if /// this could not be determined. - file: Option<&'a str>, + file: Option>, /// The line number in the source code file where the span occurred, or /// `None` if this could not be determined. @@ -132,6 +132,13 @@ impl<'a> Metadata<'a> { fields: field::FieldSet, kind: Kind, ) -> Self { + let file = { + if let Some(file) = file { + Some(Cow::Borrowed(file)) + } else { + None + } + }; Metadata { name: Cow::Borrowed(name), target: Cow::Borrowed(target), @@ -176,8 +183,8 @@ impl<'a> Metadata<'a> { /// Returns the name of the source code file where the span /// occurred, or `None` if the file is unknown - pub fn file(&self) -> Option<&'a str> { - self.file + pub fn file(&'a self) -> Option<&'a str> { + self.file.as_ref().map(|f| f.as_ref() ) } /// Returns the line number in the source code file where the span From b25010991694a7c4d4704319d6a1fcb83dbc8c7f Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 7 Oct 2020 12:53:39 +0200 Subject: [PATCH 16/20] Store module_path as Cow --- tracing-core/src/metadata.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index ef1f79683f..816c9cd28d 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -72,7 +72,7 @@ pub struct Metadata<'a> { /// The name of the Rust module where the span occurred, or `None` if this /// could not be determined. - module_path: Option<&'a str>, + module_path: Option>, /// The name of the source code file where the span occurred, or `None` if /// this could not be determined. @@ -139,6 +139,13 @@ impl<'a> Metadata<'a> { None } }; + let module_path = { + if let Some(module_path) = module_path { + Some(Cow::Borrowed(module_path)) + } else { + None + } + }; Metadata { name: Cow::Borrowed(name), target: Cow::Borrowed(target), @@ -177,8 +184,8 @@ impl<'a> Metadata<'a> { /// Returns the path to the Rust module where the span occurred, or /// `None` if the module path is unknown. - pub fn module_path(&self) -> Option<&'a str> { - self.module_path + pub fn module_path(&'a self) -> Option<&'a str> { + self.module_path.as_ref().map(|p| p.as_ref() ) } /// Returns the name of the source code file where the span From b3bec865aeee77220403f63ccda909fe43ff7378 Mon Sep 17 00:00:00 2001 From: David Palm Date: Mon, 19 Oct 2020 22:34:44 +0200 Subject: [PATCH 17/20] Feature gate usage of Cow in Metadata --- tracing-core/src/metadata.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 816c9cd28d..18a6e69847 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -1,5 +1,6 @@ //! Metadata describing trace data. use super::{callsite, field}; +#[cfg(feature = "alloc")] use alloc::borrow::Cow; use core::{ cmp, fmt, @@ -61,22 +62,34 @@ use core::{ /// [callsite identifier]: super::callsite::Identifier pub struct Metadata<'a> { /// The name of the span described by this metadata. + #[cfg(feature = "alloc")] name: Cow<'a, str>, + #[cfg(not(feature = "alloc"))] + name: &'a str, /// The part of the system that the span that this metadata describes /// occurred in. + #[cfg(feature = "alloc")] target: Cow<'a, str>, + #[cfg(not(feature = "alloc"))] + target: &'a str, /// The level of verbosity of the described span. level: Level, /// The name of the Rust module where the span occurred, or `None` if this /// could not be determined. + #[cfg(feature = "alloc")] module_path: Option>, + #[cfg(not(feature = "alloc"))] + module_path: Option<&'a str>, /// The name of the source code file where the span occurred, or `None` if /// this could not be determined. + #[cfg(feature = "alloc")] file: Option>, + #[cfg(not(feature = "alloc"))] + file: Option<&'a str>, /// The line number in the source code file where the span occurred, or /// `None` if this could not be determined. @@ -132,6 +145,7 @@ impl<'a> Metadata<'a> { fields: field::FieldSet, kind: Kind, ) -> Self { + #[cfg(feature = "alloc")] let file = { if let Some(file) = file { Some(Cow::Borrowed(file)) @@ -139,6 +153,7 @@ impl<'a> Metadata<'a> { None } }; + #[cfg(feature = "alloc")] let module_path = { if let Some(module_path) = module_path { Some(Cow::Borrowed(module_path)) @@ -146,9 +161,16 @@ impl<'a> Metadata<'a> { None } }; + Metadata { + #[cfg(feature = "alloc")] name: Cow::Borrowed(name), + #[cfg(not(feature = "alloc"))] + name, + #[cfg(feature = "alloc")] target: Cow::Borrowed(target), + #[cfg(not(feature = "alloc"))] + target, level, module_path, file, From fda189b976dc7bf42f6ef2e7b61e64f0077f7838 Mon Sep 17 00:00:00 2001 From: David Palm Date: Mon, 19 Oct 2020 23:44:51 +0200 Subject: [PATCH 18/20] Add constructor for dynamic data --- tracing-core/src/metadata.rs | 80 ++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 18a6e69847..1255726840 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -180,6 +180,35 @@ impl<'a> Metadata<'a> { } } + /// Construct new metadata for a span or event, with a name, target, level, field + /// names, and optional source code location using dynamically allocated data. + #[cfg(feature = "alloc")] + #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))] + pub fn from_cow( + name: S, + target: S, + level: Level, + file: Option, + line: Option, + module_path: Option, + fields: field::FieldSet, + kind: Kind, + ) -> Self + where + S: Into>, + { + Metadata { + name: name.into(), + target: target.into(), + level, + module_path: module_path.map(Into::into), + file: file.map(Into::into), + line, + fields, + kind + } + } + /// Returns the names of the fields on the described span or event. pub fn fields(&self) -> &field::FieldSet { &self.fields @@ -837,6 +866,13 @@ impl PartialOrd for LevelFilter { mod tests { use super::*; use core::mem; + #[cfg(feature = "alloc")] + use crate::{ + callsite::{Callsite, Identifier}, + field::FieldSet, + Interest, + }; + #[test] fn level_from_str() { @@ -900,4 +936,48 @@ mod tests { assert_eq!(expected, repr, "repr changed for {:?}", filter) } } + + #[cfg(feature = "alloc")] + #[test] + fn create_metadata_from_dynamic_data() { + struct TestCallsite; + static CS1: TestCallsite = TestCallsite; + + impl Callsite for TestCallsite { + fn set_interest(&self, _interest: Interest) {} + fn metadata(&self) -> &Metadata<'_> { + unimplemented!("not needed for this test") + } + } + let callsite_id = Identifier(&CS1); + let field_set = FieldSet::new(&["one", "fine", "day"], callsite_id); + + // Use `String`s + let _metadata = Metadata::from_cow( + "a name".to_string(), + "a target".to_string(), + Level::TRACE, + None, + None, + None, + field_set, + Kind::EVENT, + ); + + let callsite_id = Identifier(&CS1); + let field_set = FieldSet::new(&["one", "fine", "day"], callsite_id); + // Use `str`s + let _metadata = Metadata::from_cow( + "a name", + "a target", + Level::TRACE, + None, + None, + None, + field_set, + Kind::EVENT, + ); + + // TODO dp: come up with a test that makes sense. + } } From c46c352b035e279edd2e304f5479a1230e1d4362 Mon Sep 17 00:00:00 2001 From: David Palm Date: Mon, 19 Oct 2020 23:54:13 +0200 Subject: [PATCH 19/20] No need for extra lifetime --- tracing-log/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tracing-log/src/lib.rs b/tracing-log/src/lib.rs index 90fe0650d5..3c696eea49 100644 --- a/tracing-log/src/lib.rs +++ b/tracing-log/src/lib.rs @@ -213,7 +213,7 @@ pub trait AsLog<'a>: crate::sealed::Sealed { /// The `log` type that this type can be converted into. type Log; /// Returns the `log` equivalent of `self`. - fn as_log<'b: 'a>(&'b self) -> Self::Log; + fn as_log(&'a self) -> Self::Log; } /// Trait implemented for `log` types that can be converted to a `tracing` @@ -229,7 +229,7 @@ impl<'a> crate::sealed::Sealed for Metadata<'a> {} impl<'a> AsLog<'a> for Metadata<'a> { type Log = log::Metadata<'a>; - fn as_log<'b: 'a>(&'b self) -> Self::Log { + fn as_log(&'a self) -> Self::Log { log::Metadata::builder() .level(self.level().as_log()) .target(&self.target()) @@ -354,7 +354,7 @@ impl crate::sealed::Sealed for tracing_core::Level {} impl<'a> AsLog<'a> for tracing_core::Level { type Log = log::Level; - fn as_log<'b: 'a>(&'b self) -> log::Level { + fn as_log(&self) -> log::Level { match *self { tracing_core::Level::ERROR => log::Level::Error, tracing_core::Level::WARN => log::Level::Warn, From 280f0bfd1dcfdae4823e8b7e0d784fcfaa7fe637 Mon Sep 17 00:00:00 2001 From: David Palm Date: Tue, 20 Oct 2020 09:30:10 +0200 Subject: [PATCH 20/20] Use impl Into --- tracing-core/src/metadata.rs | 45 ++++++++++-------------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 1255726840..a8de8bfa1e 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -184,19 +184,16 @@ impl<'a> Metadata<'a> { /// names, and optional source code location using dynamically allocated data. #[cfg(feature = "alloc")] #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))] - pub fn from_cow( - name: S, - target: S, + pub fn from_cow( + name: impl Into>, + target: impl Into>, level: Level, - file: Option, + file: Option>>, line: Option, - module_path: Option, + module_path: Option>>, fields: field::FieldSet, kind: Kind, - ) -> Self - where - S: Into>, - { + ) -> Self { Metadata { name: name.into(), target: target.into(), @@ -205,7 +202,7 @@ impl<'a> Metadata<'a> { file: file.map(Into::into), line, fields, - kind + kind, } } @@ -236,13 +233,13 @@ impl<'a> Metadata<'a> { /// Returns the path to the Rust module where the span occurred, or /// `None` if the module path is unknown. pub fn module_path(&'a self) -> Option<&'a str> { - self.module_path.as_ref().map(|p| p.as_ref() ) + self.module_path.as_ref().map(|p| p.as_ref()) } /// Returns the name of the source code file where the span /// occurred, or `None` if the file is unknown pub fn file(&'a self) -> Option<&'a str> { - self.file.as_ref().map(|f| f.as_ref() ) + self.file.as_ref().map(|f| f.as_ref()) } /// Returns the line number in the source code file where the span @@ -865,14 +862,13 @@ impl PartialOrd for LevelFilter { #[cfg(test)] mod tests { use super::*; - use core::mem; #[cfg(feature = "alloc")] use crate::{ callsite::{Callsite, Identifier}, field::FieldSet, Interest, }; - + use core::mem; #[test] fn level_from_str() { @@ -952,32 +948,15 @@ mod tests { let callsite_id = Identifier(&CS1); let field_set = FieldSet::new(&["one", "fine", "day"], callsite_id); - // Use `String`s let _metadata = Metadata::from_cow( "a name".to_string(), - "a target".to_string(), - Level::TRACE, - None, - None, - None, - field_set, - Kind::EVENT, - ); - - let callsite_id = Identifier(&CS1); - let field_set = FieldSet::new(&["one", "fine", "day"], callsite_id); - // Use `str`s - let _metadata = Metadata::from_cow( - "a name", "a target", Level::TRACE, + Some("a file".to_string()), None, - None, - None, + None::>, field_set, Kind::EVENT, ); - - // TODO dp: come up with a test that makes sense. } }