ref: Introduce Attributes struct (#4839)

This struct reduces the amount of ceremony required to insert an
attribute into a collection or get an attribute's value. I only added
the methods I needed, there's probably more that could be added.

This PR also removes some redundant debug snapshots, or in some cases
replaces them with JSON snapshots.

Author: loewenheim

relay-event-normalization/src/normalize/nel.rs

@@ -2,9 +2,9 @@
 
 use chrono::{DateTime, Duration, Utc};
 use relay_event_schema::protocol::{
-    Attribute, AttributeValue, NetworkReportRaw, OurLog, OurLogLevel, Timestamp, TraceId,
+    Attributes, NetworkReportRaw, OurLog, OurLogLevel, Timestamp, TraceId,
 };
-use relay_protocol::{Annotated, Object};
+use relay_protocol::Annotated;
 
 /// Creates a [`OurLog`] from the provided [`NetworkReportRaw`].
 pub fn create_log(nel: Annotated<NetworkReportRaw>, received_at: DateTime<Utc>) -> Option<OurLog> {
@@ -30,18 +30,12 @@ pub fn create_log(nel: Annotated<NetworkReportRaw>, received_at: DateTime<Utc>)
         .checked_sub_signed(Duration::milliseconds(*nel.age.value().unwrap_or(&0)))
         .unwrap_or(received_at);
 
-    let mut attributes: Object<Attribute> = Default::default();
+    let mut attributes: Attributes = Default::default();
 
     macro_rules! add_attribute {
         ($name:literal, $value:expr) => {{
             if let Some(value) = $value.into_value() {
-                attributes.insert(
-                    $name.to_owned(),
-                    Annotated::new(Attribute {
-                        value: AttributeValue::from(value),
-                        other: Default::default(),
-                    }),
-                );
+                attributes.insert($name.to_owned(), value);
             }
         }};
     }

relay-event-schema/src/protocol/attributes.rs

@@ -1,5 +1,5 @@
 use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, SkipSerialization, Value};
-use std::fmt;
+use std::{borrow::Borrow, fmt};
 
 use crate::processor::ProcessValue;
 
@@ -159,3 +159,82 @@ impl IntoValue for AttributeType {
         serde::ser::Serialize::serialize(self.as_str(), s)
     }
 }
+
+/// Wrapper struct around a collection of attributes with some convenience methods.
+#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+pub struct Attributes(Object<Attribute>);
+
+impl Attributes {
+    /// Creates an empty collection of attributes.
+    pub fn new() -> Self {
+        Self(Object::new())
+    }
+
+    /// Returns the value of the attribute with the given key.
+    pub fn get_value<Q>(&self, key: &Q) -> Option<&Value>
+    where
+        String: Borrow<Q>,
+        Q: Ord + ?Sized,
+    {
+        self.get_attribute(key)?.value.value.value()
+    }
+
+    /// Returns the attribute with the given key.
+    pub fn get_attribute<Q>(&self, key: &Q) -> Option<&Attribute>
+    where
+        String: Borrow<Q>,
+        Q: Ord + ?Sized,
+    {
+        self.0.get(key)?.value()
+    }
+
+    /// Inserts an attribute with the given value into this collection.
+    pub fn insert<V: Into<AttributeValue>>(&mut self, key: String, value: V) {
+        fn inner(slf: &mut Attributes, key: String, value: AttributeValue) {
+            let attribute = Annotated::new(Attribute {
+                value,
+                other: Default::default(),
+            });
+            slf.insert_raw(key, attribute);
+        }
+        let value = value.into();
+        inner(self, key, value);
+    }
+
+    /// Inserts an annotated attribute into this collection.
+    pub fn insert_raw(&mut self, key: String, attribute: Annotated<Attribute>) {
+        self.0.insert(key, attribute);
+    }
+
+    /// Checks whether this collection contains an attribute with the given key.
+    pub fn contains_key<Q>(&self, key: &Q) -> bool
+    where
+        String: Borrow<Q>,
+        Q: Ord + ?Sized,
+    {
+        self.0.contains_key(key)
+    }
+
+    /// Iterates mutably over this collection's attribute keys and values
+    pub fn iter_mut(
+        &mut self,
+    ) -> std::collections::btree_map::IterMut<String, Annotated<Attribute>> {
+        self.0.iter_mut()
+    }
+}
+
+impl IntoIterator for Attributes {
+    type Item = (String, Annotated<Attribute>);
+
+    type IntoIter = std::collections::btree_map::IntoIter<String, Annotated<Attribute>>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
+impl FromIterator<(String, Annotated<Attribute>)> for Attributes {
+    fn from_iter<T: IntoIterator<Item = (String, Annotated<Attribute>)>>(iter: T) -> Self {
+        Self(Object::from_iter(iter))
+    }
+}

relay-event-schema/src/protocol/ourlog.rs

@@ -4,7 +4,7 @@ use std::fmt::{self, Display};
 use serde::{Serialize, Serializer};
 
 use crate::processor::ProcessValue;
-use crate::protocol::{Attribute, SpanId, Timestamp, TraceId};
+use crate::protocol::{Attributes, SpanId, Timestamp, TraceId};
 
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 #[metastructure(process_func = "process_ourlog", value_type = "OurLog")]
@@ -31,19 +31,13 @@ pub struct OurLog {
 
     /// Arbitrary attributes on a log.
     #[metastructure(pii = "true", trim = false)]
-    pub attributes: Annotated<Object<Attribute>>,
+    pub attributes: Annotated<Attributes>,
 
     /// Additional arbitrary fields for forwards compatibility.
     #[metastructure(additional_properties, retain = true, pii = "maybe")]
     pub other: Object<Value>,
 }
 
-impl OurLog {
-    pub fn attribute(&self, key: &str) -> Option<&Annotated<Value>> {
-        Some(&self.attributes.value()?.get(key)?.value()?.value.value)
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum OurLogLevel {
     Trace,
@@ -168,69 +162,6 @@ mod tests {
         }"#;
 
         let data = Annotated::<OurLog>::from_json(json).unwrap();
-        insta::assert_debug_snapshot!(data, @r###"
-        OurLog {
-            timestamp: Timestamp(
-                2018-12-13T16:51:00Z,
-            ),
-            trace_id: TraceId("5b8efff798038103d269b633813fc60c"),
-            span_id: SpanId("eee19b7ec3c1b174"),
-            level: Info,
-            body: "Example log record",
-            attributes: {
-                "boolean.attribute": Attribute {
-                    value: Bool(
-                        true,
-                    ),
-                    type: Boolean,
-                    other: {},
-                },
-                "double.attribute": Attribute {
-                    value: F64(
-                        1.23,
-                    ),
-                    type: Double,
-                    other: {},
-                },
-                "sentry.observed_timestamp_nanos": Attribute {
-                    value: String(
-                        "1544712660300000000",
-                    ),
-                    type: Integer,
-                    other: {},
-                },
-                "sentry.severity_number": Attribute {
-                    value: String(
-                        "10",
-                    ),
-                    type: Integer,
-                    other: {},
-                },
-                "sentry.severity_text": Attribute {
-                    value: String(
-                        "info",
-                    ),
-                    type: String,
-                    other: {},
-                },
-                "sentry.trace_flags": Attribute {
-                    value: String(
-                        "10",
-                    ),
-                    type: Integer,
-                    other: {},
-                },
-                "string.attribute": Attribute {
-                    value: String(
-                        "some string",
-                    ),
-                    type: String,
-                    other: {},
-                },
-            },
-            other: {},
-        }
-        "###);
 
         insta::assert_json_snapshot!(SerializableAnnotated(&data), @r###"
         {

relay-event-schema/src/protocol/span_v2.rs

@@ -6,9 +6,7 @@ use std::str::FromStr;
 use serde::Serialize;
 
 use crate::processor::ProcessValue;
-use crate::protocol::{Attribute, SpanId, Timestamp, TraceId};
-
-use super::OperationType;
+use crate::protocol::{Attributes, OperationType, SpanId, Timestamp, TraceId};
 
 /// A version 2 (transactionless) span.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue)]
@@ -64,20 +62,13 @@ pub struct SpanV2 {
 
     /// Arbitrary attributes on a span.
     #[metastructure(pii = "true", trim = false)]
-    pub attributes: Annotated<Object<Attribute>>,
+    pub attributes: Annotated<Attributes>,
 
     /// Additional arbitrary fields for forwards compatibility.
     #[metastructure(additional_properties, pii = "maybe")]
     pub other: Object<Value>,
 }
 
-impl SpanV2 {
-    /// Returns the value of the attribute with the given name.
-    pub fn attribute(&self, key: &str) -> Option<&Annotated<Value>> {
-        Some(&self.attributes.value()?.get(key)?.value()?.value.value)
-    }
-}
-
 /// Status of a V2 span.
 ///
 /// This is a subset of OTEL's statuses (unset, ok, error), plus
@@ -294,7 +285,7 @@ pub struct SpanV2Link {
 
     /// Span link attributes, similar to span attributes/data.
     #[metastructure(pii = "maybe", trim = false)]
-    pub attributes: Annotated<Object<Attribute>>,
+    pub attributes: Annotated<Attributes>,
 
     /// Additional arbitrary fields for forwards compatibility.
     #[metastructure(additional_properties, pii = "maybe", trim = false)]
@@ -308,20 +299,6 @@ mod tests {
 
     use super::*;
 
-    macro_rules! attrs {
-        ($($name:expr => $val:expr , $ty:ident),* $(,)?) => {
-            std::collections::BTreeMap::from([$((
-                $name.to_owned(),
-                relay_protocol::Annotated::new(
-                    $crate::protocol::Attribute::new(
-                        $crate::protocol::AttributeType::$ty,
-                        $val.into()
-                    )
-                )
-            ),)*])
-        };
-    }
-
     #[test]
     fn test_span_serialization() {
         let json = r#"{
@@ -394,27 +371,31 @@ mod tests {
   }
 }"#;
 
-        let attributes = attrs!(
-            "custom.error_rate" => 0.5, Double,
-            "custom.is_green" => true, Boolean,
-            "sentry.release" => "1.0.0" , String,
-            "sentry.environment" => "local", String,
-            "sentry.platform" => "php", String,
-            "sentry.sdk.name" => "sentry.php", String,
-            "sentry.sdk.version" => "4.10.0", String,
-            "sentry.transaction_info.source" => "url", String,
-            "sentry.origin" => "manual", String,
-            "server.address" => "DHWKN7KX6N.local", String,
-            "http.response.status_code" => 200i64, Integer,
+        let mut attributes = Attributes::new();
+
+        attributes.insert("custom.error_rate".to_owned(), 0.5);
+        attributes.insert("custom.is_green".to_owned(), true);
+        attributes.insert("sentry.release".to_owned(), "1.0.0".to_owned());
+        attributes.insert("sentry.environment".to_owned(), "local".to_owned());
+        attributes.insert("sentry.platform".to_owned(), "php".to_owned());
+        attributes.insert("sentry.sdk.name".to_owned(), "sentry.php".to_owned());
+        attributes.insert("sentry.sdk.version".to_owned(), "4.10.0".to_owned());
+        attributes.insert(
+            "sentry.transaction_info.source".to_owned(),
+            "url".to_owned(),
         );
+        attributes.insert("sentry.origin".to_owned(), "manual".to_owned());
+        attributes.insert("server.address".to_owned(), "DHWKN7KX6N.local".to_owned());
+        attributes.insert("http.response.status_code".to_owned(), 200i64);
+
+        let mut link_attributes = Attributes::new();
+        link_attributes.insert("sentry.link.type".to_owned(), "previous_trace".to_owned());
 
         let links = vec![Annotated::new(SpanV2Link {
             trace_id: Annotated::new("627a2885119dcc8184fae7eef09438cb".parse().unwrap()),
             span_id: Annotated::new("6c71fc6b09b8b716".parse().unwrap()),
             sampled: Annotated::new(true),
-            attributes: Annotated::new(attrs!(
-                "sentry.link.type" => "previous_trace", String
-            )),
+            attributes: Annotated::new(link_attributes),
             ..Default::default()
         })];
         let span = Annotated::new(SpanV2 {