Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Cow<'static, str> for Metadata strings #1020

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5868a6d
[WIP] Metadata name is Cow<'static, str>
dvdplm Sep 28, 2020
f40de7b
Fetch patch from git
dvdplm Sep 29, 2020
ed04e30
Clean up MockSpan
dvdplm Sep 29, 2020
645c133
Don't clone the name in Metadata::name
dvdplm Sep 30, 2020
5a69f07
Use Cow in assert_last_closed
dvdplm Sep 30, 2020
f922cc1
If Metadata is always used with a 'static lifetime it might make sens…
dvdplm Sep 30, 2020
fa4a808
cleanup
dvdplm Sep 30, 2020
183f75f
Merge branch 'master' into dp-use-cow
dvdplm Oct 5, 2020
97e5b31
Metadata.target is Cow – breaks AsLog impl
dvdplm Oct 6, 2020
028ae65
Make it build (ty Ben)
dvdplm Oct 6, 2020
a3661f8
Make tests pass
dvdplm Oct 6, 2020
f0a6443
Undo changes to TestTracer and patched opentelemetry, not needed
dvdplm Oct 6, 2020
a9a0568
Remove patch
dvdplm Oct 6, 2020
2ee5da9
cleanup
dvdplm Oct 6, 2020
d8afcbe
cleanup
dvdplm Oct 6, 2020
381d574
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 7, 2020
8913759
Store file as Cow
dvdplm Oct 7, 2020
b250109
Store module_path as Cow
dvdplm Oct 7, 2020
e29d3ca
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 16, 2020
f7274dc
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 19, 2020
b3bec86
Feature gate usage of Cow in Metadata
dvdplm Oct 19, 2020
fda189b
Add constructor for dynamic data
dvdplm Oct 19, 2020
c46c352
No need for extra lifetime
dvdplm Oct 19, 2020
7b0dfe5
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 19, 2020
280f0bf
Use impl Into<Cow<'a, str>
dvdplm Oct 20, 2020
4225d96
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 22, 2020
f42a644
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
dvdplm Oct 23, 2020
47e9cd5
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
Oct 28, 2020
de588f4
Merge remote-tracking branch 'origin/master' into dp-target-is-cow
dvdplm Dec 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 106 additions & 10 deletions tracing-core/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metadata describing trace data.
use super::{callsite, field};
#[cfg(feature = "alloc")]
use alloc::borrow::Cow;
use core::{
cmp, fmt,
str::FromStr,
Expand Down Expand Up @@ -60,21 +62,33 @@ use core::{
/// [callsite identifier]: super::callsite::Identifier
pub struct Metadata<'a> {
/// The name of the span described by this metadata.
name: &'static str,
#[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,
dvdplm marked this conversation as resolved.
Show resolved Hide resolved

/// The name of the Rust module where the span occurred, or `None` if this
/// could not be determined.
#[cfg(feature = "alloc")]
module_path: Option<Cow<'a, str>>,
#[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<Cow<'a, str>>,
#[cfg(not(feature = "alloc"))]
file: Option<&'a str>,

/// The line number in the source code file where the span occurred, or
Expand Down Expand Up @@ -122,7 +136,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(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while with this we now have general support for dynamic creation, the current API, still won't allow for that. Probably want to add another fn dynamic_new where we can pass Cow<'a, str> into for dynamic dispatching.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid so ....

name: &'static str,
name: &'a str,
target: &'a str,
level: Level,
file: Option<&'a str>,
Expand All @@ -131,8 +145,31 @@ impl<'a> Metadata<'a> {
fields: field::FieldSet,
kind: Kind,
) -> Self {
#[cfg(feature = "alloc")]
let file = {
if let Some(file) = file {
Some(Cow::Borrowed(file))
} else {
None
}
};
#[cfg(feature = "alloc")]
let module_path = {
if let Some(module_path) = module_path {
Some(Cow::Borrowed(module_path))
} else {
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,
Expand All @@ -143,6 +180,32 @@ 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: impl Into<Cow<'a, str>>,
target: impl Into<Cow<'a, str>>,
level: Level,
file: Option<impl Into<Cow<'a, str>>>,
line: Option<u32>,
module_path: Option<impl Into<Cow<'a, str>>>,
fields: field::FieldSet,
kind: Kind,
) -> Self {
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
Expand All @@ -154,29 +217,29 @@ impl<'a> Metadata<'a> {
}

/// Returns the name of the span.
pub fn name(&self) -> &'static str {
self.name
pub fn name(&self) -> &str {
&self.name
}

/// Returns a string describing the part of the system where the span or
/// event that this metadata describes occurred.
///
/// 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(&self) -> &str {
&self.target
}

/// 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
/// 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
Expand Down Expand Up @@ -799,6 +862,12 @@ impl PartialOrd<Level> for LevelFilter {
#[cfg(test)]
mod tests {
use super::*;
#[cfg(feature = "alloc")]
use crate::{
callsite::{Callsite, Identifier},
field::FieldSet,
Interest,
};
use core::mem;

#[test]
Expand Down Expand Up @@ -863,4 +932,31 @@ 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);

let _metadata = Metadata::from_cow(
"a name".to_string(),
"a target",
Level::TRACE,
Some("a file".to_string()),
None,
None::<Cow<'_, str>>,
field_set,
Kind::EVENT,
);
}
}
12 changes: 6 additions & 6 deletions tracing-log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,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(&'a self) -> Self::Log;
}

/// Trait implemented for `log` types that can be converted to a `tracing`
Expand All @@ -204,12 +204,12 @@ pub trait AsTrace: crate::sealed::Sealed {

impl<'a> crate::sealed::Sealed for Metadata<'a> {}

impl<'a> AsLog for Metadata<'a> {
impl<'a> AsLog<'a> for Metadata<'a> {
type Log = log::Metadata<'a>;
fn as_log(&self) -> Self::Log {
fn as_log(&'a self) -> Self::Log {
log::Metadata::builder()
.level(self.level().as_log())
.target(self.target())
.target(&self.target())
.build()
}
}
Expand Down Expand Up @@ -329,7 +329,7 @@ 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 {
match *self {
Expand Down
Loading