feat(err): add python frame support, tag frames (#26386)

Co-authored-by: David Newell <[email protected]>

rust/cymbal/src/frames/mod.rs

@@ -5,7 +5,9 @@ use serde_json::Value;
 use sha2::{Digest, Sha512};
 
 use crate::{
-    error::UnhandledError, langs::js::RawJSFrame, metric_consts::PER_FRAME_TIME,
+    error::UnhandledError,
+    langs::{js::RawJSFrame, python::RawPythonFrame},
+    metric_consts::PER_FRAME_TIME,
     symbol_store::Catalog,
 };
 
@@ -15,17 +17,27 @@ pub mod resolver;
 // We consume a huge variety of differently shaped stack frames, which we have special-case
 // transformation for, to produce a single, unified representation of a frame.
 #[derive(Debug, Deserialize, Serialize, Clone)]
-#[serde(untagged)]
+#[serde(tag = "platform")]
 pub enum RawFrame {
+    #[serde(rename = "python")]
+    Python(RawPythonFrame),
+    #[serde(rename = "web:javascript")]
     JavaScript(RawJSFrame),
+    // TODO - remove once we're happy no clients are using this anymore
+    #[serde(rename = "javascript")]
+    LegacyJS(RawJSFrame),
 }
 
 impl RawFrame {
     pub async fn resolve(&self, team_id: i32, catalog: &Catalog) -> Result<Frame, UnhandledError> {
-        let RawFrame::JavaScript(raw) = self;
-
         let frame_resolve_time = common_metrics::timing_guard(PER_FRAME_TIME, &[]);
-        let res = raw.resolve(team_id, catalog).await;
+        let (res, lang_tag) = match self {
+            RawFrame::JavaScript(frame) | RawFrame::LegacyJS(frame) => {
+                (frame.resolve(team_id, catalog).await, "javascript")
+            }
+
+            RawFrame::Python(frame) => (Ok(frame.into()), "python"),
+        };
 
         // The raw id of the frame is set after it's resolved
         let res = res.map(|mut f| {
@@ -38,19 +50,26 @@ impl RawFrame {
         } else {
             frame_resolve_time.label("outcome", "success")
         }
+        .label("lang", lang_tag)
         .fin();
 
         res
     }
 
     pub fn symbol_set_ref(&self) -> Option<String> {
-        let RawFrame::JavaScript(raw) = self;
-        raw.source_url().map(String::from).ok()
+        match self {
+            RawFrame::JavaScript(frame) | RawFrame::LegacyJS(frame) => {
+                frame.source_url().map(String::from).ok()
+            }
+            RawFrame::Python(_) => None, // Python frames don't have symbol sets
+        }
     }
 
     pub fn frame_id(&self) -> String {
-        let RawFrame::JavaScript(raw) = self;
-        raw.frame_id()
+        match self {
+            RawFrame::JavaScript(raw) | RawFrame::LegacyJS(raw) => raw.frame_id(),
+            RawFrame::Python(raw) => raw.frame_id(),
+        }
     }
 }
 
@@ -89,8 +108,8 @@ pub struct Context {
 
 #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)]
 pub struct ContextLine {
-    number: u32,
-    line: String,
+    pub number: u32,
+    pub line: String,
 }
 
 impl Frame {

rust/cymbal/src/frames/resolver.rs

@@ -143,12 +143,16 @@ mod test {
         // We're going to pretend out stack consists exclusively of JS frames whose source
         // we have locally
         test_stack.retain(|s| {
-            let RawFrame::JavaScript(s) = s;
+            let RawFrame::JavaScript(s) = s else {
+                return false;
+            };
             s.source_url.as_ref().unwrap().contains(CHUNK_PATH)
         });
 
         for frame in test_stack.iter_mut() {
-            let RawFrame::JavaScript(frame) = frame;
+            let RawFrame::JavaScript(frame) = frame else {
+                panic!("Expected a JavaScript frame")
+            };
             // Our test data contains our /actual/ source urls - we need to swap that to localhost
             // When I first wrote this test, I forgot to do this, and it took me a while to figure out
             // why the test was passing before I'd even set up the mockserver - which was pretty cool, tbh

rust/cymbal/src/langs/js.rs

@@ -15,7 +15,6 @@ use crate::{
 
 // A minifed JS stack frame. Just the minimal information needed to lookup some
 // sourcemap for it and produce a "real" stack frame.
-// TODO - how do we know if this frame is minified? If it isn't, we can skip a lot of work, but I think we have to guess? Based on whether we can get a sourcemap for it?
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct RawJSFrame {
     #[serde(flatten)]

rust/cymbal/src/langs/mod.rs

@@ -1 +1,2 @@
 pub mod js;
+pub mod python;

rust/cymbal/src/langs/python.rs

@@ -0,0 +1,94 @@
+use serde::{Deserialize, Serialize};
+use sha2::{Digest, Sha512};
+
+use crate::frames::{Context, ContextLine, Frame};
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct RawPythonFrame {
+    #[serde(rename = "abs_path")]
+    pub path: Option<String>, // Absolute path to the file - unused for now
+    pub context_line: Option<String>, // The line of code the exception came from
+    pub filename: String,             // The relative path of the file the context line is in
+    pub function: String,             // The name of the function the exception came from
+    pub lineno: Option<u32>,          // The line number of the context line
+    pub module: Option<String>,       // The python-import style module name the function is in
+    #[serde(default)]
+    pub pre_context: Vec<String>, // The lines of code before the context line
+    #[serde(default)]
+    pub post_context: Vec<String>, // The lines of code after the context line
+    // Default to false as sometimes not present on library code
+    #[serde(default)]
+    pub in_app: bool, // Whether the frame is in the user's code
+}
+
+impl RawPythonFrame {
+    pub fn frame_id(&self) -> String {
+        // We don't have version info for python frames, so we rely on
+        // the module, function, line number and surrounding context to
+        // uniquely identify a frame, with the intuition being that even
+        // if two frames are from two different library versions, if the
+        // files they're in are sufficiently similar we can consider
+        // them to be the same frame
+        let mut hasher = Sha512::new();
+        self.context_line
+            .as_ref()
+            .inspect(|c| hasher.update(c.as_bytes()));
+        hasher.update(self.filename.as_bytes());
+        hasher.update(self.function.as_bytes());
+        hasher.update(self.lineno.unwrap_or_default().to_be_bytes());
+        self.module
+            .as_ref()
+            .inspect(|m| hasher.update(m.as_bytes()));
+        self.pre_context
+            .iter()
+            .chain(self.post_context.iter())
+            .for_each(|line| {
+                hasher.update(line.as_bytes());
+            });
+        format!("{:x}", hasher.finalize())
+    }
+
+    pub fn get_context(&self) -> Option<Context> {
+        let context_line = self.context_line.as_ref()?;
+        let lineno = self.lineno?;
+
+        let line = ContextLine::new(lineno, context_line);
+
+        let before = self
+            .pre_context
+            .iter()
+            .enumerate()
+            .map(|(i, line)| ContextLine::new(lineno - i as u32 - 1, line.clone()))
+            .collect();
+        let after = self
+            .post_context
+            .iter()
+            .enumerate()
+            .map(|(i, line)| ContextLine::new(lineno + i as u32 + 1, line.clone()))
+            .collect();
+        Some(Context {
+            before,
+            line,
+            after,
+        })
+    }
+}
+
+impl From<&RawPythonFrame> for Frame {
+    fn from(raw: &RawPythonFrame) -> Self {
+        Frame {
+            raw_id: String::new(),
+            mangled_name: raw.function.clone(),
+            line: raw.lineno,
+            column: None,
+            source: Some(raw.filename.clone()),
+            in_app: raw.in_app,
+            resolved_name: Some(raw.function.clone()),
+            lang: "python".to_string(),
+            resolved: true,
+            resolve_failure: None,
+            junk_drawer: None,
+            context: raw.get_context(),
+        }
+    }
+}

rust/cymbal/src/types/mod.rs

@@ -197,7 +197,9 @@ mod test {
             panic!("Expected a Raw stacktrace")
         };
         assert_eq!(frames.len(), 2);
-        let RawFrame::JavaScript(frame) = &frames[0];
+        let RawFrame::JavaScript(frame) = &frames[0] else {
+            panic!("Expected a JavaScript frame")
+        };
 
         assert_eq!(
             frame.source_url,
@@ -208,7 +210,9 @@ mod test {
         assert_eq!(frame.location.as_ref().unwrap().line, 64);
         assert_eq!(frame.location.as_ref().unwrap().column, 25112);
 
-        let RawFrame::JavaScript(frame) = &frames[1];
+        let RawFrame::JavaScript(frame) = &frames[1] else {
+            panic!("Expected a JavaScript frame")
+        };
         assert_eq!(
             frame.source_url,
             Some("https://app-static.eu.posthog.com/static/chunk-PGUQKT6S.js".to_string())

rust/cymbal/tests/resolve.rs

@@ -46,12 +46,16 @@ async fn end_to_end_resolver_test() {
     // We're going to pretend out stack consists exclusively of JS frames whose source
     // we have locally
     test_stack.retain(|s| {
-        let RawFrame::JavaScript(s) = s;
+        let RawFrame::JavaScript(s) = s else {
+            panic!("Expected a JavaScript frame")
+        };
         s.source_url.as_ref().unwrap().contains(CHUNK_PATH)
     });
 
     for frame in test_stack.iter_mut() {
-        let RawFrame::JavaScript(frame) = frame;
+        let RawFrame::JavaScript(frame) = frame else {
+            panic!("Expected a JavaScript frame")
+        };
         // Our test data contains our /actual/ source urls - we need to swap that to localhost
         // When I first wrote this test, I forgot to do this, and it took me a while to figure out
         // why the test was passing before I'd even set up the mockserver - which was pretty cool, tbh