feat(feedback): add User Feedbacks as Events (#2604)

Relay changes for the revamped User Feedback feature. User Feedbacks (Known as `UserFeedbackV2`), will be events that flow through our system instead of attachments. See [here](https://www.notion.so/sentry/User-Feedback-Beta-11d68db039bc436bae017ad56c0c20f9?pvs=4) for more information on the tech spec (and the link at the bottom for what we do once they're ingested). Note that within this code, we're calling it `UserReportV2`. This is to avoid confusion with the existing `UserReport`. A PR to shim the previous `UserReports` as new `UserReportV2`'s will follow, at which point we'll rename this in the code to be `UserFeedback`. The string `feedback` is used for the context and ItemType / EventType parsing. - [x] Adds a feature flag for ingest `UserReportV2Ingest`. This is backed by options getsentry/getsentry#11573 - [x] Creates a new EventType `UserReportV2` - [x] Creates a new ItemType `UserReportV2` - [x] Creates a new DataCategory `UserReportV2` - [x] Adds a new context `UserReportV2Context` that for now contains two properties, `contact_email` (no pii stripping), and the `message` which contains the feedback text. --------- Co-authored-by: Iker Barriocanal <32816711+iker-barriocanal@users.noreply.github.com>
getsentry · Oct 30, 2023 · 20f72be · 20f72be
1 parent 939dab7
commit 20f72be
Show file tree

Hide file tree

Showing 20 changed files with 307 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 **Features**:
 
 - Add inbound filters option to filter legacy Edge browsers (i.e. versions 12-18 ) ([#2650](https://github.com/getsentry/relay/pull/2650))
+- Add User Feedback Ingestion. ([#2604](https://github.com/getsentry/relay/pull/2604))
 - Group resource spans by scrubbed domain and filename. ([#2654](https://github.com/getsentry/relay/pull/2654))
 - Convert transactions to spans for all organizations. ([#2659](https://github.com/getsentry/relay/pull/2659))
 - Filter outliers (>180s) for mobile measurements. ([#2649](https://github.com/getsentry/relay/pull/2649))

diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md
@@ -7,6 +7,7 @@
 - Remove event breadcrumbs dating before January 1, 1970 UTC. ([#2635](https://github.com/getsentry/relay/pull/2635))
 - Add `PerformanceScoreConfig` config and performance score calculations to measurements for frontend events. ([#2632](https://github.com/getsentry/relay/pull/2632))
 - Add `locale` ,`screen_width_pixels`, `screen_height_pixels`, and `uuid` to the device context. ([#2640](https://github.com/getsentry/relay/pull/2640))
+- Add feedback DataCategory. ([#2604](https://github.com/getsentry/relay/pull/2604))
 
 ## 0.8.32
 

diff --git a/py/sentry_relay/consts.py b/py/sentry_relay/consts.py
@@ -24,6 +24,7 @@ class DataCategory(IntEnum):
     PROFILE_INDEXED = 11
     SPAN = 12
     MONITOR_SEAT = 13
+    USER_REPORT_V2 = 14
     UNKNOWN = -1
     # end generated
 

diff --git a/relay-base-schema/src/data_category.rs b/relay-base-schema/src/data_category.rs
@@ -57,6 +57,12 @@ pub enum DataCategory {
     /// but we define it here to prevent clashing values since this data category enumeration
     /// is also used outside of Relay via the Python package.
     MonitorSeat = 13,
+    /// User Feedback
+    ///
+    /// Represents a User Feedback processed.
+    /// Currently standardized on name UserReportV2 to avoid clashing with the old UserReport.
+    /// TODO(jferg): Rename this to UserFeedback once old UserReport is deprecated.
+    UserReportV2 = 14,
     //
     // IMPORTANT: After adding a new entry to DataCategory, go to the `relay-cabi` subfolder and run
     // `make header` to regenerate the C-binding. This allows using the data category from Python.
@@ -86,6 +92,7 @@ impl DataCategory {
             "monitor" => Self::Monitor,
             "span" => Self::Span,
             "monitor_seat" => Self::MonitorSeat,
+            "feedback" => Self::UserReportV2,
             _ => Self::Unknown,
         }
     }
@@ -108,6 +115,7 @@ impl DataCategory {
             Self::Monitor => "monitor",
             Self::Span => "span",
             Self::MonitorSeat => "monitor_seat",
+            Self::UserReportV2 => "feedback",
             Self::Unknown => "unknown",
         }
     }
@@ -157,6 +165,7 @@ impl From<EventType> for DataCategory {
             EventType::Csp | EventType::Hpkp | EventType::ExpectCt | EventType::ExpectStaple => {
                 Self::Security
             }
+            EventType::UserReportV2 => Self::UserReportV2,
         }
     }
 }
diff --git a/relay-base-schema/src/events.rs b/relay-base-schema/src/events.rs
@@ -41,6 +41,10 @@ pub enum EventType {
     ExpectStaple,
     /// Performance monitoring transactions carrying spans.
     Transaction,
+    /// User feedback payload.
+    ///
+    /// TODO(Jferg): Change this to UserFeedback once old UserReport logic is deprecated.
+    UserReportV2,
     /// All events that do not qualify as any other type.
     #[serde(other)]
     #[default]
@@ -71,6 +75,7 @@ impl FromStr for EventType {
             "expectct" => EventType::ExpectCt,
             "expectstaple" => EventType::ExpectStaple,
             "transaction" => EventType::Transaction,
+            "feedback" => EventType::UserReportV2,
             _ => return Err(ParseEventTypeError),
         })
     }
@@ -86,6 +91,7 @@ impl fmt::Display for EventType {
             EventType::ExpectCt => write!(f, "expectct"),
             EventType::ExpectStaple => write!(f, "expectstaple"),
             EventType::Transaction => write!(f, "transaction"),
+            EventType::UserReportV2 => write!(f, "feedback"),
         }
     }
 }

diff --git a/relay-cabi/include/relay.h b/relay-cabi/include/relay.h
@@ -3,7 +3,7 @@
 #ifndef RELAY_H_INCLUDED
 #define RELAY_H_INCLUDED
 
-/* Generated with cbindgen:0.25.0 */
+/* Generated with cbindgen:0.26.0 */
 
 /* Warning, this file is autogenerated. Do not modify this manually. */
 
@@ -88,6 +88,14 @@ enum RelayDataCategory {
    * is also used outside of Relay via the Python package.
    */
   RELAY_DATA_CATEGORY_MONITOR_SEAT = 13,
+  /**
+   * User Feedback
+   *
+   * Represents a User Feedback processed.
+   * Currently standardized on name UserReportV2 to avoid clashing with the old UserReport.
+   * TODO(jferg): Rename this to UserFeedback once old UserReport is deprecated.
+   */
+  RELAY_DATA_CATEGORY_USER_REPORT_V2 = 14,
   /**
    * Any other data category not known by this Relay.
    */
@@ -616,14 +624,4 @@ struct RelayStr relay_validate_project_config(const struct RelayStr *value,
  */
 struct RelayStr normalize_global_config(const struct RelayStr *value);
 
-/**
- * Runs dynamic sampling given the sampling config, root sampling config, DSC and event.
- *
- * Returns the sampling decision containing the sample_rate and the list of matched rule ids.
- */
-struct RelayStr run_dynamic_sampling(const struct RelayStr *sampling_config,
-                                     const struct RelayStr *root_sampling_config,
-                                     const struct RelayStr *dsc,
-                                     const struct RelayStr *event);
-
 #endif /* RELAY_H_INCLUDED */
diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs
@@ -11,6 +11,11 @@ pub enum Feature {
     /// Enables data scrubbing of replay recording payloads.
     #[serde(rename = "organizations:session-replay-recording-scrubbing")]
     SessionReplayRecordingScrubbing,
+    /// Enables new User Feedback ingest.
+    ///
+    /// TODO(jferg): rename to UserFeedbackIngest once old UserReport logic is deprecated.
+    #[serde(rename = "organizations:user-feedback-ingest")]
+    UserReportV2Ingest,
     /// Enables device.class synthesis
     ///
     /// Enables device.class tag synthesis on mobile events.

diff --git a/relay-event-normalization/src/normalize/mod.rs b/relay-event-normalization/src/normalize/mod.rs
@@ -248,6 +248,9 @@ impl<'a> NormalizeProcessor<'a> {
         if event.ty.value() == Some(&EventType::Transaction) {
             return EventType::Transaction;
         }
+        if event.ty.value() == Some(&EventType::UserReportV2) {
+            return EventType::UserReportV2;
+        }
 
         // The SDKs do not describe event types, and we must infer them from available attributes.
         let has_exceptions = event

diff --git a/relay-event-schema/src/protocol/contexts/mod.rs b/relay-event-schema/src/protocol/contexts/mod.rs
@@ -12,7 +12,7 @@ mod reprocessing;
 mod response;
 mod runtime;
 mod trace;
-
+mod user_report_v2;
 pub use app::*;
 pub use browser::*;
 pub use cloud_resource::*;
@@ -27,6 +27,7 @@ pub use reprocessing::*;
 pub use response::*;
 pub use runtime::*;
 pub use trace::*;
+pub use user_report_v2::*;
 
 #[cfg(feature = "jsonschema")]
 use relay_jsonschema_derive::JsonSchema;
@@ -67,6 +68,9 @@ pub enum Context {
     Profile(Box<ProfileContext>),
     /// Information related to Replay.
     Replay(Box<ReplayContext>),
+    /// Information related to User Report V2. TODO:(jferg): rename to UserFeedbackContext
+    #[metastructure(tag = "feedback")]
+    UserReportV2(Box<UserReportV2Context>),
     /// Information related to Monitors feature.
     Monitor(Box<MonitorContext>),
     /// Auxilliary information for reprocessing.

diff --git a/relay-event-schema/src/protocol/contexts/replay.rs b/relay-event-schema/src/protocol/contexts/replay.rs
@@ -59,7 +59,7 @@ mod tests {
     use crate::protocol::Context;
 
     #[test]
-    fn test_trace_context_roundtrip() {
+    fn test_replay_context() {
         let json = r#"{
   "replay_id": "4c79f60c11214eb38604f4ae0781bfb2",
   "type": "replay"

diff --git a/relay-event-schema/src/protocol/contexts/user_report_v2.rs b/relay-event-schema/src/protocol/contexts/user_report_v2.rs
@@ -0,0 +1,78 @@
+#[cfg(feature = "jsonschema")]
+use relay_jsonschema_derive::JsonSchema;
+use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, Value};
+
+use crate::processor::ProcessValue;
+
+/// Feedback context.
+///
+/// This contexts contains user feedback specific attributes.
+/// We don't PII scrub contact_email as that is provided by the user.
+/// TODO(jferg): rename to FeedbackContext once old UserReport logic is deprecated.
+#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
+pub struct UserReportV2Context {
+    /// The feedback message which contains what the user has to say.
+    pub message: Annotated<String>,
+
+    /// an email optionally provided by the user, which can be different from user.email
+    #[metastructure(pii = "false")]
+    pub contact_email: Annotated<String>,
+    /// Additional arbitrary fields for forwards compatibility.
+    #[metastructure(additional_properties, retain = "true")]
+    pub other: Object<Value>,
+}
+
+impl super::DefaultContext for UserReportV2Context {
+    fn default_key() -> &'static str {
+        "feedback"
+    }
+
+    fn from_context(context: super::Context) -> Option<Self> {
+        match context {
+            super::Context::UserReportV2(c) => Some(*c),
+            _ => None,
+        }
+    }
+
+    fn cast(context: &super::Context) -> Option<&Self> {
+        match context {
+            super::Context::UserReportV2(c) => Some(c),
+            _ => None,
+        }
+    }
+
+    fn cast_mut(context: &mut super::Context) -> Option<&mut Self> {
+        match context {
+            super::Context::UserReportV2(c) => Some(c),
+            _ => None,
+        }
+    }
+
+    fn into_context(self) -> super::Context {
+        super::Context::UserReportV2(Box::new(self))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::protocol::Context;
+
+    #[test]
+    fn test_feedback_context() {
+        let json = r#"{
+  "message": "test message",
+  "contact_email": "test@test.com",
+  "type": "feedback"
+}"#;
+        let context = Annotated::new(Context::UserReportV2(Box::new(UserReportV2Context {
+            message: Annotated::new("test message".to_string()),
+            contact_email: Annotated::new("test@test.com".to_string()),
+            other: Object::default(),
+        })));
+
+        assert_eq!(context, Annotated::from_json(json).unwrap());
+        assert_eq!(json, context.to_json_pretty().unwrap());
+    }
+}
diff --git a/relay-quotas/src/quota.rs b/relay-quotas/src/quota.rs
@@ -110,7 +110,8 @@ impl CategoryUnit {
             | DataCategory::TransactionIndexed
             | DataCategory::Span
             | DataCategory::MonitorSeat
-            | DataCategory::Monitor => Some(Self::Count),
+            | DataCategory::Monitor
+            | DataCategory::UserReportV2 => Some(Self::Count),
             DataCategory::Attachment => Some(Self::Bytes),
             DataCategory::Session => Some(Self::Batched),
 

diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs
@@ -1641,6 +1641,7 @@ impl EnvelopeProcessorService {
             ItemType::Security => true,
             ItemType::FormData => true,
             ItemType::RawSecurity => true,
+            ItemType::UserReportV2 => true,
 
             // These should be removed conditionally:
             ItemType::UnrealReport => self.inner.config.processing_enabled(),
@@ -1684,6 +1685,9 @@ impl EnvelopeProcessorService {
         let transaction_item = envelope.take_item_by(|item| item.ty() == &ItemType::Transaction);
         let security_item = envelope.take_item_by(|item| item.ty() == &ItemType::Security);
         let raw_security_item = envelope.take_item_by(|item| item.ty() == &ItemType::RawSecurity);
+        let user_report_v2_item =
+            envelope.take_item_by(|item| item.ty() == &ItemType::UserReportV2);
+
         let form_item = envelope.take_item_by(|item| item.ty() == &ItemType::FormData);
         let attachment_item = envelope
             .take_item_by(|item| item.attachment_type() == Some(&AttachmentType::EventPayload));
@@ -1715,6 +1719,14 @@ impl EnvelopeProcessorService {
                 // hint to normalization that we're dealing with a transaction now.
                 self.event_from_json_payload(item, Some(EventType::Transaction))?
             })
+        } else if let Some(item) = user_report_v2_item {
+            relay_log::trace!("processing user_report_v2");
+            let project_state = &state.project_state;
+            let user_report_v2_ingest = project_state.has_feature(Feature::UserReportV2Ingest);
+            if !user_report_v2_ingest {
+                return Err(ProcessingError::NoEventPayload);
+            }
+            self.event_from_json_payload(item, Some(EventType::UserReportV2))?
         } else if let Some(mut item) = raw_security_item {
             relay_log::trace!("processing security report");
             sample_rates = item.take_sample_rates();

diff --git a/relay-server/src/actors/store.rs b/relay-server/src/actors/store.rs
@@ -121,7 +121,10 @@ impl StoreService {
         let event_item = envelope.get_item_by(|item| {
             matches!(
                 item.ty(),
-                ItemType::Event | ItemType::Transaction | ItemType::Security
+                ItemType::Event
+                    | ItemType::Transaction
+                    | ItemType::Security
+                    | ItemType::UserReportV2
             )
         });
 

diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs
@@ -112,6 +112,8 @@ pub enum ItemType {
     CheckIn,
     /// A standalone span.
     Span,
+    /// UserReport as an Event
+    UserReportV2,
     /// A new item type that is yet unknown by this version of Relay.
     ///
     /// By default, items of this type are forwarded without modification. Processing Relays and
@@ -127,6 +129,7 @@ impl ItemType {
         match event_type {
             EventType::Default | EventType::Error => ItemType::Event,
             EventType::Transaction => ItemType::Transaction,
+            EventType::UserReportV2 => ItemType::UserReportV2,
             EventType::Csp | EventType::Hpkp | EventType::ExpectCt | EventType::ExpectStaple => {
                 ItemType::Security
             }
@@ -145,6 +148,7 @@ impl fmt::Display for ItemType {
             Self::RawSecurity => write!(f, "raw_security"),
             Self::UnrealReport => write!(f, "unreal_report"),
             Self::UserReport => write!(f, "user_report"),
+            Self::UserReportV2 => write!(f, "feedback"),
             Self::Session => write!(f, "session"),
             Self::Sessions => write!(f, "sessions"),
             Self::Statsd => write!(f, "statsd"),
@@ -173,6 +177,7 @@ impl std::str::FromStr for ItemType {
             "raw_security" => Self::RawSecurity,
             "unreal_report" => Self::UnrealReport,
             "user_report" => Self::UserReport,
+            "feedback" => Self::UserReportV2,
             "session" => Self::Session,
             "sessions" => Self::Sessions,
             "statsd" => Self::Statsd,
@@ -556,6 +561,7 @@ impl Item {
             ItemType::Statsd | ItemType::MetricBuckets => None,
             ItemType::FormData => None,
             ItemType::UserReport => None,
+            ItemType::UserReportV2 => None,
             ItemType::Profile => Some(if indexed {
                 DataCategory::ProfileIndexed
             } else {
@@ -700,7 +706,8 @@ impl Item {
             | ItemType::Transaction
             | ItemType::Security
             | ItemType::RawSecurity
-            | ItemType::UnrealReport => true,
+            | ItemType::UnrealReport
+            | ItemType::UserReportV2 => true,
 
             // Attachments are only event items if they are crash reports or if they carry partial
             // event payloads. Plain attachments never create event payloads.
@@ -758,6 +765,8 @@ impl Item {
             ItemType::RawSecurity => true,
             ItemType::UnrealReport => true,
             ItemType::UserReport => true,
+            ItemType::UserReportV2 => true,
+
             ItemType::ReplayEvent => true,
             ItemType::Session => false,
             ItemType::Sessions => false,

diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs
@@ -94,6 +94,7 @@ fn infer_event_category(item: &Item) -> Option<DataCategory> {
         ItemType::Transaction => Some(DataCategory::Transaction),
         ItemType::Security | ItemType::RawSecurity => Some(DataCategory::Security),
         ItemType::UnrealReport => Some(DataCategory::Error),
+        ItemType::UserReportV2 => Some(DataCategory::UserReportV2),
         ItemType::Attachment if item.creates_event() => Some(DataCategory::Error),
         ItemType::Attachment => None,
         ItemType::Session => None,

diff --git a/relay-server/src/utils/sizes.rs b/relay-server/src/utils/sizes.rs
@@ -29,6 +29,7 @@ pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool
             | ItemType::Security
             | ItemType::ReplayEvent
             | ItemType::RawSecurity
+            | ItemType::UserReportV2
             | ItemType::FormData => {
                 event_size += item.len();
             }