Skip to content

Commit 9d890c4

Browse files
committed
Add kind to crash ping and clarify requirements
1 parent 367c8b2 commit 9d890c4

File tree

8 files changed

+65
-27
lines changed

8 files changed

+65
-27
lines changed

bin_tests/tests/crashtracker_bin_test.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1365,11 +1365,16 @@ fn test_receiver_emits_debug_logs_on_receiver_issue() -> anyhow::Result<()> {
13651365
.context("spawning receiver process")?;
13661366

13671367
{
1368+
use libdd_crashtracker::ErrorKind;
1369+
13681370
let mut stdin = BufWriter::new(child.stdin.take().context("child stdin missing")?);
13691371
for line in [
13701372
"DD_CRASHTRACK_BEGIN_CONFIG".to_string(),
13711373
serde_json::to_string(&config)?,
13721374
"DD_CRASHTRACK_END_CONFIG".to_string(),
1375+
"DD_CRASHTRACK_BEGIN_KIND".to_string(),
1376+
serde_json::to_string(&ErrorKind::UnixSignal)?,
1377+
"DD_CRASHTRACK_END_KIND".to_string(),
13731378
"DD_CRASHTRACK_BEGIN_METADATA".to_string(),
13741379
serde_json::to_string(&metadata)?,
13751380
"DD_CRASHTRACK_END_METADATA".to_string(),
@@ -1567,7 +1572,7 @@ fn assert_crash_ping_message(body: &str) {
15671572

15681573
assert_eq!(message_json["version"].as_str(), Some("1.0"));
15691574

1570-
assert_eq!(message_json["kind"].as_str(), Some("Crash ping"));
1575+
assert_eq!(message_json["kind"].as_str(), Some("UnixSignal"));
15711576
}
15721577

15731578
// Old TestFixtures struct kept for UDS socket tests that weren't migrated

libdd-crashtracker-ffi/src/crash_info/builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread_name(
428428
/// The `builder` can be null, but if non-null it must point to a Builder made by this module,
429429
/// which has not previously been dropped.
430430
/// All arguments must be valid.
431-
/// This method requires that the builder has a UUID and metadata set
431+
/// This method requires that the builder has `metadata` and `kind` set
432432
#[no_mangle]
433433
#[must_use]
434434
#[named]

libdd-crashtracker/src/collector/emitters.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use crate::runtime_callback::{
99
CallbackData,
1010
};
1111
use crate::shared::constants::*;
12-
use crate::{translate_si_code, CrashtrackerConfiguration, SignalNames, StacktraceCollection};
12+
use crate::{
13+
translate_si_code, CrashtrackerConfiguration, ErrorKind, SignalNames, StacktraceCollection,
14+
};
1315
use backtrace::Frame;
1416
use libc::{siginfo_t, ucontext_t};
1517
use std::{
@@ -145,13 +147,15 @@ pub(crate) fn emit_crashreport(
145147
crashing_tid: libc::pid_t,
146148
) -> Result<(), EmitterError> {
147149
// The following order is important in order to emit the crash ping:
148-
// - receiver expects the config
150+
// - receiver expects the config because the endpoint to emit to is there
149151
// - then message if any
150-
// - then siginfo (if the message is not set, we use the siginfo to generate the message)
152+
// - then siginfo if any
153+
// - then the kind if any
151154
// - then metadata
152155
emit_config(pipe, config_str)?;
153156
emit_message(pipe, message_ptr)?;
154157
emit_siginfo(pipe, sig_info)?;
158+
emit_kind(pipe, &ErrorKind::UnixSignal)?;
155159
emit_metadata(pipe, metadata_string)?;
156160
// after the metadata the ping should have been sent
157161
emit_ucontext(pipe, ucontext)?;
@@ -193,6 +197,15 @@ fn emit_config(w: &mut impl Write, config_str: &str) -> Result<(), EmitterError>
193197
Ok(())
194198
}
195199

200+
fn emit_kind<W: std::io::Write>(w: &mut W, kind: &ErrorKind) -> Result<(), EmitterError> {
201+
writeln!(w, "{DD_CRASHTRACK_BEGIN_KIND}")?;
202+
let _ = serde_json::to_writer(&mut *w, kind);
203+
writeln!(w)?;
204+
writeln!(w, "{DD_CRASHTRACK_END_KIND}")?;
205+
w.flush()?;
206+
Ok(())
207+
}
208+
196209
fn emit_metadata(w: &mut impl Write, metadata_str: &str) -> Result<(), EmitterError> {
197210
writeln!(w, "{DD_CRASHTRACK_BEGIN_METADATA}")?;
198211
writeln!(w, "{metadata_str}")?;

libdd-crashtracker/src/crash_info/builder.rs

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,16 @@ impl CrashInfoBuilder {
401401
Ok(())
402402
}
403403

404-
/// This method requires that the builder has a UUID and metadata set.
405-
/// Siginfo is optional for platforms that don't support it (like Windows)
404+
/// This method requires that the builder has Metadata and Kind set
406405
pub fn build_crash_ping(&self) -> anyhow::Result<CrashPing> {
406+
let metadata = self.metadata.clone().context("metadata is required")?;
407+
let kind = self.error.kind.clone().context("kind is required")?;
407408
let message = self.error.message.clone();
408409
let sig_info = self.sig_info.clone();
409-
let metadata = self.metadata.clone().context("metadata is required")?;
410410

411-
let mut builder = CrashPingBuilder::new(self.uuid).with_metadata(metadata);
411+
let mut builder = CrashPingBuilder::new(self.uuid)
412+
.with_metadata(metadata)
413+
.with_kind(kind);
412414
if let Some(sig_info) = sig_info {
413415
builder = builder.with_sig_info(sig_info);
414416
}
@@ -419,16 +421,7 @@ impl CrashInfoBuilder {
419421
}
420422

421423
pub fn is_ping_ready(&self) -> bool {
422-
// On Unix platforms, wait for both metadata and siginfo
423-
// On Windows, siginfo is not available, so only wait for metadata
424-
#[cfg(unix)]
425-
{
426-
self.metadata.is_some() && self.sig_info.is_some()
427-
}
428-
#[cfg(windows)]
429-
{
430-
self.metadata.is_some()
431-
}
424+
self.metadata.is_some() && self.error.kind.is_some()
432425
}
433426

434427
pub fn has_message(&self) -> bool {

libdd-crashtracker/src/crash_info/error_data.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,16 @@ pub enum ErrorKind {
189189
UnixSignal,
190190
}
191191

192+
impl ErrorKind {
193+
pub fn to_json_string(&self) -> &str {
194+
match self {
195+
ErrorKind::Panic => "\"Panic\"",
196+
ErrorKind::UnhandledException => "\"UnhandledException\"",
197+
ErrorKind::UnixSignal => "\"UnixSignal\"",
198+
}
199+
}
200+
}
201+
192202
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
193203
pub struct ThreadData {
194204
pub crashed: bool,

libdd-crashtracker/src/crash_info/telemetry.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33
use std::{fmt::Write, time::SystemTime};
44

5-
use crate::SigInfo;
5+
use crate::{ErrorKind, SigInfo};
66

77
use super::{CrashInfo, Metadata};
88
use anyhow::Context;
@@ -26,6 +26,7 @@ struct TelemetryMetadata {
2626
pub struct CrashPingBuilder {
2727
crash_uuid: Uuid,
2828
custom_message: Option<String>,
29+
kind: Option<ErrorKind>,
2930
metadata: Option<Metadata>,
3031
sig_info: Option<SigInfo>,
3132
}
@@ -38,6 +39,7 @@ impl CrashPingBuilder {
3839
Self {
3940
crash_uuid,
4041
custom_message: None,
42+
kind: None,
4143
metadata: None,
4244
sig_info: None,
4345
}
@@ -53,6 +55,11 @@ impl CrashPingBuilder {
5355
self
5456
}
5557

58+
pub fn with_kind(mut self, kind: ErrorKind) -> Self {
59+
self.kind = Some(kind);
60+
self
61+
}
62+
5663
pub fn with_metadata(mut self, metadata: Metadata) -> Self {
5764
self.metadata = Some(metadata);
5865
self
@@ -62,6 +69,7 @@ impl CrashPingBuilder {
6269
let crash_uuid = self.crash_uuid;
6370
let sig_info = self.sig_info;
6471
let metadata = self.metadata.context("metadata is required")?;
72+
let kind = self.kind.context("kind is required")?;
6573

6674
let message = if let Some(custom_message) = self.custom_message {
6775
format!("Crashtracker crash ping: crash processing started - {custom_message}")
@@ -71,13 +79,13 @@ impl CrashPingBuilder {
7179
sig_info.si_code_human_readable, sig_info.si_signo_human_readable
7280
)
7381
} else {
74-
"Crashtracker crash ping: crash processing started - Process terminated".to_string()
82+
format!("Crashtracker crash ping: crash processing started - Process terminated due to {:?}", kind)
7583
};
7684

7785
Ok(CrashPing {
7886
crash_uuid: crash_uuid.to_string(),
79-
kind: "Crash ping".to_string(),
8087
message,
88+
kind,
8189
metadata,
8290
siginfo: sig_info,
8391
version: CrashPing::current_schema_version(),
@@ -88,11 +96,11 @@ impl CrashPingBuilder {
8896
#[derive(Debug, Serialize)]
8997
pub struct CrashPing {
9098
crash_uuid: String,
99+
kind: ErrorKind,
100+
message: String,
91101
#[serde(skip_serializing_if = "Option::is_none")]
92102
siginfo: Option<SigInfo>,
93-
message: String,
94103
version: String,
95-
kind: String,
96104
metadata: Metadata,
97105
}
98106

libdd-crashtracker/src/receiver/receive_report.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ pub(crate) enum StdinState {
108108
Counters,
109109
Done,
110110
File(String, Vec<String>),
111+
Kind,
111112
Metadata,
112113
ProcInfo,
113114
SigInfo,
@@ -186,6 +187,13 @@ fn process_line(
186187
StdinState::File(name, contents)
187188
}
188189

190+
StdinState::Kind if line.starts_with(DD_CRASHTRACK_END_KIND) => StdinState::Waiting,
191+
StdinState::Kind => {
192+
let kind: ErrorKind = serde_json::from_str(line)?;
193+
builder.with_kind(kind)?;
194+
StdinState::Kind
195+
}
196+
189197
StdinState::Metadata if line.starts_with(DD_CRASHTRACK_END_METADATA) => StdinState::Waiting,
190198
StdinState::Metadata => {
191199
let metadata = serde_json::from_str(line)?;
@@ -311,6 +319,7 @@ fn process_line(
311319
let (_, filename) = line.split_once(' ').unwrap_or(("", "MISSING_FILENAME"));
312320
StdinState::File(filename.to_string(), vec![])
313321
}
322+
StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_KIND) => StdinState::Kind,
314323
StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_METADATA) => {
315324
StdinState::Metadata
316325
}
@@ -392,7 +401,7 @@ pub(crate) async fn receive_report_from_stream(
392401
}
393402
}
394403

395-
// We need to wait until at least we receive config, metadata, and siginfo (on non-Windows
404+
// We need to wait until at least we receive config, metadata, and kind (on non-Windows
396405
// platforms) before sending the crash ping
397406
if !crash_ping_sent && builder.is_ping_ready() {
398407
if let Some(ref config_ref) = config {
@@ -485,8 +494,6 @@ pub(crate) async fn receive_report_from_stream(
485494
return Ok(None);
486495
}
487496

488-
// For now, we only support Signal based crash detection in the receiver.
489-
builder.with_kind(ErrorKind::UnixSignal)?;
490497
enrich_thread_name(&mut builder)?;
491498
builder.with_os_info_this_machine()?;
492499

libdd-crashtracker/src/shared/constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub const DD_CRASHTRACK_BEGIN_ADDITIONAL_TAGS: &str = "DD_CRASHTRACK_BEGIN_ADDIT
77
pub const DD_CRASHTRACK_BEGIN_CONFIG: &str = "DD_CRASHTRACK_BEGIN_CONFIG";
88
pub const DD_CRASHTRACK_BEGIN_COUNTERS: &str = "DD_CRASHTRACK_BEGIN_COUNTERS";
99
pub const DD_CRASHTRACK_BEGIN_FILE: &str = "DD_CRASHTRACK_BEGIN_FILE";
10+
pub const DD_CRASHTRACK_BEGIN_KIND: &str = "DD_CRASHTRACK_BEGIN_KIND";
1011
pub const DD_CRASHTRACK_BEGIN_METADATA: &str = "DD_CRASHTRACK_BEGIN_METADATA";
1112
pub const DD_CRASHTRACK_BEGIN_PROCINFO: &str = "DD_CRASHTRACK_BEGIN_PROCESSINFO";
1213
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
2425
pub const DD_CRASHTRACK_END_CONFIG: &str = "DD_CRASHTRACK_END_CONFIG";
2526
pub const DD_CRASHTRACK_END_COUNTERS: &str = "DD_CRASHTRACK_END_COUNTERS";
2627
pub const DD_CRASHTRACK_END_FILE: &str = "DD_CRASHTRACK_END_FILE";
28+
pub const DD_CRASHTRACK_END_KIND: &str = "DD_CRASHTRACK_END_KIND";
2729
pub const DD_CRASHTRACK_END_METADATA: &str = "DD_CRASHTRACK_END_METADATA";
2830
pub const DD_CRASHTRACK_END_PROCINFO: &str = "DD_CRASHTRACK_END_PROCESSINFO";
2931
pub const DD_CRASHTRACK_END_THREAD_NAME: &str = "DD_CRASHTRACK_END_THREAD_NAME";

0 commit comments

Comments
 (0)