Skip to content

Commit

Permalink
Move profiling ffi crate back out of profiling
Browse files Browse the repository at this point in the history
Due to a [rust bug][1], LTO was not being applied, and we need the lib
crate type in order to run tests and clippy lints. For now, keep these
as separate crates.

  [1]: rust-lang/rust#51009
  • Loading branch information
morrisonlevi committed Aug 19, 2022
1 parent d61002f commit 6c51598
Show file tree
Hide file tree
Showing 16 changed files with 94 additions and 50 deletions.
15 changes: 14 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
[workspace]
members = [
"profiling",
"profiling-ffi",
"ddcommon",
"ddcommon-ffi",
"ddtelemetry",
Expand Down
2 changes: 1 addition & 1 deletion LICENSE-3rdparty.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
root_name: "datadog-profiling, ddcommon, ddcommon-ffi, ddtelemetry"
root_name: "datadog-profiling, ddcommon, datadog-profiling-ffi, ddcommon-ffi, ddtelemetry"
third_party_libraries:
- package_name: aho-corasick
package_version: 0.7.18
Expand Down
33 changes: 23 additions & 10 deletions build-profiling-ffi.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ case "$target" in
esac

echo "Recognized platform '${target}'. Adding libs: ${native_static_libs}"
cd profiling
cd profiling-ffi
sed < datadog_profiling.pc.in "s/@Datadog_VERSION@/${version}/g" \
> "$destdir/lib/pkgconfig/datadog_profiling.pc"

Expand All @@ -86,12 +86,21 @@ cp -v LICENSE LICENSE-3rdparty.yml NOTICE "$destdir/"

export RUSTFLAGS="${RUSTFLAGS:- -C relocation-model=pic}"

echo "Building the datadog profiling library (may take some time)..."
cargo build --package=datadog-profiling --features=datadog_profiling_ffi --release --target "${target}"
datadog_profiling_ffi="datadog-profiling-ffi"
echo "Building the ${datadog_profiling_ffi} crate (may take some time)..."
cargo build --package="${datadog_profiling_ffi}" --release --target "${target}"

shared_library_name="${library_prefix}datadog_profiling${shared_library_suffix}"
static_library_name="${library_prefix}datadog_profiling${static_library_suffix}"
cp -v "target/${target}/release/$static_library_name" "target/${target}/release/$shared_library_name" "$destdir/lib/"
# Remove _ffi suffix when copying
shared_library_name="${library_prefix}datadog_profiling_ffi${shared_library_suffix}"
static_library_name="${library_prefix}datadog_profiling_ffi${static_library_suffix}"
shared_library_rename="${library_prefix}datadog_profiling${shared_library_suffix}"
static_library_rename="${library_prefix}datadog_profiling${static_library_suffix}"

cp -v "target/${target}/release/${shared_library_name}" "$destdir/lib/${shared_library_rename}"
cp -v "target/${target}/release/${static_library_rename}" "$destdir/lib/${static_library_rename}"

shared_library_name="${shared_library_rename}"
static_library_name="${static_library_rename}"

if [[ "$remove_rpath" -eq 1 ]]; then
patchelf --remove-rpath "$destdir/lib/${shared_library_name}"
Expand All @@ -114,8 +123,8 @@ if command -v objcopy > /dev/null && [[ "$target" != "x86_64-pc-windows-msvc" ]]
fi

echo "Checking that native-static-libs are as expected for this platform..."
cd profiling
actual_native_static_libs="$(cargo rustc --features=datadog_profiling_ffi --release --target "${target}" -- --print=native-static-libs 2>&1 | awk -F ':' '/note: native-static-libs:/ { print $3 }')"
cd profiling-ffi
actual_native_static_libs="$(cargo rustc --release --target "${target}" -- --print=native-static-libs 2>&1 | awk -F ':' '/note: native-static-libs:/ { print $3 }')"
echo "Actual native-static-libs:${actual_native_static_libs}"
echo "Expected native-static-libs:${expected_native_static_libs}"

Expand All @@ -140,8 +149,12 @@ echo "Building tools"
cargo build --package tools --bins

echo "Generating $destdir/include/libdatadog headers..."
cbindgen --crate ddcommon-ffi --config ddcommon-ffi/cbindgen.toml --output "$destdir/include/datadog/common.h"
cbindgen --crate datadog-profiling --config profiling/cbindgen.toml --output "$destdir/include/datadog/profiling.h"
cbindgen --crate ddcommon-ffi \
--config ddcommon-ffi/cbindgen.toml \
--output "$destdir/include/datadog/common.h"
cbindgen --crate "${datadog_profiling_ffi}" \
--config profiling-ffi/cbindgen.toml \
--output "$destdir/include/datadog/profiling.h"
./target/debug/dedup_headers "$destdir/include/datadog/common.h" "$destdir/include/datadog/profiling.h"

echo "Done."
3 changes: 2 additions & 1 deletion cmake/DatadogConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ if(Datadog_FOUND)
wsock32
ws2_32
shlwapi
Secur32)
Secur32
Ncrypt)
endif()

add_library(Datadog::Profiling ALIAS datadog_profiling)
Expand Down
23 changes: 23 additions & 0 deletions profiling-ffi/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# 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.

[package]
name = "datadog-profiling-ffi"
version = "0.8.0-alpha.1"
edition = "2021"
license = "Apache-2.0"

[lib]
# LTO is ignored if "lib" is added as crate type
# cf. https://github.com/rust-lang/rust/issues/51009
crate-type = ["staticlib", "cdylib"]

[dependencies]
anyhow = "1.0"
chrono = "0.4"
datadog-profiling = { path = "../profiling"}
hyper = {version = "0.14", default-features = false}
ddcommon = { path = "../ddcommon"}
ddcommon-ffi = { path = "../ddcommon-ffi"}
libc = "0.2"
tokio-util = "0.7.1"
1 change: 1 addition & 0 deletions profiling-ffi/NOTICE
5 changes: 1 addition & 4 deletions profiling/cbindgen.toml → profiling-ffi/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ no_includes = true
sys_includes = ["stdbool.h", "stddef.h", "stdint.h"]
includes = ["datadog/common.h"]

[defines]
"feature = datadog_profiling_ffi" = "DATADOG_PROFILING_FFI"

[export]
prefix = "ddog_"

Expand All @@ -31,4 +28,4 @@ must_use = "DDOG_CHECK_RETURN"

[parse]
parse_deps = true
include = ["ddcommon", "ddcommon-ffi", "ux"]
include = ["ddcommon", "ddcommon-ffi", "datadog-profiling", "ux"]
File renamed without changes.
File renamed without changes.
16 changes: 9 additions & 7 deletions profiling/src/ffi/exporter.rs → profiling-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
#![allow(renamed_and_removed_lints)]
#![allow(clippy::box_vec)]

use super::Timespec;
use crate::exporter;
use crate::exporter::ProfileExporter;
use crate::Timespec;
use datadog_profiling::exporter;
use ddcommon::tag::Tag;
use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice, Slice};
use exporter::ProfileExporter;
use std::borrow::Cow;
use std::ptr::NonNull;
use std::str::FromStr;
Expand Down Expand Up @@ -304,12 +304,12 @@ pub extern "C" fn ddog_CancellationToken_drop(_cancel: Option<Box<CancellationTo

#[export_name = "ddog_SendResult_drop"]
pub unsafe extern "C" fn send_result_drop(result: SendResult) {
drop(result)
std::mem::drop(result)
}

#[cfg(test)]
mod test {
use super::*;
use crate::exporter::*;
use ddcommon_ffi::Slice;

fn family() -> CharSlice<'static> {
Expand All @@ -336,7 +336,8 @@ mod test {
NewProfileExporterResult::Ok(exporter) => unsafe {
profile_exporter_delete(Some(Box::from_raw(exporter)))
},
NewProfileExporterResult::Err(_) => {
NewProfileExporterResult::Err(message) => {
std::mem::drop(message);
panic!("Should not occur!")
}
}
Expand All @@ -350,7 +351,8 @@ mod test {
NewProfileExporterResult::Ok(exporter) => unsafe {
Some(NonNull::new_unchecked(exporter))
},
NewProfileExporterResult::Err(_) => {
NewProfileExporterResult::Err(message) => {
std::mem::drop(message);
panic!("Should not occur!")
}
};
Expand Down
File renamed without changes.
33 changes: 18 additions & 15 deletions profiling/src/ffi/profiles.rs → profiling-ffi/src/profiles.rs
Original file line number Diff line number Diff line change
@@ -1,8 +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 super::Timespec;
use crate::profile as profiles;
use crate::Timespec;
use datadog_profiling::profile as profiles;
use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice};
use std::convert::{TryFrom, TryInto};
use std::str::Utf8Error;
Expand Down Expand Up @@ -303,11 +303,11 @@ pub unsafe extern "C" fn ddog_Profile_new(
sample_types: Slice<ValueType>,
period: Option<&Period>,
start_time: Option<&Timespec>,
) -> Box<crate::profile::Profile> {
let types: Vec<crate::profile::api::ValueType> =
) -> Box<datadog_profiling::profile::Profile> {
let types: Vec<datadog_profiling::profile::api::ValueType> =
sample_types.into_slice().iter().map(Into::into).collect();

let builder = crate::profile::Profile::builder()
let builder = datadog_profiling::profile::Profile::builder()
.period(period.map(Into::into))
.sample_types(types)
.start_time(start_time.map(SystemTime::from));
Expand All @@ -319,15 +319,18 @@ pub unsafe extern "C" fn ddog_Profile_new(
/// # Safety
/// The `profile` must point to an object created by another FFI routine in this
/// module, such as `ddog_Profile_with_sample_types`.
pub unsafe extern "C" fn ddog_Profile_free(_profile: Box<crate::profile::Profile>) {}
pub unsafe extern "C" fn ddog_Profile_free(_profile: Box<datadog_profiling::profile::Profile>) {}

#[no_mangle]
/// # Safety
/// The `profile` ptr must point to a valid Profile object created by this
/// module. All pointers inside the `sample` need to be valid for the duration
/// of this call.
/// This call is _NOT_ thread-safe.
pub extern "C" fn ddog_Profile_add(profile: &mut crate::profile::Profile, sample: Sample) -> u64 {
pub extern "C" fn ddog_Profile_add(
profile: &mut datadog_profiling::profile::Profile,
sample: Sample,
) -> u64 {
match sample.try_into().map(|s| profile.add(s)) {
Ok(r) => match r {
Ok(id) => id.into(),
Expand Down Expand Up @@ -355,7 +358,7 @@ pub extern "C" fn ddog_Profile_add(profile: &mut crate::profile::Profile, sample
/// This call is _NOT_ thread-safe.
#[no_mangle]
pub unsafe extern "C" fn ddog_Profile_set_endpoint<'a>(
profile: &mut crate::profile::Profile,
profile: &mut datadog_profiling::profile::Profile,
local_root_span_id: CharSlice<'a>,
endpoint: CharSlice<'a>,
) {
Expand All @@ -372,8 +375,8 @@ pub struct EncodedProfile {
buffer: ddcommon_ffi::Vec<u8>,
}

impl From<crate::profile::EncodedProfile> for EncodedProfile {
fn from(value: crate::profile::EncodedProfile) -> Self {
impl From<datadog_profiling::profile::EncodedProfile> for EncodedProfile {
fn from(value: datadog_profiling::profile::EncodedProfile) -> Self {
let start = value.start.into();
let end = value.end.into();
let buffer = value.buffer.into();
Expand Down Expand Up @@ -406,7 +409,7 @@ pub enum SerializeResult {
/// The `duration_nanos` must be null or otherwise point to a valid i64.
#[no_mangle]
pub unsafe extern "C" fn ddog_Profile_serialize(
profile: &crate::profile::Profile,
profile: &datadog_profiling::profile::Profile,
end_time: Option<&Timespec>,
duration_nanos: Option<&i64>,
) -> SerializeResult {
Expand All @@ -416,7 +419,7 @@ pub unsafe extern "C" fn ddog_Profile_serialize(
Some(x) if *x < 0 => None,
Some(x) => Some(Duration::from_nanos((*x) as u64)),
};
match || -> anyhow::Result<crate::profile::EncodedProfile> {
match || -> anyhow::Result<datadog_profiling::profile::EncodedProfile> {
Ok(profile.serialize(end_time, duration)?)
}() {
Ok(ok) => SerializeResult::Ok(ok.into()),
Expand Down Expand Up @@ -447,15 +450,15 @@ pub unsafe extern "C" fn ddog_Vec_u8_as_slice(vec: &ddcommon_ffi::Vec<u8>) -> Sl
/// If `time` is not null, it must point to a valid Timespec object.
#[no_mangle]
pub unsafe extern "C" fn ddog_Profile_reset(
profile: &mut crate::profile::Profile,
profile: &mut datadog_profiling::profile::Profile,
start_time: Option<&Timespec>,
) -> bool {
profile.reset(start_time.map(SystemTime::from)).is_some()
}

#[cfg(test)]
mod test {
use super::*;
use crate::profiles::*;
use ddcommon_ffi::Slice;

#[test]
Expand Down Expand Up @@ -518,7 +521,7 @@ mod test {
}
}

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

Expand Down
9 changes: 1 addition & 8 deletions profiling/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ build = "build.rs"
license = "Apache-2.0"

[lib]
# LTO is ignored if "lib" is added as crate type
# cf. https://github.com/rust-lang/rust/issues/51009
crate-type = ["cdylib", "staticlib", "lib"]

[features]
default = []
datadog_profiling_ffi = ["dep:ddcommon-ffi"]
crate-type = ["lib"]

[build-dependencies]
prost-build = "0.10"
Expand All @@ -25,7 +19,6 @@ anyhow = "1.0"
bytes = "1.1"
chrono = "0.4"
ddcommon = {path = "../ddcommon"}
ddcommon-ffi = { path = "../ddcommon-ffi", optional = true }
futures = "0.3"
futures-core = {version = "0.3.0", default-features = false}
futures-util = {version = "0.3.0", default-features = false}
Expand Down
3 changes: 0 additions & 3 deletions profiling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,3 @@

pub mod exporter;
pub mod profile;

#[cfg(feature = "datadog_profiling_ffi")]
pub mod ffi;

0 comments on commit 6c51598

Please sign in to comment.