Skip to content

Commit

Permalink
tracing: allow owned values and fat pointers in Span::record (#2212)
Browse files Browse the repository at this point in the history
Previously, using `record("x", "y")` would give an error:
```
error[E0277]: the size for values of type `str` cannot be known at compilation time
    --> src/main.rs:3:22
     |
243  |                 span.record("x", "y");
     |                      ^^^^^^ doesn't have a size known at compile-time
     |
     = help: the trait `Sized` is not implemented for `str`
note: required by a bound in `Span::record`
    --> /home/jnelson/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.32/src/span.rs:1184:30
     |
1184 |     pub fn record<Q: ?Sized, V>(&self, field: &Q, value: &V) -> &Self
     |                              ^ required by this bound in `Span::record`
```

Now it works fine, as tested by the doc-example.

This doesn't break any existing code, because there's a generic `impl<T: Value> Value for &T`: https://docs.rs/tracing/0.1.35/tracing/trait.Value.html#impl-Value-for-%26%27a%20T

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
jyn514 and hawkw committed Jul 20, 2022
1 parent 2c39b60 commit b2501d5
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions tracing/src/span.rs
Expand Up @@ -1136,7 +1136,7 @@ impl Span {
///
/// // Now, record a value for parting as well.
/// // (note that the field name is passed as a string slice)
/// span.record("parting", &"goodbye world!");
/// span.record("parting", "goodbye world!");
/// ```
/// However, it may also be used to record a _new_ value for a field whose
/// value was already recorded:
Expand All @@ -1154,7 +1154,7 @@ impl Span {
/// }
/// Err(_) => {
/// // Things are no longer okay!
/// span.record("is_okay", &false);
/// span.record("is_okay", false);
/// }
/// }
/// ```
Expand All @@ -1181,17 +1181,17 @@ impl Span {
/// // Now, you try to record a value for a new field, `new_field`, which was not
/// // declared as `Empty` or populated when you created `span`.
/// // You won't get any error, but the assignment will have no effect!
/// span.record("new_field", &"interesting_value_you_really_need");
/// span.record("new_field", "interesting_value_you_really_need");
///
/// // Instead, all fields that may be recorded after span creation should be declared up front,
/// // using field::Empty when a value is not known, as we did for `parting`.
/// // This `record` call will indeed replace field::Empty with "you will be remembered".
/// span.record("parting", &"you will be remembered");
/// span.record("parting", "you will be remembered");
/// ```
///
/// [`field::Empty`]: super::field::Empty
/// [`Metadata`]: super::Metadata
pub fn record<Q: ?Sized, V>(&self, field: &Q, value: &V) -> &Self
pub fn record<Q: ?Sized, V>(&self, field: &Q, value: V) -> &Self
where
Q: field::AsField,
V: field::Value,
Expand All @@ -1201,7 +1201,7 @@ impl Span {
self.record_all(
&meta
.fields()
.value_set(&[(&field, Some(value as &dyn field::Value))]),
.value_set(&[(&field, Some(&value as &dyn field::Value))]),
);
}
}
Expand Down Expand Up @@ -1614,4 +1614,10 @@ mod test {
impl AssertSync for Span {}
impl AssertSync for Entered<'_> {}
impl AssertSync for EnteredSpan {}

#[test]
fn test_record_backwards_compat() {
Span::current().record("some-key", &"some text");
Span::current().record("some-key", &false);
}
}

0 comments on commit b2501d5

Please sign in to comment.