From 4b4d2fd51148dbb28fcb85eaa599f5d70f455920 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 21 Mar 2025 16:57:20 +0000 Subject: [PATCH] [PROF-11524] Fix not being able to set profile start_time at serialization **What does this PR do?** In https://github.com/DataDog/libdatadog/pull/926 we removed the `start_time` argument from * `ddog_prof_Profile_new` * `ddog_prof_Profile_with_string_storage` * `ddog_prof_Profile_reset` The intention was that having it as an argument in `ddog_prof_Profile_serialize` was enough, and anyway almost everyone was passing in `null`s in the APIs above. I missed when suggesting in that PR that the `start_time` for serialize was the `start_time` for the next profile, not the one being serialized. In this PR I'm changing that behavior: the `start_time` argument to serialize now controls the time for the profile being serialized, allowing the profiling library to have exact control over this value. I've also removed the duration. [As can be seen in this github search](https://github.com/search?q=org%3ADataDog+ddog_prof_Profile_serialize&type=code) this is not expected to impact anyone: everyone's passing `NULL` for `start_time` and `duration` when calling serialize already. **Motivation:** Allow Ruby profiler to set the start_time of profiles. **Additional Notes:** N/A **How to test the change?** I've tested this with my experimental libdatadog 17 branch for Ruby. --- examples/ffi/exporter.cpp | 2 +- profiling-ffi/src/profiles.rs | 25 +++++++------------------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/examples/ffi/exporter.cpp b/examples/ffi/exporter.cpp index 4da9cc554..f8d7f4c67 100644 --- a/examples/ffi/exporter.cpp +++ b/examples/ffi/exporter.cpp @@ -88,7 +88,7 @@ int main(int argc, char *argv[]) { } ddog_prof_Profile_SerializeResult serialize_result = - ddog_prof_Profile_serialize(profile.get(), nullptr, nullptr, nullptr); + ddog_prof_Profile_serialize(profile.get(), nullptr, nullptr); if (serialize_result.tag == DDOG_PROF_PROFILE_SERIALIZE_RESULT_ERR) { print_error("Failed to serialize profile: ", serialize_result.err); ddog_Error_drop(&serialize_result.err); diff --git a/profiling-ffi/src/profiles.rs b/profiling-ffi/src/profiles.rs index 35c2bb55d..6f636b28f 100644 --- a/profiling-ffi/src/profiles.rs +++ b/profiling-ffi/src/profiles.rs @@ -11,7 +11,7 @@ use ddcommon_ffi::{wrap_with_ffi_result, Error, Handle, Timespec, ToInner}; use function_name::named; use std::num::NonZeroI64; use std::str::Utf8Error; -use std::time::{Duration, SystemTime}; +use std::time::SystemTime; /// Represents a profile. Do not access its member for any reason, only use /// the C API functions on this struct. @@ -742,42 +742,31 @@ pub unsafe extern "C" fn ddog_prof_EncodedProfile_bytes<'a>( /// /// # Arguments /// * `profile` - a reference to the profile being serialized. +/// * `start_time` - start time for the serialized profile. /// * `end_time` - optional end time of the profile. If None/null is passed, the current time will /// be used. -/// * `duration_nanos` - Optional duration of the profile. Passing None or a negative duration will -/// mean the duration will based on the end time minus the start time, but under anomalous -/// conditions this may fail as system clocks can be adjusted, or the programmer accidentally -/// passed an earlier time. The duration of the serialized profile will be set to zero for these -/// cases. -/// * `start_time` - Optional start time for the next profile. /// /// # Safety /// The `profile` must point to a valid profile object. -/// The `end_time` must be null or otherwise point to a valid TimeSpec object. -/// The `duration_nanos` must be null or otherwise point to a valid i64. +/// The `start_time` and `end_time` must be null or otherwise point to a valid TimeSpec object. #[must_use] #[no_mangle] pub unsafe extern "C" fn ddog_prof_Profile_serialize( profile: *mut Profile, - end_time: Option<&Timespec>, - duration_nanos: Option<&i64>, start_time: Option<&Timespec>, + end_time: Option<&Timespec>, ) -> SerializeResult { (|| { let profile = profile_ptr_to_inner(profile)?; - let old_profile = profile.reset_and_return_previous()?; if let Some(start_time) = start_time { profile.set_start_time(start_time.into())?; } + let old_profile = profile.reset_and_return_previous()?; + let end_time = end_time.map(SystemTime::from); - let duration = match duration_nanos { - None => None, - Some(x) if *x < 0 => None, - Some(x) => Some(Duration::from_nanos((*x) as u64)), - }; - old_profile.serialize_into_compressed_pprof(end_time, duration) + old_profile.serialize_into_compressed_pprof(end_time, None) })() .context("ddog_prof_Profile_serialize failed") .into()