From 3b474e109dbe87ee2886a1e6da097f82774a1706 Mon Sep 17 00:00:00 2001 From: "erwan.viollet" Date: Tue, 7 Jul 2026 10:12:19 +0200 Subject: [PATCH 1/5] feat(profiling): add extensible profile type API for prototyping Allow callers to create profiles with arbitrary (type, unit) string pairs without adding a variant to SampleType or cutting a libdatadog release. - Internal Profile storage changed from Box<[SampleType]> to Box<[ValueType<'static>]>, making the enum a pure conversion layer - Profile::try_new_with_value_types / try_new_with_value_types_and_dictionary added as the Rust raw-string path - ddog_prof_Profile_new_custom + CustomValueType / CustomPeriod structs added to the C FFI - Profile::create_with_value_types added to the CXX bridge - Replayer updated to round-trip unknown types via Box::leak (short-lived tool) - ExperimentalCount / ExperimentalNanoseconds / ExperimentalBytes removed: they mapped to fixed strings and could not distinguish multiple custom measurements of the same unit - examples/ffi/custom_profile_types.c demonstrates the new C API - Four new unit tests cover round-trip serialization, reset survival, value-count validation, and parity with the SampleType path --- datadog-profiling-replayer/src/main.rs | 8 +- datadog-profiling-replayer/src/replayer.rs | 37 ++- examples/cxx/profiling.cpp | 5 +- examples/ffi/CMakeLists.txt | 4 + examples/ffi/custom_profile_types.c | 128 ++++++++++ libdd-profiling-ffi/src/profiles/datatypes.rs | 151 ++++++++++++ libdd-profiling/src/api/sample_type.rs | 17 +- libdd-profiling/src/cxx.rs | 89 ++++++- libdd-profiling/src/internal/profile/mod.rs | 224 ++++++++++++++++-- 9 files changed, 623 insertions(+), 40 deletions(-) create mode 100644 examples/ffi/custom_profile_types.c diff --git a/datadog-profiling-replayer/src/main.rs b/datadog-profiling-replayer/src/main.rs index 6a8ae2b32f..ed444bf4ed 100644 --- a/datadog-profiling-replayer/src/main.rs +++ b/datadog-profiling-replayer/src/main.rs @@ -163,9 +163,11 @@ fn main() -> anyhow::Result<()> { let mut replayer = Replayer::try_from(&pprof)?; - let mut outprof = - libdd_profiling::internal::Profile::try_new(&replayer.sample_types, replayer.period)? - .with_start_time(replayer.start_time)?; + let mut outprof = libdd_profiling::internal::Profile::try_new_with_value_types( + &replayer.sample_types, + replayer.period, + )? + .with_start_time(replayer.start_time)?; // Before benchmarking, let's calculate some statistics. // No point doing that if there aren't at least 4 samples though. diff --git a/datadog-profiling-replayer/src/replayer.rs b/datadog-profiling-replayer/src/replayer.rs index 6908645ee7..e694f57008 100644 --- a/datadog-profiling-replayer/src/replayer.rs +++ b/datadog-profiling-replayer/src/replayer.rs @@ -24,8 +24,8 @@ pub struct Replayer<'pprof> { pub start_time: SystemTime, pub duration: Duration, pub end_time: SystemTime, // start_time + duration - pub sample_types: Vec, - pub period: Option, + pub sample_types: Vec>, + pub period: Option<(api::ValueType<'static>, i64)>, pub endpoints: Vec<(u64, &'pprof str)>, pub samples: Vec<(Option, api::Sample<'pprof>)>, } @@ -55,29 +55,48 @@ impl<'pprof> Replayer<'pprof> { } } + /// Convert a (type_str, unit) pair read from a pprof file into a + /// `ValueType<'static>`. + /// + /// Known types are resolved through [`api::SampleType`] so the returned + /// references point to existing static string literals (zero allocation). + /// Strings that are not yet in the enum – i.e. custom / prototype types – + /// are promoted to `'static` via [`Box::leak`]. The replayer is a + /// short-lived tool; the bounded number of leaked allocations is acceptable. + fn to_static_value_type(type_str: &str, unit: &str) -> api::ValueType<'static> { + let borrowed = api::ValueType::new(type_str, unit); + // Fast path: resolve via the stable enum (returns 'static string literals). + if let Ok(st) = api::SampleType::try_from(borrowed) { + return api::ValueType::from(st); + } + // Slow path: custom type not yet in the enum – own the strings. + let ty: &'static str = Box::leak(type_str.to_string().into_boxed_str()); + let unit: &'static str = Box::leak(unit.to_string().into_boxed_str()); + api::ValueType::new(ty, unit) + } + fn sample_types<'a>( profile_index: &'a ProfileIndex<'pprof>, - ) -> anyhow::Result> { + ) -> anyhow::Result>> { let mut sample_types = Vec::with_capacity(profile_index.pprof.sample_types.len()); for sample_type in profile_index.pprof.sample_types.iter() { let type_str = profile_index.get_string(sample_type.r#type)?; let unit = profile_index.get_string(sample_type.unit)?; - let vt = api::ValueType::new(type_str, unit); - sample_types.push(vt.try_into()?); + sample_types.push(Self::to_static_value_type(type_str, unit)); } Ok(sample_types) } - fn period<'a>(profile_index: &'a ProfileIndex<'pprof>) -> anyhow::Result> { + fn period<'a>( + profile_index: &'a ProfileIndex<'pprof>, + ) -> anyhow::Result, i64)>> { let value = profile_index.pprof.period; match profile_index.pprof.period_type { Some(period_type) => { let type_str = profile_index.get_string(period_type.r#type)?; let unit = profile_index.get_string(period_type.unit)?; - let vt = api::ValueType::new(type_str, unit); - let sample_type = vt.try_into()?; - Ok(Some(api::Period { sample_type, value })) + Ok(Some((Self::to_static_value_type(type_str, unit), value))) } None => Ok(None), } diff --git a/examples/cxx/profiling.cpp b/examples/cxx/profiling.cpp index a309712b18..8ea27fa128 100644 --- a/examples/cxx/profiling.cpp +++ b/examples/cxx/profiling.cpp @@ -21,9 +21,8 @@ int main() { .value = 60 }; - // Create profile with predefined sample types - // Note: ExperimentalCount, ExperimentalNanoseconds, and ExperimentalBytes - // are available for custom profiling metrics + // Create profile with predefined sample types. For prototyping a type + // not yet in SampleType, use Profile::create_with_value_types instead. auto profile = Profile::create({SampleType::WallTime}, period); std::cout << "✅ Profile created" << std::endl; diff --git a/examples/ffi/CMakeLists.txt b/examples/ffi/CMakeLists.txt index c6e1988001..60a061278f 100644 --- a/examples/ffi/CMakeLists.txt +++ b/examples/ffi/CMakeLists.txt @@ -56,6 +56,10 @@ add_executable(profiles profiles.c) target_link_libraries(profiles PRIVATE Datadog::Profiling) set_vcruntime_link_type(profiles ${VCRUNTIME_LINK_TYPE}) +add_executable(custom_profile_types custom_profile_types.c) +target_link_libraries(custom_profile_types PRIVATE Datadog::Profiling) +set_vcruntime_link_type(custom_profile_types ${VCRUNTIME_LINK_TYPE}) + if(BUILD_SYMBOLIZER) add_executable(symbolizer symbolizer.cpp) target_link_libraries(symbolizer PRIVATE Datadog::Profiling) diff --git a/examples/ffi/custom_profile_types.c b/examples/ffi/custom_profile_types.c new file mode 100644 index 0000000000..82a403c48d --- /dev/null +++ b/examples/ffi/custom_profile_types.c @@ -0,0 +1,128 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +// Example: creating a profile with custom (type, unit) pairs. +// +// Use ddog_prof_Profile_new_custom when the desired sample type is not yet in +// the ddog_prof_SampleType enum. All type/unit strings must be program-lifetime +// constants (string literals in practice). +// +// TODO: Once custom profile types are stable and agreed upon across profiler +// teams, add them to ddog_prof_SampleType and switch callers to +// ddog_prof_Profile_new / ddog_prof_Profile_with_dictionary. + +#include +#include +#include + +static int check_profile_result(ddog_prof_Profile_Result result, const char *context) { + if (result.tag != DDOG_PROF_PROFILE_RESULT_OK) { + ddog_CharSlice msg = ddog_Error_message(&result.err); + fprintf(stderr, "%s: %.*s\n", context, (int)msg.len, msg.ptr); + ddog_Error_drop(&result.err); + return 0; + } + return 1; +} + +int main(void) { + // ------------------------------------------------------------------------- + // 1. Declare custom sample types as static string literals. + // These bypass the ddog_prof_SampleType enum so that new types can be + // prototyped without a libdatadog release. + // ------------------------------------------------------------------------- + const ddog_prof_CustomValueType sample_types[] = { + {.type_str = DDOG_CHARSLICE_C("memory-breakdown"), + .unit = DDOG_CHARSLICE_C("bytes")}, + }; + const ddog_prof_Slice_CustomValueType sample_types_slice = { + .ptr = sample_types, + .len = sizeof(sample_types) / sizeof(sample_types[0]), + }; + + // The profile period is optional. Pass NULL when there is no meaningful + // sampling interval to report for the custom type. + + // ------------------------------------------------------------------------- + // 2. Create the profile. + // ------------------------------------------------------------------------- + ddog_prof_Profile_NewResult new_result = + ddog_prof_Profile_new_custom(sample_types_slice, NULL); + if (new_result.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { + ddog_CharSlice msg = ddog_Error_message(&new_result.err); + fprintf(stderr, "ddog_prof_Profile_new_custom failed: %.*s\n", (int)msg.len, msg.ptr); + ddog_Error_drop(&new_result.err); + return EXIT_FAILURE; + } + ddog_prof_Profile profile = new_result.ok; + + if (!check_profile_result( + ddog_prof_Profile_set_omit_local_root_span_id_when_serializing(&profile, true), + "set_omit_local_root_span_id")) { + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + + // ------------------------------------------------------------------------- + // 3. Add a sample (one value per sample type). + // Separate stacks / labels can distinguish anonymous, file-backed, JIT, + // or other memory categories while sharing the same sample type. + // ------------------------------------------------------------------------- + ddog_prof_Location location = { + .mapping = (ddog_prof_Mapping){0}, + .function = {.name = DDOG_CHARSLICE_C("my_alloc_function"), + .filename = DDOG_CHARSLICE_C("/src/allocator.c")}, + }; + int64_t values[] = {4096}; + const ddog_prof_Sample sample = { + .locations = {&location, 1}, + .values = {values, sizeof(values) / sizeof(values[0])}, + .labels = {NULL, 0}, + }; + + if (!check_profile_result(ddog_prof_Profile_add(&profile, sample, 0), + "ddog_prof_Profile_add")) { + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + + // ------------------------------------------------------------------------- + // 4. Serialize and verify we get back a non-empty buffer. + // ------------------------------------------------------------------------- + ddog_prof_Profile_SerializeResult ser_result = + ddog_prof_Profile_serialize(&profile, NULL, NULL); + if (ser_result.tag != DDOG_PROF_PROFILE_SERIALIZE_RESULT_OK) { + ddog_CharSlice msg = ddog_Error_message(&ser_result.err); + fprintf(stderr, "serialize failed: %.*s\n", (int)msg.len, msg.ptr); + ddog_Error_drop(&ser_result.err); + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + ddog_prof_EncodedProfile encoded = ser_result.ok; + + ddog_prof_Result_ByteSlice buf_result = ddog_prof_EncodedProfile_bytes(&encoded); + if (buf_result.tag != DDOG_PROF_RESULT_BYTE_SLICE_OK_BYTE_SLICE) { + ddog_CharSlice msg = ddog_Error_message(&buf_result.err); + fprintf(stderr, "EncodedProfile_bytes failed: %.*s\n", (int)msg.len, msg.ptr); + ddog_Error_drop(&buf_result.err); + ddog_prof_EncodedProfile_drop(&encoded); + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + + if (buf_result.ok.len == 0) { + fprintf(stderr, "serialize returned an empty buffer\n"); + ddog_prof_EncodedProfile_drop(&encoded); + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + + fprintf(stdout, "custom_profile_types: serialized %zu bytes\n", (size_t)buf_result.ok.len); + + // ------------------------------------------------------------------------- + // 5. Clean up. + // ------------------------------------------------------------------------- + ddog_prof_EncodedProfile_drop(&encoded); + ddog_prof_Profile_drop(&profile); + return EXIT_SUCCESS; +} diff --git a/libdd-profiling-ffi/src/profiles/datatypes.rs b/libdd-profiling-ffi/src/profiles/datatypes.rs index 22d00ef9c0..d88f0c463b 100644 --- a/libdd-profiling-ffi/src/profiles/datatypes.rs +++ b/libdd-profiling-ffi/src/profiles/datatypes.rs @@ -105,6 +105,31 @@ pub use api::SampleType; /// The FFI Period is identical to the API Period. pub use api::Period; +/// A raw (type, unit) pair for profile types not yet in [`SampleType`]. +/// +/// Both `type_str` and `unit` must point to program-lifetime memory (C string +/// literals are the typical case). This is the escape hatch for prototyping +/// new profile types without a libdatadog release. +/// +/// # Stability note +/// Once a custom type is agreed upon across profiler teams, add it to +/// [`SampleType`] and migrate callers to [`ddog_prof_Profile_new`] instead. +#[repr(C)] +#[derive(Copy, Clone)] +pub struct CustomValueType<'a> { + pub type_str: CharSlice<'a>, + pub unit: CharSlice<'a>, +} + +/// Period expressed as a raw (type, unit) string pair for use with +/// [`ddog_prof_Profile_new_custom`]. +#[repr(C)] +#[derive(Copy, Clone)] +pub struct CustomPeriod<'a> { + pub value_type: CustomValueType<'a>, + pub value: i64, +} + #[repr(C)] #[derive(Copy, Clone, Default)] pub struct Label<'a> { @@ -409,6 +434,79 @@ pub unsafe extern "C" fn ddog_prof_Profile_new( profile_new(sample_types, period, None) } +/// Create a new profile using raw `(type, unit)` string pairs, bypassing the +/// [`SampleType`] enum. Must call `ddog_prof_Profile_drop` when done. +/// +/// Use this when the desired profile type is not yet in [`SampleType`]. Once +/// the type is stable add it to [`SampleType`] and switch to +/// [`ddog_prof_Profile_new`]. +/// +/// # Arguments +/// * `sample_types` - Slice of [`CustomValueType`]. Every `type_str` and `unit` pointer must be +/// valid for the lifetime of the program (C string literals are the typical case). +/// * `period` - Optional period. Same lifetime requirement as `sample_types`. +/// +/// # Safety +/// All slices must have pointers that are suitably aligned for their type and +/// must have the correct number of elements. The string pointers inside each +/// [`CustomValueType`] must point to valid UTF-8 memory that lives at least as +/// long as the program (i.e. string literals). +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_prof_Profile_new_custom( + sample_types: Slice>, + period: Option<&CustomPeriod<'_>>, +) -> ProfileNewResult { + // SAFETY: Profile type strings are program-lifetime constants (string + // literals). Extending to 'static is sound because the strings must + // outlive the Profile, and a Profile is always dropped before the program + // exits. The caller documents this requirement. + unsafe fn to_static_str(slice: CharSlice<'_>) -> Result<&'static str, anyhow::Error> { + let s = slice + .try_to_utf8() + .map_err(|e| anyhow::anyhow!("invalid UTF-8 in profile type string: {}", e))?; + Ok(std::mem::transmute::<&str, &'static str>(s)) + } + + let raw_types = match sample_types.try_as_slice() { + Ok(s) => s, + Err(e) => return ProfileNewResult::Err(anyhow::Error::from(e).into()), + }; + + let mut vts: Vec> = Vec::with_capacity(raw_types.len()); + for ct in raw_types { + let ty = match to_static_str(ct.type_str) { + Ok(s) => s, + Err(e) => return ProfileNewResult::Err(e.into()), + }; + let unit = match to_static_str(ct.unit) { + Ok(s) => s, + Err(e) => return ProfileNewResult::Err(e.into()), + }; + vts.push(api::ValueType::new(ty, unit)); + } + + let period_vt = match period { + None => None, + Some(p) => { + let ty = match to_static_str(p.value_type.type_str) { + Ok(s) => s, + Err(e) => return ProfileNewResult::Err(e.into()), + }; + let unit = match to_static_str(p.value_type.unit) { + Ok(s) => s, + Err(e) => return ProfileNewResult::Err(e.into()), + }; + Some((api::ValueType::new(ty, unit), p.value)) + } + }; + + match internal::Profile::try_new_with_value_types(&vts, period_vt) { + Ok(inner) => ProfileNewResult::Ok(Profile::new(inner)), + Err(e) => ProfileNewResult::Err(anyhow::Error::from(e).into()), + } +} + /// Create a new profile with the given sample types. Must call /// `ddog_prof_Profile_drop` when you are done with the profile. /// @@ -956,6 +1054,59 @@ mod tests { } } + #[test] + fn profile_new_custom_accepts_raw_value_type_without_period() -> Result<(), Error> { + unsafe { + let sample_type = CustomValueType { + type_str: "memory-breakdown".into(), + unit: "bytes".into(), + }; + let mut profile = Result::from(ddog_prof_Profile_new_custom( + Slice::from_raw_parts(&sample_type, 1), + None, + ))?; + + let values = [4096_i64]; + let sample = Sample { + locations: Slice::empty(), + values: Slice::from(&values[..]), + labels: Slice::empty(), + }; + Result::from(ddog_prof_Profile_add(&mut profile, sample, None))?; + ddog_prof_Profile_drop(&mut profile); + Ok(()) + } + } + + #[test] + fn profile_new_custom_invalid_sample_types_slice_returns_err() { + unsafe { + let bad_slice: Slice<'_, CustomValueType<'_>> = + Slice::from_raw_parts(std::ptr::null(), 1); + let result = ddog_prof_Profile_new_custom(bad_slice, None); + assert!( + matches!(result, ProfileNewResult::Err(_)), + "expected Err for null pointer with non-zero length (SliceConversionError::NullPointer)" + ); + } + } + + #[test] + fn profile_new_custom_invalid_utf8_returns_err() { + unsafe { + let invalid = [0xff_u8 as std::ffi::c_char]; + let sample_type = CustomValueType { + type_str: Slice::from_raw_parts(invalid.as_ptr(), invalid.len()), + unit: "bytes".into(), + }; + let result = ddog_prof_Profile_new_custom(Slice::from_raw_parts(&sample_type, 1), None); + assert!( + matches!(result, ProfileNewResult::Err(_)), + "expected Err for invalid UTF-8 in custom profile type" + ); + } + } + #[test] fn add_failure() -> Result<(), Error> { unsafe { diff --git a/libdd-profiling/src/api/sample_type.rs b/libdd-profiling/src/api/sample_type.rs index 4e1156b4bb..02c936c53d 100644 --- a/libdd-profiling/src/api/sample_type.rs +++ b/libdd-profiling/src/api/sample_type.rs @@ -12,6 +12,15 @@ use super::ValueType; /// - **dd-trace-dotnet**: Sample type definitions for allocations, locks, CPU, walltime, /// exceptions, live objects, HTTP requests /// - **pprof-nodejs**: `profile-serializer.ts` (value type functions) +/// +/// # Adding new types +/// +/// To prototype a new profile type without a libdatadog release, use +/// [`crate::internal::Profile::try_new_with_value_types`] (Rust) or the +/// `create_with_value_types` CXX / `ddog_prof_Profile_new_custom` C functions +/// with a raw `(type, unit)` string pair. +/// Once the type is stable and agreed upon across profiler teams, add a +/// variant here so callers can use the type-safe path. #[cfg_attr(test, derive(bolero::generator::TypeGenerator, strum::EnumIter))] #[repr(C)] #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] @@ -74,10 +83,14 @@ pub enum SampleType { WallTime, /// Legacy: Use `WallTime` instead for consistency with naming scheme WallLegacy, - - // Experimental sample types for testing and development. + /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` + /// pair instead. This fixed name cannot distinguish multiple custom count measurements. ExperimentalCount, + /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` + /// pair instead. This fixed name cannot distinguish multiple custom time measurements. ExperimentalNanoseconds, + /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` + /// pair instead. This fixed name cannot distinguish multiple custom byte measurements. ExperimentalBytes, } diff --git a/libdd-profiling/src/cxx.rs b/libdd-profiling/src/cxx.rs index 312b1c636d..c9b6a0bce5 100644 --- a/libdd-profiling/src/cxx.rs +++ b/libdd-profiling/src/cxx.rs @@ -26,8 +26,7 @@ pub mod ffi { // live objects, HTTP requests) // - pprof-nodejs (profile-serializer.ts value type functions) // - // Experimental variants allow profilers to use custom sample types for testing - // and development without modifying this enum. They map to fixed (type, unit) pairs. + // To use a type not yet in this enum, call create_with_value_types() instead. // // LEGACY VARIANTS (prefer alternatives for consistency): // - CpuLegacy (use CpuTime) @@ -88,10 +87,10 @@ pub mod ffi { Timeline, WallSamples, WallTime, - WallLegacy, // LEGACY: Use WallTime instead - ExperimentalCount, - ExperimentalNanoseconds, - ExperimentalBytes, + WallLegacy, // LEGACY: Use WallTime instead + ExperimentalCount, // DEPRECATED: Use create_with_value_types instead + ExperimentalNanoseconds, // DEPRECATED: Use create_with_value_types instead + ExperimentalBytes, // DEPRECATED: Use create_with_value_types instead } struct Period { @@ -99,6 +98,24 @@ pub mod ffi { value: i64, } + /// A raw (type, unit) pair for profile types not yet in the [`SampleType`] enum. + /// + /// Strings are copied during profile construction. This is the escape hatch for + /// prototyping new profile types without a libdatadog release. + /// + /// # Stability note + /// Once a custom type is agreed upon across profiler teams, add it to + /// [`SampleType`] and migrate callers to [`Profile::create`] instead. + struct CustomValueType<'a> { + type_: &'a str, + unit: &'a str, + } + + struct CustomPeriod<'a> { + value_type: CustomValueType<'a>, + value: i64, + } + struct Mapping<'a> { memory_start: u64, memory_limit: u64, @@ -160,6 +177,26 @@ pub mod ffi { #[Self = "Profile"] fn create(sample_types: Vec, period: &Period) -> Result>; + /// Create a profile using raw `(type, unit)` string pairs. + /// + /// Use this when the desired profile type is not yet in [`SampleType`]. + /// Strings are copied during profile construction. + /// + /// # Stability note + /// Once the type is stable, add it to [`SampleType`] and switch to + /// [`Profile::create`]. + #[Self = "Profile"] + fn create_with_value_types( + sample_types: Vec, + period: &CustomPeriod, + ) -> Result>; + + /// Create a profile using raw `(type, unit)` string pairs and no period. + #[Self = "Profile"] + fn create_with_value_types_no_period( + sample_types: Vec, + ) -> Result>; + // Profile methods fn add_sample(self: &mut Profile, sample: &Sample) -> Result<()>; fn add_endpoint(self: &mut Profile, local_root_span_id: u64, endpoint: &str) -> Result<()>; @@ -524,6 +561,12 @@ pub struct Profile { } impl Profile { + fn owned_value_type(type_: &str, unit: &str) -> api::ValueType<'static> { + let type_: &'static str = Box::leak(type_.to_string().into_boxed_str()); + let unit: &'static str = Box::leak(unit.to_string().into_boxed_str()); + api::ValueType::new(type_, unit) + } + pub fn create( sample_types: Vec, period: &ffi::Period, @@ -541,6 +584,40 @@ impl Profile { Ok(Box::new(Profile { inner })) } + /// Create a profile with raw `(type, unit)` string pairs. + /// + /// This is the escape hatch for profile types not yet in [`ffi::SampleType`]. + /// Use [`Profile::create`] once the type has been promoted to [`ffi::SampleType`]. + pub fn create_with_value_types( + sample_types: Vec>, + period: &ffi::CustomPeriod<'_>, + ) -> anyhow::Result> { + let vts: Vec> = sample_types + .iter() + .map(|st| Self::owned_value_type(st.type_, st.unit)) + .collect(); + let period_value = ( + Self::owned_value_type(period.value_type.type_, period.value_type.unit), + period.value, + ); + + let inner = internal::Profile::try_new_with_value_types(&vts, Some(period_value))?; + Ok(Box::new(Profile { inner })) + } + + /// Create a profile with raw `(type, unit)` string pairs and no period. + pub fn create_with_value_types_no_period( + sample_types: Vec>, + ) -> anyhow::Result> { + let vts: Vec> = sample_types + .iter() + .map(|st| Self::owned_value_type(st.type_, st.unit)) + .collect(); + + let inner = internal::Profile::try_new_with_value_types(&vts, None)?; + Ok(Box::new(Profile { inner })) + } + pub fn add_sample(&mut self, sample: &ffi::Sample) -> anyhow::Result<()> { let api_sample = api::Sample { locations: sample.locations.iter().map(Into::into).collect(), diff --git a/libdd-profiling/src/internal/profile/mod.rs b/libdd-profiling/src/internal/profile/mod.rs index e92ad4e393..aef8bf5007 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -45,8 +45,8 @@ pub struct Profile { locations: FxIndexSet, mappings: FxIndexSet, observations: Observations, - period: Option, - sample_types: Box<[api::SampleType]>, + period: Option<(api::ValueType<'static>, i64)>, + sample_types: Box<[api::ValueType<'static>]>, stack_traces: FxIndexSet, start_time: SystemTime, strings: StringTable, @@ -383,6 +383,16 @@ impl Profile { Self::try_new(sample_types, period).unwrap() } + /// Test helper for the raw value-type path. + #[cfg(test)] + pub fn new_with_value_types( + sample_types: &[api::ValueType<'static>], + period: Option<(api::ValueType<'static>, i64)>, + ) -> Self { + #[allow(clippy::unwrap_used)] + Self::try_new_with_value_types(sample_types, period).unwrap() + } + /// Tries to create a profile with the given `period`. /// Initializes the string table to hold common strings such as: /// - "" (the empty string) @@ -397,7 +407,17 @@ impl Profile { sample_types: &[api::SampleType], period: Option, ) -> io::Result { - Self::try_new_internal(period, sample_types.to_vec().into_boxed_slice(), None, None) + Self::try_new_internal( + period.map(|p| (api::ValueType::from(p.sample_type), p.value)), + sample_types + .iter() + .copied() + .map(api::ValueType::from) + .collect::>() + .into_boxed_slice(), + None, + None, + ) } /// Tries to create a profile with the given period and sample types. @@ -409,8 +429,13 @@ impl Profile { profiles_dictionary: crate::profiles::collections::Arc, ) -> io::Result { Self::try_new_internal( - period, - sample_types.to_vec().into_boxed_slice(), + period.map(|p| (api::ValueType::from(p.sample_type), p.value)), + sample_types + .iter() + .copied() + .map(api::ValueType::from) + .collect::>() + .into_boxed_slice(), None, Some(ProfilesDictionaryTranslator::new(profiles_dictionary)), ) @@ -421,12 +446,52 @@ impl Profile { sample_types: &[api::SampleType], period: Option, string_storage: Arc>, + ) -> io::Result { + Self::try_new_internal( + period.map(|p| (api::ValueType::from(p.sample_type), p.value)), + sample_types + .iter() + .copied() + .map(api::ValueType::from) + .collect::>() + .into_boxed_slice(), + Some(string_storage), + None, + ) + } + + /// Creates a profile using raw `(type, unit)` string pairs, bypassing the + /// [`SampleType`][api::SampleType] enum. + /// + /// This is the escape hatch for **prototyping** new profile types before + /// they are added to [`SampleType`][api::SampleType]. The string slices + /// must have `'static` lifetime (string literals are the typical case). + /// + /// # Stability note + /// Once a custom type is stable and agreed upon across profiler teams, + /// add a variant to [`SampleType`][api::SampleType] and migrate callers + /// to [`Profile::try_new`] instead. + pub fn try_new_with_value_types( + sample_types: &[api::ValueType<'static>], + period: Option<(api::ValueType<'static>, i64)>, + ) -> io::Result { + Self::try_new_internal(period, sample_types.to_vec().into_boxed_slice(), None, None) + } + + /// Like [`Profile::try_new_with_value_types`] but with a shared dictionary. + /// + /// # Stability note + /// Prefer [`Profile::try_new_with_dictionary`] once the types are stable. + pub fn try_new_with_value_types_and_dictionary( + sample_types: &[api::ValueType<'static>], + period: Option<(api::ValueType<'static>, i64)>, + profiles_dictionary: crate::profiles::collections::Arc, ) -> io::Result { Self::try_new_internal( period, sample_types.to_vec().into_boxed_slice(), - Some(string_storage), None, + Some(ProfilesDictionaryTranslator::new(profiles_dictionary)), ) } @@ -451,7 +516,7 @@ impl Profile { let mut profile = Profile::try_new_internal( self.period, - self.sample_types.clone(), + self.sample_types.clone(), // Box<[ValueType<'static>]> is cheap to clone (static strs) self.string_storage.clone(), profiles_dictionary_translator, ) @@ -611,16 +676,16 @@ impl Profile { // so we must serialize `Sample` before `SampleType`. // Clone to avoid holding an immutable borrow of `self.sample_types` while interning. let sample_types = self.sample_types.clone(); - for sample_type in sample_types.iter().copied() { - let sample_type = self.intern_sample_type(sample_type); - Record::<_, 1, NO_OPT_ZERO>::from(sample_type).encode(writer)?; + for vt in sample_types.iter().copied() { + let vt = self.intern_value_type(vt); + Record::<_, 1, NO_OPT_ZERO>::from(vt).encode(writer)?; } // Period type needs string interning, which must happen before we start moving fields out // of `self`. (Many fields below are consumed via `into_iter()`.) let period_type_and_value: Option<(ValueType, i64)> = self .period - .map(|period| (self.intern_sample_type(period.sample_type), period.value)); + .map(|(vt, value)| (self.intern_value_type(vt), value)); for (offset, item) in self.mappings.into_iter().enumerate() { let mapping = protobuf::Mapping { @@ -843,8 +908,7 @@ impl Profile { } #[inline(always)] - fn intern_sample_type(&mut self, sample_type: api::SampleType) -> ValueType { - let vt: api::ValueType<'static> = sample_type.into(); + fn intern_value_type(&mut self, vt: api::ValueType<'static>) -> ValueType { ValueType { r#type: Record::from(self.intern(vt.r#type)), unit: Record::from(self.intern(vt.unit)), @@ -940,8 +1004,8 @@ impl Profile { /// Creates a profile from the period, sample types, and start time using /// the owned values. fn try_new_internal( - period: Option, - sample_types: Box<[api::SampleType]>, + period: Option<(api::ValueType<'static>, i64)>, + sample_types: Box<[api::ValueType<'static>]>, string_storage: Option>>, profiles_dictionary_translator: Option, ) -> io::Result { @@ -1387,8 +1451,9 @@ mod api_tests { .reset_and_return_previous() .expect("reset to succeed"); - assert_eq!(profile.period, Some(period)); - assert_eq!(prev.period, Some(period)); + let expected_period = Some((api::ValueType::from(period.sample_type), period.value)); + assert_eq!(profile.period, expected_period); + assert_eq!(prev.period, expected_period); } #[test] @@ -3104,4 +3169,129 @@ mod api_tests { .expect("Expected serialization error to be an io::Error"); assert_eq!(io_err.kind(), io::ErrorKind::InvalidData); } + + // ----------------------------------------------------------------------- + // Custom value type tests + // ----------------------------------------------------------------------- + + /// Basic smoke test: a profile created with custom types serializes without + /// error and round-trips the type/unit strings through pprof. + #[test] + fn custom_value_types_round_trip() -> anyhow::Result<()> { + const MEMORY_BREAKDOWN: api::ValueType<'static> = + api::ValueType::new("memory-breakdown", "bytes"); + + let period = (MEMORY_BREAKDOWN, 4096_i64); + let mut profile = Profile::new_with_value_types(&[MEMORY_BREAKDOWN], Some(period)); + + let sample = api::Sample { + locations: vec![], + values: &[4096], + labels: vec![], + }; + profile.try_add_sample(sample, None)?; + + let pprof = roundtrip_to_pprof(profile)?; + + // Verify the custom sample type is present in the serialized pprof. + let st_strings: Vec<(&str, &str)> = pprof + .sample_types + .iter() + .map(|st| { + ( + string_table_fetch(&pprof, st.r#type).as_str(), + string_table_fetch(&pprof, st.unit).as_str(), + ) + }) + .collect(); + + assert_eq!(st_strings, vec![("memory-breakdown", "bytes")]); + + // Verify the period type. + let period_type = pprof.period_type.expect("period type should be present"); + assert_eq!( + string_table_fetch(&pprof, period_type.r#type).as_str(), + "memory-breakdown" + ); + assert_eq!( + string_table_fetch(&pprof, period_type.unit).as_str(), + "bytes" + ); + + Ok(()) + } + + /// Custom types survive a profile reset: the new profile keeps the same + /// (type, unit) strings. + #[test] + fn custom_value_types_survive_reset() -> anyhow::Result<()> { + const MEMORY_BREAKDOWN: api::ValueType<'static> = + api::ValueType::new("memory-breakdown", "bytes"); + + let mut profile = Profile::new_with_value_types(&[MEMORY_BREAKDOWN], None); + + let sample = api::Sample { + locations: vec![], + values: &[1024], + labels: vec![], + }; + profile.try_add_sample(sample, None)?; + profile.reset_and_return_previous()?; // discards old, keeps type config + + // Profile is now empty but should still accept samples with the same + // value count. + let sample2 = api::Sample { + locations: vec![], + values: &[2048], + labels: vec![], + }; + profile.try_add_sample(sample2, None)?; + + let pprof = roundtrip_to_pprof(profile)?; + let st_strings: Vec<&str> = pprof + .sample_types + .iter() + .map(|st| string_table_fetch(&pprof, st.r#type).as_str()) + .collect(); + assert_eq!(st_strings, vec!["memory-breakdown"]); + + Ok(()) + } + + /// A profile created with custom types rejects samples that have the wrong + /// number of values (same guard as for SampleType-based profiles). + #[test] + fn custom_value_types_wrong_value_count_is_rejected() { + const T: api::ValueType<'static> = api::ValueType::new("my-custom-type", "count"); + let mut profile = Profile::new_with_value_types(&[T], None); + + let bad_sample = api::Sample { + locations: vec![], + values: &[1, 2], // two values, but only one type declared + labels: vec![], + }; + assert!( + profile.try_add_sample(bad_sample, None).is_err(), + "expected error for mismatched value count" + ); + } + + /// SampleType-based and value-type-based profiles produce identical pprof + /// output for the same well-known type. + #[test] + fn custom_value_types_match_sample_type_output() -> anyhow::Result<()> { + // Build profile via the SampleType enum path. + let enum_profile = Profile::new(&[api::SampleType::WallTime], None); + let enum_pprof = roundtrip_to_pprof(enum_profile)?; + + // Build profile via the raw ValueType path using the same type/unit. + let vt = api::ValueType::from(api::SampleType::WallTime); + let vt_profile = Profile::new_with_value_types(&[vt], None); + let vt_pprof = roundtrip_to_pprof(vt_profile)?; + + // Both should produce identical sample_types in the pprof output. + assert_eq!(enum_pprof.sample_types, vt_pprof.sample_types); + + Ok(()) + } } From 84771716f4de044143df6fa344e4d7089bb8598d Mon Sep 17 00:00:00 2001 From: "erwan.viollet" Date: Tue, 7 Jul 2026 11:59:30 +0200 Subject: [PATCH 2/5] docs(profiling): clarify custom profile period semantics --- examples/ffi/custom_profile_types.c | 8 ++++++-- libdd-profiling-ffi/src/profiles/datatypes.rs | 19 ++++++++++++++++--- libdd-profiling/src/cxx.rs | 19 +++++++++++++++++++ libdd-profiling/src/internal/profile/mod.rs | 12 ++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/examples/ffi/custom_profile_types.c b/examples/ffi/custom_profile_types.c index 82a403c48d..39c04fc976 100644 --- a/examples/ffi/custom_profile_types.c +++ b/examples/ffi/custom_profile_types.c @@ -40,8 +40,12 @@ int main(void) { .len = sizeof(sample_types) / sizeof(sample_types[0]), }; - // The profile period is optional. Pass NULL when there is no meaningful - // sampling interval to report for the custom type. + // The profile period is optional and is profile-level sampling metadata, not + // per-sample-type metadata. It describes the sampling distance/cadence; the + // profile duration describes the upload/reporting window. + // + // memory-breakdown is a point-in-time byte measurement in this example, so + // there is no meaningful sampling cadence to report and we pass NULL. // ------------------------------------------------------------------------- // 2. Create the profile. diff --git a/libdd-profiling-ffi/src/profiles/datatypes.rs b/libdd-profiling-ffi/src/profiles/datatypes.rs index d88f0c463b..bf921a29a6 100644 --- a/libdd-profiling-ffi/src/profiles/datatypes.rs +++ b/libdd-profiling-ffi/src/profiles/datatypes.rs @@ -121,8 +121,19 @@ pub struct CustomValueType<'a> { pub unit: CharSlice<'a>, } -/// Period expressed as a raw (type, unit) string pair for use with -/// [`ddog_prof_Profile_new_custom`]. +/// Profile-level sampling period expressed as a raw (type, unit) string pair +/// for use with [`ddog_prof_Profile_new_custom`]. +/// +/// This is not per-sample-type metadata. It describes the sampling +/// distance/cadence for the whole profile (for example, every 10ms of CPU +/// time), while profile duration describes the upload or reporting window. +/// Pass `NULL` to [`ddog_prof_Profile_new_custom`] when the custom type has no +/// meaningful sampling cadence. +/// +/// The raw period form exists for future/custom profile types whose sampling +/// trigger is not yet represented by [`SampleType`], and for compatibility with +/// formats such as OpenTelemetry Profiles where each profile has a single +/// sample type and period. #[repr(C)] #[derive(Copy, Clone)] pub struct CustomPeriod<'a> { @@ -444,7 +455,9 @@ pub unsafe extern "C" fn ddog_prof_Profile_new( /// # Arguments /// * `sample_types` - Slice of [`CustomValueType`]. Every `type_str` and `unit` pointer must be /// valid for the lifetime of the program (C string literals are the typical case). -/// * `period` - Optional period. Same lifetime requirement as `sample_types`. +/// * `period` - Optional profile-level sampling period. This is not per-sample-type metadata; pass +/// `None`/`NULL` when the custom type has no meaningful sampling cadence. Same lifetime +/// requirement as `sample_types`. /// /// # Safety /// All slices must have pointers that are suitably aligned for their type and diff --git a/libdd-profiling/src/cxx.rs b/libdd-profiling/src/cxx.rs index c9b6a0bce5..00907142f4 100644 --- a/libdd-profiling/src/cxx.rs +++ b/libdd-profiling/src/cxx.rs @@ -111,6 +111,17 @@ pub mod ffi { unit: &'a str, } + /// Profile-level sampling period expressed as a raw `(type, unit)` pair. + /// + /// This is not per-sample-type metadata. It describes the sampling + /// distance/cadence for the whole profile, while profile duration describes + /// the upload or reporting window. Use `create_with_value_types_no_period` + /// when the custom type has no meaningful sampling cadence. + /// + /// The raw period form exists for future/custom profile types whose + /// sampling trigger is not yet represented by `SampleType`, and for + /// compatibility with formats such as OpenTelemetry Profiles where each + /// profile has a single sample type and period. struct CustomPeriod<'a> { value_type: CustomValueType<'a>, value: i64, @@ -182,6 +193,12 @@ pub mod ffi { /// Use this when the desired profile type is not yet in [`SampleType`]. /// Strings are copied during profile construction. /// + /// The period is profile-level sampling metadata, not per-sample-type + /// metadata. It describes the sampling distance/cadence; profile + /// duration describes the upload or reporting window. Use + /// `create_with_value_types_no_period` when there is no meaningful + /// sampling cadence for the custom type. + /// /// # Stability note /// Once the type is stable, add it to [`SampleType`] and switch to /// [`Profile::create`]. @@ -192,6 +209,8 @@ pub mod ffi { ) -> Result>; /// Create a profile using raw `(type, unit)` string pairs and no period. + /// + /// Use this when the custom type has no meaningful sampling cadence. #[Self = "Profile"] fn create_with_value_types_no_period( sample_types: Vec, diff --git a/libdd-profiling/src/internal/profile/mod.rs b/libdd-profiling/src/internal/profile/mod.rs index aef8bf5007..c645f5d480 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -467,6 +467,18 @@ impl Profile { /// they are added to [`SampleType`][api::SampleType]. The string slices /// must have `'static` lifetime (string literals are the typical case). /// + /// # Period + /// The `period` is profile-level sampling metadata, not per-sample-type + /// metadata. It describes the sampling distance/cadence (for example, + /// every 10ms of CPU time), while the profile duration describes the upload + /// or reporting window. Pass `None` when the custom type has no meaningful + /// sampling cadence. + /// + /// The raw period form exists for future/custom profile types whose + /// sampling trigger is not yet represented by [`SampleType`][api::SampleType], + /// and for compatibility with formats such as OpenTelemetry Profiles where + /// each profile has a single sample type and period. + /// /// # Stability note /// Once a custom type is stable and agreed upon across profiler teams, /// add a variant to [`SampleType`][api::SampleType] and migrate callers From c3582628b4e4ee151a2d228a0065adc7413f1785 Mon Sep 17 00:00:00 2001 From: "erwan.viollet" Date: Tue, 7 Jul 2026 18:39:09 +0200 Subject: [PATCH 3/5] feat(profiling)!: add custom profile type slots Use Daniel's slot-based design for custom profile types: callers create profiles with SampleType::Custom1 through Custom5, then configure each slot with its concrete (type, unit) pair before serialization. This keeps the normal profile constructors as the primary API while still allowing profiler teams to prototype new profile types before promoting them to stable SampleType variants. BREAKING CHANGE: remove the ExperimentalCount, ExperimentalNanoseconds, and ExperimentalBytes SampleType variants in favor of Custom1 through Custom5. --- datadog-profiling-replayer/src/main.rs | 11 +- datadog-profiling-replayer/src/replayer.rs | 74 ++++-- examples/cxx/profiling.cpp | 3 +- examples/ffi/custom_profile_types.c | 36 +-- libdd-profiling-ffi/src/profiles/datatypes.rs | 211 +++++++----------- libdd-profiling/src/api/sample_type.rs | 67 ++++-- libdd-profiling/src/cxx.rs | 132 ++++------- .../src/internal/profile/fuzz_tests.rs | 27 ++- libdd-profiling/src/internal/profile/mod.rs | 208 +++++++++-------- 9 files changed, 384 insertions(+), 385 deletions(-) diff --git a/datadog-profiling-replayer/src/main.rs b/datadog-profiling-replayer/src/main.rs index ed444bf4ed..8fe036acbb 100644 --- a/datadog-profiling-replayer/src/main.rs +++ b/datadog-profiling-replayer/src/main.rs @@ -163,11 +163,12 @@ fn main() -> anyhow::Result<()> { let mut replayer = Replayer::try_from(&pprof)?; - let mut outprof = libdd_profiling::internal::Profile::try_new_with_value_types( - &replayer.sample_types, - replayer.period, - )? - .with_start_time(replayer.start_time)?; + let mut outprof = + libdd_profiling::internal::Profile::try_new(&replayer.sample_types, replayer.period)? + .with_start_time(replayer.start_time)?; + for (slot, value_type) in replayer.custom_type_mappings.iter().copied() { + outprof.set_custom_sample_type(slot, value_type)?; + } // Before benchmarking, let's calculate some statistics. // No point doing that if there aren't at least 4 samples though. diff --git a/datadog-profiling-replayer/src/replayer.rs b/datadog-profiling-replayer/src/replayer.rs index e694f57008..6041d6d212 100644 --- a/datadog-profiling-replayer/src/replayer.rs +++ b/datadog-profiling-replayer/src/replayer.rs @@ -17,6 +17,7 @@ type SamplesAndEndpointInfo<'pprof> = ( Vec<(Option, api::Sample<'pprof>)>, Vec<(u64, &'pprof str)>, ); +type CustomTypeMappings = Vec<(api::SampleType, api::ValueType<'static>)>; pub struct Replayer<'pprof> { pub profile_index: ProfileIndex<'pprof>, @@ -24,8 +25,9 @@ pub struct Replayer<'pprof> { pub start_time: SystemTime, pub duration: Duration, pub end_time: SystemTime, // start_time + duration - pub sample_types: Vec>, - pub period: Option<(api::ValueType<'static>, i64)>, + pub sample_types: Vec, + pub period: Option, + pub custom_type_mappings: CustomTypeMappings, pub endpoints: Vec<(u64, &'pprof str)>, pub samples: Vec<(Option, api::Sample<'pprof>)>, } @@ -55,48 +57,72 @@ impl<'pprof> Replayer<'pprof> { } } - /// Convert a (type_str, unit) pair read from a pprof file into a - /// `ValueType<'static>`. - /// - /// Known types are resolved through [`api::SampleType`] so the returned - /// references point to existing static string literals (zero allocation). - /// Strings that are not yet in the enum – i.e. custom / prototype types – - /// are promoted to `'static` via [`Box::leak`]. The replayer is a - /// short-lived tool; the bounded number of leaked allocations is acceptable. - fn to_static_value_type(type_str: &str, unit: &str) -> api::ValueType<'static> { + fn custom_slots() -> [api::SampleType; 5] { + [ + api::SampleType::Custom1, + api::SampleType::Custom2, + api::SampleType::Custom3, + api::SampleType::Custom4, + api::SampleType::Custom5, + ] + } + + fn resolve_sample_type( + type_str: &str, + unit: &str, + custom_type_mappings: &mut CustomTypeMappings, + ) -> anyhow::Result { let borrowed = api::ValueType::new(type_str, unit); - // Fast path: resolve via the stable enum (returns 'static string literals). - if let Ok(st) = api::SampleType::try_from(borrowed) { - return api::ValueType::from(st); + if let Ok(sample_type) = api::SampleType::try_from(borrowed) { + return Ok(sample_type); + } + + if let Some((slot, _)) = custom_type_mappings + .iter() + .find(|(_, value_type)| value_type.r#type == type_str && value_type.unit == unit) + { + return Ok(*slot); } - // Slow path: custom type not yet in the enum – own the strings. - let ty: &'static str = Box::leak(type_str.to_string().into_boxed_str()); + + let slots = Self::custom_slots(); + let slot = *slots + .get(custom_type_mappings.len()) + .ok_or_else(|| anyhow::anyhow!("pprof uses more than 5 custom sample types"))?; + let type_str: &'static str = Box::leak(type_str.to_string().into_boxed_str()); let unit: &'static str = Box::leak(unit.to_string().into_boxed_str()); - api::ValueType::new(ty, unit) + custom_type_mappings.push((slot, api::ValueType::new(type_str, unit))); + Ok(slot) } fn sample_types<'a>( profile_index: &'a ProfileIndex<'pprof>, - ) -> anyhow::Result>> { + custom_type_mappings: &mut CustomTypeMappings, + ) -> anyhow::Result> { let mut sample_types = Vec::with_capacity(profile_index.pprof.sample_types.len()); for sample_type in profile_index.pprof.sample_types.iter() { let type_str = profile_index.get_string(sample_type.r#type)?; let unit = profile_index.get_string(sample_type.unit)?; - sample_types.push(Self::to_static_value_type(type_str, unit)); + sample_types.push(Self::resolve_sample_type( + type_str, + unit, + custom_type_mappings, + )?); } Ok(sample_types) } fn period<'a>( profile_index: &'a ProfileIndex<'pprof>, - ) -> anyhow::Result, i64)>> { + custom_type_mappings: &mut CustomTypeMappings, + ) -> anyhow::Result> { let value = profile_index.pprof.period; match profile_index.pprof.period_type { Some(period_type) => { let type_str = profile_index.get_string(period_type.r#type)?; let unit = profile_index.get_string(period_type.unit)?; - Ok(Some((Self::to_static_value_type(type_str, unit), value))) + let sample_type = Self::resolve_sample_type(type_str, unit, custom_type_mappings)?; + Ok(Some(api::Period { sample_type, value })) } None => Ok(None), } @@ -271,8 +297,9 @@ impl<'pprof> TryFrom<&'pprof prost_impls::Profile> for Replayer<'pprof> { let start_time = Self::start_time(pprof); let duration = Self::duration(pprof)?; let end_time = start_time.add(duration); - let sample_types = Self::sample_types(&profile_index)?; - let period = Self::period(&profile_index)?; + let mut custom_type_mappings = Vec::new(); + let sample_types = Self::sample_types(&profile_index, &mut custom_type_mappings)?; + let period = Self::period(&profile_index, &mut custom_type_mappings)?; let (samples, endpoints) = Self::samples(&profile_index)?; Ok(Self { @@ -282,6 +309,7 @@ impl<'pprof> TryFrom<&'pprof prost_impls::Profile> for Replayer<'pprof> { end_time, sample_types, period, + custom_type_mappings, endpoints, samples, }) diff --git a/examples/cxx/profiling.cpp b/examples/cxx/profiling.cpp index 8ea27fa128..34fe85b49f 100644 --- a/examples/cxx/profiling.cpp +++ b/examples/cxx/profiling.cpp @@ -22,7 +22,8 @@ int main() { }; // Create profile with predefined sample types. For prototyping a type - // not yet in SampleType, use Profile::create_with_value_types instead. + // not yet in SampleType, use Custom1..Custom5 and configure the slot + // with Profile::set_custom_sample_type before serialization. auto profile = Profile::create({SampleType::WallTime}, period); std::cout << "✅ Profile created" << std::endl; diff --git a/examples/ffi/custom_profile_types.c b/examples/ffi/custom_profile_types.c index 39c04fc976..df75805da5 100644 --- a/examples/ffi/custom_profile_types.c +++ b/examples/ffi/custom_profile_types.c @@ -1,15 +1,14 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -// Example: creating a profile with custom (type, unit) pairs. +// Example: creating a profile with a custom sample type slot. // -// Use ddog_prof_Profile_new_custom when the desired sample type is not yet in -// the ddog_prof_SampleType enum. All type/unit strings must be program-lifetime -// constants (string literals in practice). +// Use DDOG_PROF_SAMPLE_TYPE_CUSTOM1 through DDOG_PROF_SAMPLE_TYPE_CUSTOM5 when +// the desired sample type is not yet in the stable ddog_prof_SampleType enum, +// then configure the slot with ddog_prof_Profile_set_custom_sample_type. // // TODO: Once custom profile types are stable and agreed upon across profiler -// teams, add them to ddog_prof_SampleType and switch callers to -// ddog_prof_Profile_new / ddog_prof_Profile_with_dictionary. +// teams, add a dedicated ddog_prof_SampleType variant and switch callers to it. #include #include @@ -27,15 +26,12 @@ static int check_profile_result(ddog_prof_Profile_Result result, const char *con int main(void) { // ------------------------------------------------------------------------- - // 1. Declare custom sample types as static string literals. - // These bypass the ddog_prof_SampleType enum so that new types can be - // prototyped without a libdatadog release. + // 1. Select one of the custom sample type slots. // ------------------------------------------------------------------------- - const ddog_prof_CustomValueType sample_types[] = { - {.type_str = DDOG_CHARSLICE_C("memory-breakdown"), - .unit = DDOG_CHARSLICE_C("bytes")}, + const ddog_prof_SampleType sample_types[] = { + DDOG_PROF_SAMPLE_TYPE_CUSTOM1, }; - const ddog_prof_Slice_CustomValueType sample_types_slice = { + const ddog_prof_Slice_SampleType sample_types_slice = { .ptr = sample_types, .len = sizeof(sample_types) / sizeof(sample_types[0]), }; @@ -51,15 +47,25 @@ int main(void) { // 2. Create the profile. // ------------------------------------------------------------------------- ddog_prof_Profile_NewResult new_result = - ddog_prof_Profile_new_custom(sample_types_slice, NULL); + ddog_prof_Profile_new(sample_types_slice, NULL); if (new_result.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { ddog_CharSlice msg = ddog_Error_message(&new_result.err); - fprintf(stderr, "ddog_prof_Profile_new_custom failed: %.*s\n", (int)msg.len, msg.ptr); + fprintf(stderr, "ddog_prof_Profile_new failed: %.*s\n", (int)msg.len, msg.ptr); ddog_Error_drop(&new_result.err); return EXIT_FAILURE; } ddog_prof_Profile profile = new_result.ok; + if (!check_profile_result( + ddog_prof_Profile_set_custom_sample_type(&profile, + DDOG_PROF_SAMPLE_TYPE_CUSTOM1, + DDOG_CHARSLICE_C("memory-breakdown"), + DDOG_CHARSLICE_C("bytes")), + "set_custom_sample_type")) { + ddog_prof_Profile_drop(&profile); + return EXIT_FAILURE; + } + if (!check_profile_result( ddog_prof_Profile_set_omit_local_root_span_id_when_serializing(&profile, true), "set_omit_local_root_span_id")) { diff --git a/libdd-profiling-ffi/src/profiles/datatypes.rs b/libdd-profiling-ffi/src/profiles/datatypes.rs index bf921a29a6..711054853c 100644 --- a/libdd-profiling-ffi/src/profiles/datatypes.rs +++ b/libdd-profiling-ffi/src/profiles/datatypes.rs @@ -105,42 +105,6 @@ pub use api::SampleType; /// The FFI Period is identical to the API Period. pub use api::Period; -/// A raw (type, unit) pair for profile types not yet in [`SampleType`]. -/// -/// Both `type_str` and `unit` must point to program-lifetime memory (C string -/// literals are the typical case). This is the escape hatch for prototyping -/// new profile types without a libdatadog release. -/// -/// # Stability note -/// Once a custom type is agreed upon across profiler teams, add it to -/// [`SampleType`] and migrate callers to [`ddog_prof_Profile_new`] instead. -#[repr(C)] -#[derive(Copy, Clone)] -pub struct CustomValueType<'a> { - pub type_str: CharSlice<'a>, - pub unit: CharSlice<'a>, -} - -/// Profile-level sampling period expressed as a raw (type, unit) string pair -/// for use with [`ddog_prof_Profile_new_custom`]. -/// -/// This is not per-sample-type metadata. It describes the sampling -/// distance/cadence for the whole profile (for example, every 10ms of CPU -/// time), while profile duration describes the upload or reporting window. -/// Pass `NULL` to [`ddog_prof_Profile_new_custom`] when the custom type has no -/// meaningful sampling cadence. -/// -/// The raw period form exists for future/custom profile types whose sampling -/// trigger is not yet represented by [`SampleType`], and for compatibility with -/// formats such as OpenTelemetry Profiles where each profile has a single -/// sample type and period. -#[repr(C)] -#[derive(Copy, Clone)] -pub struct CustomPeriod<'a> { - pub value_type: CustomValueType<'a>, - pub value: i64, -} - #[repr(C)] #[derive(Copy, Clone, Default)] pub struct Label<'a> { @@ -445,81 +409,6 @@ pub unsafe extern "C" fn ddog_prof_Profile_new( profile_new(sample_types, period, None) } -/// Create a new profile using raw `(type, unit)` string pairs, bypassing the -/// [`SampleType`] enum. Must call `ddog_prof_Profile_drop` when done. -/// -/// Use this when the desired profile type is not yet in [`SampleType`]. Once -/// the type is stable add it to [`SampleType`] and switch to -/// [`ddog_prof_Profile_new`]. -/// -/// # Arguments -/// * `sample_types` - Slice of [`CustomValueType`]. Every `type_str` and `unit` pointer must be -/// valid for the lifetime of the program (C string literals are the typical case). -/// * `period` - Optional profile-level sampling period. This is not per-sample-type metadata; pass -/// `None`/`NULL` when the custom type has no meaningful sampling cadence. Same lifetime -/// requirement as `sample_types`. -/// -/// # Safety -/// All slices must have pointers that are suitably aligned for their type and -/// must have the correct number of elements. The string pointers inside each -/// [`CustomValueType`] must point to valid UTF-8 memory that lives at least as -/// long as the program (i.e. string literals). -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_prof_Profile_new_custom( - sample_types: Slice>, - period: Option<&CustomPeriod<'_>>, -) -> ProfileNewResult { - // SAFETY: Profile type strings are program-lifetime constants (string - // literals). Extending to 'static is sound because the strings must - // outlive the Profile, and a Profile is always dropped before the program - // exits. The caller documents this requirement. - unsafe fn to_static_str(slice: CharSlice<'_>) -> Result<&'static str, anyhow::Error> { - let s = slice - .try_to_utf8() - .map_err(|e| anyhow::anyhow!("invalid UTF-8 in profile type string: {}", e))?; - Ok(std::mem::transmute::<&str, &'static str>(s)) - } - - let raw_types = match sample_types.try_as_slice() { - Ok(s) => s, - Err(e) => return ProfileNewResult::Err(anyhow::Error::from(e).into()), - }; - - let mut vts: Vec> = Vec::with_capacity(raw_types.len()); - for ct in raw_types { - let ty = match to_static_str(ct.type_str) { - Ok(s) => s, - Err(e) => return ProfileNewResult::Err(e.into()), - }; - let unit = match to_static_str(ct.unit) { - Ok(s) => s, - Err(e) => return ProfileNewResult::Err(e.into()), - }; - vts.push(api::ValueType::new(ty, unit)); - } - - let period_vt = match period { - None => None, - Some(p) => { - let ty = match to_static_str(p.value_type.type_str) { - Ok(s) => s, - Err(e) => return ProfileNewResult::Err(e.into()), - }; - let unit = match to_static_str(p.value_type.unit) { - Ok(s) => s, - Err(e) => return ProfileNewResult::Err(e.into()), - }; - Some((api::ValueType::new(ty, unit), p.value)) - } - }; - - match internal::Profile::try_new_with_value_types(&vts, period_vt) { - Ok(inner) => ProfileNewResult::Ok(Profile::new(inner)), - Err(e) => ProfileNewResult::Err(anyhow::Error::from(e).into()), - } -} - /// Create a new profile with the given sample types. Must call /// `ddog_prof_Profile_drop` when you are done with the profile. /// @@ -619,6 +508,46 @@ unsafe fn profile_new( } } +/// Configure one of the custom sample type slots (`Custom1` through `Custom5`) +/// with its concrete `(type, unit)` string pair. +/// +/// Use this after creating a profile with a custom slot in its `sample_types` +/// or `period`. The strings are copied during this call. A profile that uses a +/// custom slot must configure it before serialization. +/// +/// # Safety +/// The `profile` ptr must point to a valid Profile object created by this +/// module. The `type_str` and `unit` slices must point to valid UTF-8 memory for +/// the duration of this call. +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_prof_Profile_set_custom_sample_type( + profile: *mut Profile, + slot: SampleType, + type_str: CharSlice, + unit: CharSlice, +) -> ProfileResult { + (|| { + let profile = profile_ptr_to_inner(profile)?; + let type_str: &'static str = Box::leak( + type_str + .try_to_utf8() + .context("invalid UTF-8 in custom profile type")? + .to_string() + .into_boxed_str(), + ); + let unit: &'static str = Box::leak( + unit.try_to_utf8() + .context("invalid UTF-8 in custom profile unit")? + .to_string() + .into_boxed_str(), + ); + profile.set_custom_sample_type(slot, api::ValueType::new(type_str, unit)) + })() + .context("ddog_prof_Profile_set_custom_sample_type failed") + .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. @@ -1068,17 +997,21 @@ mod tests { } #[test] - fn profile_new_custom_accepts_raw_value_type_without_period() -> Result<(), Error> { + fn profile_set_custom_sample_type_accepts_slot() -> Result<(), Error> { unsafe { - let sample_type = CustomValueType { - type_str: "memory-breakdown".into(), - unit: "bytes".into(), - }; - let mut profile = Result::from(ddog_prof_Profile_new_custom( + let sample_type = SampleType::Custom1; + let mut profile = Result::from(ddog_prof_Profile_new( Slice::from_raw_parts(&sample_type, 1), None, ))?; + Result::from(ddog_prof_Profile_set_custom_sample_type( + &mut profile, + SampleType::Custom1, + "memory-breakdown".into(), + "bytes".into(), + ))?; + let values = [4096_i64]; let sample = Sample { locations: Slice::empty(), @@ -1092,31 +1025,49 @@ mod tests { } #[test] - fn profile_new_custom_invalid_sample_types_slice_returns_err() { + fn profile_set_custom_sample_type_rejects_non_custom_slot() -> Result<(), Error> { unsafe { - let bad_slice: Slice<'_, CustomValueType<'_>> = - Slice::from_raw_parts(std::ptr::null(), 1); - let result = ddog_prof_Profile_new_custom(bad_slice, None); + let sample_type = SampleType::CpuSamples; + let mut profile = Result::from(ddog_prof_Profile_new( + Slice::from_raw_parts(&sample_type, 1), + None, + ))?; + let result = ddog_prof_Profile_set_custom_sample_type( + &mut profile, + SampleType::CpuSamples, + "memory-breakdown".into(), + "bytes".into(), + ); assert!( - matches!(result, ProfileNewResult::Err(_)), - "expected Err for null pointer with non-zero length (SliceConversionError::NullPointer)" + matches!(result, ProfileResult::Err(_)), + "expected Err when configuring a non-custom slot" ); + ddog_prof_Profile_drop(&mut profile); + Ok(()) } } #[test] - fn profile_new_custom_invalid_utf8_returns_err() { + fn profile_set_custom_sample_type_invalid_utf8_returns_err() -> Result<(), Error> { unsafe { + let sample_type = SampleType::Custom1; + let mut profile = Result::from(ddog_prof_Profile_new( + Slice::from_raw_parts(&sample_type, 1), + None, + ))?; let invalid = [0xff_u8 as std::ffi::c_char]; - let sample_type = CustomValueType { - type_str: Slice::from_raw_parts(invalid.as_ptr(), invalid.len()), - unit: "bytes".into(), - }; - let result = ddog_prof_Profile_new_custom(Slice::from_raw_parts(&sample_type, 1), None); + let result = ddog_prof_Profile_set_custom_sample_type( + &mut profile, + SampleType::Custom1, + Slice::from_raw_parts(invalid.as_ptr(), invalid.len()), + "bytes".into(), + ); assert!( - matches!(result, ProfileNewResult::Err(_)), + matches!(result, ProfileResult::Err(_)), "expected Err for invalid UTF-8 in custom profile type" ); + ddog_prof_Profile_drop(&mut profile); + Ok(()) } } diff --git a/libdd-profiling/src/api/sample_type.rs b/libdd-profiling/src/api/sample_type.rs index 02c936c53d..3c79584cf0 100644 --- a/libdd-profiling/src/api/sample_type.rs +++ b/libdd-profiling/src/api/sample_type.rs @@ -15,12 +15,11 @@ use super::ValueType; /// /// # Adding new types /// -/// To prototype a new profile type without a libdatadog release, use -/// [`crate::internal::Profile::try_new_with_value_types`] (Rust) or the -/// `create_with_value_types` CXX / `ddog_prof_Profile_new_custom` C functions -/// with a raw `(type, unit)` string pair. -/// Once the type is stable and agreed upon across profiler teams, add a -/// variant here so callers can use the type-safe path. +/// To prototype a new profile type without a libdatadog release, use one of +/// the `Custom1` through `Custom5` slots and configure its concrete `(type, +/// unit)` string pair on the profile before serialization. Once the type is +/// stable and agreed upon across profiler teams, add a dedicated variant here +/// so callers can use the type-safe path. #[cfg_attr(test, derive(bolero::generator::TypeGenerator, strum::EnumIter))] #[repr(C)] #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] @@ -83,15 +82,35 @@ pub enum SampleType { WallTime, /// Legacy: Use `WallTime` instead for consistency with naming scheme WallLegacy, - /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` - /// pair instead. This fixed name cannot distinguish multiple custom count measurements. - ExperimentalCount, - /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` - /// pair instead. This fixed name cannot distinguish multiple custom time measurements. - ExperimentalNanoseconds, - /// Deprecated: use `Profile::try_new_with_value_types` with a descriptive raw `(type, unit)` - /// pair instead. This fixed name cannot distinguish multiple custom byte measurements. - ExperimentalBytes, + /// Custom profile type slot. Configure its concrete `(type, unit)` on the profile before + /// serialization. + Custom1, + /// Custom profile type slot. Configure its concrete `(type, unit)` on the profile before + /// serialization. + Custom2, + /// Custom profile type slot. Configure its concrete `(type, unit)` on the profile before + /// serialization. + Custom3, + /// Custom profile type slot. Configure its concrete `(type, unit)` on the profile before + /// serialization. + Custom4, + /// Custom profile type slot. Configure its concrete `(type, unit)` on the profile before + /// serialization. + Custom5, +} + +impl SampleType { + #[inline(always)] + pub fn is_custom(self) -> bool { + matches!( + self, + SampleType::Custom1 + | SampleType::Custom2 + | SampleType::Custom3 + | SampleType::Custom4 + | SampleType::Custom5 + ) + } } impl From<&SampleType> for ValueType<'static> { @@ -173,11 +192,11 @@ impl From for ValueType<'static> { SampleType::WallSamples => ValueType::new("wall-samples", "count"), SampleType::WallTime => ValueType::new("wall-time", "nanoseconds"), SampleType::WallLegacy => ValueType::new("wall", "nanoseconds"), - SampleType::ExperimentalCount => ValueType::new("experimental-count", "count"), - SampleType::ExperimentalNanoseconds => { - ValueType::new("experimental-nanoseconds", "nanoseconds") - } - SampleType::ExperimentalBytes => ValueType::new("experimental-bytes", "bytes"), + SampleType::Custom1 => ValueType::new("custom-1", "count"), + SampleType::Custom2 => ValueType::new("custom-2", "count"), + SampleType::Custom3 => ValueType::new("custom-3", "count"), + SampleType::Custom4 => ValueType::new("custom-4", "count"), + SampleType::Custom5 => ValueType::new("custom-5", "count"), } } } @@ -239,9 +258,11 @@ impl<'a> TryFrom> for SampleType { ("wall-samples", "count") => SampleType::WallSamples, ("wall-time", "nanoseconds") => SampleType::WallTime, ("wall", "nanoseconds") => SampleType::WallLegacy, - ("experimental-count", "count") => SampleType::ExperimentalCount, - ("experimental-nanoseconds", "nanoseconds") => SampleType::ExperimentalNanoseconds, - ("experimental-bytes", "bytes") => SampleType::ExperimentalBytes, + ("custom-1", "count") => SampleType::Custom1, + ("custom-2", "count") => SampleType::Custom2, + ("custom-3", "count") => SampleType::Custom3, + ("custom-4", "count") => SampleType::Custom4, + ("custom-5", "count") => SampleType::Custom5, _ => anyhow::bail!("Unknown sample type: ({}, {})", vt.r#type, vt.unit), }) } diff --git a/libdd-profiling/src/cxx.rs b/libdd-profiling/src/cxx.rs index 00907142f4..c5b34bc805 100644 --- a/libdd-profiling/src/cxx.rs +++ b/libdd-profiling/src/cxx.rs @@ -26,7 +26,8 @@ pub mod ffi { // live objects, HTTP requests) // - pprof-nodejs (profile-serializer.ts value type functions) // - // To use a type not yet in this enum, call create_with_value_types() instead. + // To use a type not yet in this enum, use Custom1 through Custom5 and configure + // the slot on the Profile before serialization. // // LEGACY VARIANTS (prefer alternatives for consistency): // - CpuLegacy (use CpuTime) @@ -87,10 +88,12 @@ pub mod ffi { Timeline, WallSamples, WallTime, - WallLegacy, // LEGACY: Use WallTime instead - ExperimentalCount, // DEPRECATED: Use create_with_value_types instead - ExperimentalNanoseconds, // DEPRECATED: Use create_with_value_types instead - ExperimentalBytes, // DEPRECATED: Use create_with_value_types instead + WallLegacy, // LEGACY: Use WallTime instead + Custom1, + Custom2, + Custom3, + Custom4, + Custom5, } struct Period { @@ -98,35 +101,6 @@ pub mod ffi { value: i64, } - /// A raw (type, unit) pair for profile types not yet in the [`SampleType`] enum. - /// - /// Strings are copied during profile construction. This is the escape hatch for - /// prototyping new profile types without a libdatadog release. - /// - /// # Stability note - /// Once a custom type is agreed upon across profiler teams, add it to - /// [`SampleType`] and migrate callers to [`Profile::create`] instead. - struct CustomValueType<'a> { - type_: &'a str, - unit: &'a str, - } - - /// Profile-level sampling period expressed as a raw `(type, unit)` pair. - /// - /// This is not per-sample-type metadata. It describes the sampling - /// distance/cadence for the whole profile, while profile duration describes - /// the upload or reporting window. Use `create_with_value_types_no_period` - /// when the custom type has no meaningful sampling cadence. - /// - /// The raw period form exists for future/custom profile types whose - /// sampling trigger is not yet represented by `SampleType`, and for - /// compatibility with formats such as OpenTelemetry Profiles where each - /// profile has a single sample type and period. - struct CustomPeriod<'a> { - value_type: CustomValueType<'a>, - value: i64, - } - struct Mapping<'a> { memory_start: u64, memory_limit: u64, @@ -188,36 +162,18 @@ pub mod ffi { #[Self = "Profile"] fn create(sample_types: Vec, period: &Period) -> Result>; - /// Create a profile using raw `(type, unit)` string pairs. - /// - /// Use this when the desired profile type is not yet in [`SampleType`]. - /// Strings are copied during profile construction. - /// - /// The period is profile-level sampling metadata, not per-sample-type - /// metadata. It describes the sampling distance/cadence; profile - /// duration describes the upload or reporting window. Use - /// `create_with_value_types_no_period` when there is no meaningful - /// sampling cadence for the custom type. - /// - /// # Stability note - /// Once the type is stable, add it to [`SampleType`] and switch to - /// [`Profile::create`]. - #[Self = "Profile"] - fn create_with_value_types( - sample_types: Vec, - period: &CustomPeriod, - ) -> Result>; - - /// Create a profile using raw `(type, unit)` string pairs and no period. - /// - /// Use this when the custom type has no meaningful sampling cadence. + /// Create a profile without a sampling period. #[Self = "Profile"] - fn create_with_value_types_no_period( - sample_types: Vec, - ) -> Result>; + fn create_no_period(sample_types: Vec) -> Result>; // Profile methods fn add_sample(self: &mut Profile, sample: &Sample) -> Result<()>; + fn set_custom_sample_type( + self: &mut Profile, + slot: SampleType, + type_: &str, + unit: &str, + ) -> Result<()>; fn add_endpoint(self: &mut Profile, local_root_span_id: u64, endpoint: &str) -> Result<()>; fn add_endpoint_count(self: &mut Profile, endpoint: &str, value: i64) -> Result<()>; @@ -447,9 +403,11 @@ impl TryFrom for api::SampleType { ffi::SampleType::WallSamples => api::SampleType::WallSamples, ffi::SampleType::WallTime => api::SampleType::WallTime, ffi::SampleType::WallLegacy => api::SampleType::WallLegacy, - ffi::SampleType::ExperimentalCount => api::SampleType::ExperimentalCount, - ffi::SampleType::ExperimentalNanoseconds => api::SampleType::ExperimentalNanoseconds, - ffi::SampleType::ExperimentalBytes => api::SampleType::ExperimentalBytes, + ffi::SampleType::Custom1 => api::SampleType::Custom1, + ffi::SampleType::Custom2 => api::SampleType::Custom2, + ffi::SampleType::Custom3 => api::SampleType::Custom3, + ffi::SampleType::Custom4 => api::SampleType::Custom4, + ffi::SampleType::Custom5 => api::SampleType::Custom5, _ => anyhow::bail!("invalid SampleType discriminant from C++"), }) } @@ -603,37 +561,12 @@ impl Profile { Ok(Box::new(Profile { inner })) } - /// Create a profile with raw `(type, unit)` string pairs. - /// - /// This is the escape hatch for profile types not yet in [`ffi::SampleType`]. - /// Use [`Profile::create`] once the type has been promoted to [`ffi::SampleType`]. - pub fn create_with_value_types( - sample_types: Vec>, - period: &ffi::CustomPeriod<'_>, - ) -> anyhow::Result> { - let vts: Vec> = sample_types - .iter() - .map(|st| Self::owned_value_type(st.type_, st.unit)) - .collect(); - let period_value = ( - Self::owned_value_type(period.value_type.type_, period.value_type.unit), - period.value, - ); - - let inner = internal::Profile::try_new_with_value_types(&vts, Some(period_value))?; - Ok(Box::new(Profile { inner })) - } - - /// Create a profile with raw `(type, unit)` string pairs and no period. - pub fn create_with_value_types_no_period( - sample_types: Vec>, - ) -> anyhow::Result> { - let vts: Vec> = sample_types - .iter() - .map(|st| Self::owned_value_type(st.type_, st.unit)) - .collect(); - - let inner = internal::Profile::try_new_with_value_types(&vts, None)?; + pub fn create_no_period(sample_types: Vec) -> anyhow::Result> { + let types: Vec = sample_types + .into_iter() + .map(TryInto::try_into) + .collect::, _>>()?; + let inner = internal::Profile::try_new(&types, None)?; Ok(Box::new(Profile { inner })) } @@ -649,6 +582,17 @@ impl Profile { Ok(()) } + pub fn set_custom_sample_type( + &mut self, + slot: ffi::SampleType, + type_: &str, + unit: &str, + ) -> anyhow::Result<()> { + let slot: api::SampleType = slot.try_into()?; + self.inner + .set_custom_sample_type(slot, Self::owned_value_type(type_, unit)) + } + pub fn add_endpoint(&mut self, local_root_span_id: u64, endpoint: &str) -> anyhow::Result<()> { self.inner .add_endpoint(local_root_span_id, std::borrow::Cow::Borrowed(endpoint)) diff --git a/libdd-profiling/src/internal/profile/fuzz_tests.rs b/libdd-profiling/src/internal/profile/fuzz_tests.rs index a134462254..73fb9ec917 100644 --- a/libdd-profiling/src/internal/profile/fuzz_tests.rs +++ b/libdd-profiling/src/internal/profile/fuzz_tests.rs @@ -290,6 +290,27 @@ impl<'a> From<&'a Sample> for api::Sample<'a> { } } +fn configured_custom_value_type(sample_type: api::SampleType) -> Option> { + match sample_type { + api::SampleType::Custom1 => Some(api::ValueType::new("custom-1-test", "count")), + api::SampleType::Custom2 => Some(api::ValueType::new("custom-2-test", "count")), + api::SampleType::Custom3 => Some(api::ValueType::new("custom-3-test", "count")), + api::SampleType::Custom4 => Some(api::ValueType::new("custom-4-test", "count")), + api::SampleType::Custom5 => Some(api::ValueType::new("custom-5-test", "count")), + _ => None, + } +} + +fn configure_custom_sample_types(profile: &mut Profile, sample_types: &[api::SampleType]) { + for sample_type in sample_types.iter().copied() { + if let Some(value_type) = configured_custom_value_type(sample_type) { + // Ignore errors: duplicate custom slots are expected in fuzz input, and the first + // successful configuration updates every occurrence of the placeholder in the profile. + let _ = profile.set_custom_sample_type(sample_type, value_type); + } + } +} + #[track_caller] fn assert_sample_types_eq(profile: &pprof::Profile, expected_sample_types: &[api::SampleType]) { assert_eq!( @@ -302,7 +323,8 @@ fn assert_sample_types_eq(profile: &pprof::Profile, expected_sample_types: &[api .iter() .zip(expected_sample_types.iter()) { - let expected_vt: api::ValueType<'static> = (*expected_typ).into(); + let expected_vt = configured_custom_value_type(*expected_typ) + .unwrap_or_else(|| api::ValueType::from(*expected_typ)); assert_eq!(*string_table_fetch(profile, typ.r#type), expected_vt.r#type); assert_eq!(*string_table_fetch(profile, typ.unit), expected_vt.unit); } @@ -515,6 +537,7 @@ fn test_fuzz_add_sample() { .collect::>(); let mut expected_profile = Profile::new(expected_sample_types, None); + configure_custom_sample_types(&mut expected_profile, expected_sample_types); let mut samples_with_timestamps = Vec::new(); let mut samples_without_timestamps: HashMap<(&[Location], &[Label]), Vec> = HashMap::new(); @@ -569,6 +592,7 @@ fn fuzz_add_sample_with_fixed_sample_length() { }) .for_each(|(sample_types, samples)| { let mut profile = Profile::new(sample_types, None); + configure_custom_sample_types(&mut profile, sample_types); let mut samples_with_timestamps = Vec::new(); let mut samples_without_timestamps: HashMap<(&[Location], &[Label]), Vec> = HashMap::new(); @@ -683,6 +707,7 @@ fn fuzz_api_function_calls() { let operations = operations.iter().map(Operation::from).collect::>(); let mut profile = Profile::new(sample_types, None); + configure_custom_sample_types(&mut profile, sample_types); let mut samples_with_timestamps: Vec<&Sample> = Vec::new(); let mut samples_without_timestamps: HashMap<(&[Location], &[Label]), Vec> = HashMap::new(); diff --git a/libdd-profiling/src/internal/profile/mod.rs b/libdd-profiling/src/internal/profile/mod.rs index c645f5d480..281c652880 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -383,16 +383,6 @@ impl Profile { Self::try_new(sample_types, period).unwrap() } - /// Test helper for the raw value-type path. - #[cfg(test)] - pub fn new_with_value_types( - sample_types: &[api::ValueType<'static>], - period: Option<(api::ValueType<'static>, i64)>, - ) -> Self { - #[allow(clippy::unwrap_used)] - Self::try_new_with_value_types(sample_types, period).unwrap() - } - /// Tries to create a profile with the given `period`. /// Initializes the string table to hold common strings such as: /// - "" (the empty string) @@ -460,51 +450,44 @@ impl Profile { ) } - /// Creates a profile using raw `(type, unit)` string pairs, bypassing the - /// [`SampleType`][api::SampleType] enum. - /// - /// This is the escape hatch for **prototyping** new profile types before - /// they are added to [`SampleType`][api::SampleType]. The string slices - /// must have `'static` lifetime (string literals are the typical case). - /// - /// # Period - /// The `period` is profile-level sampling metadata, not per-sample-type - /// metadata. It describes the sampling distance/cadence (for example, - /// every 10ms of CPU time), while the profile duration describes the upload - /// or reporting window. Pass `None` when the custom type has no meaningful - /// sampling cadence. - /// - /// The raw period form exists for future/custom profile types whose - /// sampling trigger is not yet represented by [`SampleType`][api::SampleType], - /// and for compatibility with formats such as OpenTelemetry Profiles where - /// each profile has a single sample type and period. + /// Configure one of the custom sample type slots with its concrete `(type, unit)` pair. /// - /// # Stability note - /// Once a custom type is stable and agreed upon across profiler teams, - /// add a variant to [`SampleType`][api::SampleType] and migrate callers - /// to [`Profile::try_new`] instead. - pub fn try_new_with_value_types( - sample_types: &[api::ValueType<'static>], - period: Option<(api::ValueType<'static>, i64)>, - ) -> io::Result { - Self::try_new_internal(period, sample_types.to_vec().into_boxed_slice(), None, None) - } + /// Custom slots are placeholders (`Custom1` through `Custom5`) that can be used to + /// prototype new profile types without a libdatadog release. A profile that uses a + /// custom slot must configure it before serialization. Once a type is stable, add a + /// dedicated [`SampleType`][api::SampleType] variant and migrate callers back to the + /// type-safe path. + pub fn set_custom_sample_type( + &mut self, + slot: api::SampleType, + value_type: api::ValueType<'static>, + ) -> anyhow::Result<()> { + anyhow::ensure!( + slot.is_custom(), + "{slot:?} is not a custom sample type slot" + ); - /// Like [`Profile::try_new_with_value_types`] but with a shared dictionary. - /// - /// # Stability note - /// Prefer [`Profile::try_new_with_dictionary`] once the types are stable. - pub fn try_new_with_value_types_and_dictionary( - sample_types: &[api::ValueType<'static>], - period: Option<(api::ValueType<'static>, i64)>, - profiles_dictionary: crate::profiles::collections::Arc, - ) -> io::Result { - Self::try_new_internal( - period, - sample_types.to_vec().into_boxed_slice(), - None, - Some(ProfilesDictionaryTranslator::new(profiles_dictionary)), - ) + let placeholder = api::ValueType::from(slot); + let mut found = false; + for sample_type in self.sample_types.iter_mut() { + if *sample_type == placeholder { + *sample_type = value_type; + found = true; + } + } + + if let Some((period_type, _)) = &mut self.period { + if *period_type == placeholder { + *period_type = value_type; + found = true; + } + } + + anyhow::ensure!( + found, + "{slot:?} is not used by this profile's sample types or period" + ); + Ok(()) } /// Resets all data except the sample types and period. @@ -676,6 +659,21 @@ impl Profile { Record::<_, 2, NO_OPT_ZERO>::from(item).encode(writer)?; } + for vt in self.sample_types.iter().copied() { + anyhow::ensure!( + !Self::is_unresolved_custom_value_type(vt), + "custom sample type slot {} was not configured before serialization", + vt.r#type + ); + } + if let Some((vt, _)) = self.period { + anyhow::ensure!( + !Self::is_unresolved_custom_value_type(vt), + "custom period type slot {} was not configured before serialization", + vt.r#type + ); + } + // `Sample`s must be emitted before `SampleTypes` since we consume // fields as we convert (using `into_iter`). This allows Rust to // release memory faster, reducing our peak RSS, but means that we @@ -919,6 +917,18 @@ impl Profile { self.stack_traces.try_dedup(StackTrace { locations }) } + #[inline(always)] + fn is_unresolved_custom_value_type(vt: api::ValueType<'static>) -> bool { + matches!( + (vt.r#type, vt.unit), + ("custom-1", "count") + | ("custom-2", "count") + | ("custom-3", "count") + | ("custom-4", "count") + | ("custom-5", "count") + ) + } + #[inline(always)] fn intern_value_type(&mut self, vt: api::ValueType<'static>) -> ValueType { ValueType { @@ -3183,18 +3193,23 @@ mod api_tests { } // ----------------------------------------------------------------------- - // Custom value type tests + // Custom sample type slot tests // ----------------------------------------------------------------------- - /// Basic smoke test: a profile created with custom types serializes without - /// error and round-trips the type/unit strings through pprof. + /// Basic smoke test: a profile created with a custom slot serializes without + /// error after the slot is configured, and round-trips the configured + /// type/unit strings through pprof. #[test] - fn custom_value_types_round_trip() -> anyhow::Result<()> { - const MEMORY_BREAKDOWN: api::ValueType<'static> = - api::ValueType::new("memory-breakdown", "bytes"); - - let period = (MEMORY_BREAKDOWN, 4096_i64); - let mut profile = Profile::new_with_value_types(&[MEMORY_BREAKDOWN], Some(period)); + fn custom_sample_type_slot_round_trip() -> anyhow::Result<()> { + let period = api::Period { + sample_type: api::SampleType::Custom1, + value: 4096_i64, + }; + let mut profile = Profile::new(&[api::SampleType::Custom1], Some(period)); + profile.set_custom_sample_type( + api::SampleType::Custom1, + api::ValueType::new("memory-breakdown", "bytes"), + )?; let sample = api::Sample { locations: vec![], @@ -3205,7 +3220,6 @@ mod api_tests { let pprof = roundtrip_to_pprof(profile)?; - // Verify the custom sample type is present in the serialized pprof. let st_strings: Vec<(&str, &str)> = pprof .sample_types .iter() @@ -3219,7 +3233,6 @@ mod api_tests { assert_eq!(st_strings, vec![("memory-breakdown", "bytes")]); - // Verify the period type. let period_type = pprof.period_type.expect("period type should be present"); assert_eq!( string_table_fetch(&pprof, period_type.r#type).as_str(), @@ -3233,14 +3246,15 @@ mod api_tests { Ok(()) } - /// Custom types survive a profile reset: the new profile keeps the same - /// (type, unit) strings. + /// Custom slot configuration survives a profile reset: the new profile keeps + /// the configured (type, unit) strings. #[test] - fn custom_value_types_survive_reset() -> anyhow::Result<()> { - const MEMORY_BREAKDOWN: api::ValueType<'static> = - api::ValueType::new("memory-breakdown", "bytes"); - - let mut profile = Profile::new_with_value_types(&[MEMORY_BREAKDOWN], None); + fn custom_sample_type_slot_survives_reset() -> anyhow::Result<()> { + let mut profile = Profile::new(&[api::SampleType::Custom1], None); + profile.set_custom_sample_type( + api::SampleType::Custom1, + api::ValueType::new("memory-breakdown", "bytes"), + )?; let sample = api::Sample { locations: vec![], @@ -3250,8 +3264,6 @@ mod api_tests { profile.try_add_sample(sample, None)?; profile.reset_and_return_previous()?; // discards old, keeps type config - // Profile is now empty but should still accept samples with the same - // value count. let sample2 = api::Sample { locations: vec![], values: &[2048], @@ -3270,12 +3282,17 @@ mod api_tests { Ok(()) } - /// A profile created with custom types rejects samples that have the wrong - /// number of values (same guard as for SampleType-based profiles). + /// A profile created with custom slots rejects samples that have the wrong + /// number of values (same guard as for stable SampleType-based profiles). #[test] - fn custom_value_types_wrong_value_count_is_rejected() { - const T: api::ValueType<'static> = api::ValueType::new("my-custom-type", "count"); - let mut profile = Profile::new_with_value_types(&[T], None); + fn custom_sample_type_slot_wrong_value_count_is_rejected() { + let mut profile = Profile::new(&[api::SampleType::Custom1], None); + profile + .set_custom_sample_type( + api::SampleType::Custom1, + api::ValueType::new("my-custom-type", "count"), + ) + .unwrap(); let bad_sample = api::Sample { locations: vec![], @@ -3288,22 +3305,27 @@ mod api_tests { ); } - /// SampleType-based and value-type-based profiles produce identical pprof - /// output for the same well-known type. #[test] - fn custom_value_types_match_sample_type_output() -> anyhow::Result<()> { - // Build profile via the SampleType enum path. - let enum_profile = Profile::new(&[api::SampleType::WallTime], None); - let enum_pprof = roundtrip_to_pprof(enum_profile)?; - - // Build profile via the raw ValueType path using the same type/unit. - let vt = api::ValueType::from(api::SampleType::WallTime); - let vt_profile = Profile::new_with_value_types(&[vt], None); - let vt_pprof = roundtrip_to_pprof(vt_profile)?; - - // Both should produce identical sample_types in the pprof output. - assert_eq!(enum_pprof.sample_types, vt_pprof.sample_types); + fn unresolved_custom_sample_type_slot_is_rejected_on_serialize() { + let profile = Profile::new(&[api::SampleType::Custom1], None); + let result = profile.serialize_into_compressed_pprof(None, None); + assert!( + result.is_err(), + "expected serialization to fail for unresolved custom sample type slot" + ); + } - Ok(()) + #[test] + fn custom_sample_type_setter_rejects_non_custom_slot() { + let mut profile = Profile::new(&[api::SampleType::WallTime], None); + assert!( + profile + .set_custom_sample_type( + api::SampleType::WallTime, + api::ValueType::new("memory-breakdown", "bytes"), + ) + .is_err(), + "expected setter to reject non-custom sample type slots" + ); } } From f876ccefced6b73715ab71b1d08564263e94320c Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 14 Jul 2026 18:49:01 +0000 Subject: [PATCH 4/5] Cleanup PR to avoid changing types and leaking --- datadog-profiling-replayer/src/replayer.rs | 24 ++- libdd-profiling-ffi/src/profiles/datatypes.rs | 19 +-- libdd-profiling-heap-gotter/src/elf.rs | 4 +- libdd-profiling/src/cxx.rs | 8 +- .../src/internal/profile/fuzz_tests.rs | 2 +- libdd-profiling/src/internal/profile/mod.rs | 147 +++++++++--------- 6 files changed, 95 insertions(+), 109 deletions(-) diff --git a/datadog-profiling-replayer/src/replayer.rs b/datadog-profiling-replayer/src/replayer.rs index 6041d6d212..58fc445751 100644 --- a/datadog-profiling-replayer/src/replayer.rs +++ b/datadog-profiling-replayer/src/replayer.rs @@ -17,7 +17,7 @@ type SamplesAndEndpointInfo<'pprof> = ( Vec<(Option, api::Sample<'pprof>)>, Vec<(u64, &'pprof str)>, ); -type CustomTypeMappings = Vec<(api::SampleType, api::ValueType<'static>)>; +type CustomTypeMappings<'pprof> = Vec<(api::SampleType, api::ValueType<'pprof>)>; pub struct Replayer<'pprof> { pub profile_index: ProfileIndex<'pprof>, @@ -27,7 +27,7 @@ pub struct Replayer<'pprof> { pub end_time: SystemTime, // start_time + duration pub sample_types: Vec, pub period: Option, - pub custom_type_mappings: CustomTypeMappings, + pub custom_type_mappings: CustomTypeMappings<'pprof>, pub endpoints: Vec<(u64, &'pprof str)>, pub samples: Vec<(Option, api::Sample<'pprof>)>, } @@ -68,9 +68,9 @@ impl<'pprof> Replayer<'pprof> { } fn resolve_sample_type( - type_str: &str, - unit: &str, - custom_type_mappings: &mut CustomTypeMappings, + type_str: &'pprof str, + unit: &'pprof str, + custom_type_mappings: &mut CustomTypeMappings<'pprof>, ) -> anyhow::Result { let borrowed = api::ValueType::new(type_str, unit); if let Ok(sample_type) = api::SampleType::try_from(borrowed) { @@ -88,15 +88,13 @@ impl<'pprof> Replayer<'pprof> { let slot = *slots .get(custom_type_mappings.len()) .ok_or_else(|| anyhow::anyhow!("pprof uses more than 5 custom sample types"))?; - let type_str: &'static str = Box::leak(type_str.to_string().into_boxed_str()); - let unit: &'static str = Box::leak(unit.to_string().into_boxed_str()); custom_type_mappings.push((slot, api::ValueType::new(type_str, unit))); Ok(slot) } - fn sample_types<'a>( - profile_index: &'a ProfileIndex<'pprof>, - custom_type_mappings: &mut CustomTypeMappings, + fn sample_types( + profile_index: &ProfileIndex<'pprof>, + custom_type_mappings: &mut CustomTypeMappings<'pprof>, ) -> anyhow::Result> { let mut sample_types = Vec::with_capacity(profile_index.pprof.sample_types.len()); for sample_type in profile_index.pprof.sample_types.iter() { @@ -111,9 +109,9 @@ impl<'pprof> Replayer<'pprof> { Ok(sample_types) } - fn period<'a>( - profile_index: &'a ProfileIndex<'pprof>, - custom_type_mappings: &mut CustomTypeMappings, + fn period( + profile_index: &ProfileIndex<'pprof>, + custom_type_mappings: &mut CustomTypeMappings<'pprof>, ) -> anyhow::Result> { let value = profile_index.pprof.period; diff --git a/libdd-profiling-ffi/src/profiles/datatypes.rs b/libdd-profiling-ffi/src/profiles/datatypes.rs index 711054853c..18ae88cf3f 100644 --- a/libdd-profiling-ffi/src/profiles/datatypes.rs +++ b/libdd-profiling-ffi/src/profiles/datatypes.rs @@ -529,19 +529,12 @@ pub unsafe extern "C" fn ddog_prof_Profile_set_custom_sample_type( ) -> ProfileResult { (|| { let profile = profile_ptr_to_inner(profile)?; - let type_str: &'static str = Box::leak( - type_str - .try_to_utf8() - .context("invalid UTF-8 in custom profile type")? - .to_string() - .into_boxed_str(), - ); - let unit: &'static str = Box::leak( - unit.try_to_utf8() - .context("invalid UTF-8 in custom profile unit")? - .to_string() - .into_boxed_str(), - ); + let type_str = type_str + .try_to_utf8() + .context("invalid UTF-8 in custom profile type")?; + let unit = unit + .try_to_utf8() + .context("invalid UTF-8 in custom profile unit")?; profile.set_custom_sample_type(slot, api::ValueType::new(type_str, unit)) })() .context("ddog_prof_Profile_set_custom_sample_type failed") diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 27e786b2a5..a09aaff407 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -22,8 +22,8 @@ use std::collections::HashMap; use std::ffi::CStr; use libc::{ - dl_iterate_phdr, dl_phdr_info, mprotect, sysconf, Elf64_Rel, Elf64_Rela, Elf64_Sym, PROT_EXEC, - PROT_READ, PROT_WRITE, PT_DYNAMIC, PT_LOAD, _SC_PAGESIZE, + dl_iterate_phdr, dl_phdr_info, mprotect, sysconf, Elf64_Rel, Elf64_Rela, Elf64_Sym, + _SC_PAGESIZE, PROT_EXEC, PROT_READ, PROT_WRITE, PT_DYNAMIC, PT_LOAD, }; use std::io::{BufRead, BufReader}; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/libdd-profiling/src/cxx.rs b/libdd-profiling/src/cxx.rs index c5b34bc805..c52ce8f20a 100644 --- a/libdd-profiling/src/cxx.rs +++ b/libdd-profiling/src/cxx.rs @@ -538,12 +538,6 @@ pub struct Profile { } impl Profile { - fn owned_value_type(type_: &str, unit: &str) -> api::ValueType<'static> { - let type_: &'static str = Box::leak(type_.to_string().into_boxed_str()); - let unit: &'static str = Box::leak(unit.to_string().into_boxed_str()); - api::ValueType::new(type_, unit) - } - pub fn create( sample_types: Vec, period: &ffi::Period, @@ -590,7 +584,7 @@ impl Profile { ) -> anyhow::Result<()> { let slot: api::SampleType = slot.try_into()?; self.inner - .set_custom_sample_type(slot, Self::owned_value_type(type_, unit)) + .set_custom_sample_type(slot, api::ValueType::new(type_, unit)) } pub fn add_endpoint(&mut self, local_root_span_id: u64, endpoint: &str) -> anyhow::Result<()> { diff --git a/libdd-profiling/src/internal/profile/fuzz_tests.rs b/libdd-profiling/src/internal/profile/fuzz_tests.rs index 73fb9ec917..c6912f9ee8 100644 --- a/libdd-profiling/src/internal/profile/fuzz_tests.rs +++ b/libdd-profiling/src/internal/profile/fuzz_tests.rs @@ -305,7 +305,7 @@ fn configure_custom_sample_types(profile: &mut Profile, sample_types: &[api::Sam for sample_type in sample_types.iter().copied() { if let Some(value_type) = configured_custom_value_type(sample_type) { // Ignore errors: duplicate custom slots are expected in fuzz input, and the first - // successful configuration updates every occurrence of the placeholder in the profile. + // successful configuration covers every occurrence of the slot in the profile. let _ = profile.set_custom_sample_type(sample_type, value_type); } } diff --git a/libdd-profiling/src/internal/profile/mod.rs b/libdd-profiling/src/internal/profile/mod.rs index 281c652880..6243efef2e 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -45,8 +45,9 @@ pub struct Profile { locations: FxIndexSet, mappings: FxIndexSet, observations: Observations, - period: Option<(api::ValueType<'static>, i64)>, - sample_types: Box<[api::ValueType<'static>]>, + period: Option, + sample_types: Box<[api::SampleType]>, + sample_type_overrides: HashMap, stack_traces: FxIndexSet, start_time: SystemTime, strings: StringTable, @@ -56,6 +57,21 @@ pub struct Profile { upscaling_rules: UpscalingRules, } +#[derive(Clone, Debug, Eq, PartialEq)] +struct OwnedValueType { + r#type: Box, + unit: Box, +} + +impl From> for OwnedValueType { + fn from(value_type: api::ValueType<'_>) -> Self { + Self { + r#type: value_type.r#type.into(), + unit: value_type.unit.into(), + } + } +} + pub struct EncodedProfile { pub start: SystemTime, pub end: SystemTime, @@ -398,13 +414,9 @@ impl Profile { period: Option, ) -> io::Result { Self::try_new_internal( - period.map(|p| (api::ValueType::from(p.sample_type), p.value)), - sample_types - .iter() - .copied() - .map(api::ValueType::from) - .collect::>() - .into_boxed_slice(), + period, + sample_types.to_vec().into_boxed_slice(), + HashMap::new(), None, None, ) @@ -419,13 +431,9 @@ impl Profile { profiles_dictionary: crate::profiles::collections::Arc, ) -> io::Result { Self::try_new_internal( - period.map(|p| (api::ValueType::from(p.sample_type), p.value)), - sample_types - .iter() - .copied() - .map(api::ValueType::from) - .collect::>() - .into_boxed_slice(), + period, + sample_types.to_vec().into_boxed_slice(), + HashMap::new(), None, Some(ProfilesDictionaryTranslator::new(profiles_dictionary)), ) @@ -438,13 +446,9 @@ impl Profile { string_storage: Arc>, ) -> io::Result { Self::try_new_internal( - period.map(|p| (api::ValueType::from(p.sample_type), p.value)), - sample_types - .iter() - .copied() - .map(api::ValueType::from) - .collect::>() - .into_boxed_slice(), + period, + sample_types.to_vec().into_boxed_slice(), + HashMap::new(), Some(string_storage), None, ) @@ -460,25 +464,16 @@ impl Profile { pub fn set_custom_sample_type( &mut self, slot: api::SampleType, - value_type: api::ValueType<'static>, + value_type: api::ValueType<'_>, ) -> anyhow::Result<()> { anyhow::ensure!( slot.is_custom(), "{slot:?} is not a custom sample type slot" ); - let placeholder = api::ValueType::from(slot); - let mut found = false; - for sample_type in self.sample_types.iter_mut() { - if *sample_type == placeholder { - *sample_type = value_type; - found = true; - } - } - - if let Some((period_type, _)) = &mut self.period { - if *period_type == placeholder { - *period_type = value_type; + let mut found = self.sample_types.iter().copied().any(|st| st == slot); + if let Some(period) = self.period { + if period.sample_type == slot { found = true; } } @@ -487,6 +482,8 @@ impl Profile { found, "{slot:?} is not used by this profile's sample types or period" ); + + self.sample_type_overrides.insert(slot, value_type.into()); Ok(()) } @@ -511,7 +508,8 @@ impl Profile { let mut profile = Profile::try_new_internal( self.period, - self.sample_types.clone(), // Box<[ValueType<'static>]> is cheap to clone (static strs) + self.sample_types.clone(), + self.sample_type_overrides.clone(), self.string_storage.clone(), profiles_dictionary_translator, ) @@ -659,19 +657,11 @@ impl Profile { Record::<_, 2, NO_OPT_ZERO>::from(item).encode(writer)?; } - for vt in self.sample_types.iter().copied() { - anyhow::ensure!( - !Self::is_unresolved_custom_value_type(vt), - "custom sample type slot {} was not configured before serialization", - vt.r#type - ); + for sample_type in self.sample_types.iter().copied() { + self.ensure_custom_sample_type_resolved(sample_type, "sample")?; } - if let Some((vt, _)) = self.period { - anyhow::ensure!( - !Self::is_unresolved_custom_value_type(vt), - "custom period type slot {} was not configured before serialization", - vt.r#type - ); + if let Some(period) = self.period { + self.ensure_custom_sample_type_resolved(period.sample_type, "period")?; } // `Sample`s must be emitted before `SampleTypes` since we consume @@ -686,16 +676,18 @@ impl Profile { // so we must serialize `Sample` before `SampleType`. // Clone to avoid holding an immutable borrow of `self.sample_types` while interning. let sample_types = self.sample_types.clone(); - for vt in sample_types.iter().copied() { - let vt = self.intern_value_type(vt); - Record::<_, 1, NO_OPT_ZERO>::from(vt).encode(writer)?; + for sample_type in sample_types.iter().copied() { + let sample_type = self.intern_sample_type(sample_type); + Record::<_, 1, NO_OPT_ZERO>::from(sample_type).encode(writer)?; } // Period type needs string interning, which must happen before we start moving fields out // of `self`. (Many fields below are consumed via `into_iter()`.) - let period_type_and_value: Option<(ValueType, i64)> = self - .period - .map(|(vt, value)| (self.intern_value_type(vt), value)); + let period_type_and_value: Option<(ValueType, i64)> = if let Some(period) = self.period { + Some((self.intern_sample_type(period.sample_type), period.value)) + } else { + None + }; for (offset, item) in self.mappings.into_iter().enumerate() { let mapping = protobuf::Mapping { @@ -917,23 +909,30 @@ impl Profile { self.stack_traces.try_dedup(StackTrace { locations }) } - #[inline(always)] - fn is_unresolved_custom_value_type(vt: api::ValueType<'static>) -> bool { - matches!( - (vt.r#type, vt.unit), - ("custom-1", "count") - | ("custom-2", "count") - | ("custom-3", "count") - | ("custom-4", "count") - | ("custom-5", "count") - ) + fn ensure_custom_sample_type_resolved( + &self, + sample_type: api::SampleType, + kind: &str, + ) -> anyhow::Result<()> { + anyhow::ensure!( + !sample_type.is_custom() || self.sample_type_overrides.contains_key(&sample_type), + "custom {kind} type slot {sample_type:?} was not configured before serialization" + ); + Ok(()) } #[inline(always)] - fn intern_value_type(&mut self, vt: api::ValueType<'static>) -> ValueType { + fn intern_sample_type(&mut self, sample_type: api::SampleType) -> ValueType { + let (type_, unit) = if let Some(value_type) = self.sample_type_overrides.get(&sample_type) { + (value_type.r#type.as_ref(), value_type.unit.as_ref()) + } else { + let value_type = api::ValueType::from(sample_type); + (value_type.r#type, value_type.unit) + }; + ValueType { - r#type: Record::from(self.intern(vt.r#type)), - unit: Record::from(self.intern(vt.unit)), + r#type: Record::from(self.strings.intern(type_)), + unit: Record::from(self.strings.intern(unit)), } } @@ -1011,6 +1010,7 @@ impl Profile { /// Interns the `str` as a string, returning the id in the string table. /// The empty string is guaranteed to have an id of [StringId::ZERO]. + #[cfg(test)] #[inline] fn intern(&mut self, item: &str) -> StringId { self.strings.intern(item) @@ -1026,8 +1026,9 @@ impl Profile { /// Creates a profile from the period, sample types, and start time using /// the owned values. fn try_new_internal( - period: Option<(api::ValueType<'static>, i64)>, - sample_types: Box<[api::ValueType<'static>]>, + period: Option, + sample_types: Box<[api::SampleType]>, + sample_type_overrides: HashMap, string_storage: Option>>, profiles_dictionary_translator: Option, ) -> io::Result { @@ -1047,6 +1048,7 @@ impl Profile { observations: Default::default(), period, sample_types, + sample_type_overrides, stack_traces: Default::default(), start_time, strings: Default::default(), @@ -1473,9 +1475,8 @@ mod api_tests { .reset_and_return_previous() .expect("reset to succeed"); - let expected_period = Some((api::ValueType::from(period.sample_type), period.value)); - assert_eq!(profile.period, expected_period); - assert_eq!(prev.period, expected_period); + assert_eq!(profile.period, Some(period)); + assert_eq!(prev.period, Some(period)); } #[test] From 0c7d2619f641c29f5ea6080b0543c6128aaffcb8 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 14 Jul 2026 19:07:42 +0000 Subject: [PATCH 5/5] clippy --- libdd-profiling-heap-gotter/src/elf.rs | 4 ++-- libdd-profiling/src/internal/profile/mod.rs | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index a09aaff407..27e786b2a5 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -22,8 +22,8 @@ use std::collections::HashMap; use std::ffi::CStr; use libc::{ - dl_iterate_phdr, dl_phdr_info, mprotect, sysconf, Elf64_Rel, Elf64_Rela, Elf64_Sym, - _SC_PAGESIZE, PROT_EXEC, PROT_READ, PROT_WRITE, PT_DYNAMIC, PT_LOAD, + dl_iterate_phdr, dl_phdr_info, mprotect, sysconf, Elf64_Rel, Elf64_Rela, Elf64_Sym, PROT_EXEC, + PROT_READ, PROT_WRITE, PT_DYNAMIC, PT_LOAD, _SC_PAGESIZE, }; use std::io::{BufRead, BufReader}; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/libdd-profiling/src/internal/profile/mod.rs b/libdd-profiling/src/internal/profile/mod.rs index 6243efef2e..beb3742fb9 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -683,11 +683,9 @@ impl Profile { // Period type needs string interning, which must happen before we start moving fields out // of `self`. (Many fields below are consumed via `into_iter()`.) - let period_type_and_value: Option<(ValueType, i64)> = if let Some(period) = self.period { - Some((self.intern_sample_type(period.sample_type), period.value)) - } else { - None - }; + let period_type_and_value: Option<(ValueType, i64)> = self + .period + .map(|period| (self.intern_sample_type(period.sample_type), period.value)); for (offset, item) in self.mappings.into_iter().enumerate() { let mapping = protobuf::Mapping {