From 75337b1e4e25f96679dc0c186cbd22b60f218200 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Mon, 10 Nov 2025 10:40:45 +0000 Subject: [PATCH] Make siginfo optional and update tests/examples --- .../src/crash_info/builder.rs | 27 ++++++--- .../src/crash_info/telemetry.rs | 60 ++++++++++++------- .../src/receiver/receive_report.rs | 4 +- examples/ffi/crashinfo.cpp | 12 +++- 4 files changed, 68 insertions(+), 35 deletions(-) diff --git a/datadog-crashtracker/src/crash_info/builder.rs b/datadog-crashtracker/src/crash_info/builder.rs index 0404859dd0..43c8bd0cc3 100644 --- a/datadog-crashtracker/src/crash_info/builder.rs +++ b/datadog-crashtracker/src/crash_info/builder.rs @@ -395,19 +395,30 @@ impl CrashInfoBuilder { Ok(self) } - /// This method requires that the builder has a UUID, siginfo, and metadata set + /// This method requires that the builder has a UUID and metadata set. + /// Siginfo is optional for platforms that don't support it (like Windows) pub fn build_crash_ping(&self) -> anyhow::Result { - let sig_info = self.sig_info.clone().context("sig_info is required")?; + let sig_info = self.sig_info.clone(); let metadata = self.metadata.clone().context("metadata is required")?; - CrashPingBuilder::new(self.uuid) - .with_sig_info(sig_info) - .with_metadata(metadata) - .build() + let mut builder = CrashPingBuilder::new(self.uuid).with_metadata(metadata); + if let Some(sig_info) = sig_info { + builder = builder.with_sig_info(sig_info); + } + builder.build() } pub fn is_ping_ready(&self) -> bool { - self.sig_info.is_some() && self.metadata.is_some() + // On Unix platforms, wait for both metadata and siginfo + // On Windows, siginfo is not available, so only wait for metadata + #[cfg(unix)] + { + self.metadata.is_some() && self.sig_info.is_some() + } + #[cfg(windows)] + { + self.metadata.is_some() + } } } @@ -430,7 +441,7 @@ mod tests { assert!(!crash_ping.crash_uuid().is_empty()); assert!(Uuid::parse_str(crash_ping.crash_uuid()).is_ok()); - assert_eq!(crash_ping.siginfo(), &sig_info); + assert_eq!(crash_ping.siginfo(), Some(&sig_info)); assert_eq!(crash_ping.metadata(), &metadata); assert!(crash_ping.message().contains("crash processing started")); } diff --git a/datadog-crashtracker/src/crash_info/telemetry.rs b/datadog-crashtracker/src/crash_info/telemetry.rs index 1398aadfb8..f794aa1043 100644 --- a/datadog-crashtracker/src/crash_info/telemetry.rs +++ b/datadog-crashtracker/src/crash_info/telemetry.rs @@ -60,14 +60,18 @@ impl CrashPingBuilder { pub fn build(self) -> anyhow::Result { let crash_uuid = self.crash_uuid; - let sig_info = self.sig_info.context("sig_info is required")?; + let sig_info = self.sig_info; let metadata = self.metadata.context("metadata is required")?; let message = self.custom_message.unwrap_or_else(|| { - format!( - "Crashtracker crash ping: crash processing started - Process terminated with {:?} ({:?})", - sig_info.si_code_human_readable, sig_info.si_signo_human_readable - ) + if let Some(ref sig_info) = sig_info { + format!( + "Crashtracker crash ping: crash processing started - Process terminated with {:?} ({:?})", + sig_info.si_code_human_readable, sig_info.si_signo_human_readable + ) + } else { + "Crashtracker crash ping: crash processing started - Process terminated".to_string() + } }); Ok(CrashPing { @@ -84,7 +88,8 @@ impl CrashPingBuilder { #[derive(Debug, Serialize)] pub struct CrashPing { crash_uuid: String, - siginfo: SigInfo, + #[serde(skip_serializing_if = "Option::is_none")] + siginfo: Option, message: String, version: String, kind: String, @@ -104,8 +109,8 @@ impl CrashPing { &self.metadata } - pub fn siginfo(&self) -> &SigInfo { - &self.siginfo + pub fn siginfo(&self) -> Option<&SigInfo> { + self.siginfo.as_ref() } fn current_schema_version() -> String { @@ -237,20 +242,26 @@ impl TelemetryCrashUploader { .await } - fn build_crash_ping_tags(&self, crash_uuid: &str, sig_info: &SigInfo) -> String { + fn build_crash_ping_tags(&self, crash_uuid: &str, sig_info: Option<&SigInfo>) -> String { let metadata = &self.metadata; let mut tags = format!( - "uuid:{},is_crash_ping:true,service:{},language_name:{},language_version:{},tracer_version:{},si_code_human_readable:{:?},si_signo:{},si_signo_human_readable:{:?}", + "uuid:{},is_crash_ping:true,service:{},language_name:{},language_version:{},tracer_version:{}", crash_uuid, metadata.application.service_name, metadata.application.language_name, metadata.application.language_version, - metadata.application.tracer_version, - sig_info.si_code_human_readable, - sig_info.si_signo, - sig_info.si_signo_human_readable + metadata.application.tracer_version ); + if let Some(sig_info) = sig_info { + tags.push_str(&format!( + ",si_code_human_readable:{:?},si_signo:{},si_signo_human_readable:{:?}", + sig_info.si_code_human_readable, + sig_info.si_signo, + sig_info.si_signo_human_readable + )); + } + self.append_optional_tags(&mut tags); tags } @@ -533,7 +544,7 @@ mod tests { // Structured message format let message_json: serde_json::Value = serde_json::from_str(log_entry["message"].as_str().unwrap())?; - assert_eq!(message_json["siginfo"], serde_json::to_value(sig_info)?); + assert_eq!(message_json["siginfo"], serde_json::to_value(&sig_info)?); assert!(message_json["crash_uuid"].is_string()); assert!(Uuid::parse_str(message_json["crash_uuid"].as_str().unwrap()).is_ok()); @@ -720,17 +731,20 @@ mod tests { #[test] #[cfg_attr(miri, ignore)] fn test_crash_ping_builder_validation() { + // Test that crash ping can be built with only metadata (no sig_info) let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder .with_metadata(Metadata::test_instance(1)) .unwrap(); let result = crash_info_builder.build_crash_ping(); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("sig_info is required")); + assert!(result.is_ok()); + let crash_ping = result.unwrap(); + assert!(crash_ping.siginfo().is_none()); + assert!(crash_ping + .message() + .contains("Crashtracker crash ping: crash processing started - Process terminated")); + // Test that crash ping fails without metadata let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder .with_sig_info(crate::SigInfo::test_instance(1)) @@ -742,7 +756,7 @@ mod tests { .to_string() .contains("metadata is required")); - // Test successful build with both required fields + // Test successful build with both fields let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder .with_sig_info(crate::SigInfo::test_instance(1)) @@ -752,6 +766,8 @@ mod tests { .unwrap(); let result = crash_info_builder.build_crash_ping(); assert!(result.is_ok()); + let crash_ping = result.unwrap(); + assert!(crash_ping.siginfo().is_some()); } #[test] @@ -770,7 +786,7 @@ mod tests { assert!(Uuid::parse_str(crash_ping.crash_uuid()).is_ok()); assert!(crash_ping.message().contains("crash processing started")); assert_eq!(crash_ping.metadata(), &metadata); - assert_eq!(crash_ping.siginfo(), &sig_info); + assert_eq!(crash_ping.siginfo(), Some(&sig_info)); } #[tokio::test] diff --git a/datadog-crashtracker/src/receiver/receive_report.rs b/datadog-crashtracker/src/receiver/receive_report.rs index 8ad5cd82a8..f16a24e176 100644 --- a/datadog-crashtracker/src/receiver/receive_report.rs +++ b/datadog-crashtracker/src/receiver/receive_report.rs @@ -301,8 +301,8 @@ pub(crate) async fn receive_report_from_stream( //TODO: This assumes that the input is valid UTF-8. loop { - // We need to wait until at least we receive config, metadata, and siginfo before sending - // the crash ping + // We need to wait until at least we receive config, metadata, and siginfo (on non-Windows + // platforms) before sending the crash ping if !crash_ping_sent && builder.is_ping_ready() { if let Some(ref config_ref) = config { let config_clone = config_ref.clone(); diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index c31456756e..d3ecf2e7a2 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -241,6 +241,12 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_os_info_this_machine(builder.get()), "Failed to set os_info"); + // Test uploading a crash ping without siginfo + auto ping_endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/crash_ping_test")); + check_result(ddog_crasht_CrashInfoBuilder_upload_ping_to_endpoint(builder.get(), ping_endpoint), + "Failed to upload crash ping"); + ddog_endpoint_drop(ping_endpoint); + auto sigInfo = ddog_crasht_SigInfo { .addr = "0xBABEF00D", .code = 16, @@ -252,10 +258,10 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_sig_info(builder.get(), sigInfo), "failed to add signal info"); - auto ping_endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/crash_ping_test")); - check_result(ddog_crasht_CrashInfoBuilder_upload_ping_to_endpoint(builder.get(), ping_endpoint), + auto ping_endpoint2 = ddog_endpoint_from_filename(to_slice_c_char("/tmp/crash_ping_test")); + check_result(ddog_crasht_CrashInfoBuilder_upload_ping_to_endpoint(builder.get(), ping_endpoint2), "Failed to upload crash ping"); - ddog_endpoint_drop(ping_endpoint); + ddog_endpoint_drop(ping_endpoint2); auto crashinfo = extract_result(ddog_crasht_CrashInfoBuilder_build(builder.release()), "failed to build CrashInfo");