Use StackFrame in internal code; convert FFI obj to StackFrame for runtime stack

gyuheon0h · gyuheon0h · commit d86a9c37e896 · 2025-10-28T14:26:14.000Z
diff --git a/datadog-crashtracker-ffi/src/runtime_callback.rs b/datadog-crashtracker-ffi/src/runtime_callback.rs
@@ -152,33 +152,7 @@ mod tests {
     }
 
     #[test]
-    fn test_ffi_callback_registration() {
-        let _guard = TEST_MUTEX.lock().unwrap();
-        unsafe {
-            clear_runtime_callback();
-            assert!(!ddog_crasht_is_runtime_callback_registered());
-
-            let result = ddog_crasht_register_runtime_stack_callback(
-                test_runtime_callback,
-                CallbackType::Frame,
-            );
-
-            assert_eq!(result, CallbackResult::Ok);
-            assert!(ddog_crasht_is_runtime_callback_registered());
-
-            let result = ddog_crasht_register_runtime_stack_callback(
-                test_runtime_callback,
-                CallbackType::Frame,
-            );
-            assert_eq!(result, CallbackResult::Ok);
-            assert!(ddog_crasht_is_runtime_callback_registered());
-
-            clear_runtime_callback();
-        }
-    }
-
-    #[test]
-    fn test_enum_based_registration() {
+    fn test_callback_registration() {
         let _guard = TEST_MUTEX.lock().unwrap();
         unsafe {
             clear_runtime_callback();
diff --git a/datadog-crashtracker/src/receiver/receive_report.rs b/datadog-crashtracker/src/receiver/receive_report.rs
@@ -2,8 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{
-    crash_info::{CrashInfo, CrashInfoBuilder, ErrorKind, Metadata, SigInfo, Span},
-    runtime_callback::{RuntimeFrame, RuntimeStack},
+    crash_info::{CrashInfo, CrashInfoBuilder, ErrorKind, Metadata, SigInfo, Span, StackFrame},
+    runtime_callback::RuntimeStack,
     shared::constants::*,
     CrashtrackerConfiguration,
 };
@@ -62,7 +62,7 @@ pub(crate) enum StdinState {
     Waiting,
     // StackFrame is always emitted as one stream of all the frames but StackString
     // may have lines that we need to accumulate depending on runtime (e.g. Python)
-    RuntimeStackFrame(Vec<RuntimeFrame>),
+    RuntimeStackFrame(Vec<StackFrame>),
     RuntimeStackString(Vec<String>),
 }
 
diff --git a/datadog-crashtracker/src/runtime_callback.rs b/datadog-crashtracker/src/runtime_callback.rs
@@ -6,7 +6,7 @@
 //! This module provides APIs for runtime languages (Ruby, Python, PHP, etc.) to register
 //! callbacks that can provide runtime-specific stack traces during crash handling.
 
-/// Runtime-specific stack frame representation suitable for signal-safe context
+use crate::crash_info::StackFrame;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::{
@@ -19,7 +19,6 @@ use thiserror::Error;
 static FRAME_CSTR: &std::ffi::CStr = c"frame";
 static STACKTRACE_STRING_CSTR: &std::ffi::CStr = c"stacktrace_string";
 
-/// Callback type identifier for different collection strategies
 #[repr(C)]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum CallbackType {
@@ -72,6 +71,50 @@ pub struct RuntimeStackFrame {
     pub column_number: u32,
 }
 
+impl From<&RuntimeStackFrame> for StackFrame {
+    /// Convert RuntimeStackFrame (C FFI) to StackFrame (internal representation)
+    ///
+    /// # Safety
+    /// The caller must ensure that all C string pointers are either null or point to
+    /// valid, null-terminated C strings that remain valid for the duration of this call.
+    fn from(rsf: &RuntimeStackFrame) -> Self {
+        let mut stack_frame = StackFrame::new();
+
+        // Convert C strings to Rust strings and populate StackFrame
+        if !rsf.function_name.is_null() {
+            // Safety: function_name was checked to be non-null. The caller
+            // guarantees it points to a valid, null-terminated C string.
+            let c_str = unsafe { std::ffi::CStr::from_ptr(rsf.function_name) };
+            if let Ok(s) = c_str.to_str() {
+                if !s.is_empty() {
+                    stack_frame.function = Some(s.to_string());
+                }
+            }
+        }
+
+        if !rsf.file_name.is_null() {
+            // Safety: file_name was checked to be non-null. The caller
+            // guarantees it points to a valid, null-terminated C string.
+            let c_str = unsafe { std::ffi::CStr::from_ptr(rsf.file_name) };
+            if let Ok(s) = c_str.to_str() {
+                if !s.is_empty() {
+                    stack_frame.file = Some(s.to_string());
+                }
+            }
+        }
+
+        if rsf.line_number != 0 {
+            stack_frame.line = Some(rsf.line_number);
+        }
+
+        if rsf.column_number != 0 {
+            stack_frame.column = Some(rsf.column_number);
+        }
+
+        stack_frame
+    }
+}
+
 /// Function signature for runtime stack collection callbacks
 ///
 /// This callback is invoked during crash handling in a signal context, so it must be best effort
@@ -103,30 +146,12 @@ pub struct RuntimeStack {
     /// Array of runtime-specific stack frames (optional, mutually exclusive with
     /// stacktrace_string)
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub frames: Vec<RuntimeFrame>,
+    pub frames: Vec<StackFrame>,
     /// Raw stacktrace string (optional, mutually exclusive with frames)
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub stacktrace_string: Option<String>,
 }
 
-/// JSON-serializable runtime stack frame
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
-pub struct RuntimeFrame {
-    /// Fully qualified function/method name
-    /// Examples: "my_package.submodule.TestClass.method", "MyClass::method", "namespace::function"
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub function: Option<String>,
-    /// Source file name
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub file: Option<String>,
-    /// Line number in source file
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub line: Option<u32>,
-    /// Column number in source file
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub column: Option<u32>,
-}
-
 /// Errors that can occur during callback registration
 #[derive(Debug, Error)]
 pub enum CallbackError {
@@ -346,53 +371,12 @@ unsafe fn emit_frame_as_json(
     // Safety: frame was checked to be non-null above. The caller guarantees it points
     // to a valid RuntimeStackFrame.
     let frame_ref = &*frame;
-    write!(writer, "{{")?;
-    let mut first = true;
-
-    // Convert C strings to Rust strings and write JSON fields
-    if !frame_ref.function_name.is_null() {
-        // Safety: frame_ref.function_name was checked to be non-null. The caller
-        // guarantees it points to a valid, null-terminated C string.
-        let c_str = std::ffi::CStr::from_ptr(frame_ref.function_name);
-        if let Ok(s) = c_str.to_str() {
-            if !s.is_empty() {
-                write!(writer, "\"function\": \"{}\"", s)?;
-                first = false;
-            }
-        }
-    }
-
-    if !frame_ref.file_name.is_null() {
-        // Safety: frame_ref.file_name was checked to be non-null. The caller
-        // guarantees it points to a valid, null-terminated C string.
-        let c_str = std::ffi::CStr::from_ptr(frame_ref.file_name);
-        if let Ok(s) = c_str.to_str() {
-            if !s.is_empty() {
-                if !first {
-                    write!(writer, ", ")?;
-                }
-                write!(writer, "\"file\": \"{}\"", s)?;
-                first = false;
-            }
-        }
-    }
 
-    if frame_ref.line_number != 0 {
-        if !first {
-            write!(writer, ", ")?;
-        }
-        write!(writer, "\"line\": {}", frame_ref.line_number)?;
-        first = false;
-    }
-
-    if frame_ref.column_number != 0 {
-        if !first {
-            write!(writer, ", ")?;
-        }
-        write!(writer, "\"column\": {}", frame_ref.column_number)?;
-    }
+    let stack_frame: StackFrame = frame_ref.into();
 
-    writeln!(writer, "}}")?;
+    // Serialize to JSON and write
+    let json = serde_json::to_string(&stack_frame).map_err(std::io::Error::other)?;
+    writeln!(writer, "{}", json)?;
     Ok(())
 }
 
@@ -494,9 +478,12 @@ mod tests {
         );
         assert!(json_output.contains("\"file\""), "Missing file field");
         assert!(json_output.contains("test.rb"), "Missing file name");
-        assert!(json_output.contains("\"line\": 42"), "Missing line number");
         assert!(
-            json_output.contains("\"column\": 10"),
+            json_output.contains("\"line\":42") || json_output.contains("\"line\": 42"),
+            "Missing line number"
+        );
+        assert!(
+            json_output.contains("\"column\":10") || json_output.contains("\"column\": 10"),
             "Missing column number"
         );
 
@@ -586,9 +573,12 @@ mod tests {
         );
         assert!(json_output.contains("\"file\""), "Missing file field");
         assert!(json_output.contains("test.rb"), "Missing file name");
-        assert!(json_output.contains("\"line\": 42"), "Missing line number");
         assert!(
-            json_output.contains("\"column\": 10"),
+            json_output.contains("\"line\":42") || json_output.contains("\"line\": 42"),
+            "Missing line number"
+        );
+        assert!(
+            json_output.contains("\"column\":10") || json_output.contains("\"column\": 10"),
             "Missing column number"
         );
 
diff --git a/docs/RFCs/artifacts/0011-crashtracker-unified-runtime-stack-schema.json b/docs/RFCs/artifacts/0011-crashtracker-unified-runtime-stack-schema.json
@@ -254,44 +254,6 @@
         }
       }
     },
-    "RuntimeFrame": {
-      "description": "JSON-serializable runtime stack frame",
-      "type": "object",
-      "properties": {
-        "column": {
-          "description": "Column number in source file",
-          "type": [
-            "integer",
-            "null"
-          ],
-          "format": "uint32",
-          "minimum": 0.0
-        },
-        "file": {
-          "description": "Source file name",
-          "type": [
-            "string",
-            "null"
-          ]
-        },
-        "function": {
-          "description": "Fully qualified function/method name Examples: \"my_package.submodule.TestClass.method\", \"MyClass::method\", \"namespace::function\"",
-          "type": [
-            "string",
-            "null"
-          ]
-        },
-        "line": {
-          "description": "Line number in source file",
-          "type": [
-            "integer",
-            "null"
-          ],
-          "format": "uint32",
-          "minimum": 0.0
-        }
-      }
-    },
     "RuntimeStack": {
       "description": "Runtime stack representation for JSON serialization",
       "type": "object",
@@ -307,7 +269,7 @@
           "description": "Array of runtime-specific stack frames (optional, mutually exclusive with stacktrace_string)",
           "type": "array",
           "items": {
-            "$ref": "#/definitions/RuntimeFrame"
+            "$ref": "#/definitions/StackFrame"
           }
         },
         "stacktrace_string": {
@@ -599,4 +561,4 @@
       }
     }
   }
-}
+}
