Skip to content

Commit 4828b75

Browse files
authored
[HandsOffConfig] Remove eprintln and return debug messages inside Result (#1201)
* Change return type to get log messages with succes result Signed-off-by: Anna <[email protected]> * Change return type to LoggedResult Signed-off-by: Anna <[email protected]> * Adapt ffi, dnot use generic for 2 values as cbindgen generates numbered fields Signed-off-by: Anna <[email protected]> * Adapt c example Signed-off-by: Anna <[email protected]> * Defensive free model Signed-off-by: Anna <[email protected]> * clarify library_config.c Signed-off-by: Anna <[email protected]> * Wrap main treatment in catch panic Indenting Signed-off-by: Anna <[email protected]> --------- Signed-off-by: Anna <[email protected]>
1 parent f9b7f23 commit 4828b75

File tree

5 files changed

+328
-108
lines changed

5 files changed

+328
-108
lines changed

datadog-library-config-ffi/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ anyhow = "1.0"
1818
constcat = "0.4.1"
1919

2020
[features]
21-
default = ["cbindgen"]
21+
default = ["cbindgen", "catch_panic"]
2222
cbindgen = ["build_common/cbindgen", "ddcommon-ffi/cbindgen"]
23+
catch_panic = []
2324

2425
[build-dependencies]
2526
build_common = { path = "../build-common" }

datadog-library-config-ffi/src/lib.rs

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,52 @@
44
pub mod tracer_metadata;
55

66
use datadog_library_config::{self as lib_config, LibraryConfigSource};
7-
use ddcommon_ffi::{self as ffi, slice::AsBytes, CharSlice};
7+
use ddcommon_ffi::{self as ffi, slice::AsBytes, CString, CharSlice, Error};
8+
9+
#[cfg(all(feature = "catch_panic", panic = "unwind"))]
10+
use std::panic::{catch_unwind, AssertUnwindSafe};
11+
12+
#[cfg(all(feature = "catch_panic", panic = "unwind"))]
13+
macro_rules! catch_panic {
14+
($f:expr) => {
15+
match catch_unwind(AssertUnwindSafe(|| $f)) {
16+
Ok(ret) => ret,
17+
Err(info) => {
18+
let panic_msg = if let Some(s) = info.downcast_ref::<&'static str>() {
19+
s.to_string()
20+
} else if let Some(s) = info.downcast_ref::<String>() {
21+
s.clone()
22+
} else {
23+
"Unable to retrieve panic context".to_string()
24+
};
25+
LibraryConfigLoggedResult::Err(Error::from(format!(
26+
"FFI function panicked: {}",
27+
panic_msg
28+
)))
29+
}
30+
}
31+
};
32+
}
33+
34+
#[cfg(any(not(feature = "catch_panic"), panic = "abort"))]
35+
macro_rules! catch_panic {
36+
($f:expr) => {
37+
$f
38+
};
39+
}
40+
41+
/// A result type that includes debug/log messages along with the data
42+
#[repr(C)]
43+
pub struct OkResult {
44+
pub value: ffi::Vec<LibraryConfig>,
45+
pub logs: CString,
46+
}
47+
48+
#[repr(C)]
49+
pub enum LibraryConfigLoggedResult {
50+
Ok(OkResult),
51+
Err(Error),
52+
}
853

954
// TODO: Centos 6 build
1055
// Trust me it works bro 😉😉😉
@@ -53,6 +98,29 @@ impl LibraryConfig {
5398
.collect::<Result<Vec<_>, std::ffi::NulError>>()?;
5499
Ok(ffi::Vec::from_std(cfg))
55100
}
101+
102+
fn logged_result_to_ffi_with_messages(
103+
result: datadog_library_config::LoggedResult<Vec<lib_config::LibraryConfig>, anyhow::Error>,
104+
) -> LibraryConfigLoggedResult {
105+
match result {
106+
datadog_library_config::LoggedResult::Ok(configs, logs) => {
107+
match Self::rs_vec_to_ffi(configs) {
108+
Ok(ffi_configs) => {
109+
let messages = logs.join("\n");
110+
let cstring_logs = CString::new_or_empty(messages);
111+
LibraryConfigLoggedResult::Ok(OkResult {
112+
value: ffi_configs,
113+
logs: cstring_logs,
114+
})
115+
}
116+
Err(err) => LibraryConfigLoggedResult::Err(err.into()),
117+
}
118+
}
119+
datadog_library_config::LoggedResult::Err(err) => {
120+
LibraryConfigLoggedResult::Err(err.into())
121+
}
122+
}
123+
}
56124
}
57125

58126
pub struct Configurator<'a> {
@@ -116,19 +184,17 @@ pub extern "C" fn ddog_library_configurator_drop(_: Box<Configurator>) {}
116184
#[no_mangle]
117185
pub extern "C" fn ddog_library_configurator_get(
118186
configurator: &Configurator,
119-
) -> ffi::Result<ffi::Vec<LibraryConfig>> {
120-
(|| {
187+
) -> LibraryConfigLoggedResult {
188+
catch_panic!({
121189
let local_path = configurator
122190
.local_path
123191
.as_ref()
124-
.map(|p| p.into_std().to_str())
125-
.transpose()?
192+
.and_then(|p| p.into_std().to_str().ok())
126193
.unwrap_or(lib_config::Configurator::LOCAL_STABLE_CONFIGURATION_PATH);
127194
let fleet_path = configurator
128195
.fleet_path
129196
.as_ref()
130-
.map(|p| p.into_std().to_str())
131-
.transpose()?
197+
.and_then(|p| p.into_std().to_str().ok())
132198
.unwrap_or(lib_config::Configurator::FLEET_STABLE_CONFIGURATION_PATH);
133199
let detected_process_info;
134200
let process_info = match configurator.process_info {
@@ -141,14 +207,14 @@ pub extern "C" fn ddog_library_configurator_get(
141207
}
142208
};
143209

144-
configurator.inner.get_config_from_file(
210+
let result = configurator.inner.get_config_from_file(
145211
local_path.as_ref(),
146212
fleet_path.as_ref(),
147213
process_info,
148-
)
149-
})()
150-
.and_then(LibraryConfig::rs_vec_to_ffi)
151-
.into()
214+
);
215+
216+
LibraryConfig::logged_result_to_ffi_with_messages(result)
217+
})
152218
}
153219

154220
#[no_mangle]
@@ -190,4 +256,12 @@ pub extern "C" fn ddog_library_config_local_stable_config_path() -> ffi::CStr<'s
190256
}
191257

192258
#[no_mangle]
193-
pub extern "C" fn ddog_library_config_drop(_: ffi::Vec<LibraryConfig>) {}
259+
pub extern "C" fn ddog_library_config_drop(mut config_result: LibraryConfigLoggedResult) {
260+
match &mut config_result {
261+
LibraryConfigLoggedResult::Ok(_) => {}
262+
LibraryConfigLoggedResult::Err(err) => {
263+
// Use the internal error clearing function for defensive cleanup
264+
ddcommon_ffi::clear_error(err);
265+
}
266+
}
267+
}

0 commit comments

Comments
 (0)