[oximeter] use Cow<'static, str> for FieldValue::String (#5670)

Fixes #5664.

Currently, Oximeter `FieldValue::String` fields are always represented
by a `String` value. This is somewhat unfortunate, as it requires
allocating a new `String` for every string field value that's emitted to
Oximeter, even when the string value is fixed at compile-time --- for
example, when an enum is used to generate a metric label with `&'static
str` values, they must be alloced into new `String`s to record the
metric.

This commit changes `FieldValue::String` from holding a `String` to
holding a `Cow<'static, str>`, so that static string constants need not
be heap-allocated.

Additionally, I've changed Oximeter's `self_stats` module to use
`Cow<'static, str>` to represent the field value generated by the
`FailureReason` enum. This way, a heap-allocated string is only
necessary for dynamically formatted variants (the `FailureReason::Other`
variant, which owns an HTTP `StatusCode`). For the
`FailureReason::Unreachable` and `FailureReason::Deserialization`
variants, we can just emit a `&'static str` without allocating.
Hopefully, this reduces resident memory for the collector process
somewhat when encountering a lot of failures.

nexus/db-queries/src/transaction_retry.rs

@@ -353,7 +353,9 @@ mod test {
             assert_eq!(
                 target_fields["name"].value,
                 FieldValue::String(
-                    "test_transaction_retry_produces_samples".to_string()
+                    "test_transaction_retry_produces_samples"
+                        .to_string()
+                        .into()
                 )
             );
 

oximeter/collector/src/self_stats.rs

@@ -14,6 +14,7 @@ use oximeter::MetricsError;
 use oximeter::Sample;
 use oximeter::Target;
 use reqwest::StatusCode;
+use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::net::IpAddr;
 use std::time::Duration;
@@ -65,13 +66,26 @@ pub enum FailureReason {
 impl std::fmt::Display for FailureReason {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
-            Self::Unreachable => write!(f, "unreachable"),
-            Self::Deserialization => write!(f, "deserialization"),
+            Self::Unreachable => f.write_str(Self::UNREACHABLE),
+            Self::Deserialization => f.write_str(Self::DESERIALIZATION),
             Self::Other(c) => write!(f, "{}", c.as_u16()),
         }
     }
 }
 
+impl FailureReason {
+    const UNREACHABLE: &'static str = "unreachable";
+    const DESERIALIZATION: &'static str = "deserialization";
+
+    fn as_string(&self) -> Cow<'static, str> {
+        match self {
+            Self::Unreachable => Cow::Borrowed(Self::UNREACHABLE),
+            Self::Deserialization => Cow::Borrowed(Self::DESERIALIZATION),
+            Self::Other(c) => Cow::Owned(c.as_u16().to_string()),
+        }
+    }
+}
+
 /// The number of failed collections from a single producer.
 #[derive(Clone, Debug, Metric)]
 pub struct FailedCollections {
@@ -88,7 +102,7 @@ pub struct FailedCollections {
     /// The reason we could not collect.
     //
     // NOTE: This should always be generated through a `FailureReason`.
-    pub reason: String,
+    pub reason: Cow<'static, str>,
     pub datum: Cumulative<u64>,
 }
 
@@ -128,7 +142,7 @@ impl CollectionTaskStats {
                 producer_ip: self.collections.producer_ip,
                 producer_port: self.collections.producer_port,
                 base_route: self.collections.base_route.clone(),
-                reason: reason.to_string(),
+                reason: reason.as_string(),
                 datum: Cumulative::new(0),
             }
         })
@@ -200,7 +214,7 @@ mod tests {
             producer_ip: IpAddr::V6(Ipv6Addr::LOCALHOST),
             producer_port: 12345,
             base_route: String::from("/"),
-            reason: FailureReason::Unreachable.to_string(),
+            reason: FailureReason::Unreachable.as_string(),
             datum: Cumulative::new(0),
         }
     }

oximeter/db/src/lib.rs

@@ -296,6 +296,7 @@ mod tests {
     use super::*;
     use crate::model::DbFieldList;
     use crate::model::DbTimeseriesSchema;
+    use std::borrow::Cow;
     use uuid::Uuid;
 
     // Validates that the timeseries_key stability for a sample is stable.
@@ -332,7 +333,7 @@ mod tests {
         use strum::EnumCount;
 
         let values = [
-            ("string", FieldValue::String(String::default())),
+            ("string", FieldValue::String(Cow::Owned(String::default()))),
             ("i8", FieldValue::I8(-0x0A)),
             ("u8", FieldValue::U8(0x0A)),
             ("i16", FieldValue::I16(-0x0ABC)),

oximeter/db/src/model.rs

@@ -391,7 +391,7 @@ declare_field_row! {I32FieldRow, i32, "i32"}
 declare_field_row! {U32FieldRow, u32, "u32"}
 declare_field_row! {I64FieldRow, i64, "i64"}
 declare_field_row! {U64FieldRow, u64, "u64"}
-declare_field_row! {StringFieldRow, String, "string"}
+declare_field_row! {StringFieldRow, std::borrow::Cow<'static, str>, "string"}
 declare_field_row! {IpAddrFieldRow, Ipv6Addr, "ipaddr"}
 declare_field_row! {UuidFieldRow, Uuid, "uuid"}
 
@@ -1716,6 +1716,7 @@ pub(crate) fn parse_field_select_row(
                         .as_str()
                         .expect("Expected a UUID string for a Uuid field from the database")
                         .to_string()
+                        .into()
                     )
             }
         };

oximeter/db/src/oxql/ast/literal.rs

@@ -14,6 +14,7 @@ use chrono::Utc;
 use oximeter::FieldType;
 use oximeter::FieldValue;
 use regex::Regex;
+use std::borrow::Borrow;
 use std::fmt;
 use std::net::IpAddr;
 use std::time::Duration;
@@ -127,20 +128,24 @@ impl Literal {
             (FieldValue::Bool(lhs), Literal::Boolean(rhs)) => {
                 generate_cmp_match!(rhs, lhs)
             }
-            (FieldValue::String(lhs), Literal::String(rhs)) => match cmp {
-                Comparison::Eq => Ok(Some(lhs == rhs)),
-                Comparison::Ne => Ok(Some(lhs != rhs)),
-                Comparison::Gt => Ok(Some(lhs > rhs)),
-                Comparison::Ge => Ok(Some(lhs >= rhs)),
-                Comparison::Lt => Ok(Some(lhs < rhs)),
-                Comparison::Le => Ok(Some(lhs <= rhs)),
-                Comparison::Like => {
-                    let re = Regex::new(rhs).context(
-                        "failed to create regex for string matching",
-                    )?;
-                    Ok(Some(re.is_match(lhs)))
+            (FieldValue::String(lhs), Literal::String(rhs)) => {
+                let lhs = lhs.borrow();
+                let rhs = rhs.as_ref();
+                match cmp {
+                    Comparison::Eq => Ok(Some(lhs == rhs)),
+                    Comparison::Ne => Ok(Some(lhs != rhs)),
+                    Comparison::Gt => Ok(Some(lhs > rhs)),
+                    Comparison::Ge => Ok(Some(lhs >= rhs)),
+                    Comparison::Lt => Ok(Some(lhs < rhs)),
+                    Comparison::Le => Ok(Some(lhs <= rhs)),
+                    Comparison::Like => {
+                        let re = Regex::new(rhs).context(
+                            "failed to create regex for string matching",
+                        )?;
+                        Ok(Some(re.is_match(lhs)))
+                    }
                 }
-            },
+            }
             (FieldValue::IpAddr(lhs), Literal::IpAddr(rhs)) => {
                 generate_cmp_match!(rhs, lhs)
             }
@@ -377,7 +382,8 @@ mod tests {
 
     #[test]
     fn test_literal_compare_field_wrong_type() {
-        let value = FieldValue::String(String::from("foo"));
+        let value =
+            FieldValue::String(std::borrow::Cow::Owned(String::from("foo")));
         let lit = Literal::Integer(4);
         assert!(lit.compare_field(&value, Comparison::Eq).is_err());
     }

oximeter/oximeter/src/types.rs

@@ -17,6 +17,7 @@ use num::traits::Zero;
 use schemars::JsonSchema;
 use serde::Deserialize;
 use serde::Serialize;
+use std::borrow::Cow;
 use std::boxed::Box;
 use std::collections::BTreeMap;
 use std::fmt;
@@ -77,6 +78,8 @@ macro_rules! impl_field_type_from {
 }
 
 impl_field_type_from! { String, FieldType::String }
+impl_field_type_from! { &'static str, FieldType::String }
+impl_field_type_from! { Cow<'static, str>, FieldType::String }
 impl_field_type_from! { i8, FieldType::I8 }
 impl_field_type_from! { u8, FieldType::U8 }
 impl_field_type_from! { i16, FieldType::I16 }
@@ -103,7 +106,7 @@ impl_field_type_from! { bool, FieldType::Bool }
 )]
 #[serde(tag = "type", content = "value", rename_all = "snake_case")]
 pub enum FieldValue {
-    String(String),
+    String(Cow<'static, str>),
     I8(i8),
     U8(u8),
     I16(i16),
@@ -147,7 +150,9 @@ impl FieldValue {
             typ: field_type.to_string(),
         };
         match field_type {
-            FieldType::String => Ok(FieldValue::String(s.to_string())),
+            FieldType::String => {
+                Ok(FieldValue::String(Cow::Owned(s.to_string())))
+            }
             FieldType::I8 => {
                 Ok(FieldValue::I8(s.parse().map_err(|_| make_err())?))
             }
@@ -222,14 +227,20 @@ impl_field_value_from! { i32, FieldValue::I32 }
 impl_field_value_from! { u32, FieldValue::U32 }
 impl_field_value_from! { i64, FieldValue::I64 }
 impl_field_value_from! { u64, FieldValue::U64 }
-impl_field_value_from! { String, FieldValue::String }
+impl_field_value_from! { Cow<'static, str>, FieldValue::String }
 impl_field_value_from! { IpAddr, FieldValue::IpAddr }
 impl_field_value_from! { Uuid, FieldValue::Uuid }
 impl_field_value_from! { bool, FieldValue::Bool }
 
 impl From<&str> for FieldValue {
     fn from(value: &str) -> Self {
-        FieldValue::String(String::from(value))
+        FieldValue::String(Cow::Owned(String::from(value)))
+    }
+}
+
+impl From<String> for FieldValue {
+    fn from(value: String) -> Self {
+        FieldValue::String(Cow::Owned(value))
     }
 }