core: add support for visiting floating point values (#1507)

## Motivation

Tracing is a really useful framework but a lack of floating point value
support for the core visitors means these get coerced unnecessarily to
strings.

## Solution

This change adds support for floating point (`f64`) visitors.

Author: maxburke

tracing-core/src/field.rs

@@ -16,9 +16,9 @@
 //! will contain any fields attached to each event.
 //!
 //! `tracing` represents values as either one of a set of Rust primitives
-//! (`i64`, `u64`, `bool`, and `&str`) or using a `fmt::Display` or `fmt::Debug`
-//! implementation. `Subscriber`s are provided these primitive value types as
-//! `dyn Value` trait objects.
+//! (`i64`, `u64`, `f64`, `bool`, and `&str`) or using a `fmt::Display` or
+//! `fmt::Debug` implementation. `Subscriber`s are provided these primitive
+//! value types as `dyn Value` trait objects.
 //!
 //! These trait objects can be formatted using `fmt::Debug`, but may also be
 //! recorded as typed data by calling the [`Value::record`] method on these
@@ -184,6 +184,11 @@ pub struct Iter {
 /// [`Event`]: ../event/struct.Event.html
 /// [`ValueSet`]: struct.ValueSet.html
 pub trait Visit {
+    /// Visit a double-precision floating point value.
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        self.record_debug(field, &value)
+    }
+
     /// Visit a signed 64-bit integer value.
     fn record_i64(&mut self, field: &Field, value: i64) {
         self.record_debug(field, &value)
@@ -335,6 +340,12 @@ macro_rules! ty_to_nonzero {
 }
 
 macro_rules! impl_one_value {
+    (f32, $op:expr, $record:ident) => {
+        impl_one_value!(normal, f32, $op, $record);
+    };
+    (f64, $op:expr, $record:ident) => {
+        impl_one_value!(normal, f64, $op, $record);
+    };
     (bool, $op:expr, $record:ident) => {
         impl_one_value!(normal, bool, $op, $record);
     };
@@ -387,7 +398,8 @@ impl_values! {
     record_u64(usize, u32, u16, u8 as u64),
     record_i64(i64),
     record_i64(isize, i32, i16, i8 as i64),
-    record_bool(bool)
+    record_bool(bool),
+    record_f64(f64, f32 as f64)
 }
 
 impl<T: crate::sealed::Sealed> crate::sealed::Sealed for Wrapping<T> {}

tracing-serde/src/lib.rs

@@ -384,6 +384,12 @@ where
         }
     }
 
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        if self.state.is_ok() {
+            self.state = self.serializer.serialize_entry(field.name(), &value)
+        }
+    }
+
     fn record_str(&mut self, field: &Field, value: &str) {
         if self.state.is_ok() {
             self.state = self.serializer.serialize_entry(field.name(), &value)
@@ -430,6 +436,12 @@ where
         }
     }
 
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        if self.state.is_ok() {
+            self.state = self.serializer.serialize_field(field.name(), &value)
+        }
+    }
+
     fn record_str(&mut self, field: &Field, value: &str) {
         if self.state.is_ok() {
             self.state = self.serializer.serialize_field(field.name(), &value)

tracing-subscriber/src/field/debug.rs

@@ -38,6 +38,11 @@ impl<V> Visit for Alt<V>
 where
     V: Visit,
 {
+    #[inline]
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        self.0.record_f64(field, value)
+    }
+
     #[inline]
     fn record_i64(&mut self, field: &Field, value: i64) {
         self.0.record_i64(field, value)

tracing-subscriber/src/field/display.rs

@@ -40,6 +40,11 @@ impl<V> Visit for Messages<V>
 where
     V: Visit,
 {
+    #[inline]
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        self.0.record_f64(field, value)
+    }
+
     #[inline]
     fn record_i64(&mut self, field: &Field, value: i64) {
         self.0.record_i64(field, value)

tracing-subscriber/src/filter/env/field.rs

@@ -36,14 +36,77 @@ pub(crate) struct MatchVisitor<'a> {
     inner: &'a SpanMatch,
 }
 
-#[derive(Debug, Clone, PartialOrd, Ord, Eq, PartialEq)]
+#[derive(Debug, Clone)]
 pub(crate) enum ValueMatch {
     Bool(bool),
+    F64(f64),
     U64(u64),
     I64(i64),
+    NaN,
     Pat(Box<MatchPattern>),
 }
 
+impl Eq for ValueMatch {}
+
+impl PartialEq for ValueMatch {
+    fn eq(&self, other: &Self) -> bool {
+        use ValueMatch::*;
+        match (self, other) {
+            (Bool(a), Bool(b)) => a.eq(b),
+            (F64(a), F64(b)) => {
+                debug_assert!(!a.is_nan());
+                debug_assert!(!b.is_nan());
+
+                a.eq(b)
+            }
+            (U64(a), U64(b)) => a.eq(b),
+            (I64(a), I64(b)) => a.eq(b),
+            (NaN, NaN) => true,
+            (Pat(a), Pat(b)) => a.eq(b),
+            _ => false,
+        }
+    }
+}
+
+impl Ord for ValueMatch {
+    fn cmp(&self, other: &Self) -> Ordering {
+        use ValueMatch::*;
+        match (self, other) {
+            (Bool(this), Bool(that)) => this.cmp(that),
+            (Bool(_), _) => Ordering::Less,
+
+            (F64(this), F64(that)) => this
+                .partial_cmp(that)
+                .expect("`ValueMatch::F64` may not contain `NaN` values"),
+            (F64(_), Bool(_)) => Ordering::Greater,
+            (F64(_), _) => Ordering::Less,
+
+            (NaN, NaN) => Ordering::Equal,
+            (NaN, Bool(_)) | (NaN, F64(_)) => Ordering::Greater,
+            (NaN, _) => Ordering::Less,
+
+            (U64(this), U64(that)) => this.cmp(that),
+            (U64(_), Bool(_)) | (U64(_), F64(_)) | (U64(_), NaN) => Ordering::Greater,
+            (U64(_), _) => Ordering::Less,
+
+            (I64(this), I64(that)) => this.cmp(that),
+            (I64(_), Bool(_)) | (I64(_), F64(_)) | (I64(_), NaN) | (I64(_), U64(_)) => {
+                Ordering::Greater
+            }
+            (I64(_), _) => Ordering::Less,
+
+            (Pat(this), Pat(that)) => this.cmp(that),
+            (Pat(_), _) => Ordering::Greater,
+        }
+    }
+}
+
+impl PartialOrd for ValueMatch {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 #[derive(Debug, Clone)]
 pub(crate) struct MatchPattern {
     pub(crate) matcher: Pattern,
@@ -127,13 +190,22 @@ impl PartialOrd for Match {
 
 // === impl ValueMatch ===
 
+fn value_match_f64(v: f64) -> ValueMatch {
+    if v.is_nan() {
+        ValueMatch::NaN
+    } else {
+        ValueMatch::F64(v)
+    }
+}
+
 impl FromStr for ValueMatch {
     type Err = matchers::Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         s.parse::<bool>()
             .map(ValueMatch::Bool)
             .or_else(|_| s.parse::<u64>().map(ValueMatch::U64))
             .or_else(|_| s.parse::<i64>().map(ValueMatch::I64))
+            .or_else(|_| s.parse::<f64>().map(value_match_f64))
             .or_else(|_| {
                 s.parse::<MatchPattern>()
                     .map(|p| ValueMatch::Pat(Box::new(p)))
@@ -145,6 +217,8 @@ impl fmt::Display for ValueMatch {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             ValueMatch::Bool(ref inner) => fmt::Display::fmt(inner, f),
+            ValueMatch::F64(ref inner) => fmt::Display::fmt(inner, f),
+            ValueMatch::NaN => fmt::Display::fmt(&f64::NAN, f),
             ValueMatch::I64(ref inner) => fmt::Display::fmt(inner, f),
             ValueMatch::U64(ref inner) => fmt::Display::fmt(inner, f),
             ValueMatch::Pat(ref inner) => fmt::Display::fmt(inner, f),
@@ -275,6 +349,18 @@ impl SpanMatch {
 }
 
 impl<'a> Visit for MatchVisitor<'a> {
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        match self.inner.fields.get(field) {
+            Some((ValueMatch::NaN, ref matched)) if value.is_nan() => {
+                matched.store(true, Release);
+            }
+            Some((ValueMatch::F64(ref e), ref matched)) if (value - *e).abs() < f64::EPSILON => {
+                matched.store(true, Release);
+            }
+            _ => {}
+        }
+    }
+
     fn record_i64(&mut self, field: &Field, value: i64) {
         use std::convert::TryInto;
 

tracing-subscriber/src/fmt/format/json.rs

@@ -431,6 +431,12 @@ impl<'a> crate::field::VisitOutput<fmt::Result> for JsonVisitor<'a> {
 }
 
 impl<'a> field::Visit for JsonVisitor<'a> {
+    /// Visit a double precision floating point value.
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        self.values
+            .insert(field.name(), serde_json::Value::from(value));
+    }
+
     /// Visit a signed 64-bit integer value.
     fn record_i64(&mut self, field: &Field, value: i64) {
         self.values

tracing/tests/event.rs

@@ -145,6 +145,7 @@ fn one_with_everything() {
                         )))
                         .and(field::mock("foo").with_value(&666))
                         .and(field::mock("bar").with_value(&false))
+                        .and(field::mock("like_a_butterfly").with_value(&42.0))
                         .only(),
                 )
                 .at_level(Level::ERROR)
@@ -157,7 +158,7 @@ fn one_with_everything() {
         event!(
             target: "whatever",
             Level::ERROR,
-            { foo = 666, bar = false },
+            { foo = 666, bar = false, like_a_butterfly = 42.0 },
              "{:#x} make me one with{what:.>20}", 4_277_009_102u64, what = "everything"
         );
     });

tracing/tests/span.rs

@@ -458,6 +458,28 @@ fn move_field_out_of_struct() {
     handle.assert_finished();
 }
 
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+#[test]
+fn float_values() {
+    let (collector, handle) = collector::mock()
+        .new_span(
+            span::mock().named("foo").with_field(
+                field::mock("x")
+                    .with_value(&3.234)
+                    .and(field::mock("y").with_value(&-1.223))
+                    .only(),
+            ),
+        )
+        .run_with_handle();
+
+    with_default(collector, || {
+        let foo = span!(Level::TRACE, "foo", x = 3.234, y = -1.223);
+        foo.in_scope(|| {});
+    });
+
+    handle.assert_finished();
+}
+
 // TODO(#1138): determine a new syntax for uninitialized span fields, and
 // re-enable these.
 /*

tracing/tests/support/field.rs

@@ -19,8 +19,9 @@ pub struct MockField {
     value: MockValue,
 }
 
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Debug)]
 pub enum MockValue {
+    F64(f64),
     I64(i64),
     U64(u64),
     Bool(bool),
@@ -29,6 +30,31 @@ pub enum MockValue {
     Any,
 }
 
+impl Eq for MockValue {}
+
+impl PartialEq for MockValue {
+    fn eq(&self, other: &Self) -> bool {
+        use MockValue::*;
+
+        match (self, other) {
+            (F64(a), F64(b)) => {
+                debug_assert!(!a.is_nan());
+                debug_assert!(!b.is_nan());
+
+                a.eq(b)
+            }
+            (I64(a), I64(b)) => a.eq(b),
+            (U64(a), U64(b)) => a.eq(b),
+            (Bool(a), Bool(b)) => a.eq(b),
+            (Str(a), Str(b)) => a.eq(b),
+            (Debug(a), Debug(b)) => a.eq(b),
+            (Any, _) => true,
+            (_, Any) => true,
+            _ => false,
+        }
+    }
+}
+
 pub fn mock<K>(name: K) -> MockField
 where
     String: From<K>,
@@ -120,6 +146,7 @@ impl Expect {
 impl fmt::Display for MockValue {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
+            MockValue::F64(v) => write!(f, "f64 = {:?}", v),
             MockValue::I64(v) => write!(f, "i64 = {:?}", v),
             MockValue::U64(v) => write!(f, "u64 = {:?}", v),
             MockValue::Bool(v) => write!(f, "bool = {:?}", v),
@@ -136,6 +163,11 @@ pub struct CheckVisitor<'a> {
 }
 
 impl<'a> Visit for CheckVisitor<'a> {
+    fn record_f64(&mut self, field: &Field, value: f64) {
+        self.expect
+            .compare_or_panic(field.name(), &value, &self.ctx[..])
+    }
+
     fn record_i64(&mut self, field: &Field, value: i64) {
         self.expect
             .compare_or_panic(field.name(), &value, &self.ctx[..])
@@ -180,6 +212,10 @@ impl<'a> From<&'a dyn Value> for MockValue {
         }
 
         impl Visit for MockValueBuilder {
+            fn record_f64(&mut self, _: &Field, value: f64) {
+                self.value = Some(MockValue::F64(value));
+            }
+
             fn record_i64(&mut self, _: &Field, value: i64) {
                 self.value = Some(MockValue::I64(value));
             }