From 31cd819d33f3b73746650f863d15ad541f6e7b5d Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Mon, 9 Dec 2024 17:52:56 -0500 Subject: [PATCH] [profiling] Refactor: use the new FFI macros and types --- Cargo.lock | 1 + ddcommon-ffi/src/handle.rs | 13 + ddcommon-ffi/src/result.rs | 18 + examples/ffi/exporter.cpp | 93 +++-- examples/ffi/profiles.c | 12 +- profiling-ffi/Cargo.toml | 1 + profiling-ffi/cbindgen.toml | 6 +- profiling-ffi/src/datatypes.rs | 257 ++++++++++++++ profiling-ffi/src/lib.rs | 1 + profiling-ffi/src/profiles.rs | 616 ++++++--------------------------- 10 files changed, 459 insertions(+), 559 deletions(-) create mode 100644 profiling-ffi/src/datatypes.rs diff --git a/Cargo.lock b/Cargo.lock index 0c5684b83..afa7da7f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1525,6 +1525,7 @@ dependencies = [ "ddcommon", "ddcommon-ffi", "ddtelemetry-ffi", + "function_name", "futures", "hyper 0.14.31", "libc", diff --git a/ddcommon-ffi/src/handle.rs b/ddcommon-ffi/src/handle.rs index c71cca5d5..92ea2c341 100644 --- a/ddcommon-ffi/src/handle.rs +++ b/ddcommon-ffi/src/handle.rs @@ -12,6 +12,9 @@ pub struct Handle { } pub trait ToInner { + /// # Safety + /// The Handle must hold a valid `inner` which has been allocated and not freed. + unsafe fn to_inner(&self) -> anyhow::Result<&T>; /// # Safety /// The Handle must hold a valid `inner` which has been allocated and not freed. unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T>; @@ -21,6 +24,10 @@ pub trait ToInner { } impl ToInner for *mut Handle { + unsafe fn to_inner(&self) -> anyhow::Result<&T> { + self.as_ref().context("Null pointer")?.to_inner() + } + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T> { self.as_mut().context("Null pointer")?.to_inner_mut() } @@ -31,6 +38,12 @@ impl ToInner for *mut Handle { } impl ToInner for Handle { + unsafe fn to_inner(&self) -> anyhow::Result<&T> { + self.inner + .as_ref() + .context("inner pointer was null, indicates use after free") + } + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T> { self.inner .as_mut() diff --git a/ddcommon-ffi/src/result.rs b/ddcommon-ffi/src/result.rs index 37c3a56b2..bc9da657c 100644 --- a/ddcommon-ffi/src/result.rs +++ b/ddcommon-ffi/src/result.rs @@ -24,6 +24,15 @@ impl From> for VoidResult { } } +impl From for anyhow::Result<()> { + fn from(value: VoidResult) -> Self { + match value { + VoidResult::Ok(_) => Self::Ok(()), + VoidResult::Err(err) => Self::Err(err.into()), + } + } +} + /// A generic result type for when an operation may fail, /// or may return in case of success. #[repr(C)] @@ -49,3 +58,12 @@ impl From> for Result { } } } + +impl From> for anyhow::Result { + fn from(value: Result) -> Self { + match value { + Result::Ok(v) => Self::Ok(v), + Result::Err(err) => Self::Err(err.into()), + } + } +} diff --git a/examples/ffi/exporter.cpp b/examples/ffi/exporter.cpp index a6d61d457..efb21a9f2 100644 --- a/examples/ffi/exporter.cpp +++ b/examples/ffi/exporter.cpp @@ -8,16 +8,50 @@ #include static ddog_CharSlice to_slice_c_char(const char *s) { return {.ptr = s, .len = strlen(s)}; } - -struct Deleter { - void operator()(ddog_prof_Profile *object) { ddog_prof_Profile_drop(object); } -}; +static ddog_CharSlice to_slice_c_char(const char *s, std::size_t size) { + return {.ptr = s, .len = size}; +} +static ddog_CharSlice to_slice_string(std::string const &s) { + return {.ptr = s.data(), .len = s.length()}; +} void print_error(const char *s, const ddog_Error &err) { auto charslice = ddog_Error_message(&err); printf("%s (%.*s)\n", s, static_cast(charslice.len), charslice.ptr); } +#define CHECK_RESULT(typ, ok_tag) \ + void check_result(typ result, const char *msg) { \ + if (result.tag != ok_tag) { \ + print_error(msg, result.err); \ + ddog_Error_drop(&result.err); \ + exit(EXIT_FAILURE); \ + } \ + } + +CHECK_RESULT(ddog_VoidResult, DDOG_VOID_RESULT_OK) + +#define EXTRACT_RESULT(typ, ok_tag) \ + struct typ##Deleter { \ + void operator()(ddog_prof_Handle_##typ *object) { \ + ddog_prof_##typ##_drop(object); \ + delete object; \ + } \ + }; \ + std::unique_ptr extract_result( \ + ddog_prof_Result_Handle##typ result, const char *msg) { \ + if (result.tag != ok_tag) { \ + print_error(msg, result.err); \ + ddog_Error_drop(&result.err); \ + exit(EXIT_FAILURE); \ + } \ + std::unique_ptr rval{ \ + new ddog_prof_Handle_##typ{result.ok}}; \ + return rval; \ + } + +EXTRACT_RESULT(Profile, DDOG_PROF_RESULT_HANDLE_PROFILE_OK_HANDLE_PROFILE) + int main(int argc, char *argv[]) { if (argc != 2) { printf("Usage: exporter SERVICE_NAME\n"); @@ -38,14 +72,8 @@ int main(int argc, char *argv[]) { const ddog_prof_Slice_ValueType sample_types = {&wall_time, 1}; const ddog_prof_Period period = {wall_time, 60}; - ddog_prof_Profile_NewResult profile_new_result = - ddog_prof_Profile_new(sample_types, &period, nullptr); - if (profile_new_result.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { - print_error("Failed to make new profile: ", profile_new_result.err); - ddog_Error_drop(&profile_new_result.err); - exit(EXIT_FAILURE); - } - std::unique_ptr profile{&profile_new_result.ok}; + auto profile = + extract_result(ddog_prof_Profile_new(sample_types, &period, nullptr), "Can't get profile"); ddog_prof_Location root_location = { // yes, a zero-initialized mapping is valid @@ -67,30 +95,18 @@ int main(int argc, char *argv[]) { .values = {&value, 1}, .labels = {&label, 1}, }; - auto add_result = ddog_prof_Profile_add(profile.get(), sample, 0); - if (add_result.tag != DDOG_PROF_PROFILE_RESULT_OK) { - print_error("Failed to add sample to profile: ", add_result.err); - ddog_Error_drop(&add_result.err); - return 1; - } + check_result(ddog_prof_Profile_add(profile.get(), sample, 0), "failed to add"); uintptr_t offset[1] = {0}; ddog_prof_Slice_Usize offsets_slice = {.ptr = offset, .len = 1}; ddog_CharSlice empty_charslice = DDOG_CHARSLICE_C_BARE(""); - auto upscaling_addresult = ddog_prof_Profile_add_upscaling_rule_proportional( - profile.get(), offsets_slice, empty_charslice, empty_charslice, 1, 1); + check_result(ddog_prof_Profile_add_upscaling_rule_proportional( + profile.get(), offsets_slice, empty_charslice, empty_charslice, 1, 1), + "failed to add upscaling rule"); - if (upscaling_addresult.tag == DDOG_PROF_PROFILE_RESULT_ERR) { - print_error("Failed to add an upscaling rule: ", upscaling_addresult.err); - ddog_Error_drop(&upscaling_addresult.err); - // in this specific case, we want to fail the execution. But in general, we should not - return 1; - } - - ddog_prof_Profile_SerializeResult serialize_result = - ddog_prof_Profile_serialize(profile.get(), nullptr, nullptr, nullptr); - if (serialize_result.tag == DDOG_PROF_PROFILE_SERIALIZE_RESULT_ERR) { + auto serialize_result = ddog_prof_Profile_serialize(profile.get(), nullptr, nullptr, nullptr); + if (serialize_result.tag != DDOG_PROF_RESULT_ENCODED_PROFILE_OK_ENCODED_PROFILE) { print_error("Failed to serialize profile: ", serialize_result.err); ddog_Error_drop(&serialize_result.err); return 1; @@ -110,9 +126,9 @@ int main(int argc, char *argv[]) { return 1; } - ddog_prof_Exporter_NewResult exporter_new_result = - ddog_prof_Exporter_new(DDOG_CHARSLICE_C_BARE("exporter-example"), DDOG_CHARSLICE_C_BARE("1.2.3"), - DDOG_CHARSLICE_C_BARE("native"), &tags, endpoint); + ddog_prof_Exporter_NewResult exporter_new_result = ddog_prof_Exporter_new( + DDOG_CHARSLICE_C_BARE("exporter-example"), DDOG_CHARSLICE_C_BARE("1.2.3"), + DDOG_CHARSLICE_C_BARE("native"), &tags, endpoint); ddog_Vec_Tag_drop(tags); if (exporter_new_result.tag == DDOG_PROF_EXPORTER_NEW_RESULT_ERR) { @@ -137,14 +153,15 @@ int main(int argc, char *argv[]) { ddog_CharSlice internal_metadata_example = DDOG_CHARSLICE_C_BARE( "{\"no_signals_workaround_enabled\": \"true\", \"execution_trace_enabled\": \"false\"}"); - ddog_CharSlice info_example = DDOG_CHARSLICE_C_BARE( - "{\"application\": {\"start_time\": \"2024-01-24T11:17:22+0000\"}, \"platform\": {\"kernel\": \"Darwin Kernel 22.5.0\"}}"); + ddog_CharSlice info_example = + DDOG_CHARSLICE_C_BARE("{\"application\": {\"start_time\": \"2024-01-24T11:17:22+0000\"}, " + "\"platform\": {\"kernel\": \"Darwin Kernel 22.5.0\"}}"); auto res = ddog_prof_Exporter_set_timeout(exporter, 30000); if (res.tag == DDOG_PROF_OPTION_ERROR_SOME_ERROR) { - print_error("Failed to set the timeout", res.some); - ddog_Error_drop(&res.some); - return 1; + print_error("Failed to set the timeout", res.some); + ddog_Error_drop(&res.some); + return 1; } ddog_prof_Exporter_Request_BuildResult build_result = ddog_prof_Exporter_Request_build( diff --git a/examples/ffi/profiles.c b/examples/ffi/profiles.c index 05ae61c9f..8fd1d9b63 100644 --- a/examples/ffi/profiles.c +++ b/examples/ffi/profiles.c @@ -14,8 +14,8 @@ int main(void) { const ddog_prof_Slice_ValueType sample_types = {&wall_time, 1}; const ddog_prof_Period period = {wall_time, 60}; - ddog_prof_Profile_NewResult new_result = ddog_prof_Profile_new(sample_types, &period, NULL); - if (new_result.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { + ddog_prof_Result_HandleProfile new_result = ddog_prof_Profile_new(sample_types, &period, NULL); + if (new_result.tag != DDOG_PROF_RESULT_HANDLE_PROFILE_OK_HANDLE_PROFILE) { ddog_CharSlice message = ddog_Error_message(&new_result.err); fprintf(stderr, "%.*s", (int)message.len, message.ptr); ddog_Error_drop(&new_result.err); @@ -47,8 +47,8 @@ int main(void) { for (int i = 0; i < 10000000; i++) { label.num = i; - ddog_prof_Profile_Result add_result = ddog_prof_Profile_add(profile, sample, 0); - if (add_result.tag != DDOG_PROF_PROFILE_RESULT_OK) { + ddog_VoidResult add_result = ddog_prof_Profile_add(profile, sample, 0); + if (add_result.tag == DDOG_VOID_RESULT_ERR) { ddog_CharSlice message = ddog_Error_message(&add_result.err); fprintf(stderr, "%.*s", (int)message.len, message.ptr); ddog_Error_drop(&add_result.err); @@ -58,8 +58,8 @@ int main(void) { // printf("Press any key to reset and drop..."); // getchar(); - ddog_prof_Profile_Result reset_result = ddog_prof_Profile_reset(profile, NULL); - if (reset_result.tag != DDOG_PROF_PROFILE_RESULT_OK) { + ddog_VoidResult reset_result = ddog_prof_Profile_reset(profile, NULL); + if (reset_result.tag == DDOG_VOID_RESULT_ERR) { ddog_CharSlice message = ddog_Error_message(&reset_result.err); fprintf(stderr, "%.*s", (int)message.len, message.ptr); ddog_Error_drop(&reset_result.err); diff --git a/profiling-ffi/Cargo.toml b/profiling-ffi/Cargo.toml index 2e1e62c5f..2faf6307a 100644 --- a/profiling-ffi/Cargo.toml +++ b/profiling-ffi/Cargo.toml @@ -46,3 +46,4 @@ symbolizer-ffi = { path = "../symbolizer-ffi", optional = true, default-features symbolic-demangle = { version = "12.8.0", default-features = false, features = ["rust", "cpp", "msvc"] } symbolic-common = "12.8.0" data-pipeline-ffi = { path = "../data-pipeline-ffi", default-features = false, optional = true } +function_name = "0.3.0" diff --git a/profiling-ffi/cbindgen.toml b/profiling-ffi/cbindgen.toml index 4c3c485dd..d41472962 100644 --- a/profiling-ffi/cbindgen.toml +++ b/profiling-ffi/cbindgen.toml @@ -33,17 +33,15 @@ renaming_overrides_prefixing = true "Timespec" = "ddog_Timespec" "Vec_Tag" = "ddog_Vec_Tag" "Vec_U8" = "ddog_Vec_U8" +"VoidResult" = "ddog_VoidResult" -"ProfilingEndpoint" = "ddog_prof_Endpoint" "ExporterNewResult" = "ddog_prof_Exporter_NewResult" "File" = "ddog_prof_Exporter_File" "ProfileExporter" = "ddog_prof_Exporter" -"ProfileNewResult" = "ddog_prof_Profile_NewResult" -"ProfileResult" = "ddog_prof_Profile_Result" +"ProfilingEndpoint" = "ddog_prof_Endpoint" "Request" = "ddog_prof_Exporter_Request" "RequestBuildResult" = "ddog_prof_Exporter_Request_BuildResult" "SendResult" = "ddog_prof_Exporter_SendResult" -"SerializeResult" = "ddog_prof_Profile_SerializeResult" "Slice_File" = "ddog_prof_Exporter_Slice_File" [export.mangle] diff --git a/profiling-ffi/src/datatypes.rs b/profiling-ffi/src/datatypes.rs new file mode 100644 index 000000000..5fea75e2c --- /dev/null +++ b/profiling-ffi/src/datatypes.rs @@ -0,0 +1,257 @@ +// Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice}; +use std::str::Utf8Error; +use datadog_profiling::api; +use std::result::Result; + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct ValueType<'a> { + pub type_: CharSlice<'a>, + pub unit: CharSlice<'a>, +} + +impl<'a> ValueType<'a> { + pub fn new(type_: &'a str, unit: &'a str) -> Self { + Self { + type_: type_.into(), + unit: unit.into(), + } + } +} + +#[repr(C)] +pub struct Period<'a> { + pub type_: ValueType<'a>, + pub value: i64, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +pub struct Label<'a> { + pub key: CharSlice<'a>, + + /// At most one of the following must be present + pub str: CharSlice<'a>, + pub num: i64, + + /// Should only be present when num is present. + /// Specifies the units of num. + /// Use arbitrary string (for example, "requests") as a custom count unit. + /// If no unit is specified, consumer may apply heuristic to deduce the unit. + /// Consumers may also interpret units like "bytes" and "kilobytes" as memory + /// units and units like "seconds" and "nanoseconds" as time units, + /// and apply appropriate unit conversions to these. + pub num_unit: CharSlice<'a>, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +pub struct Function<'a> { + /// Name of the function, in human-readable form if available. + pub name: CharSlice<'a>, + + /// Name of the function, as identified by the system. + /// For instance, it can be a C++ mangled name. + pub system_name: CharSlice<'a>, + + /// Source file containing the function. + pub filename: CharSlice<'a>, + + /// Line number in source file. + pub start_line: i64, +} + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct Line<'a> { + /// The corresponding profile.Function for this line. + pub function: Function<'a>, + + /// Line number in source code. + pub line: i64, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +pub struct Location<'a> { + /// todo: how to handle unknown mapping? + pub mapping: Mapping<'a>, + pub function: Function<'a>, + + /// The instruction address for this location, if available. It + /// should be within [Mapping.memory_start...Mapping.memory_limit] + /// for the corresponding mapping. A non-leaf address may be in the + /// middle of a call instruction. It is up to display tools to find + /// the beginning of the instruction if necessary. + pub address: u64, + pub line: i64, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +pub struct Mapping<'a> { + /// Address at which the binary (or DLL) is loaded into memory. + pub memory_start: u64, + + /// The limit of the address range occupied by this mapping. + pub memory_limit: u64, + + /// Offset in the binary that corresponds to the first mapped address. + pub file_offset: u64, + + /// The object this entry is loaded from. This can be a filename on + /// disk for the main binary and shared libraries, or virtual + /// abstractions like "[vdso]". + pub filename: CharSlice<'a>, + + /// A string that uniquely identifies a particular program version + /// with high probability. E.g., for binaries generated by GNU tools, + /// it could be the contents of the .note.gnu.build-id field. + pub build_id: CharSlice<'a>, +} + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct Sample<'a> { + /// The leaf is at locations[0]. + pub locations: Slice<'a, Location<'a>>, + + /// The type and unit of each value is defined by the corresponding + /// entry in Profile.sample_type. All samples must have the same + /// number of values, the same as the length of Profile.sample_type. + /// When aggregating multiple samples into a single sample, the + /// result has a list of values that is the element-wise sum of the + /// lists of the originals. + pub values: Slice<'a, i64>, + + /// label includes additional context for this sample. It can include + /// things like a thread id, allocation size, etc + pub labels: Slice<'a, Label<'a>>, +} + +impl<'a> TryFrom<&'a Mapping<'a>> for api::Mapping<'a> { + type Error = Utf8Error; + + fn try_from(mapping: &'a Mapping<'a>) -> Result { + let filename = mapping.filename.try_to_utf8()?; + let build_id = mapping.build_id.try_to_utf8()?; + Ok(Self { + memory_start: mapping.memory_start, + memory_limit: mapping.memory_limit, + file_offset: mapping.file_offset, + filename, + build_id, + }) + } +} + +impl<'a> From<&'a ValueType<'a>> for api::ValueType<'a> { + fn from(vt: &'a ValueType<'a>) -> Self { + Self::new( + vt.type_.try_to_utf8().unwrap_or(""), + vt.unit.try_to_utf8().unwrap_or(""), + ) + } +} + +impl<'a> From<&'a Period<'a>> for api::Period<'a> { + fn from(period: &'a Period<'a>) -> Self { + Self { + r#type: api::ValueType::from(&period.type_), + value: period.value, + } + } +} + +impl<'a> TryFrom<&'a Function<'a>> for api::Function<'a> { + type Error = Utf8Error; + + fn try_from(function: &'a Function<'a>) -> Result { + let name = function.name.try_to_utf8()?; + let system_name = function.system_name.try_to_utf8()?; + let filename = function.filename.try_to_utf8()?; + Ok(Self { + name, + system_name, + filename, + start_line: function.start_line, + }) + } +} + +impl<'a> TryFrom<&'a Line<'a>> for api::Line<'a> { + type Error = Utf8Error; + + fn try_from(line: &'a Line<'a>) -> Result { + Ok(Self { + function: api::Function::try_from(&line.function)?, + line: line.line, + }) + } +} + +impl<'a> TryFrom<&'a Location<'a>> for api::Location<'a> { + type Error = Utf8Error; + + fn try_from(location: &'a Location<'a>) -> Result { + let mapping = api::Mapping::try_from(&location.mapping)?; + let function = api::Function::try_from(&location.function)?; + Ok(Self { + mapping, + function, + address: location.address, + line: location.line, + }) + } +} + +impl<'a> TryFrom<&'a Label<'a>> for api::Label<'a> { + type Error = Utf8Error; + + fn try_from(label: &'a Label<'a>) -> Result { + let key = label.key.try_to_utf8()?; + let str = label.str.try_to_utf8()?; + let str = if str.is_empty() { None } else { Some(str) }; + let num_unit = label.num_unit.try_to_utf8()?; + let num_unit = if num_unit.is_empty() { + None + } else { + Some(num_unit) + }; + + Ok(Self { + key, + str, + num: label.num, + num_unit, + }) + } +} + +impl<'a> TryFrom> for api::Sample<'a> { + type Error = Utf8Error; + + fn try_from(sample: Sample<'a>) -> Result { + let mut locations: Vec = Vec::with_capacity(sample.locations.len()); + + for location in sample.locations.as_slice().iter() { + locations.push(location.try_into()?) + } + + let values: Vec = sample.values.into_slice().to_vec(); + + let mut labels: Vec = Vec::with_capacity(sample.labels.len()); + for label in sample.labels.as_slice().iter() { + labels.push(label.try_into()?); + } + + Ok(Self { + locations, + values, + labels, + }) + } +} diff --git a/profiling-ffi/src/lib.rs b/profiling-ffi/src/lib.rs index f25481447..96458c6e8 100644 --- a/profiling-ffi/src/lib.rs +++ b/profiling-ffi/src/lib.rs @@ -4,6 +4,7 @@ #[cfg(all(feature = "symbolizer", not(target_os = "windows")))] pub use symbolizer_ffi::*; +mod datatypes; mod exporter; mod profiles; diff --git a/profiling-ffi/src/profiles.rs b/profiling-ffi/src/profiles.rs index 6f3b4716b..162e29b4e 100644 --- a/profiling-ffi/src/profiles.rs +++ b/profiling-ffi/src/profiles.rs @@ -1,356 +1,17 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use anyhow::Context; use datadog_profiling::api; use datadog_profiling::internal; +use datadog_profiling::internal::Profile; use datadog_profiling::internal::ProfiledEndpointsStats; use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice}; -use ddcommon_ffi::{Error, Timespec}; +use ddcommon_ffi::{wrap_with_ffi_result, Handle, Timespec}; +use ddcommon_ffi::{wrap_with_void_ffi_result, ToInner, VoidResult}; +use function_name::named; use std::num::NonZeroI64; -use std::str::Utf8Error; use std::time::{Duration, SystemTime}; - -/// Represents a profile. Do not access its member for any reason, only use -/// the C API functions on this struct. -#[repr(C)] -pub struct Profile { - // This may be null, but if not it will point to a valid Profile. - inner: *mut internal::Profile, -} - -impl Profile { - fn new(profile: internal::Profile) -> Self { - Profile { - inner: Box::into_raw(Box::new(profile)), - } - } - - fn take(&mut self) -> Option> { - // Leaving a null will help with double-free issues that can - // arise in C. Of course, it's best to never get there in the - // first place! - let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } - } -} - -impl Drop for Profile { - fn drop(&mut self) { - drop(self.take()) - } -} - -/// A generic result type for when a profiling operation may fail, but there's -/// nothing to return in the case of success. -#[allow(dead_code)] -#[repr(C)] -pub enum ProfileResult { - Ok( - /// Do not use the value of Ok. This value only exists to overcome - /// Rust -> C code generation. - bool, - ), - Err(Error), -} - -impl From> for ProfileResult { - fn from(value: anyhow::Result<()>) -> Self { - match value { - Ok(_) => Self::Ok(true), - Err(err) => Self::Err(err.into()), - } - } -} - -/// Returned by [ddog_prof_Profile_new]. -#[allow(dead_code)] -#[repr(C)] -pub enum ProfileNewResult { - Ok(Profile), - #[allow(dead_code)] - Err(Error), -} - -#[allow(dead_code)] -#[repr(C)] -pub enum SerializeResult { - Ok(EncodedProfile), - Err(Error), -} - -impl From> for SerializeResult { - fn from(value: anyhow::Result) -> Self { - match value { - Ok(e) => Self::Ok(e), - Err(err) => Self::Err(err.into()), - } - } -} - -impl From> for SerializeResult { - fn from(value: anyhow::Result) -> Self { - match value { - Ok(e) => Self::Ok(e.into()), - Err(err) => Self::Err(err.into()), - } - } -} - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct ValueType<'a> { - pub type_: CharSlice<'a>, - pub unit: CharSlice<'a>, -} - -impl<'a> ValueType<'a> { - pub fn new(type_: &'a str, unit: &'a str) -> Self { - Self { - type_: type_.into(), - unit: unit.into(), - } - } -} - -#[repr(C)] -pub struct Period<'a> { - pub type_: ValueType<'a>, - pub value: i64, -} - -#[repr(C)] -#[derive(Copy, Clone, Default)] -pub struct Label<'a> { - pub key: CharSlice<'a>, - - /// At most one of the following must be present - pub str: CharSlice<'a>, - pub num: i64, - - /// Should only be present when num is present. - /// Specifies the units of num. - /// Use arbitrary string (for example, "requests") as a custom count unit. - /// If no unit is specified, consumer may apply heuristic to deduce the unit. - /// Consumers may also interpret units like "bytes" and "kilobytes" as memory - /// units and units like "seconds" and "nanoseconds" as time units, - /// and apply appropriate unit conversions to these. - pub num_unit: CharSlice<'a>, -} - -#[repr(C)] -#[derive(Copy, Clone, Default)] -pub struct Function<'a> { - /// Name of the function, in human-readable form if available. - pub name: CharSlice<'a>, - - /// Name of the function, as identified by the system. - /// For instance, it can be a C++ mangled name. - pub system_name: CharSlice<'a>, - - /// Source file containing the function. - pub filename: CharSlice<'a>, - - /// Line number in source file. - pub start_line: i64, -} - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct Line<'a> { - /// The corresponding profile.Function for this line. - pub function: Function<'a>, - - /// Line number in source code. - pub line: i64, -} - -#[repr(C)] -#[derive(Copy, Clone, Default)] -pub struct Location<'a> { - /// todo: how to handle unknown mapping? - pub mapping: Mapping<'a>, - pub function: Function<'a>, - - /// The instruction address for this location, if available. It - /// should be within [Mapping.memory_start...Mapping.memory_limit] - /// for the corresponding mapping. A non-leaf address may be in the - /// middle of a call instruction. It is up to display tools to find - /// the beginning of the instruction if necessary. - pub address: u64, - pub line: i64, -} - -#[repr(C)] -#[derive(Copy, Clone, Default)] -pub struct Mapping<'a> { - /// Address at which the binary (or DLL) is loaded into memory. - pub memory_start: u64, - - /// The limit of the address range occupied by this mapping. - pub memory_limit: u64, - - /// Offset in the binary that corresponds to the first mapped address. - pub file_offset: u64, - - /// The object this entry is loaded from. This can be a filename on - /// disk for the main binary and shared libraries, or virtual - /// abstractions like "[vdso]". - pub filename: CharSlice<'a>, - - /// A string that uniquely identifies a particular program version - /// with high probability. E.g., for binaries generated by GNU tools, - /// it could be the contents of the .note.gnu.build-id field. - pub build_id: CharSlice<'a>, -} - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct Sample<'a> { - /// The leaf is at locations[0]. - pub locations: Slice<'a, Location<'a>>, - - /// The type and unit of each value is defined by the corresponding - /// entry in Profile.sample_type. All samples must have the same - /// number of values, the same as the length of Profile.sample_type. - /// When aggregating multiple samples into a single sample, the - /// result has a list of values that is the element-wise sum of the - /// lists of the originals. - pub values: Slice<'a, i64>, - - /// label includes additional context for this sample. It can include - /// things like a thread id, allocation size, etc - pub labels: Slice<'a, Label<'a>>, -} - -impl<'a> TryFrom<&'a Mapping<'a>> for api::Mapping<'a> { - type Error = Utf8Error; - - fn try_from(mapping: &'a Mapping<'a>) -> Result { - let filename = mapping.filename.try_to_utf8()?; - let build_id = mapping.build_id.try_to_utf8()?; - Ok(Self { - memory_start: mapping.memory_start, - memory_limit: mapping.memory_limit, - file_offset: mapping.file_offset, - filename, - build_id, - }) - } -} - -impl<'a> From<&'a ValueType<'a>> for api::ValueType<'a> { - fn from(vt: &'a ValueType<'a>) -> Self { - Self::new( - vt.type_.try_to_utf8().unwrap_or(""), - vt.unit.try_to_utf8().unwrap_or(""), - ) - } -} - -impl<'a> From<&'a Period<'a>> for api::Period<'a> { - fn from(period: &'a Period<'a>) -> Self { - Self { - r#type: api::ValueType::from(&period.type_), - value: period.value, - } - } -} - -impl<'a> TryFrom<&'a Function<'a>> for api::Function<'a> { - type Error = Utf8Error; - - fn try_from(function: &'a Function<'a>) -> Result { - let name = function.name.try_to_utf8()?; - let system_name = function.system_name.try_to_utf8()?; - let filename = function.filename.try_to_utf8()?; - Ok(Self { - name, - system_name, - filename, - start_line: function.start_line, - }) - } -} - -impl<'a> TryFrom<&'a Line<'a>> for api::Line<'a> { - type Error = Utf8Error; - - fn try_from(line: &'a Line<'a>) -> Result { - Ok(Self { - function: api::Function::try_from(&line.function)?, - line: line.line, - }) - } -} - -impl<'a> TryFrom<&'a Location<'a>> for api::Location<'a> { - type Error = Utf8Error; - - fn try_from(location: &'a Location<'a>) -> Result { - let mapping = api::Mapping::try_from(&location.mapping)?; - let function = api::Function::try_from(&location.function)?; - Ok(Self { - mapping, - function, - address: location.address, - line: location.line, - }) - } -} - -impl<'a> TryFrom<&'a Label<'a>> for api::Label<'a> { - type Error = Utf8Error; - - fn try_from(label: &'a Label<'a>) -> Result { - let key = label.key.try_to_utf8()?; - let str = label.str.try_to_utf8()?; - let str = if str.is_empty() { None } else { Some(str) }; - let num_unit = label.num_unit.try_to_utf8()?; - let num_unit = if num_unit.is_empty() { - None - } else { - Some(num_unit) - }; - - Ok(Self { - key, - str, - num: label.num, - num_unit, - }) - } -} - -impl<'a> TryFrom> for api::Sample<'a> { - type Error = Utf8Error; - - fn try_from(sample: Sample<'a>) -> Result { - let mut locations: Vec = Vec::with_capacity(sample.locations.len()); - - for location in sample.locations.as_slice().iter() { - locations.push(location.try_into()?) - } - - let values: Vec = sample.values.into_slice().to_vec(); - - let mut labels: Vec = Vec::with_capacity(sample.labels.len()); - for label in sample.labels.as_slice().iter() { - labels.push(label.try_into()?); - } - - Ok(Self { - locations, - values, - labels, - }) - } -} +use super::datatypes::*; /// Create a new profile with the given sample types. Must call /// `ddog_prof_Profile_drop` when you are done with the profile. @@ -366,25 +27,26 @@ impl<'a> TryFrom> for api::Sample<'a> { /// and must have the correct number of elements for the slice. #[no_mangle] #[must_use] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_new( sample_types: Slice, period: Option<&Period>, start_time: Option<&Timespec>, -) -> ProfileNewResult { - let types: Vec = sample_types.into_slice().iter().map(Into::into).collect(); - let start_time = start_time.map_or_else(SystemTime::now, SystemTime::from); - let period = period.map(Into::into); - - let internal_profile = internal::Profile::new(start_time, &types, period); - let ffi_profile = Profile::new(internal_profile); - ProfileNewResult::Ok(ffi_profile) +) -> ddcommon_ffi::Result> { + wrap_with_ffi_result!({ + let types: Vec = sample_types.into_slice().iter().map(Into::into).collect(); + let start_time = start_time.map_or_else(SystemTime::now, SystemTime::from); + let period = period.map(Into::into); + + anyhow::Ok(Profile::new(start_time, &types, period).into()) + }) } /// # Safety /// The `profile` can be null, but if non-null it must point to a Profile /// made by this module, which has not previously been dropped. #[no_mangle] -pub unsafe extern "C" fn ddog_prof_Profile_drop(profile: *mut Profile) { +pub unsafe extern "C" fn ddog_prof_Profile_drop(profile: *mut Handle) { // Technically, this function has been designed so if it's double-dropped // then it's okay, but it's not something that should be relied on. if !profile.is_null() { @@ -392,26 +54,6 @@ pub unsafe extern "C" fn ddog_prof_Profile_drop(profile: *mut Profile) { } } -#[cfg(test)] -impl From for Result<(), Error> { - fn from(result: ProfileResult) -> Self { - match result { - ProfileResult::Ok(_) => Ok(()), - ProfileResult::Err(err) => Err(err), - } - } -} - -#[cfg(test)] -impl From for Result { - fn from(result: ProfileNewResult) -> Self { - match result { - ProfileNewResult::Ok(p) => Ok(p), - ProfileNewResult::Err(err) => Err(err), - } - } -} - /// # Safety /// The `profile` ptr must point to a valid Profile object created by this /// module. All pointers inside the `sample` need to be valid for the duration @@ -426,30 +68,17 @@ impl From for Result { /// This call is _NOT_ thread-safe. #[must_use] #[no_mangle] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_add( - profile: *mut Profile, + mut profile: *mut Handle, sample: Sample, timestamp: Option, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; - let sample = sample.try_into()?; - profile.add_sample(sample, timestamp) - })() - .context("ddog_prof_Profile_add failed") - .into() -} - -unsafe fn profile_ptr_to_inner<'a>( - profile_ptr: *mut Profile, -) -> anyhow::Result<&'a mut internal::Profile> { - match profile_ptr.as_mut() { - None => anyhow::bail!("profile pointer was null"), - Some(inner_ptr) => match inner_ptr.inner.as_mut() { - Some(profile) => Ok(profile), - None => anyhow::bail!("profile's inner pointer was null (indicates use-after-free)"), - }, - } +) -> VoidResult { + wrap_with_void_ffi_result!({ + profile + .to_inner_mut()? + .add_sample(sample.try_into()?, timestamp)? + }) } /// Associate an endpoint to a given local root span id. @@ -470,18 +99,17 @@ unsafe fn profile_ptr_to_inner<'a>( /// This call is _NOT_ thread-safe. #[no_mangle] #[must_use] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_set_endpoint( - profile: *mut Profile, + mut profile: *mut Handle, local_root_span_id: u64, endpoint: CharSlice, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; - let endpoint = endpoint.to_utf8_lossy(); - profile.add_endpoint(local_root_span_id, endpoint) - })() - .context("ddog_prof_Profile_set_endpoint failed") - .into() +) -> VoidResult { + wrap_with_void_ffi_result!({ + profile + .to_inner_mut()? + .add_endpoint(local_root_span_id, endpoint.to_utf8_lossy())? + }) } /// Count the number of times an endpoint has been seen. @@ -496,18 +124,17 @@ pub unsafe extern "C" fn ddog_prof_Profile_set_endpoint( /// This call is _NOT_ thread-safe. #[no_mangle] #[must_use] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_add_endpoint_count( - profile: *mut Profile, + mut profile: *mut Handle, endpoint: CharSlice, value: i64, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; - let endpoint = endpoint.to_utf8_lossy(); - profile.add_endpoint_count(endpoint, value) - })() - .context("ddog_prof_Profile_set_endpoint failed") - .into() +) -> VoidResult { + wrap_with_void_ffi_result!({ + profile + .to_inner_mut()? + .add_endpoint_count(endpoint.to_utf8_lossy(), value)? + }) } /// Add a poisson-based upscaling rule which will be use to adjust values and make them @@ -532,33 +159,29 @@ pub unsafe extern "C" fn ddog_prof_Profile_add_endpoint_count( /// This call is _NOT_ thread-safe. #[must_use] #[no_mangle] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_add_upscaling_rule_poisson( - profile: *mut Profile, + mut profile: *mut Handle, offset_values: Slice, label_name: CharSlice, label_value: CharSlice, sum_value_offset: usize, count_value_offset: usize, sampling_distance: u64, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; +) -> VoidResult { + wrap_with_void_ffi_result!({ anyhow::ensure!(sampling_distance != 0, "sampling_distance must not be 0"); - let upscaling_info = api::UpscalingInfo::Poisson { - sum_value_offset, - count_value_offset, - sampling_distance, - }; - add_upscaling_rule( - profile, - offset_values, - label_name, - label_value, - upscaling_info, - ) - })() - .context("ddog_prof_Profile_add_upscaling_rule_proportional failed") - .into() + profile.to_inner_mut()?.add_upscaling_rule( + offset_values.as_slice(), + &label_name.try_to_string()?, + &label_value.try_to_string()?, + api::UpscalingInfo::Poisson { + sum_value_offset, + count_value_offset, + sampling_distance, + }, + )? + }) } /// Add a proportional-based upscaling rule which will be use to adjust values and make them @@ -581,48 +204,27 @@ pub unsafe extern "C" fn ddog_prof_Profile_add_upscaling_rule_poisson( /// This call is _NOT_ thread-safe. #[must_use] #[no_mangle] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_add_upscaling_rule_proportional( - profile: *mut Profile, + mut profile: *mut Handle, offset_values: Slice, label_name: CharSlice, label_value: CharSlice, total_sampled: u64, total_real: u64, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; +) -> VoidResult { + wrap_with_void_ffi_result!({ anyhow::ensure!(total_sampled != 0, "total_sampled must not be 0"); anyhow::ensure!(total_real != 0, "total_real must not be 0"); - let upscaling_info = api::UpscalingInfo::Proportional { - scale: total_real as f64 / total_sampled as f64, - }; - add_upscaling_rule( - profile, - offset_values, - label_name, - label_value, - upscaling_info, - ) - })() - .context("ddog_prof_Profile_add_upscaling_rule_proportional failed") - .into() -} - -unsafe fn add_upscaling_rule( - profile: &mut internal::Profile, - offset_values: Slice, - label_name: CharSlice, - label_value: CharSlice, - upscaling_info: api::UpscalingInfo, -) -> anyhow::Result<()> { - let label_name_n = label_name.to_utf8_lossy(); - let label_value_n = label_value.to_utf8_lossy(); - profile.add_upscaling_rule( - offset_values.as_slice(), - label_name_n.as_ref(), - label_value_n.as_ref(), - upscaling_info, - ) + profile.to_inner_mut()?.add_upscaling_rule( + offset_values.as_slice(), + &label_name.try_to_string()?, + &label_value.try_to_string()?, + api::UpscalingInfo::Proportional { + scale: total_real as f64 / total_sampled as f64, + }, + )? + }) } #[repr(C)] @@ -686,27 +288,28 @@ impl From for EncodedProfile { /// The `duration_nanos` must be null or otherwise point to a valid i64. #[must_use] #[no_mangle] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_serialize( - profile: *mut Profile, + mut profile: *mut Handle, end_time: Option<&Timespec>, duration_nanos: Option<&i64>, start_time: Option<&Timespec>, -) -> SerializeResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; - - let start_time = start_time.map(SystemTime::from); - let old_profile = profile.reset_and_return_previous(start_time)?; - let end_time = end_time.map(SystemTime::from); +) -> ddcommon_ffi::Result { + wrap_with_ffi_result!({ + let old_profile = profile + .to_inner_mut()? + .reset_and_return_previous(start_time.map(SystemTime::from))?; let duration = match duration_nanos { None => None, Some(x) if *x < 0 => None, Some(x) => Some(Duration::from_nanos((*x) as u64)), }; - old_profile.serialize_into_compressed_pprof(end_time, duration) - })() - .context("ddog_prof_Profile_serialize failed") - .into() + anyhow::Ok( + old_profile + .serialize_into_compressed_pprof(end_time.map(SystemTime::from), duration)? + .into(), + ) + }) } #[must_use] @@ -729,17 +332,16 @@ pub unsafe extern "C" fn ddog_Vec_U8_as_slice(vec: &ddcommon_ffi::Vec) -> Sl /// If `time` is not null, it must point to a valid Timespec object. #[no_mangle] #[must_use] +#[named] pub unsafe extern "C" fn ddog_prof_Profile_reset( - profile: *mut Profile, + mut profile: *mut Handle, start_time: Option<&Timespec>, -) -> ProfileResult { - (|| { - let profile = profile_ptr_to_inner(profile)?; - profile.reset_and_return_previous(start_time.map(SystemTime::from))?; - anyhow::Ok(()) - })() - .context("ddog_prof_Profile_reset failed") - .into() +) -> VoidResult { + wrap_with_void_ffi_result!({ + profile + .to_inner_mut()? + .reset_and_return_previous(start_time.map(SystemTime::from))? + }) } #[cfg(test)] @@ -747,10 +349,10 @@ mod tests { use super::*; #[test] - fn ctor_and_dtor() -> Result<(), Error> { + fn ctor_and_dtor() -> anyhow::Result<()> { unsafe { let sample_type: *const ValueType = &ValueType::new("samples", "count"); - let mut profile = Result::from(ddog_prof_Profile_new( + let mut profile = anyhow::Result::from(ddog_prof_Profile_new( Slice::from_raw_parts(sample_type, 1), None, None, @@ -761,10 +363,10 @@ mod tests { } #[test] - fn add_failure() -> Result<(), Error> { + fn add_failure() -> anyhow::Result<()> { unsafe { let sample_type: *const ValueType = &ValueType::new("samples", "count"); - let mut profile = Result::from(ddog_prof_Profile_new( + let mut profile = anyhow::Result::from(ddog_prof_Profile_new( Slice::from_raw_parts(sample_type, 1), None, None, @@ -779,7 +381,7 @@ mod tests { labels: Slice::empty(), }; - let result = Result::from(ddog_prof_Profile_add(&mut profile, sample, None)); + let result = anyhow::Result::from(ddog_prof_Profile_add(&mut profile, sample, None)); result.unwrap_err(); ddog_prof_Profile_drop(&mut profile); Ok(()) @@ -793,7 +395,7 @@ mod tests { fn aggregate_samples() -> anyhow::Result<()> { unsafe { let sample_type: *const ValueType = &ValueType::new("samples", "count"); - let mut profile = Result::from(ddog_prof_Profile_new( + let mut profile = anyhow::Result::from(ddog_prof_Profile_new( Slice::from_raw_parts(sample_type, 1), None, None, @@ -827,22 +429,18 @@ mod tests { labels: Slice::from(&labels), }; - Result::from(ddog_prof_Profile_add(&mut profile, sample, None))?; + anyhow::Result::from(ddog_prof_Profile_add(&mut profile, sample, None))?; assert_eq!( profile - .inner - .as_ref() - .unwrap() + .to_inner()? .only_for_testing_num_aggregated_samples(), 1 ); - Result::from(ddog_prof_Profile_add(&mut profile, sample, None))?; + anyhow::Result::from(ddog_prof_Profile_add(&mut profile, sample, None))?; assert_eq!( profile - .inner - .as_ref() - .unwrap() + .to_inner()? .only_for_testing_num_aggregated_samples(), 1 ); @@ -852,7 +450,7 @@ mod tests { } } - unsafe fn provide_distinct_locations_ffi() -> Profile { + unsafe fn provide_distinct_locations_ffi() -> anyhow::Result> { let sample_type: *const ValueType = &ValueType::new("samples", "count"); let mut profile = Result::from(ddog_prof_Profile_new( Slice::from_raw_parts(sample_type, 1), @@ -907,33 +505,29 @@ mod tests { labels: Slice::from(labels.as_slice()), }; - Result::from(ddog_prof_Profile_add(&mut profile, main_sample, None)).unwrap(); + anyhow::Result::from(ddog_prof_Profile_add(&mut profile, main_sample, None)).unwrap(); assert_eq!( profile - .inner - .as_ref() - .unwrap() + .to_inner()? .only_for_testing_num_aggregated_samples(), 1 ); - Result::from(ddog_prof_Profile_add(&mut profile, test_sample, None)).unwrap(); + anyhow::Result::from(ddog_prof_Profile_add(&mut profile, test_sample, None)).unwrap(); assert_eq!( profile - .inner - .as_ref() - .unwrap() + .to_inner()? .only_for_testing_num_aggregated_samples(), 2 ); - profile + Ok(profile) } #[test] fn distinct_locations_ffi() { unsafe { - ddog_prof_Profile_drop(&mut provide_distinct_locations_ffi()); + ddog_prof_Profile_drop(&mut provide_distinct_locations_ffi().unwrap()); } } }