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`. 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 committed Jun 22, 2022
1 parent 5a90095 commit 1008395
Show file tree
Hide file tree
Showing 3 changed files with 43 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 (subscriber, handle) = subscriber::mock()
.new_span(span)
Expand Down
8 changes: 8 additions & 0 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ use crate::stdlib::{
hash::{Hash, Hasher},
num,
ops::Range,
string::String,
};

use self::private::ValidLen;
Expand Down Expand Up @@ -596,6 +597,13 @@ where
}
}

impl crate::sealed::Sealed for String {}
impl Value for 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
22 changes: 22 additions & 0 deletions tracing/tests/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,28 @@ fn option_ref_mut_values() {
some_u64 = some_u64,
none_u64 = none_u64
);
})
}

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
#[test]
fn string_field() {
let (subscriber, handle) = subscriber::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(subscriber, || {
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();
Expand Down

0 comments on commit 1008395

Please sign in to comment.