From d482c08f561926c19ff98173cc1ab923786eea11 Mon Sep 17 00:00:00 2001 From: Julio Date: Tue, 23 Jun 2026 11:40:00 +0200 Subject: [PATCH] refactor(libdd-telemetry)!: avoid leaking libdd-common types in the public API. --- datadog-sidecar/src/service/sidecar_server.rs | 7 +- examples/ffi/telemetry.c | 5 +- examples/ffi/telemetry_metrics.c | 5 +- .../src/crash_info/telemetry.rs | 7 +- libdd-data-pipeline/src/telemetry/mod.rs | 4 +- libdd-telemetry-ffi/src/builder.rs | 93 ++++++++++++++++--- libdd-telemetry-ffi/src/lib.rs | 28 ++++-- .../examples/tm-metrics-worker-test.rs | 4 +- libdd-telemetry/examples/tm-send-sketch.rs | 12 +-- libdd-telemetry/examples/tm-worker-test.rs | 5 +- libdd-telemetry/src/config.rs | 69 ++++++++++---- libdd-telemetry/src/worker/mod.rs | 16 +--- 12 files changed, 173 insertions(+), 82 deletions(-) diff --git a/datadog-sidecar/src/service/sidecar_server.rs b/datadog-sidecar/src/service/sidecar_server.rs index 7a5e095d13..cd86f3b565 100644 --- a/datadog-sidecar/src/service/sidecar_server.rs +++ b/datadog-sidecar/src/service/sidecar_server.rs @@ -709,7 +709,12 @@ impl SidecarInterface for ConnectionSidecarHandler { libdd_telemetry::config::PROD_INTAKE_SUBDOMAIN, &config.endpoint, ); - cfg.set_endpoint(endpoint).ok(); + // Set the api key before the uri so the telemetry path is resolved correctly. + cfg.set_endpoint_api_key(endpoint.api_key.as_deref()).ok(); + cfg.set_endpoint_uri(endpoint.url.clone()).ok(); + cfg.set_endpoint_timeout_ms(endpoint.timeout_ms); + cfg.set_endpoint_test_token(endpoint.test_token.clone()); + cfg.set_endpoint_use_system_resolver(endpoint.use_system_resolver); cfg.telemetry_heartbeat_interval = config.telemetry_heartbeat_interval; cfg.telemetry_extended_heartbeat_interval = config.telemetry_extended_heartbeat_interval; diff --git a/examples/ffi/telemetry.c b/examples/ffi/telemetry.c index 2987381fc6..b9dff330b6 100644 --- a/examples/ffi/telemetry.c +++ b/examples/ffi/telemetry.c @@ -23,9 +23,8 @@ int main(void) { TRY(ddog_telemetry_builder_instantiate(&builder, service, lang, lang_version, tracer_version)); ddog_CharSlice endpoint_char = DDOG_CHARSLICE_C("file://./examples_telemetry.out"); - struct ddog_Endpoint *endpoint = ddog_endpoint_from_url(endpoint_char); - TRY(ddog_telemetry_builder_with_endpoint_config_endpoint(builder, endpoint)); - ddog_endpoint_drop(endpoint); + TRY(ddog_telemetry_builder_with_endpoint_config_endpoint( + builder, endpoint_char, DDOG_CHARSLICE_C(""), 0, DDOG_CHARSLICE_C(""), false)); ddog_CharSlice runtime_id = DDOG_CHARSLICE_C("fa1f0ed0-8a3a-49e8-8f23-46fb44e24579"), service_version = DDOG_CHARSLICE_C("1.0"), env = DDOG_CHARSLICE_C("test"); diff --git a/examples/ffi/telemetry_metrics.c b/examples/ffi/telemetry_metrics.c index 8b1138f8d9..1d86f7ff2e 100644 --- a/examples/ffi/telemetry_metrics.c +++ b/examples/ffi/telemetry_metrics.c @@ -49,9 +49,8 @@ int main(void) { TRY(ddog_telemetry_builder_instantiate(&builder, service, lang, lang_version, tracer_version)); ddog_CharSlice endpoint_char = DDOG_CHARSLICE_C("file://./examples_telemetry_metrics.out"); - struct ddog_Endpoint *endpoint = ddog_endpoint_from_url(endpoint_char); - TRY(ddog_telemetry_builder_with_endpoint_config_endpoint(builder, endpoint)); - ddog_endpoint_drop(endpoint); + TRY(ddog_telemetry_builder_with_endpoint_config_endpoint( + builder, endpoint_char, DDOG_CHARSLICE_C(""), 0, DDOG_CHARSLICE_C(""), false)); ddog_CharSlice runtime_id = DDOG_CHARSLICE_C("fa1f0ed0-8a3a-49e8-8f23-46fb44e24579"), service_version = DDOG_CHARSLICE_C("1.0"), env = DDOG_CHARSLICE_C("test"); diff --git a/libdd-crashtracker/src/crash_info/telemetry.rs b/libdd-crashtracker/src/crash_info/telemetry.rs index e4c369f06a..a43b31a745 100644 --- a/libdd-crashtracker/src/crash_info/telemetry.rs +++ b/libdd-crashtracker/src/crash_info/telemetry.rs @@ -202,7 +202,12 @@ impl TelemetryCrashUploader { .context("file path is not valid")?; cfg.set_host_from_url(&format!("file://{}.telemetry", path.display())) } else { - cfg.set_endpoint(endpoint.clone()) + cfg.set_endpoint_timeout_ms(endpoint.timeout_ms); + cfg.set_endpoint_test_token(endpoint.test_token.clone()); + cfg.set_endpoint_use_system_resolver(endpoint.use_system_resolver); + // Set the api key before the uri so the telemetry path is resolved correctly. + cfg.set_endpoint_api_key(endpoint.api_key.as_deref()) + .and_then(|()| cfg.set_endpoint_uri(endpoint.url.clone())) }; } diff --git a/libdd-data-pipeline/src/telemetry/mod.rs b/libdd-data-pipeline/src/telemetry/mod.rs index 3ea0ea2096..cd2b9e05fd 100644 --- a/libdd-data-pipeline/src/telemetry/mod.rs +++ b/libdd-data-pipeline/src/telemetry/mod.rs @@ -72,9 +72,7 @@ impl TelemetryClientBuilder { /// Sets the url where the metrics will be sent. pub fn set_url(mut self, url: &str) -> Self { - let _ = self - .config - .set_endpoint(libdd_common::Endpoint::from_slice(url)); + let _ = self.config.set_endpoint_url(url); self } diff --git a/libdd-telemetry-ffi/src/builder.rs b/libdd-telemetry-ffi/src/builder.rs index 78216e1848..df76c73e93 100644 --- a/libdd-telemetry-ffi/src/builder.rs +++ b/libdd-telemetry-ffi/src/builder.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use ffi::slice::AsBytes; -use libdd_common::Endpoint; use libdd_common_ffi as ffi; use libdd_telemetry::{ data, @@ -155,14 +154,60 @@ pub unsafe extern "C" fn ddog_telemetry_builder_run_metric_logs( MaybeError::None } +/// Applies endpoint settings to the builder's telemetry config from primitive +/// values, so `libdd_common::Endpoint` stays out of this crate's public API. +/// +/// `api_key` and `test_token` are treated as unset when empty; a `timeout_ms` of +/// 0 keeps the existing/default timeout. +fn set_builder_endpoint( + telemetry_builder: &mut TelemetryWorkerBuilder, + url: ffi::CharSlice, + api_key: ffi::CharSlice, + timeout_ms: u64, + test_token: ffi::CharSlice, + use_system_resolver: bool, +) -> ffi::MaybeError { + let url = try_c!(url.try_to_utf8()); + let api_key = api_key.to_utf8_lossy(); + let test_token = test_token.to_utf8_lossy(); + let config = &mut telemetry_builder.config; + // Set the api key before the url so the telemetry path is resolved correctly. + if !api_key.is_empty() { + try_c!(config.set_endpoint_api_key(Some(api_key.as_ref()))); + } + try_c!(config.set_endpoint_url(url)); + if timeout_ms != 0 { + config.set_endpoint_timeout_ms(timeout_ms); + } + if !test_token.is_empty() { + config.set_endpoint_test_token(Some(test_token.into_owned())); + } + config.set_endpoint_use_system_resolver(use_system_resolver); + ffi::MaybeError::None +} + #[no_mangle] #[allow(clippy::missing_safety_doc)] +/// Sets the telemetry endpoint from its component parts. +/// +/// * `api_key` / `test_token`: ignored when empty. +/// * `timeout_ms`: pass 0 to keep the existing/default timeout. pub unsafe extern "C" fn ddog_telemetry_builder_with_endpoint_config_endpoint( telemetry_builder: &mut TelemetryWorkerBuilder, - endpoint: &Endpoint, + url: ffi::CharSlice, + api_key: ffi::CharSlice, + timeout_ms: u64, + test_token: ffi::CharSlice, + use_system_resolver: bool, ) -> ffi::MaybeError { - try_c!(telemetry_builder.config.set_endpoint(endpoint.clone())); - ffi::MaybeError::None + set_builder_endpoint( + telemetry_builder, + url, + api_key, + timeout_ms, + test_token, + use_system_resolver, + ) } #[repr(C)] #[allow(dead_code)] @@ -172,7 +217,7 @@ pub enum TelemetryWorkerBuilderEndpointProperty { #[no_mangle] #[allow(clippy::missing_safety_doc)] -/// Sets a property from it's string value. +/// Sets the endpoint property from its component parts. /// /// Available properties: /// @@ -180,14 +225,24 @@ pub enum TelemetryWorkerBuilderEndpointProperty { pub unsafe extern "C" fn ddog_telemetry_builder_with_property_endpoint( telemetry_builder: &mut TelemetryWorkerBuilder, _property: TelemetryWorkerBuilderEndpointProperty, - endpoint: &Endpoint, + url: ffi::CharSlice, + api_key: ffi::CharSlice, + timeout_ms: u64, + test_token: ffi::CharSlice, + use_system_resolver: bool, ) -> ffi::MaybeError { - try_c!(telemetry_builder.config.set_endpoint(endpoint.clone())); - ffi::MaybeError::None + set_builder_endpoint( + telemetry_builder, + url, + api_key, + timeout_ms, + test_token, + use_system_resolver, + ) } #[no_mangle] #[allow(clippy::missing_safety_doc)] -/// Sets a property from it's string value. +/// Sets a named endpoint property from its component parts. /// /// Available properties: /// @@ -195,15 +250,23 @@ pub unsafe extern "C" fn ddog_telemetry_builder_with_property_endpoint( pub unsafe extern "C" fn ddog_telemetry_builder_with_endpoint_named_property( telemetry_builder: &mut TelemetryWorkerBuilder, property: ffi::CharSlice, - endpoint: &Endpoint, + url: ffi::CharSlice, + api_key: ffi::CharSlice, + timeout_ms: u64, + test_token: ffi::CharSlice, + use_system_resolver: bool, ) -> ffi::MaybeError { let property = try_c!(property.try_to_utf8()); match property { - "config . endpoint" => { - try_c!(telemetry_builder.config.set_endpoint(endpoint.clone())); - } - _ => return ffi::MaybeError::None, + "config . endpoint" => set_builder_endpoint( + telemetry_builder, + url, + api_key, + timeout_ms, + test_token, + use_system_resolver, + ), + _ => ffi::MaybeError::None, } - ffi::MaybeError::None } diff --git a/libdd-telemetry-ffi/src/lib.rs b/libdd-telemetry-ffi/src/lib.rs index c9ea548c4d..3f0bda6a2f 100644 --- a/libdd-telemetry-ffi/src/lib.rs +++ b/libdd-telemetry-ffi/src/lib.rs @@ -111,7 +111,6 @@ mod tests { use crate::{builder::*, worker_handle::*}; use ffi::tags::{ddog_Vec_Tag_new, ddog_Vec_Tag_push, PushTagResult}; use ffi::MaybeError; - use libdd_common::Endpoint; use libdd_common_ffi as ffi; use libdd_telemetry::{ data::{ @@ -137,9 +136,14 @@ mod tests { let mut builder = builder.assume_init(); let f = tempfile::NamedTempFile::new().unwrap(); + let url = format!("file://{}", f.path().to_str().unwrap()); ddog_telemetry_builder_with_endpoint_config_endpoint( &mut builder, - &Endpoint::from_slice(&format!("file://{}", f.path().to_str().unwrap())), + ffi::CharSlice::from(url.as_str()), + ffi::CharSlice::from(""), + 0, + ffi::CharSlice::from(""), + false, ) .unwrap_none(); @@ -306,13 +310,15 @@ mod tests { let mut builder = builder.assume_init(); let f = tempfile::NamedTempFile::new().unwrap(); + let url = format!("file://{}", f.path().as_os_str().to_str().unwrap()); assert_eq!( ddog_telemetry_builder_with_endpoint_config_endpoint( &mut builder, - &Endpoint::from_slice(&format!( - "file://{}", - f.path().as_os_str().to_str().unwrap() - )), + ffi::CharSlice::from(url.as_str()), + ffi::CharSlice::from(""), + 0, + ffi::CharSlice::from(""), + false, ), MaybeError::None ); @@ -351,13 +357,15 @@ mod tests { let mut builder = builder.assume_init(); let f = tempfile::NamedTempFile::new().unwrap(); + let url = format!("file://{}", f.path().as_os_str().to_str().unwrap()); assert_eq!( ddog_telemetry_builder_with_endpoint_config_endpoint( &mut builder, - &Endpoint::from_slice(&format!( - "file://{}", - f.path().as_os_str().to_str().unwrap() - )), + ffi::CharSlice::from(url.as_str()), + ffi::CharSlice::from(""), + 0, + ffi::CharSlice::from(""), + false, ), MaybeError::None ); diff --git a/libdd-telemetry/examples/tm-metrics-worker-test.rs b/libdd-telemetry/examples/tm-metrics-worker-test.rs index 8dff49964a..0a9d420a02 100644 --- a/libdd-telemetry/examples/tm-metrics-worker-test.rs +++ b/libdd-telemetry/examples/tm-metrics-worker-test.rs @@ -36,9 +36,7 @@ fn main() -> Result<(), Box> { builder.config.telemetry_debug_logging_enabled = true; builder .config - .set_endpoint(libdd_common::Endpoint::from_slice( - "file://./tm-metrics-worker-test.output", - )) + .set_endpoint_url("file://./tm-metrics-worker-test.output") .unwrap(); builder.config.telemetry_heartbeat_interval = Duration::from_secs(1); builder.config.debug_enabled = true; diff --git a/libdd-telemetry/examples/tm-send-sketch.rs b/libdd-telemetry/examples/tm-send-sketch.rs index f977a1dd22..ca78ce5ea5 100644 --- a/libdd-telemetry/examples/tm-send-sketch.rs +++ b/libdd-telemetry/examples/tm-send-sketch.rs @@ -4,13 +4,11 @@ // Datadog, Inc. use std::{ - borrow::Cow, sync::atomic::{AtomicU64, Ordering}, time::SystemTime, }; -use http::{header::CONTENT_TYPE, Uri}; -use libdd_common::Endpoint; +use http::header::CONTENT_TYPE; use libdd_telemetry::{ build_host, config::Config, @@ -111,12 +109,10 @@ async fn async_main() { let mut config = Config::from_env(); config.direct_submission_enabled = true; config.debug_enabled = true; + let api_key = std::env::var("DD_API_KEY").unwrap(); + config.set_endpoint_api_key(Some(&api_key)).unwrap(); config - .set_endpoint(Endpoint { - url: Uri::from_static("https://instrumentation-telemetry-intake.datad0g.com"), - api_key: Some(Cow::Owned(std::env::var("DD_API_KEY").unwrap())), - ..Default::default() - }) + .set_endpoint_url("https://instrumentation-telemetry-intake.datad0g.com") .unwrap(); push_telemetry(&config, &req).await.unwrap(); } diff --git a/libdd-telemetry/examples/tm-worker-test.rs b/libdd-telemetry/examples/tm-worker-test.rs index f60ef46218..7cbd012d69 100644 --- a/libdd-telemetry/examples/tm-worker-test.rs +++ b/libdd-telemetry/examples/tm-worker-test.rs @@ -39,10 +39,7 @@ fn main() -> Result<(), Box> { builder.config = libdd_telemetry::config::Config::from_env(); builder .config - .set_endpoint(libdd_common::Endpoint { - url: libdd_common::parse_uri("file://./tm-worker-test.output").unwrap(), - ..Default::default() - }) + .set_endpoint_url("file://./tm-worker-test.output") .unwrap(); builder.config.telemetry_heartbeat_interval = Duration::from_secs(1); diff --git a/libdd-telemetry/src/config.rs b/libdd-telemetry/src/config.rs index 1ecdf1c512..68ffbaea11 100644 --- a/libdd-telemetry/src/config.rs +++ b/libdd-telemetry/src/config.rs @@ -236,17 +236,58 @@ impl Config { self.endpoint.as_ref() } - pub fn set_endpoint(&mut self, endpoint: Endpoint) -> anyhow::Result<()> { - self.endpoint = Some(endpoint_with_telemetry_path( - endpoint, - self.direct_submission_enabled, - )?); + /// Rewrites the endpoint path to the telemetry path appropriate for the + /// current scheme, API key and direct-submission setting. Called by the + /// setters that can affect it (URL and API key). + fn apply_telemetry_path(&mut self) -> anyhow::Result<()> { + if let Some(endpoint) = self.endpoint.take() { + self.endpoint = Some(endpoint_with_telemetry_path( + endpoint, + self.direct_submission_enabled, + )?); + } Ok(()) } + /// Sets the endpoint URL from a string (`http(s)://`, `unix://`, `windows:` + /// or `file://`). Use [`Config::set_endpoint_uri`] if you already hold a + /// parsed [`Uri`] to avoid re-parsing. + pub fn set_endpoint_url(&mut self, url: &str) -> anyhow::Result<()> { + self.set_endpoint_uri(parse_uri(url)?) + } + + /// Sets the endpoint URL from an already-parsed [`Uri`]. + pub fn set_endpoint_uri(&mut self, uri: Uri) -> anyhow::Result<()> { + self.endpoint.get_or_insert_with(Endpoint::default).url = uri; + self.apply_telemetry_path() + } + + /// Sets (or, with `None`, clears) the endpoint API key. + pub fn set_endpoint_api_key(&mut self, api_key: Option<&str>) -> anyhow::Result<()> { + self.endpoint.get_or_insert_with(Endpoint::default).api_key = + api_key.map(|key| Cow::Owned(key.to_string())); + self.apply_telemetry_path() + } + + /// Sets the endpoint request timeout in milliseconds. + pub fn set_endpoint_timeout_ms(&mut self, timeout_ms: u64) { + if let Some(endpoint) = &mut self.endpoint { + endpoint.timeout_ms = timeout_ms; + } + } + + /// Sets (or, with `None`, clears) the `X-Datadog-Test-Session-Token` header + /// sent with requests. pub fn set_endpoint_test_token>>(&mut self, test_token: Option) { if let Some(endpoint) = &mut self.endpoint { - endpoint.test_token = test_token.map(|t| t.into()); + endpoint.test_token = test_token.map(|token| token.into()); + } + } + + /// Sets whether to use the system DNS resolver when building the HTTP client. + pub fn set_endpoint_use_system_resolver(&mut self, use_system_resolver: bool) { + if let Some(endpoint) = &mut self.endpoint { + endpoint.use_system_resolver = use_system_resolver; } } @@ -266,14 +307,9 @@ impl Config { parent_session_id: None, root_session_id: None, }; - if let Ok(url) = parse_uri(&trace_agent_url) { - let _res = this.set_endpoint(Endpoint { - url, - api_key, - ..Default::default() - }); - } + _ = this.set_endpoint_api_key(api_key.as_deref()); + _ = this.set_endpoint_url(&trace_agent_url); this } @@ -294,12 +330,7 @@ impl Config { /// If the host_url is http/https, any path will be ignored and replaced by the /// appropriate telemetry endpoint path pub fn set_host_from_url(&mut self, host_url: &str) -> anyhow::Result<()> { - let endpoint = self.endpoint.take().unwrap_or_default(); - - self.set_endpoint(Endpoint { - url: parse_uri(host_url)?, - ..endpoint - }) + self.set_endpoint_url(host_url) } } diff --git a/libdd-telemetry/src/worker/mod.rs b/libdd-telemetry/src/worker/mod.rs index 675ddad877..f14f471916 100644 --- a/libdd-telemetry/src/worker/mod.rs +++ b/libdd-telemetry/src/worker/mod.rs @@ -1307,7 +1307,7 @@ mod tests { LifecycleAction, TelemetryActions, TelemetryWorker, TelemetryWorkerBuilder, TelemetryWorkerFlavor, TelemetryWorkerHandle, }; - use libdd_common::{http_common, Endpoint}; + use libdd_common::http_common; use tokio::runtime::Runtime; fn is_send(_: T) {} @@ -1333,9 +1333,7 @@ mod tests { "1".into(), "tv".into(), ); - b.config - .set_endpoint(Endpoint::from_slice("http://127.0.0.1:1")) - .unwrap(); + b.config.set_endpoint_url("http://127.0.0.1:1").unwrap(); b.runtime_id = Some("rid".into()); b.config.session_id = session_id; b.config.parent_session_id = parent_session_id; @@ -1467,9 +1465,7 @@ mod tests { "1".into(), "tv".into(), ); - b.config - .set_endpoint(Endpoint::from_slice("http://127.0.0.1:1")) - .unwrap(); + b.config.set_endpoint_url("http://127.0.0.1:1").unwrap(); b.runtime_id = Some("rid".into()); b.flavor = flavor; b.build_worker(Some(tokio::runtime::Handle::current())).1 @@ -1795,7 +1791,6 @@ mod tests { #[test] fn test_channel_close_flushes_and_parks_via_shared_runtime() { use httpmock::prelude::*; - use libdd_common::Endpoint; use libdd_shared_runtime::SharedRuntime; use std::time::Duration; @@ -1814,10 +1809,7 @@ mod tests { "1".into(), "tv".into(), ); - builder - .config - .set_endpoint(Endpoint::from_slice(&server.url("/"))) - .unwrap(); + builder.config.set_endpoint_url(&server.url("/")).unwrap(); builder.runtime_id = Some("rid".into()); let shared_runtime = SharedRuntime::new().expect("SharedRuntime::new");