ref(normalization): remove MongoDB query scrubbing feature

MongoDB support for the Query insights module is now in GA. Remove the feature flag that controlled whether or not to enable query scrubbing for Mongo queries, as it is now always required.
getsentry · Nov 6, 2024 · eb956dc · eb956dc
1 parent 0a5a60e
commit eb956dc
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 226 deletions.
diff --git a/relay-cabi/src/processing.rs b/relay-cabi/src/processing.rs
@@ -12,7 +12,6 @@ use std::sync::OnceLock;
 use chrono::{DateTime, Utc};
 use relay_cardinality::CardinalityLimit;
 use relay_dynamic_config::{normalize_json, GlobalConfig, ProjectConfig};
-use relay_event_normalization::span::description::ScrubMongoDescription;
 use relay_event_normalization::{
     normalize_event, validate_event, BreakdownsConfig, ClientHints, EventValidationConfig,
     GeoIpLookup, NormalizationConfig, RawUserAgentInfo,
@@ -273,8 +272,7 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event(
         measurements: None,
         normalize_spans: config.normalize_spans,
         replay_id: config.replay_id,
-        span_allowed_hosts: &[], // only supported in relay
-        scrub_mongo_description: ScrubMongoDescription::Disabled, // only supported in relay
+        span_allowed_hosts: &[],              // only supported in relay
         span_op_defaults: Default::default(), // only supported in relay
     };
     normalize_event(&mut event, &normalization_config);

diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs
@@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize};
 pub const GRADUATED_FEATURE_FLAGS: &[Feature] = &[
     Feature::UserReportV2Ingest,
     Feature::IngestUnsampledProfiles,
+    Feature::ScrubMongoDbDescriptions,
 ];
 
 /// Features exposed by project config.
@@ -96,12 +97,6 @@ pub enum Feature {
     /// Serialized as `organizations:indexed-spans-extraction`.
     #[serde(rename = "organizations:indexed-spans-extraction")]
     ExtractSpansFromEvent,
-    /// Enables description scrubbing for MongoDB spans (and consequently, their presence in the
-    /// Queries module inside Sentry).
-    ///
-    /// Serialized as `organizations:performance-queries-mongodb-extraction`.
-    #[serde(rename = "organizations:performance-queries-mongodb-extraction")]
-    ScrubMongoDbDescriptions,
     /// Indicate if the EAP consumers should ingest a span.
     ///
     /// Serialized as `organizations:ingest-spans-in-eap`
@@ -116,6 +111,10 @@ pub enum Feature {
     #[doc(hidden)]
     #[serde(rename = "organizations:user-feedback-ingest")]
     UserReportV2Ingest,
+    /// This feature has graduated and is hard-coded for external Relays.
+    #[doc(hidden)]
+    #[serde(rename = "organizations:performance-queries-mongodb-extraction")]
+    ScrubMongoDbDescriptions,
     /// Forward compatibility.
     #[doc(hidden)]
     #[serde(other)]

diff --git a/relay-event-normalization/src/event.rs b/relay-event-normalization/src/event.rs
@@ -29,7 +29,6 @@ use uuid::Uuid;
 
 use crate::normalize::request;
 use crate::span::ai::normalize_ai_measurements;
-use crate::span::description::ScrubMongoDescription;
 use crate::span::tag_extraction::extract_span_tags_from_event;
 use crate::utils::{self, get_event_user_tag, MAX_DURATION_MOBILE_MS};
 use crate::{
@@ -159,9 +158,6 @@ pub struct NormalizationConfig<'a> {
     /// Controls list of hosts to be excluded from scrubbing
     pub span_allowed_hosts: &'a [String],
 
-    /// Controls whether or not MongoDB span descriptions will be scrubbed.
-    pub scrub_mongo_description: ScrubMongoDescription,
-
     /// Rules to infer `span.op` from other span fields.
     pub span_op_defaults: BorrowedSpanOpDefaults<'a>,
 }
@@ -196,7 +192,6 @@ impl<'a> Default for NormalizationConfig<'a> {
             normalize_spans: true,
             replay_id: Default::default(),
             span_allowed_hosts: Default::default(),
-            scrub_mongo_description: ScrubMongoDescription::Disabled,
             span_op_defaults: Default::default(),
         }
     }
@@ -343,7 +338,6 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) {
             event,
             config.max_tag_value_length,
             config.span_allowed_hosts,
-            config.scrub_mongo_description,
         );
     }
 

diff --git a/relay-event-normalization/src/normalize/span/description/mod.rs b/relay-event-normalization/src/normalize/span/description/mod.rs
@@ -36,22 +36,12 @@ const MAX_EXTENSION_LENGTH: usize = 10;
 /// Domain names that are preserved during scrubbing
 const DOMAIN_ALLOW_LIST: &[&str] = &["localhost"];
 
-#[derive(PartialEq, Clone, Copy, Debug)]
-/// Whether to scrub MongoDB span descriptions or not. See `Feature::ScrubMongoDBDescriptions`.
-pub enum ScrubMongoDescription {
-    /// Disable scrubbing of MongoDB span descriptions.
-    Disabled,
-    /// Enable scrubbing of MongoDB span descriptions.
-    Enabled,
-}
-
 /// Attempts to replace identifiers in the span description with placeholders.
 ///
 /// Returns `None` if no scrubbing can be performed.
 pub(crate) fn scrub_span_description(
     span: &Span,
     span_allowed_hosts: &[String],
-    scrub_mongo_description: ScrubMongoDescription,
 ) -> (Option<String>, Option<Vec<sqlparser::ast::Statement>>) {
     let Some(description) = span.description.as_str() else {
         return (None, None);
@@ -73,10 +63,7 @@ pub(crate) fn scrub_span_description(
             ("http", _) => scrub_http(description, span_allowed_hosts),
             ("cache", _) | ("db", "redis") => scrub_redis_keys(description),
             ("db", _) if db_system == Some("redis") => scrub_redis_keys(description),
-            ("db", _)
-                if scrub_mongo_description == ScrubMongoDescription::Enabled
-                    && db_system == Some("mongodb") =>
-            {
+            ("db", _) if db_system == Some("mongodb") => {
                 let command = data
                     .and_then(|data| data.db_operation.value())
                     .and_then(|command| command.as_str());
@@ -656,11 +643,7 @@ mod tests {
                     .description
                     .set_value(Some($description_in.into()));
 
-                let scrubbed = scrub_span_description(
-                    span.value_mut().as_mut().unwrap(),
-                    &[],
-                    ScrubMongoDescription::Disabled,
-                );
+                let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
                 if $expected == "" {
                     assert!(scrubbed.0.is_none());
@@ -1219,7 +1202,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
         let span = span.value_mut().as_mut().unwrap();
-        let scrubbed = scrub_span_description(span, &[], ScrubMongoDescription::Disabled);
+        let scrubbed = scrub_span_description(span, &[]);
         assert_eq!(scrubbed.0.as_deref(), Some("SELECT %s"));
     }
 
@@ -1232,11 +1215,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         // When db.system is missing, no scrubbed description (i.e. no group) is set.
         assert!(scrubbed.0.is_none());
@@ -1254,11 +1233,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         // Can be scrubbed with db system.
         assert_eq!(scrubbed.0.as_deref(), Some("SELECT a FROM b"));
@@ -1276,11 +1251,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         // NOTE: this should return `DEL *`, but we cannot detect lowercase command names yet.
         assert_eq!(scrubbed.0.as_deref(), Some("*"));
@@ -1296,11 +1267,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         assert_eq!(scrubbed.0.as_deref(), Some("INSERTED * 'UAEventData'"));
     }
@@ -1315,11 +1282,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         assert_eq!(
             scrubbed.0.as_deref(),
@@ -1328,7 +1291,7 @@ mod tests {
     }
 
     #[test]
-    fn mongodb_scrubbing_enabled() {
+    fn mongodb_scrubbing() {
         let json = r#"{
             "description": "{\"find\": \"documents\", \"foo\": \"bar\"}",
             "op": "db",
@@ -1341,41 +1304,14 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Enabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         assert_eq!(
             scrubbed.0.as_deref(),
             Some(r#"{"find":"documents","foo":"?"}"#)
         )
     }
 
-    #[test]
-    fn mongodb_scrubbing_disabled() {
-        let json = r#"{
-            "description": "{\"find\": \"documents\", \"foo\": \"bar\"}",
-            "op": "db",
-            "data": {
-                "db.system": "mongodb",
-                "db.operation": "find",
-                "db.collection.name": "documents"
-            }
-        }"#;
-
-        let mut span = Annotated::<Span>::from_json(json).unwrap();
-
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
-
-        assert_eq!(scrubbed.0.as_deref(), None)
-    }
-
     #[test]
     fn mongodb_with_legacy_collection_property() {
         let json = r#"{
@@ -1390,11 +1326,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Enabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         assert_eq!(
             scrubbed.0.as_deref(),
@@ -1414,11 +1346,7 @@ mod tests {
 
         let mut span = Annotated::<Span>::from_json(json).unwrap();
 
-        let scrubbed = scrub_span_description(
-            span.value_mut().as_mut().unwrap(),
-            &[],
-            ScrubMongoDescription::Disabled,
-        );
+        let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
         // Can be scrubbed with db system.
         assert_eq!(scrubbed.0.as_deref(), Some("my-component-name"));
@@ -1460,11 +1388,8 @@ mod tests {
 
             let mut span = Annotated::<Span>::from_json(&json).unwrap();
 
-            let scrubbed = scrub_span_description(
-                span.value_mut().as_mut().unwrap(),
-                &allowed_hosts,
-                ScrubMongoDescription::Disabled,
-            );
+            let scrubbed =
+                scrub_span_description(span.value_mut().as_mut().unwrap(), &allowed_hosts);
 
             assert_eq!(
                 scrubbed.0.as_deref(),
@@ -1517,11 +1442,7 @@ mod tests {
                     .description
                     .set_value(Some($description_in.into()));
 
-                let scrubbed = scrub_span_description(
-                    span.value_mut().as_mut().unwrap(),
-                    &[],
-                    ScrubMongoDescription::Enabled,
-                );
+                let scrubbed = scrub_span_description(span.value_mut().as_mut().unwrap(), &[]);
 
                 if $expected == "" {
                     assert!(scrubbed.0.is_none());