Skip to content

Commit

Permalink
[crashtracker] Refactor: Use the new FFI helper macros to cleanup the…
Browse files Browse the repository at this point in the history
… code (#793)
  • Loading branch information
danielsn authored Dec 9, 2024
1 parent 189269d commit d111f32
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 117 deletions.
19 changes: 8 additions & 11 deletions crashtracker-ffi/src/collector/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

use super::datatypes::OpTypes;
use anyhow::Context;
use ddcommon_ffi::VoidResult;
use ::function_name::named;
use ddcommon_ffi::{wrap_with_void_ffi_result, VoidResult};

/// Resets all counters to 0.
/// Expected to be used after a fork, to reset the counters on the child
Expand All @@ -15,34 +15,31 @@ use ddcommon_ffi::VoidResult;
/// No safety concerns.
#[no_mangle]
#[must_use]
#[named]
pub unsafe extern "C" fn ddog_crasht_reset_counters() -> VoidResult {
datadog_crashtracker::reset_counters()
.context("ddog_crasht_reset_counters failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::reset_counters()? })
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically increments the count associated with `op`.
/// Useful for tracking what operations were occuring when a crash occurred.
///
/// # Safety
/// No safety concerns.
pub unsafe extern "C" fn ddog_crasht_begin_op(op: OpTypes) -> VoidResult {
datadog_crashtracker::begin_op(op)
.context("ddog_crasht_begin_op failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::begin_op(op)? })
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically decrements the count associated with `op`.
/// Useful for tracking what operations were occuring when a crash occurred.
///
/// # Safety
/// No safety concerns.
pub unsafe extern "C" fn ddog_crasht_end_op(op: OpTypes) -> VoidResult {
datadog_crashtracker::end_op(op)
.context("ddog_crasht_end_op failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::end_op(op)? })
}
16 changes: 0 additions & 16 deletions crashtracker-ffi/src/collector/datatypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,22 +98,6 @@ impl<'a> TryFrom<Config<'a>> for datadog_crashtracker::CrashtrackerConfiguration
}
}

#[repr(C)]
pub enum UsizeResult {
Ok(usize),
#[allow(dead_code)]
Err(Error),
}

impl From<anyhow::Result<usize>> for UsizeResult {
fn from(value: anyhow::Result<usize>) -> Self {
match value {
Ok(x) => Self::Ok(x),
Err(err) => Self::Err(err.into()),
}
}
}

#[repr(C)]
pub enum CrashtrackerGetCountersResult {
Ok([i64; OpTypes::SIZE as usize]),
Expand Down
70 changes: 31 additions & 39 deletions crashtracker-ffi/src/collector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use anyhow::Context;
pub use counters::*;
use datadog_crashtracker::CrashtrackerReceiverConfig;
pub use datatypes::*;
use ddcommon_ffi::VoidResult;
use ddcommon_ffi::{wrap_with_void_ffi_result, VoidResult};
use function_name::named;
pub use spans::*;

#[no_mangle]
Expand Down Expand Up @@ -37,6 +38,7 @@ pub unsafe extern "C" fn ddog_crasht_shutdown() -> VoidResult {

#[no_mangle]
#[must_use]
#[named]
/// Reinitialize the crash-tracking infrastructure after a fork.
/// This should be one of the first things done after a fork, to minimize the
/// chance that a crash occurs between the fork, and this call.
Expand All @@ -59,18 +61,18 @@ pub unsafe extern "C" fn ddog_crasht_update_on_fork(
receiver_config: ReceiverConfig,
metadata: Metadata,
) -> VoidResult {
(|| {
let config = config.try_into()?;
let receiver_config = receiver_config.try_into()?;
let metadata = metadata.try_into()?;
datadog_crashtracker::on_fork(config, receiver_config, metadata)
})()
.context("ddog_crasht_update_on_fork failed")
.into()
wrap_with_void_ffi_result!({
datadog_crashtracker::on_fork(
config.try_into()?,
receiver_config.try_into()?,
metadata.try_into()?,
)?;
})
}

#[no_mangle]
#[must_use]
#[named]
/// Initialize the crash-tracking infrastructure.
///
/// # Preconditions
Expand All @@ -86,18 +88,18 @@ pub unsafe extern "C" fn ddog_crasht_init(
receiver_config: ReceiverConfig,
metadata: Metadata,
) -> VoidResult {
(|| {
let config = config.try_into()?;
let receiver_config = receiver_config.try_into()?;
let metadata = metadata.try_into()?;
datadog_crashtracker::init(config, receiver_config, metadata)
})()
.context("ddog_crasht_init failed")
.into()
wrap_with_void_ffi_result!({
datadog_crashtracker::init(
config.try_into()?,
receiver_config.try_into()?,
metadata.try_into()?,
)?;
})
}

#[no_mangle]
#[must_use]
#[named]
/// Initialize the crash-tracking infrastructure without launching the receiver.
///
/// # Preconditions
Expand All @@ -112,30 +114,20 @@ pub unsafe extern "C" fn ddog_crasht_init_without_receiver(
config: Config,
metadata: Metadata,
) -> VoidResult {
(|| {
let config: datadog_crashtracker::CrashtrackerConfiguration = config.try_into()?;
let metadata = metadata.try_into()?;

wrap_with_void_ffi_result!({
// If the unix domain socket path is not set, then we throw an error--there's currently no
// other way to specify communication between an async receiver and a collector, so this
// isn't a valid configuration.
if config.unix_socket_path.is_none() {
return Err(anyhow::anyhow!("config.unix_socket_path must be set"));
}
if config.unix_socket_path.as_ref().unwrap().is_empty() {
return Err(anyhow::anyhow!("config.unix_socket_path can't be empty"));
}
anyhow::ensure!(
!config.optional_unix_socket_filename.is_empty(),
"config.optional_unix_socket_filename must be set in this configuration"
);

// Populate an empty receiver config
let receiver_config = CrashtrackerReceiverConfig {
args: vec![],
env: vec![],
path_to_receiver_binary: "".to_string(),
stderr_filename: None,
stdout_filename: None,
};
datadog_crashtracker::init(config, receiver_config, metadata)
})()
.context("ddog_crasht_init failed")
.into()
// No receiver, use an empty receiver config
datadog_crashtracker::init(
config.try_into()?,
CrashtrackerReceiverConfig::default(),
metadata.try_into()?,
)?
})
}
56 changes: 28 additions & 28 deletions crashtracker-ffi/src/collector/spans.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
// SPDX-License-Identifier: Apache-2.0

use crate::UsizeResult;
use anyhow::Context;
use ddcommon_ffi::VoidResult;

use ddcommon_ffi::{wrap_with_ffi_result, wrap_with_void_ffi_result, Result, VoidResult};
use function_name::named;
/// Resets all stored spans to 0.
/// Expected to be used after a fork, to reset the spans on the child
/// ATOMICITY:
Expand All @@ -15,10 +13,9 @@ use ddcommon_ffi::VoidResult;
/// No safety concerns.
#[no_mangle]
#[must_use]
#[named]
pub unsafe extern "C" fn ddog_crasht_clear_span_ids() -> VoidResult {
datadog_crashtracker::clear_spans()
.context("ddog_crasht_clear_span_ids failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::clear_spans()? })
}

/// Resets all stored traces to 0.
Expand All @@ -31,14 +28,14 @@ pub unsafe extern "C" fn ddog_crasht_clear_span_ids() -> VoidResult {
/// No safety concerns.
#[no_mangle]
#[must_use]
#[named]
pub unsafe extern "C" fn ddog_crasht_clear_trace_ids() -> VoidResult {
datadog_crashtracker::clear_traces()
.context("ddog_crasht_clear_trace_ids failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::clear_traces()? })
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically registers an active traceId.
/// Useful for tracking what operations were occurring when a crash occurred.
/// 0 is reserved for "NoId"
Expand All @@ -58,15 +55,16 @@ pub unsafe extern "C" fn ddog_crasht_clear_trace_ids() -> VoidResult {
///
/// # Safety
/// No safety concerns.
pub unsafe extern "C" fn ddog_crasht_insert_trace_id(id_high: u64, id_low: u64) -> UsizeResult {
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::insert_trace(id)
.context("ddog_crasht_insert_trace_id failed")
.into()
pub unsafe extern "C" fn ddog_crasht_insert_trace_id(id_high: u64, id_low: u64) -> Result<usize> {
wrap_with_ffi_result!({
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::insert_trace(id)
})
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically registers an active SpanId.
/// Useful for tracking what operations were occurring when a crash occurred.
/// 0 is reserved for "NoId".
Expand All @@ -86,15 +84,16 @@ pub unsafe extern "C" fn ddog_crasht_insert_trace_id(id_high: u64, id_low: u64)
///
/// # Safety
/// No safety concerns.
pub unsafe extern "C" fn ddog_crasht_insert_span_id(id_high: u64, id_low: u64) -> UsizeResult {
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::insert_span(id)
.context("ddog_crasht_insert_span_id failed")
.into()
pub unsafe extern "C" fn ddog_crasht_insert_span_id(id_high: u64, id_low: u64) -> Result<usize> {
wrap_with_ffi_result!({
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::insert_span(id)
})
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically removes a completed SpanId.
/// Useful for tracking what operations were occurring when a crash occurred.
/// 0 is reserved for "NoId"
Expand All @@ -120,14 +119,15 @@ pub unsafe extern "C" fn ddog_crasht_remove_span_id(
id_low: u64,
idx: usize,
) -> VoidResult {
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::remove_span(id, idx)
.context("ddog_crasht_remove_span_id failed")
.into()
wrap_with_void_ffi_result!({
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::remove_span(id, idx)?
})
}

#[no_mangle]
#[must_use]
#[named]
/// Atomically removes a completed TraceId.
/// Useful for tracking what operations were occurring when a crash occurred.
/// 0 is reserved for "NoId"
Expand All @@ -153,8 +153,8 @@ pub unsafe extern "C" fn ddog_crasht_remove_trace_id(
id_low: u64,
idx: usize,
) -> VoidResult {
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::remove_trace(id, idx)
.context("ddog_crasht_remove_trace_id failed")
.into()
wrap_with_void_ffi_result!({
let id: u128 = (id_high as u128) << 64 | (id_low as u128);
datadog_crashtracker::remove_trace(id, idx)?
})
}
22 changes: 13 additions & 9 deletions crashtracker-ffi/src/demangler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
mod datatypes;
pub use datatypes::*;

use ddcommon_ffi::{slice::AsBytes, CharSlice};
use ::function_name::named;
use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, StringWrapper};
use symbolic_common::Name;
use symbolic_demangle::Demangle;

Expand All @@ -15,17 +16,20 @@ use symbolic_demangle::Demangle;
/// The string is copied into the result, and does not need to outlive this call
#[no_mangle]
#[must_use]
#[named]
pub unsafe extern "C" fn ddog_crasht_demangle(
name: CharSlice,
options: DemangleOptions,
) -> StringWrapperResult {
let name = name.to_utf8_lossy();
let name = Name::from(name);
let options = match options {
DemangleOptions::Complete => symbolic_demangle::DemangleOptions::complete(),
DemangleOptions::NameOnly => symbolic_demangle::DemangleOptions::name_only(),
};
StringWrapperResult::Ok(name.demangle(options).unwrap_or_default().into())
) -> Result<StringWrapper> {
wrap_with_ffi_result!({
let name = name.to_utf8_lossy();
let name = Name::from(name);
let options = match options {
DemangleOptions::Complete => symbolic_demangle::DemangleOptions::complete(),
DemangleOptions::NameOnly => symbolic_demangle::DemangleOptions::name_only(),
};
anyhow::Ok(name.demangle(options).unwrap_or_default().into())
})
}

#[test]
Expand Down
19 changes: 8 additions & 11 deletions crashtracker-ffi/src/receiver.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright 2023-Present Datadog, Inc. https://www.datadoghq.com/
// SPDX-License-Identifier: Apache-2.0

use anyhow::Context;
use ddcommon_ffi::{slice::AsBytes, CharSlice, VoidResult};
use ::function_name::named;
use ddcommon_ffi::{slice::AsBytes, wrap_with_void_ffi_result, CharSlice, VoidResult};
#[no_mangle]
#[must_use]
#[named]
/// Receives data from a crash collector via a pipe on `stdin`, formats it into
/// `CrashInfo` json, and emits it to the endpoint/file defined in `config`.
///
Expand All @@ -16,13 +17,12 @@ use ddcommon_ffi::{slice::AsBytes, CharSlice, VoidResult};
/// # Safety
/// No safety concerns
pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_stdin() -> VoidResult {
datadog_crashtracker::receiver_entry_point_stdin()
.context("ddog_crasht_receiver_entry_point_stdin failed")
.into()
wrap_with_void_ffi_result!({ datadog_crashtracker::receiver_entry_point_stdin()? })
}

#[no_mangle]
#[must_use]
#[named]
/// Receives data from a crash collector via a pipe on `stdin`, formats it into
/// `CrashInfo` json, and emits it to the endpoint/file defined in `config`.
///
Expand All @@ -37,10 +37,7 @@ pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_stdin() -> VoidResult
pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_unix_socket(
socket_path: CharSlice,
) -> VoidResult {
(|| {
let socket_path = socket_path.try_to_utf8()?;
datadog_crashtracker::receiver_entry_point_unix_socket(socket_path)
})()
.context("ddog_crasht_receiver_entry_point_unix_socket failed")
.into()
wrap_with_void_ffi_result!({
datadog_crashtracker::receiver_entry_point_unix_socket(socket_path.try_to_utf8()?)?
})
}
Loading

0 comments on commit d111f32

Please sign in to comment.