Skip to content

Commit

Permalink
Drop "V3" suffix from exporter APIs (#32)
Browse files Browse the repository at this point in the history
As discussed in
<#24 (comment)>
having the "V3" suffix on APIs doesn't seem that useful because:

1. We're still iterating on APIs all the time. We just broke the API
   again by changing the prefixes to `ddog_` so it seems a bit too
   early to version things. We never supported more than a version
   at a time!

2. Since the datadog agent just proxies profiling requests, we
   probably don't need to support more than one API version at a
   time.

3. The new intake format (internally called v2.4, whereas the
   old v3 was renamed to v1.3) does not need extensive API changes,
   so it's quite possible that we can move profilers to use v2.4
   without changing the libdatadog APIs. So having v3 in the API
   names would force us to break the API even if not needed.

For these reasons, I decided to ride the wave of #24 already breaking
all our APIs and also dropping the V3 suffix. Let me know your
thoughts :)
  • Loading branch information
ivoanjo committed Jul 20, 2022
1 parent 0d0ebd6 commit c318c90
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 54 deletions.
8 changes: 4 additions & 4 deletions ddprof-exporter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ pub struct Exporter {
runtime: Runtime,
}

pub struct FieldsV3 {
pub struct Fields {
pub start: DateTime<Utc>,
pub end: DateTime<Utc>,
}

pub struct ProfileExporterV3 {
pub struct ProfileExporter {
exporter: Exporter,
endpoint: Endpoint,
family: Cow<'static, str>,
Expand Down Expand Up @@ -103,12 +103,12 @@ impl Request {
}
}

impl ProfileExporterV3 {
impl ProfileExporter {
pub fn new<IntoCow: Into<Cow<'static, str>>>(
family: IntoCow,
tags: Option<Vec<Tag>>,
endpoint: Endpoint,
) -> Result<ProfileExporterV3, Box<dyn Error>> {
) -> Result<ProfileExporter, Box<dyn Error>> {
Ok(Self {
exporter: Exporter::new()?,
endpoint,
Expand Down
8 changes: 4 additions & 4 deletions ddprof-exporter/tests/form.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// 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 ddprof_exporter::{File, ProfileExporterV3, Request};
use ddprof_exporter::{File, ProfileExporter, Request};
use std::error::Error;
use std::io::Read;
use std::ops::Sub;
Expand All @@ -16,7 +16,7 @@ fn open<P: AsRef<Path>>(path: P) -> Result<Vec<u8>, Box<dyn Error>> {
Ok(buffer)
}

fn multipart(exporter: &ProfileExporterV3) -> Request {
fn multipart(exporter: &ProfileExporter) -> Request {
let small_pprof_name = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/profile.pprof");
let buffer = open(small_pprof_name).expect("to open file and read its bytes");

Expand Down Expand Up @@ -57,7 +57,7 @@ mod tests {
fn multipart_agent() {
let base_url = "http://localhost:8126".parse().expect("url to parse");
let endpoint = config::agent(base_url).expect("endpoint to construct");
let exporter = ProfileExporterV3::new("php", Some(default_tags()), endpoint)
let exporter = ProfileExporter::new("php", Some(default_tags()), endpoint)
.expect("exporter to construct");

let request = multipart(&exporter);
Expand All @@ -75,7 +75,7 @@ mod tests {
fn multipart_agentless() {
let api_key = "1234567890123456789012";
let endpoint = config::agentless("datadoghq.com", api_key).expect("endpoint to construct");
let exporter = ProfileExporterV3::new("php", Some(default_tags()), endpoint)
let exporter = ProfileExporter::new("php", Some(default_tags()), endpoint)
.expect("exporter to construct");

let request = multipart(&exporter);
Expand Down
74 changes: 37 additions & 37 deletions ddprof-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::Timespec;
use ddcommon::tag::Tag;
use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice, Slice};
use ddprof_exporter as exporter;
use exporter::ProfileExporterV3;
use exporter::ProfileExporter;
use std::borrow::Cow;
use std::error::Error;
use std::ptr::NonNull;
Expand All @@ -21,26 +21,26 @@ pub enum SendResult {
}

#[repr(C)]
pub enum NewProfileExporterV3Result {
Ok(*mut ProfileExporterV3),
pub enum NewProfileExporterResult {
Ok(*mut ProfileExporter),
Err(ddcommon_ffi::Vec<u8>),
}

#[export_name = "ddog_NewProfileExporterV3Result_drop"]
pub unsafe extern "C" fn new_profile_exporter_v3_result_drop(result: NewProfileExporterV3Result) {
#[export_name = "ddog_NewProfileExporterResult_drop"]
pub unsafe extern "C" fn new_profile_exporter_result_drop(result: NewProfileExporterResult) {
match result {
NewProfileExporterV3Result::Ok(ptr) => {
NewProfileExporterResult::Ok(ptr) => {
let exporter = Box::from_raw(ptr);
std::mem::drop(exporter);
}
NewProfileExporterV3Result::Err(message) => {
NewProfileExporterResult::Err(message) => {
std::mem::drop(message);
}
}
}

#[repr(C)]
pub enum EndpointV3<'a> {
pub enum Endpoint<'a> {
Agent(CharSlice<'a>),
Agentless(CharSlice<'a>, CharSlice<'a>),
}
Expand All @@ -65,21 +65,21 @@ pub struct HttpStatus(u16);
/// Creates an endpoint that uses the agent.
/// # Arguments
/// * `base_url` - Contains a URL with scheme, host, and port e.g. "https://agent:8126/".
#[export_name = "ddog_EndpointV3_agent"]
pub extern "C" fn endpoint_agent(base_url: CharSlice) -> EndpointV3 {
EndpointV3::Agent(base_url)
#[export_name = "ddog_Endpoint_agent"]
pub extern "C" fn endpoint_agent(base_url: CharSlice) -> Endpoint {
Endpoint::Agent(base_url)
}

/// Creates an endpoint that uses the Datadog intake directly aka agentless.
/// # Arguments
/// * `site` - Contains a host and port e.g. "datadoghq.com".
/// * `api_key` - Contains the Datadog API key.
#[export_name = "ddog_EndpointV3_agentless"]
#[export_name = "ddog_Endpoint_agentless"]
pub extern "C" fn endpoint_agentless<'a>(
site: CharSlice<'a>,
api_key: CharSlice<'a>,
) -> EndpointV3<'a> {
EndpointV3::Agentless(site, api_key)
) -> Endpoint<'a> {
Endpoint::Agentless(site, api_key)
}

unsafe fn try_to_url(slice: CharSlice) -> Result<hyper::Uri, Box<dyn std::error::Error>> {
Expand All @@ -95,16 +95,16 @@ unsafe fn try_to_url(slice: CharSlice) -> Result<hyper::Uri, Box<dyn std::error:
}

unsafe fn try_to_endpoint(
endpoint: EndpointV3,
endpoint: Endpoint,
) -> Result<ddprof_exporter::Endpoint, Box<dyn std::error::Error>> {
// convert to utf8 losslessly -- URLs and API keys should all be ASCII, so
// a failed result is likely to be an error.
match endpoint {
EndpointV3::Agent(url) => {
Endpoint::Agent(url) => {
let base_url = try_to_url(url)?;
ddprof_exporter::config::agent(base_url)
}
EndpointV3::Agentless(site, api_key) => {
Endpoint::Agentless(site, api_key) => {
let site_str = site.try_to_utf8()?;
let api_key_str = api_key.try_to_utf8()?;
ddprof_exporter::config::agentless(
Expand All @@ -116,25 +116,25 @@ unsafe fn try_to_endpoint(
}

#[must_use]
#[export_name = "ddog_ProfileExporterV3_new"]
#[export_name = "ddog_ProfileExporter_new"]
pub extern "C" fn profile_exporter_new(
family: CharSlice,
tags: Option<&ddcommon_ffi::Vec<Tag>>,
endpoint: EndpointV3,
) -> NewProfileExporterV3Result {
match || -> Result<ProfileExporterV3, Box<dyn Error>> {
endpoint: Endpoint,
) -> NewProfileExporterResult {
match || -> Result<ProfileExporter, Box<dyn Error>> {
let family = unsafe { family.to_utf8_lossy() }.into_owned();
let converted_endpoint = unsafe { try_to_endpoint(endpoint)? };
let tags = tags.map(|tags| tags.iter().map(|tag| tag.clone().into_owned()).collect());
ProfileExporterV3::new(family, tags, converted_endpoint)
ProfileExporter::new(family, tags, converted_endpoint)
}() {
Ok(exporter) => NewProfileExporterV3Result::Ok(Box::into_raw(Box::new(exporter))),
Err(err) => NewProfileExporterV3Result::Err(err.into()),
Ok(exporter) => NewProfileExporterResult::Ok(Box::into_raw(Box::new(exporter))),
Err(err) => NewProfileExporterResult::Err(err.into()),
}
}

#[export_name = "ddog_ProfileExporterV3_delete"]
pub extern "C" fn profile_exporter_delete(exporter: Option<Box<ProfileExporterV3>>) {
#[export_name = "ddog_ProfileExporter_delete"]
pub extern "C" fn profile_exporter_delete(exporter: Option<Box<ProfileExporter>>) {
std::mem::drop(exporter)
}

Expand All @@ -155,9 +155,9 @@ unsafe fn into_vec_files<'a>(slice: Slice<'a, File>) -> Vec<ddprof_exporter::Fil
/// # Safety
/// The `exporter` and the files inside of the `files` slice need to have been
/// created by this module.
#[export_name = "ddog_ProfileExporterV3_build"]
#[export_name = "ddog_ProfileExporter_build"]
pub unsafe extern "C" fn profile_exporter_build(
exporter: Option<NonNull<ProfileExporterV3>>,
exporter: Option<NonNull<ProfileExporter>>,
start: Timespec,
end: Timespec,
files: Slice<File>,
Expand Down Expand Up @@ -194,9 +194,9 @@ pub unsafe extern "C" fn profile_exporter_build(
/// # Safety
/// All non-null arguments MUST have been created by created by apis in this module.
#[must_use]
#[export_name = "ddog_ProfileExporterV3_send"]
#[export_name = "ddog_ProfileExporter_send"]
pub unsafe extern "C" fn profile_exporter_send(
exporter: Option<NonNull<ProfileExporterV3>>,
exporter: Option<NonNull<ProfileExporter>>,
request: Option<Box<Request>>,
cancel: Option<NonNull<CancellationToken>>,
) -> SendResult {
Expand Down Expand Up @@ -263,7 +263,7 @@ pub extern "C" fn ddog_CancellationToken_new() -> *mut CancellationToken {
/// cancel_t2 = ddog_CancellationToken_clone(cancel_t1);
///
/// // On thread t1:
/// ddog_ProfileExporterV3_send(..., cancel_t1);
/// ddog_ProfileExporter_send(..., cancel_t1);
/// ddog_CancellationToken_drop(cancel_t1);
///
/// // On thread t2:
Expand Down Expand Up @@ -331,33 +331,33 @@ mod test {
}

#[test]
fn profile_exporter_v3_new_and_delete() {
fn profile_exporter_new_and_delete() {
let mut tags = ddcommon_ffi::Vec::default();
let host = Tag::new("host", "localhost").expect("static tags to be valid");
tags.push(host);

let result = profile_exporter_new(family(), Some(&tags), endpoint_agent(endpoint()));

match result {
NewProfileExporterV3Result::Ok(exporter) => unsafe {
NewProfileExporterResult::Ok(exporter) => unsafe {
profile_exporter_delete(Some(Box::from_raw(exporter)))
},
NewProfileExporterV3Result::Err(message) => {
NewProfileExporterResult::Err(message) => {
std::mem::drop(message);
panic!("Should not occur!")
}
}
}

#[test]
fn profile_exporter_v3_build() {
fn test_build() {
let exporter_result = profile_exporter_new(family(), None, endpoint_agent(endpoint()));

let exporter = match exporter_result {
NewProfileExporterV3Result::Ok(exporter) => unsafe {
NewProfileExporterResult::Ok(exporter) => unsafe {
Some(NonNull::new_unchecked(exporter))
},
NewProfileExporterV3Result::Err(message) => {
NewProfileExporterResult::Err(message) => {
std::mem::drop(message);
panic!("Should not occur!")
}
Expand Down
18 changes: 9 additions & 9 deletions examples/ffi/exporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ int main(int argc, char *argv[]) {

ddog_EncodedProfile *encoded_profile = &serialize_result.ok;

ddog_EndpointV3 endpoint = ddog_EndpointV3_agentless(
ddog_Endpoint endpoint = ddog_Endpoint_agentless(
DDOG_CHARSLICE_C("datad0g.com"), to_slice_c_char(api_key));

ddog_Vec_tag tags = ddog_Vec_tag_new();
Expand All @@ -94,13 +94,13 @@ int main(int argc, char *argv[]) {

ddog_PushTagResult_drop(tag_result);

ddog_NewProfileExporterV3Result exporter_new_result =
ddog_ProfileExporterV3_new(DDOG_CHARSLICE_C("native"), &tags, endpoint);
ddog_NewProfileExporterResult exporter_new_result =
ddog_ProfileExporter_new(DDOG_CHARSLICE_C("native"), &tags, endpoint);
ddog_Vec_tag_drop(tags);

if (exporter_new_result.tag == DDOG_NEW_PROFILE_EXPORTER_V3_RESULT_ERR) {
if (exporter_new_result.tag == DDOG_NEW_PROFILE_EXPORTER_RESULT_ERR) {
print_error("Failed to create exporter: ", exporter_new_result.err);
ddog_NewProfileExporterV3Result_drop(exporter_new_result);
ddog_NewProfileExporterResult_drop(exporter_new_result);
return 1;
}

Expand All @@ -113,7 +113,7 @@ int main(int argc, char *argv[]) {

ddog_Slice_file files = {.ptr = files_, .len = sizeof files_ / sizeof *files_};

ddog_Request *request = ddog_ProfileExporterV3_build(
ddog_Request *request = ddog_ProfileExporter_build(
exporter, encoded_profile->start, encoded_profile->end, files, nullptr, 30000);

ddog_CancellationToken *cancel = ddog_CancellationToken_new();
Expand All @@ -122,7 +122,7 @@ int main(int argc, char *argv[]) {

// As an example of CancellationToken usage, here we create a background
// thread that sleeps for some time and then cancels a request early (e.g.
// before the timeout in ddog_ProfileExporterV3_send is hit).
// before the timeout in ddog_ProfileExporter_send is hit).
//
// If the request is faster than the sleep time, no cancellation takes place.
std::thread trigger_cancel_if_request_takes_too_long_thread(
Expand All @@ -139,15 +139,15 @@ int main(int argc, char *argv[]) {
trigger_cancel_if_request_takes_too_long_thread.detach();

int exit_code = 0;
ddog_SendResult send_result = ddog_ProfileExporterV3_send(exporter, request, cancel);
ddog_SendResult send_result = ddog_ProfileExporter_send(exporter, request, cancel);
if (send_result.tag == DDOG_SEND_RESULT_ERR) {
print_error("Failed to send profile: ", send_result.err);
exit_code = 1;
} else {
printf("Response code: %d\n", send_result.http_response.code);
}

ddog_NewProfileExporterV3Result_drop(exporter_new_result);
ddog_NewProfileExporterResult_drop(exporter_new_result);
ddog_SendResult_drop(send_result);
ddog_CancellationToken_drop(cancel);
return exit_code;
Expand Down

0 comments on commit c318c90

Please sign in to comment.