diff --git a/datadog-profiling-replayer/src/main.rs b/datadog-profiling-replayer/src/main.rs index 6a8ae2b32f..8fe036acbb 100644 --- a/datadog-profiling-replayer/src/main.rs +++ b/datadog-profiling-replayer/src/main.rs @@ -166,6 +166,9 @@ fn main() -> anyhow::Result<()> { 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 6908645ee7..58fc445751 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<'pprof> = Vec<(api::SampleType, api::ValueType<'pprof>)>; pub struct Replayer<'pprof> { pub profile_index: ProfileIndex<'pprof>, @@ -26,6 +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<'pprof>, pub endpoints: Vec<(u64, &'pprof str)>, pub samples: Vec<(Option, api::Sample<'pprof>)>, } @@ -55,28 +57,69 @@ impl<'pprof> Replayer<'pprof> { } } - fn sample_types<'a>( - profile_index: &'a ProfileIndex<'pprof>, + 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: &'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) { + 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); + } + + 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"))?; + custom_type_mappings.push((slot, api::ValueType::new(type_str, unit))); + Ok(slot) + } + + 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() { 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::resolve_sample_type( + type_str, + unit, + custom_type_mappings, + )?); } Ok(sample_types) } - fn period<'a>(profile_index: &'a ProfileIndex<'pprof>) -> anyhow::Result> { + fn period( + profile_index: &ProfileIndex<'pprof>, + custom_type_mappings: &mut CustomTypeMappings<'pprof>, + ) -> 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)?; - let vt = api::ValueType::new(type_str, unit); - let sample_type = vt.try_into()?; + let sample_type = Self::resolve_sample_type(type_str, unit, custom_type_mappings)?; Ok(Some(api::Period { sample_type, value })) } None => Ok(None), @@ -252,8 +295,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 { @@ -263,6 +307,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 a309712b18..34fe85b49f 100644 --- a/examples/cxx/profiling.cpp +++ b/examples/cxx/profiling.cpp @@ -21,9 +21,9 @@ 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 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/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..df75805da5 --- /dev/null +++ b/examples/ffi/custom_profile_types.c @@ -0,0 +1,138 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +// Example: creating a profile with a custom sample type slot. +// +// 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 a dedicated ddog_prof_SampleType variant and switch callers to it. + +#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. Select one of the custom sample type slots. + // ------------------------------------------------------------------------- + const ddog_prof_SampleType sample_types[] = { + DDOG_PROF_SAMPLE_TYPE_CUSTOM1, + }; + const ddog_prof_Slice_SampleType sample_types_slice = { + .ptr = sample_types, + .len = sizeof(sample_types) / sizeof(sample_types[0]), + }; + + // 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. + // ------------------------------------------------------------------------- + ddog_prof_Profile_NewResult new_result = + 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 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")) { + 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..18ae88cf3f 100644 --- a/libdd-profiling-ffi/src/profiles/datatypes.rs +++ b/libdd-profiling-ffi/src/profiles/datatypes.rs @@ -508,6 +508,39 @@ 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 = 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") + .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. @@ -956,6 +989,81 @@ mod tests { } } + #[test] + fn profile_set_custom_sample_type_accepts_slot() -> 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, + ))?; + + 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(), + 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_set_custom_sample_type_rejects_non_custom_slot() -> Result<(), Error> { + unsafe { + 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, ProfileResult::Err(_)), + "expected Err when configuring a non-custom slot" + ); + ddog_prof_Profile_drop(&mut profile); + Ok(()) + } + } + + #[test] + 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 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, ProfileResult::Err(_)), + "expected Err for invalid UTF-8 in custom profile type" + ); + ddog_prof_Profile_drop(&mut profile); + Ok(()) + } + } + #[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..3c79584cf0 100644 --- a/libdd-profiling/src/api/sample_type.rs +++ b/libdd-profiling/src/api/sample_type.rs @@ -12,6 +12,14 @@ 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 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)] @@ -74,11 +82,35 @@ pub enum SampleType { WallTime, /// Legacy: Use `WallTime` instead for consistency with naming scheme WallLegacy, + /// 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, +} - // Experimental sample types for testing and development. - ExperimentalCount, - ExperimentalNanoseconds, - ExperimentalBytes, +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> { @@ -160,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"), } } } @@ -226,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 312b1c636d..c52ce8f20a 100644 --- a/libdd-profiling/src/cxx.rs +++ b/libdd-profiling/src/cxx.rs @@ -26,8 +26,8 @@ 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, use Custom1 through Custom5 and configure + // the slot on the Profile before serialization. // // LEGACY VARIANTS (prefer alternatives for consistency): // - CpuLegacy (use CpuTime) @@ -89,9 +89,11 @@ pub mod ffi { WallSamples, WallTime, WallLegacy, // LEGACY: Use WallTime instead - ExperimentalCount, - ExperimentalNanoseconds, - ExperimentalBytes, + Custom1, + Custom2, + Custom3, + Custom4, + Custom5, } struct Period { @@ -160,8 +162,18 @@ pub mod ffi { #[Self = "Profile"] fn create(sample_types: Vec, period: &Period) -> Result>; + /// Create a profile without a sampling period. + #[Self = "Profile"] + 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<()>; @@ -391,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++"), }) } @@ -541,6 +555,15 @@ impl Profile { Ok(Box::new(Profile { inner })) } + 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 })) + } + pub fn add_sample(&mut self, sample: &ffi::Sample) -> anyhow::Result<()> { let api_sample = api::Sample { locations: sample.locations.iter().map(Into::into).collect(), @@ -553,6 +576,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, api::ValueType::new(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..c6912f9ee8 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 covers every occurrence of the slot 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 e92ad4e393..beb3742fb9 100644 --- a/libdd-profiling/src/internal/profile/mod.rs +++ b/libdd-profiling/src/internal/profile/mod.rs @@ -47,6 +47,7 @@ pub struct Profile { observations: Observations, 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, @@ -397,7 +413,13 @@ 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, + sample_types.to_vec().into_boxed_slice(), + HashMap::new(), + None, + None, + ) } /// Tries to create a profile with the given period and sample types. @@ -411,6 +433,7 @@ impl Profile { Self::try_new_internal( period, sample_types.to_vec().into_boxed_slice(), + HashMap::new(), None, Some(ProfilesDictionaryTranslator::new(profiles_dictionary)), ) @@ -425,11 +448,45 @@ impl Profile { Self::try_new_internal( period, sample_types.to_vec().into_boxed_slice(), + HashMap::new(), Some(string_storage), None, ) } + /// Configure one of the custom sample type slots with its concrete `(type, unit)` pair. + /// + /// 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<'_>, + ) -> anyhow::Result<()> { + anyhow::ensure!( + slot.is_custom(), + "{slot:?} is not a custom sample type slot" + ); + + 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; + } + } + + anyhow::ensure!( + found, + "{slot:?} is not used by this profile's sample types or period" + ); + + self.sample_type_overrides.insert(slot, value_type.into()); + Ok(()) + } + /// Resets all data except the sample types and period. /// Returns the previous Profile on success. #[inline] @@ -452,6 +509,7 @@ impl Profile { let mut profile = Profile::try_new_internal( self.period, self.sample_types.clone(), + self.sample_type_overrides.clone(), self.string_storage.clone(), profiles_dictionary_translator, ) @@ -599,6 +657,13 @@ impl Profile { Record::<_, 2, NO_OPT_ZERO>::from(item).encode(writer)?; } + for sample_type in self.sample_types.iter().copied() { + self.ensure_custom_sample_type_resolved(sample_type, "sample")?; + } + 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 // fields as we convert (using `into_iter`). This allows Rust to // release memory faster, reducing our peak RSS, but means that we @@ -842,12 +907,30 @@ impl Profile { self.stack_traces.try_dedup(StackTrace { locations }) } + 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_sample_type(&mut self, sample_type: api::SampleType) -> ValueType { - let vt: api::ValueType<'static> = sample_type.into(); + 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)), } } @@ -925,6 +1008,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) @@ -942,6 +1026,7 @@ impl Profile { fn try_new_internal( period: Option, sample_types: Box<[api::SampleType]>, + sample_type_overrides: HashMap, string_storage: Option>>, profiles_dictionary_translator: Option, ) -> io::Result { @@ -961,6 +1046,7 @@ impl Profile { observations: Default::default(), period, sample_types, + sample_type_overrides, stack_traces: Default::default(), start_time, strings: Default::default(), @@ -3104,4 +3190,141 @@ mod api_tests { .expect("Expected serialization error to be an io::Error"); assert_eq!(io_err.kind(), io::ErrorKind::InvalidData); } + + // ----------------------------------------------------------------------- + // Custom sample type slot tests + // ----------------------------------------------------------------------- + + /// 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_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![], + values: &[4096], + labels: vec![], + }; + profile.try_add_sample(sample, None)?; + + let pprof = roundtrip_to_pprof(profile)?; + + 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")]); + + 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 slot configuration survives a profile reset: the new profile keeps + /// the configured (type, unit) strings. + #[test] + 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![], + values: &[1024], + labels: vec![], + }; + profile.try_add_sample(sample, None)?; + profile.reset_and_return_previous()?; // discards old, keeps type config + + 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 slots rejects samples that have the wrong + /// number of values (same guard as for stable SampleType-based profiles). + #[test] + 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![], + 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" + ); + } + + #[test] + 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" + ); + } + + #[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" + ); + } }