From 99181d99c7aae6fa3eb8ea99c5a752c209f58620 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Wed, 18 Feb 2026 02:40:17 +0000 Subject: [PATCH] Add kind to crash ping and clarify requirements --- bin_tests/tests/crashtracker_bin_test.rs | 7 +++- .../src/crash_info/builder.rs | 7 +++- libdd-crashtracker/src/collector/emitters.rs | 19 +++++++-- libdd-crashtracker/src/crash_info/builder.rs | 27 ++++++------ .../src/crash_info/telemetry.rs | 41 ++++++++++++++----- libdd-crashtracker/src/receiver/mod.rs | 6 ++- .../src/receiver/receive_report.rs | 13 ++++-- libdd-crashtracker/src/shared/constants.rs | 2 + 8 files changed, 89 insertions(+), 33 deletions(-) diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 60c3e503b5..5e0683a03b 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -1365,11 +1365,16 @@ fn test_receiver_emits_debug_logs_on_receiver_issue() -> anyhow::Result<()> { .context("spawning receiver process")?; { + use libdd_crashtracker::ErrorKind; + let mut stdin = BufWriter::new(child.stdin.take().context("child stdin missing")?); for line in [ "DD_CRASHTRACK_BEGIN_CONFIG".to_string(), serde_json::to_string(&config)?, "DD_CRASHTRACK_END_CONFIG".to_string(), + "DD_CRASHTRACK_BEGIN_KIND".to_string(), + serde_json::to_string(&ErrorKind::UnixSignal)?, + "DD_CRASHTRACK_END_KIND".to_string(), "DD_CRASHTRACK_BEGIN_METADATA".to_string(), serde_json::to_string(&metadata)?, "DD_CRASHTRACK_END_METADATA".to_string(), @@ -1567,7 +1572,7 @@ fn assert_crash_ping_message(body: &str) { assert_eq!(message_json["version"].as_str(), Some("1.0")); - assert_eq!(message_json["kind"].as_str(), Some("Crash ping")); + assert_eq!(message_json["kind"].as_str(), Some("UnixSignal")); } // Old TestFixtures struct kept for UDS socket tests that weren't migrated diff --git a/libdd-crashtracker-ffi/src/crash_info/builder.rs b/libdd-crashtracker-ffi/src/crash_info/builder.rs index 42c0dab1d3..8590d13f4d 100644 --- a/libdd-crashtracker-ffi/src/crash_info/builder.rs +++ b/libdd-crashtracker-ffi/src/crash_info/builder.rs @@ -428,7 +428,12 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread_name( /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. /// All arguments must be valid. -/// This method requires that the builder has a UUID and metadata set +/// This method requires that the builder has `metadata` and `kind` set +/// Applications can add `message` or `sig_info` to the builder to provide additional context. +/// If set, the data will be used to derive the crash ping message in the order of +/// - an explicit message set with `with_message` +/// - sig_info set with `with_sig_info` +/// - kind set with `with_kind` #[no_mangle] #[must_use] #[named] diff --git a/libdd-crashtracker/src/collector/emitters.rs b/libdd-crashtracker/src/collector/emitters.rs index 949cba1b0c..33de2e8ce7 100644 --- a/libdd-crashtracker/src/collector/emitters.rs +++ b/libdd-crashtracker/src/collector/emitters.rs @@ -9,7 +9,9 @@ use crate::runtime_callback::{ CallbackData, }; use crate::shared::constants::*; -use crate::{translate_si_code, CrashtrackerConfiguration, SignalNames, StacktraceCollection}; +use crate::{ + translate_si_code, CrashtrackerConfiguration, ErrorKind, SignalNames, StacktraceCollection, +}; use backtrace::Frame; use libc::{siginfo_t, ucontext_t}; use std::{ @@ -145,13 +147,15 @@ pub(crate) fn emit_crashreport( crashing_tid: libc::pid_t, ) -> Result<(), EmitterError> { // The following order is important in order to emit the crash ping: - // - receiver expects the config + // - receiver expects the config because the endpoint to emit to is there // - then message if any - // - then siginfo (if the message is not set, we use the siginfo to generate the message) + // - then siginfo if any + // - then the kind if any // - then metadata emit_config(pipe, config_str)?; emit_message(pipe, message_ptr)?; emit_siginfo(pipe, sig_info)?; + emit_kind(pipe, &ErrorKind::UnixSignal)?; emit_metadata(pipe, metadata_string)?; // after the metadata the ping should have been sent emit_ucontext(pipe, ucontext)?; @@ -193,6 +197,15 @@ fn emit_config(w: &mut impl Write, config_str: &str) -> Result<(), EmitterError> Ok(()) } +fn emit_kind(w: &mut W, kind: &ErrorKind) -> Result<(), EmitterError> { + writeln!(w, "{DD_CRASHTRACK_BEGIN_KIND}")?; + let _ = serde_json::to_writer(&mut *w, kind); + writeln!(w)?; + writeln!(w, "{DD_CRASHTRACK_END_KIND}")?; + w.flush()?; + Ok(()) +} + fn emit_metadata(w: &mut impl Write, metadata_str: &str) -> Result<(), EmitterError> { writeln!(w, "{DD_CRASHTRACK_BEGIN_METADATA}")?; writeln!(w, "{metadata_str}")?; diff --git a/libdd-crashtracker/src/crash_info/builder.rs b/libdd-crashtracker/src/crash_info/builder.rs index 71c5fa5007..a2ae68b027 100644 --- a/libdd-crashtracker/src/crash_info/builder.rs +++ b/libdd-crashtracker/src/crash_info/builder.rs @@ -401,14 +401,16 @@ impl CrashInfoBuilder { Ok(()) } - /// This method requires that the builder has a UUID and metadata set. - /// Siginfo is optional for platforms that don't support it (like Windows) + /// This method requires that the builder has Metadata and Kind set pub fn build_crash_ping(&self) -> anyhow::Result { + let metadata = self.metadata.clone().context("metadata is required")?; + let kind = self.error.kind.clone().context("kind is required")?; let message = self.error.message.clone(); let sig_info = self.sig_info.clone(); - let metadata = self.metadata.clone().context("metadata is required")?; - let mut builder = CrashPingBuilder::new(self.uuid).with_metadata(metadata); + let mut builder = CrashPingBuilder::new(self.uuid) + .with_metadata(metadata) + .with_kind(kind); if let Some(sig_info) = sig_info { builder = builder.with_sig_info(sig_info); } @@ -419,16 +421,7 @@ impl CrashInfoBuilder { } pub fn is_ping_ready(&self) -> bool { - // 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() - } + self.metadata.is_some() && self.error.kind.is_some() } pub fn has_message(&self) -> bool { @@ -463,6 +456,8 @@ mod tests { #[test] fn test_with_message() { let mut builder = CrashInfoBuilder::new(); + + builder.with_kind(ErrorKind::UnixSignal).unwrap(); let test_message = "Test error message".to_string(); let result = builder.with_message(test_message.clone()); @@ -473,6 +468,7 @@ mod tests { let sig_info = SigInfo::test_instance(42); builder.with_sig_info(sig_info).unwrap(); builder.with_metadata(Metadata::test_instance(1)).unwrap(); + builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = builder.build_crash_ping().unwrap(); assert!(crash_ping.message().contains(&test_message)); @@ -506,6 +502,7 @@ mod tests { let sig_info = SigInfo::test_instance(42); builder.with_sig_info(sig_info).unwrap(); builder.with_metadata(Metadata::test_instance(1)).unwrap(); + builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = builder.build_crash_ping().unwrap(); assert!(crash_ping.message().contains("second message")); @@ -524,6 +521,7 @@ mod tests { builder.with_message(special_message.to_string()).unwrap(); builder.with_sig_info(SigInfo::test_instance(42)).unwrap(); builder.with_metadata(Metadata::test_instance(1)).unwrap(); + builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = builder.build_crash_ping().unwrap(); assert!(crash_ping.message().contains(special_message)); @@ -543,6 +541,7 @@ mod tests { builder.with_sig_info(SigInfo::test_instance(42)).unwrap(); builder.with_metadata(Metadata::test_instance(1)).unwrap(); + builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = builder.build_crash_ping().unwrap(); assert!(crash_ping.message().len() >= 10000); diff --git a/libdd-crashtracker/src/crash_info/telemetry.rs b/libdd-crashtracker/src/crash_info/telemetry.rs index 1b94a448af..8f6def58c6 100644 --- a/libdd-crashtracker/src/crash_info/telemetry.rs +++ b/libdd-crashtracker/src/crash_info/telemetry.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::{fmt::Write, time::SystemTime}; -use crate::SigInfo; +use crate::{ErrorKind, SigInfo}; use super::{CrashInfo, Metadata}; use anyhow::Context; @@ -26,6 +26,7 @@ struct TelemetryMetadata { pub struct CrashPingBuilder { crash_uuid: Uuid, custom_message: Option, + kind: Option, metadata: Option, sig_info: Option, } @@ -38,6 +39,7 @@ impl CrashPingBuilder { Self { crash_uuid, custom_message: None, + kind: None, metadata: None, sig_info: None, } @@ -53,6 +55,11 @@ impl CrashPingBuilder { self } + pub fn with_kind(mut self, kind: ErrorKind) -> Self { + self.kind = Some(kind); + self + } + pub fn with_metadata(mut self, metadata: Metadata) -> Self { self.metadata = Some(metadata); self @@ -62,6 +69,7 @@ impl CrashPingBuilder { let crash_uuid = self.crash_uuid; let sig_info = self.sig_info; let metadata = self.metadata.context("metadata is required")?; + let kind = self.kind.context("kind is required")?; let message = if let Some(custom_message) = self.custom_message { format!("Crashtracker crash ping: crash processing started - {custom_message}") @@ -71,13 +79,13 @@ impl CrashPingBuilder { sig_info.si_code_human_readable, sig_info.si_signo_human_readable ) } else { - "Crashtracker crash ping: crash processing started - Process terminated".to_string() + format!("Crashtracker crash ping: crash processing started - Process terminated due to {:?}", kind) }; Ok(CrashPing { crash_uuid: crash_uuid.to_string(), - kind: "Crash ping".to_string(), message, + kind, metadata, siginfo: sig_info, version: CrashPing::current_schema_version(), @@ -88,11 +96,11 @@ impl CrashPingBuilder { #[derive(Debug, Serialize)] pub struct CrashPing { crash_uuid: String, + kind: ErrorKind, + message: String, #[serde(skip_serializing_if = "Option::is_none")] siginfo: Option, - message: String, version: String, - kind: String, metadata: Metadata, } @@ -446,7 +454,10 @@ fn extract_crash_info_tags(crash_info: &CrashInfo) -> anyhow::Result { #[cfg(test)] mod tests { use super::TelemetryCrashUploader; - use crate::crash_info::{test_utils::TestInstance, CrashInfo, CrashInfoBuilder, Metadata}; + use crate::{ + crash_info::{test_utils::TestInstance, CrashInfo, CrashInfoBuilder, Metadata}, + ErrorKind, + }; use libdd_common::Endpoint; use libdd_telemetry::data::LogLevel; use std::{collections::HashSet, fs}; @@ -580,6 +591,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); t.upload_crash_ping(&crash_ping).await.unwrap(); @@ -607,7 +619,7 @@ mod tests { assert!(Uuid::parse_str(message_json["crash_uuid"].as_str().unwrap()).is_ok()); assert_eq!(message_json["version"], "1.0"); - assert_eq!(message_json["kind"], "Crash ping"); + assert_eq!(message_json["kind"], "UnixSignal"); let metadata_in_message = &message_json["metadata"]; assert!( @@ -655,6 +667,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); t.upload_crash_ping(&crash_ping).await.unwrap(); @@ -707,7 +720,7 @@ mod tests { ); assert_eq!(message_json["version"], "1.0"); - assert_eq!(message_json["kind"], "Crash ping"); + assert_eq!(message_json["kind"], "UnixSignal"); let tags = log_entry["tags"].as_str().unwrap(); let uuid_str = message_json["crash_uuid"].as_str().unwrap(); @@ -738,6 +751,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping()?; let endpoint = Some(Endpoint::from_slice(&format!( @@ -781,7 +795,7 @@ mod tests { assert!(message_json["crash_uuid"].is_string()); assert!(Uuid::parse_str(message_json["crash_uuid"].as_str().unwrap()).is_ok()); assert_eq!(message_json["version"], "1.0"); - assert_eq!(message_json["kind"], "Crash ping"); + assert_eq!(message_json["kind"], "UnixSignal"); Ok(()) } @@ -794,6 +808,7 @@ mod tests { crash_info_builder .with_metadata(Metadata::test_instance(1)) .unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let result = crash_info_builder.build_crash_ping(); assert!(result.is_ok()); let crash_ping = result.unwrap(); @@ -807,6 +822,7 @@ mod tests { crash_info_builder .with_sig_info(crate::SigInfo::test_instance(1)) .unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let result = crash_info_builder.build_crash_ping(); assert!(result.is_err()); assert!(result @@ -822,6 +838,7 @@ mod tests { crash_info_builder .with_metadata(Metadata::test_instance(1)) .unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let result = crash_info_builder.build_crash_ping(); assert!(result.is_ok()); let crash_ping = result.unwrap(); @@ -838,6 +855,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); assert!(!crash_ping.crash_uuid().is_empty()); @@ -857,6 +875,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); assert!(!crash_ping.crash_uuid().is_empty()); @@ -882,6 +901,7 @@ mod tests { crash_info_builder .with_message("my process panicked".to_string()) .unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); assert!(!crash_ping.crash_uuid().is_empty()); @@ -918,6 +938,7 @@ mod tests { let mut crash_info_builder = CrashInfoBuilder::new(); crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder.with_kind(ErrorKind::UnixSignal).unwrap(); let crash_ping = crash_info_builder.build_crash_ping().unwrap(); uploader.upload_crash_ping(&crash_ping).await?; @@ -941,7 +962,7 @@ mod tests { assert!(message_json["crash_uuid"].is_string()); assert!(Uuid::parse_str(message_json["crash_uuid"].as_str().unwrap()).is_ok()); assert_eq!(message_json["version"], "1.0"); - assert_eq!(message_json["kind"], "Crash ping"); + assert_eq!(message_json["kind"], "UnixSignal"); let uploaded_siginfo = &message_json["siginfo"]; assert_eq!(uploaded_siginfo["si_signo"], sig_info.si_signo); diff --git a/libdd-crashtracker/src/receiver/mod.rs b/libdd-crashtracker/src/receiver/mod.rs index 47cc958142..05cfeb773a 100644 --- a/libdd-crashtracker/src/receiver/mod.rs +++ b/libdd-crashtracker/src/receiver/mod.rs @@ -18,7 +18,7 @@ mod tests { use crate::collector::default_signals; use crate::crash_info::{SiCodes, SigInfo, SignalNames}; use crate::shared::constants::*; - use crate::{CrashtrackerConfiguration, StacktraceCollection}; + use crate::{CrashtrackerConfiguration, ErrorKind, StacktraceCollection}; use std::time::Duration; use tokio::io::{AsyncWriteExt, BufReader}; use tokio::net::UnixStream; @@ -49,6 +49,10 @@ mod tests { .await?; to_socket(sender, DD_CRASHTRACK_END_SIGINFO).await?; + to_socket(sender, DD_CRASHTRACK_BEGIN_KIND).await?; + to_socket(sender, serde_json::to_string(&ErrorKind::UnixSignal)?).await?; + to_socket(sender, DD_CRASHTRACK_END_KIND).await?; + to_socket(sender, DD_CRASHTRACK_BEGIN_CONFIG).await?; to_socket( sender, diff --git a/libdd-crashtracker/src/receiver/receive_report.rs b/libdd-crashtracker/src/receiver/receive_report.rs index 0b8d5f5840..9dc6ea6544 100644 --- a/libdd-crashtracker/src/receiver/receive_report.rs +++ b/libdd-crashtracker/src/receiver/receive_report.rs @@ -108,6 +108,7 @@ pub(crate) enum StdinState { Counters, Done, File(String, Vec), + Kind, Metadata, ProcInfo, SigInfo, @@ -186,6 +187,13 @@ fn process_line( StdinState::File(name, contents) } + StdinState::Kind if line.starts_with(DD_CRASHTRACK_END_KIND) => StdinState::Waiting, + StdinState::Kind => { + let kind: ErrorKind = serde_json::from_str(line)?; + builder.with_kind(kind)?; + StdinState::Kind + } + StdinState::Metadata if line.starts_with(DD_CRASHTRACK_END_METADATA) => StdinState::Waiting, StdinState::Metadata => { let metadata = serde_json::from_str(line)?; @@ -311,6 +319,7 @@ fn process_line( let (_, filename) = line.split_once(' ').unwrap_or(("", "MISSING_FILENAME")); StdinState::File(filename.to_string(), vec![]) } + StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_KIND) => StdinState::Kind, StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_METADATA) => { StdinState::Metadata } @@ -392,7 +401,7 @@ pub(crate) async fn receive_report_from_stream( } } - // We need to wait until at least we receive config, metadata, and siginfo (on non-Windows + // We need to wait until at least we receive config, metadata, and kind (on non-Windows // platforms) before sending the crash ping if !crash_ping_sent && builder.is_ping_ready() { if let Some(ref config_ref) = config { @@ -485,8 +494,6 @@ pub(crate) async fn receive_report_from_stream( return Ok(None); } - // For now, we only support Signal based crash detection in the receiver. - builder.with_kind(ErrorKind::UnixSignal)?; enrich_thread_name(&mut builder)?; builder.with_os_info_this_machine()?; diff --git a/libdd-crashtracker/src/shared/constants.rs b/libdd-crashtracker/src/shared/constants.rs index 910bd5a018..207bf533b9 100644 --- a/libdd-crashtracker/src/shared/constants.rs +++ b/libdd-crashtracker/src/shared/constants.rs @@ -7,6 +7,7 @@ pub const DD_CRASHTRACK_BEGIN_ADDITIONAL_TAGS: &str = "DD_CRASHTRACK_BEGIN_ADDIT pub const DD_CRASHTRACK_BEGIN_CONFIG: &str = "DD_CRASHTRACK_BEGIN_CONFIG"; pub const DD_CRASHTRACK_BEGIN_COUNTERS: &str = "DD_CRASHTRACK_BEGIN_COUNTERS"; pub const DD_CRASHTRACK_BEGIN_FILE: &str = "DD_CRASHTRACK_BEGIN_FILE"; +pub const DD_CRASHTRACK_BEGIN_KIND: &str = "DD_CRASHTRACK_BEGIN_KIND"; pub const DD_CRASHTRACK_BEGIN_METADATA: &str = "DD_CRASHTRACK_BEGIN_METADATA"; pub const DD_CRASHTRACK_BEGIN_PROCINFO: &str = "DD_CRASHTRACK_BEGIN_PROCESSINFO"; pub const DD_CRASHTRACK_BEGIN_THREAD_NAME: &str = "DD_CRASHTRACK_BEGIN_THREAD_NAME"; @@ -24,6 +25,7 @@ pub const DD_CRASHTRACK_END_ADDITIONAL_TAGS: &str = "DD_CRASHTRACK_END_ADDITIONA pub const DD_CRASHTRACK_END_CONFIG: &str = "DD_CRASHTRACK_END_CONFIG"; pub const DD_CRASHTRACK_END_COUNTERS: &str = "DD_CRASHTRACK_END_COUNTERS"; pub const DD_CRASHTRACK_END_FILE: &str = "DD_CRASHTRACK_END_FILE"; +pub const DD_CRASHTRACK_END_KIND: &str = "DD_CRASHTRACK_END_KIND"; pub const DD_CRASHTRACK_END_METADATA: &str = "DD_CRASHTRACK_END_METADATA"; pub const DD_CRASHTRACK_END_PROCINFO: &str = "DD_CRASHTRACK_END_PROCESSINFO"; pub const DD_CRASHTRACK_END_THREAD_NAME: &str = "DD_CRASHTRACK_END_THREAD_NAME";