Skip to content

Commit

Permalink
Expose start, end, and duration in APIs (#16)
Browse files Browse the repository at this point in the history
* Expose start, end, and duration in APIs

This allows the PHP profiler to back-date the start of the profile.
When the node.js profiler was trying to use this library, they also
encountered an issue with these not being exposed.

* Fix FFI failures caused by Tag being moved into ddcommon
  • Loading branch information
morrisonlevi authored May 27, 2022
1 parent 43177b8 commit 32ca509
Show file tree
Hide file tree
Showing 10 changed files with 212 additions and 130 deletions.
2 changes: 1 addition & 1 deletion ddprof-exporter/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::error;
use std::fmt;

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, Eq, PartialEq)]
#[allow(dead_code)]
pub(crate) enum Error {
InvalidUrl,
Expand Down
4 changes: 2 additions & 2 deletions ddprof-exporter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ use std::future;
use std::io::Cursor;
use std::str::FromStr;

use ddcommon::{container_id::get_container_id, tag::Tag};

use bytes::Bytes;
pub use chrono::{DateTime, Utc};
use ddcommon::container_id::get_container_id;
pub use ddcommon::tag::Tag;
use hyper::header::HeaderValue;
pub use hyper::Uri;
use hyper_multipart_rfc7578::client::multipart;
Expand Down
2 changes: 1 addition & 1 deletion ddprof-ffi/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ must_use = "__attribute__((warn_unused_result))"

[parse]
parse_deps = true
include = ["ddprof-exporter", "ddprof-profiles", "ux"]
include = ["ddcommon", "ddprof-exporter", "ddprof-profiles", "ux"]
3 changes: 3 additions & 0 deletions ddprof-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ pub unsafe extern "C" fn profile_exporter_send(
}
}

#[no_mangle]
pub extern "C" fn ddprof_ffi_Request_drop(_request: Option<Box<Request>>) {}

fn unwrap_cancellation_token<'a>(
cancel: Option<NonNull<CancellationToken>>,
) -> Option<&'a tokio_util::sync::CancellationToken> {
Expand Down
46 changes: 31 additions & 15 deletions ddprof-ffi/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc.

use std::convert::{TryFrom, TryInto};
use std::fmt::Debug;
use std::ops::Sub;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use std::time::SystemTime;

use chrono::{DateTime, TimeZone, Utc};

Expand All @@ -31,17 +29,35 @@ impl From<Timespec> for DateTime<Utc> {
}
}

impl TryFrom<SystemTime> for Timespec {
type Error = Box<dyn std::error::Error>;

fn try_from(value: SystemTime) -> Result<Self, Self::Error> {
let mut duration = value.duration_since(UNIX_EPOCH)?;
let seconds: i64 = duration.as_secs().try_into()?;
duration = duration.sub(Duration::from_secs(seconds as u64));
let nanoseconds: u32 = duration.as_nanos().try_into()?;
Ok(Self {
seconds,
nanoseconds,
})
impl From<Timespec> for SystemTime {
fn from(value: Timespec) -> Self {
// The DateTime API is more convenient, so let's delegate.
let datetime: DateTime<Utc> = value.into();
SystemTime::from(datetime)
}
}

impl<'a> From<&'a Timespec> for SystemTime {
fn from(value: &'a Timespec) -> Self {
// The DateTime API is more convenient, so let's delegate.
let datetime: DateTime<Utc> = (*value).into();
SystemTime::from(datetime)
}
}

impl From<DateTime<Utc>> for Timespec {
fn from(value: DateTime<Utc>) -> Self {
Self {
seconds: value.timestamp(),
nanoseconds: value.timestamp_subsec_nanos(),
}
}
}

impl From<SystemTime> for Timespec {
fn from(value: SystemTime) -> Self {
// The DateTime API is more convenient, so let's delegate again.
let datetime: DateTime<Utc> = value.into();
Self::from(datetime)
}
}
85 changes: 62 additions & 23 deletions ddprof-ffi/src/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use ddprof_profiles as profiles;
use std::convert::{TryFrom, TryInto};
use std::error::Error;
use std::str::Utf8Error;
use std::time::{Duration, SystemTime};

#[repr(C)]
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -286,6 +287,13 @@ impl<'a> TryFrom<Sample<'a>> for profiles::api::Sample<'a> {

/// Create a new profile with the given sample types. Must call
/// `ddprof_ffi_Profile_free` when you are done with the profile.
///
/// # Arguments
/// * `sample_types`
/// * `period` - Optional period of the profile. Passing None/null translates to zero values.
/// * `start_time` - Optional time the profile started at. Passing None/null will use the current
/// time.
///
/// # Safety
/// All slices must be have pointers that are suitably aligned for their type
/// and must have the correct number of elements for the slice.
Expand All @@ -294,12 +302,15 @@ impl<'a> TryFrom<Sample<'a>> for profiles::api::Sample<'a> {
pub unsafe extern "C" fn ddprof_ffi_Profile_new(
sample_types: Slice<ValueType>,
period: Option<&Period>,
start_time: Option<&Timespec>,
) -> Box<ddprof_profiles::Profile> {
let types: Vec<ddprof_profiles::api::ValueType> =
sample_types.into_slice().iter().map(Into::into).collect();

let builder = ddprof_profiles::Profile::builder()
.period(period.map(Into::into))
.sample_types(types)
.period(period.map(Into::into));
.start_time(start_time.map(SystemTime::from));

Box::new(builder.build())
}
Expand All @@ -308,9 +319,7 @@ pub unsafe extern "C" fn ddprof_ffi_Profile_new(
/// # Safety
/// The `profile` must point to an object created by another FFI routine in this
/// module, such as `ddprof_ffi_Profile_with_sample_types`.
pub extern "C" fn ddprof_ffi_Profile_free(profile: Box<ddprof_profiles::Profile>) {
std::mem::drop(profile)
}
pub unsafe extern "C" fn ddprof_ffi_Profile_free(_profile: Box<ddprof_profiles::Profile>) {}

#[no_mangle]
/// # Safety
Expand Down Expand Up @@ -338,14 +347,12 @@ pub struct EncodedProfile {
buffer: crate::Vec<u8>,
}

impl TryFrom<ddprof_profiles::EncodedProfile> for EncodedProfile {
type Error = Box<dyn Error>;

fn try_from(value: ddprof_profiles::EncodedProfile) -> Result<Self, Self::Error> {
let start = value.start.try_into()?;
let end = value.end.try_into()?;
impl From<ddprof_profiles::EncodedProfile> for EncodedProfile {
fn from(value: ddprof_profiles::EncodedProfile) -> Self {
let start = value.start.into();
let end = value.end.into();
let buffer = value.buffer.into();
Ok(Self { start, end, buffer })
Self { start, end, buffer }
}
}

Expand All @@ -357,21 +364,41 @@ pub enum SerializeResult {

/// Serialize the aggregated profile. Don't forget to clean up the result by
/// calling ddprof_ffi_SerializeResult_drop.
///
/// # Arguments
/// * `profile` - a reference to the profile being serialized.
/// * `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.
///
/// # 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.
#[no_mangle]
#[must_use]
pub extern "C" fn ddprof_ffi_Profile_serialize(
pub unsafe extern "C" fn ddprof_ffi_Profile_serialize(
profile: &ddprof_profiles::Profile,
end_time: Option<&Timespec>,
duration_nanos: Option<&i64>,
) -> SerializeResult {
match || -> Result<EncodedProfile, Box<dyn Error>> { profile.serialize()?.try_into() }() {
Ok(ok) => SerializeResult::Ok(ok),
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)),
};
match || -> Result<_, Box<dyn Error>> { Ok(profile.serialize(end_time, duration)?) }() {
Ok(ok) => SerializeResult::Ok(ok.into()),
Err(err) => SerializeResult::Err(err.into()),
}
}

#[no_mangle]
pub unsafe extern "C" fn ddprof_ffi_SerializeResult_drop(result: SerializeResult) {
std::mem::drop(result)
}
pub unsafe extern "C" fn ddprof_ffi_SerializeResult_drop(_result: SerializeResult) {}

#[must_use]
#[no_mangle]
Expand All @@ -382,9 +409,21 @@ pub unsafe extern "C" fn ddprof_ffi_Vec_u8_as_slice(vec: &crate::Vec<u8>) -> Sli
/// Resets all data in `profile` except the sample types and period. Returns
/// true if it successfully reset the profile and false otherwise. The profile
/// remains valid if false is returned.
///
/// # Arguments
/// * `profile` - A mutable reference to the profile to be reset.
/// * `start_time` - The time of the profile (after reset). Pass None/null to use the current time.
///
/// # Safety
/// The `profile` must meet all the requirements of a mutable reference to the profile. Given this
/// can be called across an FFI boundary, the compiler cannot enforce this.
/// If `time` is not null, it must point to a valid Timespec object.
#[no_mangle]
pub extern "C" fn ddprof_ffi_Profile_reset(profile: &mut ddprof_profiles::Profile) -> bool {
profile.reset().is_some()
pub unsafe extern "C" fn ddprof_ffi_Profile_reset(
profile: &mut ddprof_profiles::Profile,
start_time: Option<&Timespec>,
) -> bool {
profile.reset(start_time.map(SystemTime::from)).is_some()
}

#[cfg(test)]
Expand All @@ -396,7 +435,7 @@ mod test {
fn ctor_and_dtor() {
unsafe {
let sample_type: *const ValueType = &ValueType::new("samples", "count");
let profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None);
let profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None, None);
ddprof_ffi_Profile_free(profile);
}
}
Expand All @@ -405,7 +444,7 @@ mod test {
fn aggregate_samples() {
unsafe {
let sample_type: *const ValueType = &ValueType::new("samples", "count");
let mut profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None);
let mut profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None, None);

let lines = &vec![Line {
function: Function {
Expand Down Expand Up @@ -454,7 +493,7 @@ mod test {

unsafe fn provide_distinct_locations_ffi() -> ddprof_profiles::Profile {
let sample_type: *const ValueType = &ValueType::new("samples", "count");
let mut profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None);
let mut profile = ddprof_ffi_Profile_new(Slice::new(sample_type, 1), None, None);

let main_lines = vec![Line {
function: Function {
Expand Down
3 changes: 2 additions & 1 deletion ddprof-profiles/examples/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ fn main() {
labels: vec![],
};

// Not setting .start_time intentionally to use the current time.
let mut profile: Profile = Profile::builder()
.sample_types(sample_types)
.period(Some(period))
Expand All @@ -72,7 +73,7 @@ fn main() {
Err(_) => exit(1),
}

match profile.serialize() {
match profile.serialize(None, None) {
Ok(encoded_profile) => {
let buffer = &encoded_profile.buffer;
assert!(buffer.len() > 100);
Expand Down
Loading

0 comments on commit 32ca509

Please sign in to comment.