Skip to content

Commit 1fe3798

Browse files
authored
Return agent response length. (#1170)
* Return agent response length. * Return the actual payload along with its lenght rather than a copy turned into a null terminated string.
1 parent 9fc7ee7 commit 1fe3798

File tree

2 files changed

+38
-25
lines changed

2 files changed

+38
-25
lines changed

data-pipeline-ffi/src/response.rs

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
//! Define FFI compatible AgentResponse struct
55
66
use data_pipeline::trace_exporter::agent_response::AgentResponse;
7-
use std::{
8-
ffi::{c_char, CString},
9-
ptr::null,
10-
};
7+
use std::ptr::null;
118

129
/// Structure containing the agent response to a trace payload
1310
/// MUST be freed with `ddog_trace_exporter_response_free`
@@ -18,18 +15,16 @@ use std::{
1815
#[derive(Debug, Default)]
1916
pub struct ExporterResponse {
2017
/// The body of the response, which is a string containing the response from the agent.
21-
pub body: CString,
18+
pub body: Option<Vec<u8>>,
2219
}
2320

2421
impl From<AgentResponse> for ExporterResponse {
2522
fn from(value: AgentResponse) -> Self {
2623
match value {
2724
AgentResponse::Changed { body } => ExporterResponse {
28-
body: CString::new(body).unwrap_or_default(),
29-
},
30-
AgentResponse::Unchanged => ExporterResponse {
31-
body: CString::new("").unwrap_or_default(),
25+
body: Some(body.into_bytes()),
3226
},
27+
AgentResponse::Unchanged => ExporterResponse { body: None },
3328
}
3429
}
3530
}
@@ -39,12 +34,22 @@ impl From<AgentResponse> for ExporterResponse {
3934
#[no_mangle]
4035
pub unsafe extern "C" fn ddog_trace_exporter_response_get_body(
4136
response: *const ExporterResponse,
42-
) -> *const c_char {
43-
if response.is_null() {
37+
out_len: Option<&mut usize>,
38+
) -> *const u8 {
39+
let mut len: usize = 0;
40+
let body = if response.is_null() {
4441
null()
42+
} else if let Some(body) = &(*response).body {
43+
len = body.len();
44+
body.as_ptr()
4545
} else {
46-
(*response).body.as_ptr()
46+
null()
47+
};
48+
49+
if let Some(out_len) = out_len {
50+
*out_len = len;
4751
}
52+
body
4853
}
4954

5055
/// Free `response` and all its contents. After being called response will not point to a valid
@@ -59,28 +64,30 @@ pub unsafe extern "C" fn ddog_trace_exporter_response_free(response: *mut Export
5964
#[cfg(test)]
6065
mod tests {
6166
use super::*;
62-
use std::ffi::CStr;
6367

6468
#[test]
6569
fn constructor_test_changed() {
6670
let agent_response = AgentResponse::Changed {
6771
body: "res".to_string(),
6872
};
6973
let response = &ExporterResponse::from(agent_response) as *const ExporterResponse;
70-
let body = unsafe {
71-
CStr::from_ptr(ddog_trace_exporter_response_get_body(response)).to_string_lossy()
72-
};
73-
assert_eq!(body, "res".to_string());
74+
let mut len = 0;
75+
let body = unsafe { ddog_trace_exporter_response_get_body(response, Some(&mut len)) };
76+
let response =
77+
unsafe { std::str::from_utf8(std::slice::from_raw_parts(body, len)).unwrap() };
78+
assert_eq!(response, "res");
79+
assert_eq!(len, 3);
7480
}
7581

7682
#[test]
7783
fn constructor_test_unchanged() {
7884
let agent_response = AgentResponse::Unchanged;
7985
let response = Box::into_raw(Box::new(ExporterResponse::from(agent_response)));
80-
let body = unsafe {
81-
CStr::from_ptr(ddog_trace_exporter_response_get_body(response)).to_string_lossy()
82-
};
83-
assert_eq!(body, "".to_string());
86+
let mut len = usize::MAX;
87+
let body = unsafe { ddog_trace_exporter_response_get_body(response, Some(&mut len)) };
88+
assert!(body.is_null());
89+
assert_eq!(len, 0);
90+
8491
unsafe {
8592
ddog_trace_exporter_response_free(response);
8693
}
@@ -89,7 +96,7 @@ mod tests {
8996
#[test]
9097
fn handle_null_test() {
9198
unsafe {
92-
let body = ddog_trace_exporter_response_get_body(null());
99+
let body = ddog_trace_exporter_response_get_body(null(), None);
93100
assert!(body.is_null());
94101

95102
ddog_trace_exporter_response_free(null::<ExporterResponse>() as *mut ExporterResponse);

data-pipeline-ffi/src/trace_exporter.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -979,7 +979,7 @@ mod tests {
979979
);
980980
assert_eq!(ret, None);
981981
assert_eq!(
982-
response.assume_init().body.to_string_lossy(),
982+
String::from_utf8_lossy(&response.assume_init().body.unwrap()),
983983
r#"{
984984
"rate_by_service": {
985985
"service:foo,env:staging": 1.0,
@@ -1048,7 +1048,10 @@ mod tests {
10481048
);
10491049
mock_traces.assert();
10501050
assert_eq!(ret, None);
1051-
assert_eq!(response.assume_init().body.to_string_lossy(), response_body);
1051+
assert_eq!(
1052+
String::from_utf8_lossy(&response.assume_init().body.unwrap()),
1053+
response_body
1054+
);
10521055

10531056
ddog_trace_exporter_free(exporter);
10541057
}
@@ -1123,7 +1126,10 @@ mod tests {
11231126
);
11241127
mock_traces.assert();
11251128
assert_eq!(ret, None);
1126-
assert_eq!(response.assume_init().body.to_string_lossy(), response_body);
1129+
assert_eq!(
1130+
String::from_utf8_lossy(&response.assume_init().body.unwrap()),
1131+
response_body
1132+
);
11271133

11281134
ddog_trace_exporter_free(exporter);
11291135
// It should receive 1 payloads: metrics

0 commit comments

Comments
 (0)