Merge branch 'master' into feat/metrics-aggregate-envelopes

* master: fix(server): Remove dependent items from envelope when dropping transaction item (#960) fix(clippy): Fix clippy 1.51.0 warnings (#965) feat(server): Add support for breakdowns ingestion (#934) build: Update schemars and remove workarounds (#961) feat(server): Add rule id to outcomes coming from transaction sampling (#953)
getsentry · Mar 26, 2021 · aed4194 · aed4194
2 parents 47d6fdc + f877f22
commit aed4194
Show file tree

Hide file tree

Showing 30 changed files with 1,141 additions and 383 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,13 +5,16 @@
 **Bug Fixes**:
 
 - Make request url scrubbable. ([#955](https://github.com/getsentry/relay/pull/955))
+- Remove dependent items from envelope when dropping transaction item. ([#960](https://github.com/getsentry/relay/pull/960))
 
 **Internal**:
 
 - Emit the `quantity` field for outcomes of events. This field describes the total size in bytes for attachments or the event count for all other categories. A separate outcome is emitted for attachments in a rejected envelope, if any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942))
 - Add experimental metrics ingestion without bucketing or pre-aggregation. ([#948](https://github.com/getsentry/relay/pull/948))
 - Change HTTP response for upstream timeouts from 502 to 504. ([#859](https://github.com/getsentry/relay/pull/859))
 - Disallow empty metric names, require alphabetic start. ([#952](https://github.com/getsentry/relay/pull/952))
+- Add rule id to outcomes coming from transaction sampling. ([#953](https://github.com/getsentry/relay/pull/953))
+- Add support for breakdowns ingestion. ([#934](https://github.com/getsentry/relay/pull/934))
 
 ## 21.3.0
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/relay-common/Cargo.toml b/relay-common/Cargo.toml
@@ -23,7 +23,7 @@ rand = "0.7.3"
 regex = "1.3.9"
 relay-log = { path = "../relay-log" }
 sentry-types = "0.20.0"
-schemars = { version = "0.8.0", features = ["uuid", "chrono"], optional = true }
+schemars = { version = "0.8.1", features = ["uuid", "chrono"], optional = true }
 serde = { version = "1.0.114", features = ["derive"] }
 
 [features]

diff --git a/relay-common/src/constants.rs b/relay-common/src/constants.rs
@@ -1,8 +1,5 @@
 //! Constants shared with the C-ABI and Sentry.
 
-// FIXME: Workaround for https://github.com/GREsau/schemars/pull/65
-#![allow(clippy::field_reassign_with_default)]
-
 use std::convert::TryInto;
 use std::fmt;
 use std::str::FromStr;
@@ -34,7 +31,7 @@ pub enum EventType {
     /// An HPKP violation payload.
     Hpkp,
     /// An ExpectCT violation payload.
-    ExpectCT,
+    ExpectCt,
     /// An ExpectStaple violation payload.
     ExpectStaple,
     /// Performance monitoring transactions carrying spans.
@@ -71,7 +68,7 @@ impl FromStr for EventType {
             "error" => EventType::Error,
             "csp" => EventType::Csp,
             "hpkp" => EventType::Hpkp,
-            "expectct" => EventType::ExpectCT,
+            "expectct" => EventType::ExpectCt,
             "expectstaple" => EventType::ExpectStaple,
             "transaction" => EventType::Transaction,
             _ => return Err(ParseEventTypeError),
@@ -86,7 +83,7 @@ impl fmt::Display for EventType {
             EventType::Error => write!(f, "error"),
             EventType::Csp => write!(f, "csp"),
             EventType::Hpkp => write!(f, "hpkp"),
-            EventType::ExpectCT => write!(f, "expectct"),
+            EventType::ExpectCt => write!(f, "expectct"),
             EventType::ExpectStaple => write!(f, "expectstaple"),
             EventType::Transaction => write!(f, "transaction"),
         }
@@ -172,7 +169,7 @@ impl From<EventType> for DataCategory {
         match ty {
             EventType::Default | EventType::Error => Self::Error,
             EventType::Transaction => Self::Transaction,
-            EventType::Csp | EventType::Hpkp | EventType::ExpectCT | EventType::ExpectStaple => {
+            EventType::Csp | EventType::Hpkp | EventType::ExpectCt | EventType::ExpectStaple => {
                 Self::Security
             }
         }

diff --git a/relay-general/Cargo.toml b/relay-general/Cargo.toml
@@ -28,7 +28,7 @@ pest_derive = "2.1.0"
 regex = "1.3.9"
 relay-common = { path = "../relay-common" }
 relay-general-derive = { path = "derive" }
-schemars = { version = "0.8.0", features = ["uuid", "chrono"], optional = true }
+schemars = { version = "0.8.1", features = ["uuid", "chrono"], optional = true }
 scroll = "0.10.2"
 serde = { version = "1.0.114", features = ["derive"] }
 serde_json = "1.0.55"

diff --git a/relay-general/benches/benchmarks.rs b/relay-general/benches/benchmarks.rs
@@ -90,6 +90,7 @@ fn bench_store_processor(c: &mut Criterion) {
         user_agent: None,
         sent_at: None,
         received_at: None,
+        breakdowns: None,
     };
 
     let mut processor = StoreProcessor::new(config, None);

diff --git a/relay-general/src/pii/processor.rs b/relay-general/src/pii/processor.rs
@@ -400,15 +400,16 @@ fn test_basic_stripping() {
                 Annotated::new(rv)
             },
             headers: {
-                let mut rv = Vec::new();
-                rv.push(Annotated::new((
-                    Annotated::new("Cookie".to_string().into()),
-                    Annotated::new("super secret".to_string().into()),
-                )));
-                rv.push(Annotated::new((
-                    Annotated::new("X-Forwarded-For".to_string().into()),
-                    Annotated::new("127.0.0.1".to_string().into()),
-                )));
+                let rv = vec![
+                    Annotated::new((
+                        Annotated::new("Cookie".to_string().into()),
+                        Annotated::new("super secret".to_string().into()),
+                    )),
+                    Annotated::new((
+                        Annotated::new("X-Forwarded-For".to_string().into()),
+                        Annotated::new("127.0.0.1".to_string().into()),
+                    )),
+                ];
                 Annotated::new(Headers(PairList(rv)))
             },
             ..Default::default()

diff --git a/relay-general/src/processor/attrs.rs b/relay-general/src/processor/attrs.rs
@@ -677,13 +677,11 @@ mod tests {
             let actual = $state.path().matches_selector(&$selector.parse().unwrap());
             assert!(
                 actual == $expected,
-                format!(
-                    "Matched {} against {}, expected {:?}, actually {:?}",
-                    $selector,
-                    $state.path(),
-                    $expected,
-                    actual
-                )
+                "Matched {} against {}, expected {:?}, actually {:?}",
+                $selector,
+                $state.path(),
+                $expected,
+                actual
             );
         }};
     }

diff --git a/relay-general/src/processor/selector.rs b/relay-general/src/processor/selector.rs
@@ -31,6 +31,7 @@ pub enum InvalidSelectorError {
 }
 
 mod parser {
+    #![allow(clippy::upper_case_acronyms)]
     use pest_derive::Parser;
 
     #[derive(Parser)]

diff --git a/relay-general/src/protocol/breakdowns.rs b/relay-general/src/protocol/breakdowns.rs
@@ -0,0 +1,69 @@
+use std::ops::{Deref, DerefMut};
+
+use crate::protocol::Measurements;
+use crate::types::{Annotated, Error, FromValue, Object, Value};
+
+/// A map of breakdowns.
+/// Breakdowns may be available on any event type. A breakdown are product-defined measurement values
+/// generated by the client, or materialized during ingestion. For example, for transactions, we may
+/// emit span operation breakdowns based on the attached span data.
+#[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)]
+pub struct Breakdowns(pub Object<Measurements>);
+
+impl Breakdowns {
+    pub fn is_valid_breakdown_name(name: &str) -> bool {
+        !name.is_empty()
+            && name.starts_with(|c| matches!(c, 'a'..='z' | 'A'..='Z'))
+            && name
+                .chars()
+                .all(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '.'))
+    }
+}
+
+impl FromValue for Breakdowns {
+    fn from_value(value: Annotated<Value>) -> Annotated<Self> {
+        let mut processing_errors = Vec::new();
+
+        let mut breakdowns = Object::from_value(value).map_value(|breakdowns| {
+            let breakdowns = breakdowns
+                .into_iter()
+                .filter_map(|(name, object)| {
+                    let name = name.trim();
+
+                    if Breakdowns::is_valid_breakdown_name(name) {
+                        return Some((name.into(), object));
+                    } else {
+                        processing_errors.push(Error::invalid(format!(
+                            "breakdown name '{}' can contain only characters a-z0-9.-_",
+                            name
+                        )));
+                    }
+
+                    None
+                })
+                .collect();
+
+            Self(breakdowns)
+        });
+
+        for error in processing_errors {
+            breakdowns.meta_mut().add_error(error);
+        }
+
+        breakdowns
+    }
+}
+
+impl Deref for Breakdowns {
+    type Target = Object<Measurements>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for Breakdowns {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
diff --git a/relay-general/src/protocol/event.rs b/relay-general/src/protocol/event.rs
@@ -10,9 +10,9 @@ use serde::{Serialize, Serializer};
 
 use crate::processor::ProcessValue;
 use crate::protocol::{
-    Breadcrumb, ClientSdkInfo, Contexts, Csp, DebugMeta, Exception, ExpectCt, ExpectStaple,
-    Fingerprint, Hpkp, LenientString, Level, LogEntry, Measurements, Metrics, Request, Span,
-    Stacktrace, Tags, TemplateInfo, Thread, Timestamp, User, Values,
+    Breadcrumb, Breakdowns, ClientSdkInfo, Contexts, Csp, DebugMeta, Exception, ExpectCt,
+    ExpectStaple, Fingerprint, Hpkp, LenientString, Level, LogEntry, Measurements, Metrics,
+    Request, Span, Stacktrace, Tags, TemplateInfo, Thread, Timestamp, User, Values,
 };
 use crate::types::{
     Annotated, Array, Empty, ErrorKind, FromValue, Object, SkipSerialization, ToValue, Value,
@@ -501,6 +501,11 @@ pub struct Event {
     #[metastructure(omit_from_schema)] // we only document error events for now
     pub measurements: Annotated<Measurements>,
 
+    /// Breakdowns which holds product-defined values such as span operation breakdowns.
+    #[metastructure(skip_serialization = "empty")]
+    #[metastructure(omit_from_schema)] // we only document error events for now
+    pub breakdowns: Annotated<Breakdowns>,
+
     /// Internal ingestion and processing metrics.
     ///
     /// This value should not be ingested and will be overwritten by the store normalizer.
@@ -588,11 +593,10 @@ fn test_event_roundtrip() {
         dist: Annotated::new("mydist".to_string()),
         environment: Annotated::new("myenv".to_string()),
         tags: {
-            let mut items = Array::new();
-            items.push(Annotated::new(TagEntry(
+            let items = vec![Annotated::new(TagEntry(
                 Annotated::new("tag".to_string()),
                 Annotated::new("value".to_string()),
-            )));
+            ))];
             Annotated::new(Tags(items.into()))
         },
         extra: {

diff --git a/relay-general/src/protocol/measurements.rs b/relay-general/src/protocol/measurements.rs
@@ -1,8 +1,9 @@
+use std::ops::{Deref, DerefMut};
+
 use crate::types::{Annotated, Error, FromValue, Object, Value};
 
 /// An individual observed measurement.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, ToValue, ProcessValue)]
-#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
 pub struct Measurement {
     /// Value of observed measurement value.
     #[metastructure(required = "true", skip_serialization = "never")]
@@ -11,17 +12,22 @@ pub struct Measurement {
 
 /// A map of observed measurement values.
 ///
-/// Measurements are only available on transactions. They contain measurement values of observed
-/// values such as Largest Contentful Paint (LCP).
+/// They contain measurement values of observed values such as Largest Contentful Paint (LCP).
 #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)]
-#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
 pub struct Measurements(pub Object<Measurement>);
 
+impl Measurements {
+    /// Returns the underlying object of measurements.
+    pub fn into_inner(self) -> Object<Measurement> {
+        self.0
+    }
+}
+
 impl FromValue for Measurements {
     fn from_value(value: Annotated<Value>) -> Annotated<Self> {
         let mut processing_errors = Vec::new();
 
-        let mut measurements = Object::<Measurement>::from_value(value).map_value(|measurements| {
+        let mut measurements = Object::from_value(value).map_value(|measurements| {
             let measurements = measurements
                 .into_iter()
                 .filter_map(|(name, object)| {
@@ -51,6 +57,20 @@ impl FromValue for Measurements {
     }
 }
 
+impl Deref for Measurements {
+    type Target = Object<Measurement>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for Measurements {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
 fn is_valid_measurement_name(name: &str) -> bool {
     name.chars()
         .all(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '.'))

diff --git a/relay-general/src/protocol/mod.rs b/relay-general/src/protocol/mod.rs
@@ -1,9 +1,7 @@
 //! Implements the sentry event protocol.
 
-// FIXME: Workaround for https://github.com/GREsau/schemars/pull/65
-#![allow(clippy::field_reassign_with_default)]
-
 mod breadcrumb;
+mod breakdowns;
 mod clientsdk;
 mod constants;
 mod contexts;
@@ -30,6 +28,7 @@ mod user;
 mod user_report;
 
 pub use self::breadcrumb::Breadcrumb;
+pub use self::breakdowns::Breakdowns;
 pub use self::clientsdk::{ClientSdkInfo, ClientSdkPackage};
 pub use self::constants::{INVALID_ENVIRONMENTS, INVALID_RELEASES, VALID_PLATFORMS};
 pub use self::contexts::{
@@ -47,7 +46,7 @@ pub use self::event::{
 pub use self::exception::Exception;
 pub use self::fingerprint::Fingerprint;
 pub use self::logentry::{LogEntry, Message};
-pub use self::measurements::Measurements;
+pub use self::measurements::{Measurement, Measurements};
 pub use self::mechanism::{CError, MachException, Mechanism, MechanismMeta, PosixSignal};
 pub use self::metrics::{Metrics, SampleRate};
 pub use self::request::{Cookies, HeaderName, HeaderValue, Headers, Query, Request};