fix(pattern): Fix mongodb patterns, assert all patterns in debug prof…

…ile (#4125) Assert patterns for correctness in the debug profile. This caught the mongodb patterns to be broken because of unbalanced parenthesis. Example failure of the MongoDB patterns: ``` ---- metrics_extraction::event::tests::test_metrics_summaries_on_transaction_and_spans stdout ---- thread 'metrics_extraction::event::tests::test_metrics_summaries_on_transaction_and_spans' panicked at /home/runner/work/relay/relay/relay-pattern/src/typed.rs:185:18: all patterns should be valid patterns: Error { pattern: "*[{*", kind: UnbalancedCharacterClass } ``` Fixing the mongodb patterns surfaced a bug in matching of the `db.system`, which is now also fixed.
getsentry · Oct 9, 2024 · 469e104 · 469e104
1 parent 0f88b37
commit 469e104
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 5 deletions.
diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs
@@ -24,7 +24,7 @@ const MOBILE_OPS: &[&str] = &[
 const APP_START_ROOT_SPAN_DESCRIPTIONS: &[&str] = &["Cold Start", "Warm Start"];
 
 /// A list of patterns found in MongoDB queries.
-const MONGODB_QUERIES: &[&str] = &["*\"$*", "{*", "*({*", "*[{*"];
+const MONGODB_QUERIES: &[&str] = &["*\"$*", r"\{*", r"*(\{*", r"*\[\{*"];
 
 /// A list of patterns for resource span ops we'd like to ingest.
 const RESOURCE_SPAN_OPS: &[&str] = &["resource.script", "resource.css", "resource.img"];
@@ -117,7 +117,7 @@ pub fn hardcoded_span_metrics() -> Vec<(GroupKey, Vec<MetricSpec>, Vec<TagMappin
     let is_db = RuleCondition::eq("span.sentry_tags.category", "db")
         & !RuleCondition::glob("span.op", DISABLED_DATABASES)
         // MongoDB queries are only allowed when `span.system` is set to `mongodb`.
-        & (RuleCondition::eq("span.system", "mongodb")
+        & (RuleCondition::eq("span.data.db\\.system", "mongodb")
             | !RuleCondition::glob("span.description", MONGODB_QUERIES));
     let is_resource = RuleCondition::glob("span.op", RESOURCE_SPAN_OPS);
 

diff --git a/relay-pattern/src/typed.rs b/relay-pattern/src/typed.rs
@@ -180,7 +180,9 @@ impl<C: PatternConfig> FromIterator<String> for TypedPatterns<C> {
     fn from_iter<T: IntoIterator<Item = String>>(iter: T) -> Self {
         let mut builder = Self::builder();
         for pattern in iter.into_iter() {
-            let _ = builder.add(pattern);
+            let _err = builder.add(pattern);
+            #[cfg(debug_assertions)]
+            _err.expect("all patterns should be valid patterns");
         }
         builder.build()
     }
@@ -248,7 +250,9 @@ impl<'de, C: PatternConfig> serde::Deserialize<'de> for TypedPatterns<C> {
 
                 while let Some(item) = seq.next_element()? {
                     // Ignore invalid patterns as documented.
-                    let _ = builder.add(item);
+                    let _err = builder.add(item);
+                    #[cfg(debug_assertions)]
+                    _err.expect("all patterns should be valid patterns");
                 }
 
                 Ok(builder.build())
@@ -398,7 +402,7 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "serde")]
+    #[cfg(all(feature = "serde", not(debug_assertions)))]
     fn test_patterns_deserialize_err() {
         let r: TypedPatterns<CaseInsensitive> =
             serde_json::from_str(r#"["[invalid","foobar"]"#).unwrap();

diff --git a/...snapshots/relay_server__metrics_extraction__event__tests__both_feature_flags_enabled.snap b/...snapshots/relay_server__metrics_extraction__event__tests__both_feature_flags_enabled.snap
@@ -3682,8 +3682,12 @@ expression: metrics
         ),
         tags: {
             "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
             "span.op": "db.sql.query",
+            "span.system": "mongodb",
             "transaction": "gEt /api/:version/users/",
+            "transaction.method": "POST",
             "transaction.op": "myop",
         },
         metadata: BucketMetadata {
@@ -3694,6 +3698,32 @@ expression: metrics
             extracted_from_indexed: false,
         },
     },
+    Bucket {
+        timestamp: UnixTimestamp(1597976302),
+        width: 0,
+        name: MetricName(
+            "d:spans/exclusive_time_light@millisecond",
+        ),
+        value: Distribution(
+            [
+                2000.0,
+            ],
+        ),
+        tags: {
+            "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
+            "span.op": "db.sql.query",
+            "span.system": "mongodb",
+        },
+        metadata: BucketMetadata {
+            merges: 1,
+            received_at: Some(
+                UnixTimestamp(0),
+            ),
+            extracted_from_indexed: false,
+        },
+    },
     Bucket {
         timestamp: UnixTimestamp(1597976302),
         width: 0,
@@ -3707,8 +3737,12 @@ expression: metrics
         ),
         tags: {
             "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
             "span.op": "db.sql.query",
+            "span.system": "mongodb",
             "transaction": "gEt /api/:version/users/",
+            "transaction.method": "POST",
             "transaction.op": "myop",
         },
         metadata: BucketMetadata {
@@ -3719,6 +3753,32 @@ expression: metrics
             extracted_from_indexed: false,
         },
     },
+    Bucket {
+        timestamp: UnixTimestamp(1597976302),
+        width: 0,
+        name: MetricName(
+            "d:spans/duration_light@millisecond",
+        ),
+        value: Distribution(
+            [
+                2000.0,
+            ],
+        ),
+        tags: {
+            "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
+            "span.op": "db.sql.query",
+            "span.system": "mongodb",
+        },
+        metadata: BucketMetadata {
+            merges: 1,
+            received_at: Some(
+                UnixTimestamp(0),
+            ),
+            extracted_from_indexed: false,
+        },
+    },
     Bucket {
         timestamp: UnixTimestamp(1597976302),
         width: 0,

diff --git a/...ics_extraction/snapshots/relay_server__metrics_extraction__event__tests__only_common.snap b/...ics_extraction/snapshots/relay_server__metrics_extraction__event__tests__only_common.snap
@@ -3388,8 +3388,11 @@ expression: metrics
         ),
         tags: {
             "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
             "span.op": "db.sql.query",
             "transaction": "gEt /api/:version/users/",
+            "transaction.method": "POST",
             "transaction.op": "myop",
         },
         metadata: BucketMetadata {
@@ -3400,6 +3403,31 @@ expression: metrics
             extracted_from_indexed: false,
         },
     },
+    Bucket {
+        timestamp: UnixTimestamp(1597976302),
+        width: 0,
+        name: MetricName(
+            "d:spans/exclusive_time_light@millisecond",
+        ),
+        value: Distribution(
+            [
+                2000.0,
+            ],
+        ),
+        tags: {
+            "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
+            "span.op": "db.sql.query",
+        },
+        metadata: BucketMetadata {
+            merges: 1,
+            received_at: Some(
+                UnixTimestamp(0),
+            ),
+            extracted_from_indexed: false,
+        },
+    },
     Bucket {
         timestamp: UnixTimestamp(1597976302),
         width: 0,
@@ -3413,8 +3441,11 @@ expression: metrics
         ),
         tags: {
             "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
             "span.op": "db.sql.query",
             "transaction": "gEt /api/:version/users/",
+            "transaction.method": "POST",
             "transaction.op": "myop",
         },
         metadata: BucketMetadata {
@@ -3425,6 +3456,31 @@ expression: metrics
             extracted_from_indexed: false,
         },
     },
+    Bucket {
+        timestamp: UnixTimestamp(1597976302),
+        width: 0,
+        name: MetricName(
+            "d:spans/duration_light@millisecond",
+        ),
+        value: Distribution(
+            [
+                2000.0,
+            ],
+        ),
+        tags: {
+            "environment": "fake_environment",
+            "span.action": "COUNT",
+            "span.category": "db",
+            "span.op": "db.sql.query",
+        },
+        metadata: BucketMetadata {
+            merges: 1,
+            received_at: Some(
+                UnixTimestamp(0),
+            ),
+            extracted_from_indexed: false,
+        },
+    },
     Bucket {
         timestamp: UnixTimestamp(1597976302),
         width: 0,