Skip to content

Commit

Permalink
core: impl field::Value for String (#2164)
Browse files Browse the repository at this point in the history
## Motivation

Currently, it is rather difficult to record `String`s as field values,
even though `&str` is a primitive `Value` type. For example, this code
does not currently compile:

```rust
let my_string = String::from("hello world!");
tracing::debug!(my_string);
```

Instead, it is necessary to explicitly call `String::as_str` or a
similar conversion method:

```rust
let my_string = String::from("hello world!");
tracing::debug!(my_string = my_string.as_str());
```

This is unfortunate, as it makes a fairly straightforward, commomplace
task (recording a `String` as a field) unnecessarily verbose.

## Solution

This branch adds an `impl Value for String` in `tracing-core` when the
"alloc" feature flag is enabled. The impl simply calls `String::as_str`
and then calls `record_str`. Because `Value` takes an `&self`, and there
are preexisting `impl<T: Value> Value` for `&T` and `&mut T`, the string
is not consumed, and `&String` or `&mut String`s can also be used as
`Value`s.

I've also added tests validating that this actually works.
  • Loading branch information
hawkw authored Jun 16, 2022
1 parent 0c863e4 commit af5edf8
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
13 changes: 13 additions & 0 deletions tracing-attributes/tests/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ fn fn_clashy_expr_field2(s: &str) {
let _ = s;
}

#[instrument(fields(s = &s))]
fn fn_string(s: String) {
let _ = s;
}

#[derive(Debug)]
struct HasField {
my_field: &'static str,
Expand Down Expand Up @@ -134,6 +139,14 @@ fn empty_field() {
});
}

#[test]
fn string_field() {
let span = span::mock().with_field(mock("s").with_value(&"hello world").only());
run_test(span, || {
fn_string(String::from("hello world"));
});
}

fn run_test<F: FnOnce() -> T, T>(span: NewSpan, fun: F) {
let (collector, handle) = collector::mock()
.new_span(span)
Expand Down
12 changes: 12 additions & 0 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,18 @@ where
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl crate::sealed::Sealed for alloc::string::String {}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl Value for alloc::string::String {
fn record(&self, key: &Field, visitor: &mut dyn Visit) {
visitor.record_str(key, self.as_str())
}
}

impl fmt::Debug for dyn Value {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// We are only going to be recording the field value, so we don't
Expand Down
24 changes: 24 additions & 0 deletions tracing/tests/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,27 @@ fn explicit_child_at_levels() {

handle.assert_finished();
}

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
#[test]
fn string_field() {
let (collector, handle) = collector::mock()
.event(event::mock().with_fields(field::mock("my_string").with_value(&"hello").only()))
.event(
event::mock().with_fields(field::mock("my_string").with_value(&"hello world!").only()),
)
.done()
.run_with_handle();
with_default(collector, || {
let mut my_string = String::from("hello");

tracing::event!(Level::INFO, my_string);

// the string is not moved by using it as a field!
my_string.push_str(" world!");

tracing::event!(Level::INFO, my_string);
});

handle.assert_finished();
}

0 comments on commit af5edf8

Please sign in to comment.