From 5f197a9460c9ec13815ed6d8e9cc3734cb913feb Mon Sep 17 00:00:00 2001 From: Alexandre Rulleau Date: Fri, 31 Oct 2025 17:16:03 -0400 Subject: [PATCH 1/4] feat: implement threaded sidecar connection for PHP tracer Signed-off-by: Alexandre Rulleau --- Cargo.lock | 111 ++++++++++ components-rs/common.h | 179 +++++++++------ components-rs/lib.rs | 2 +- components-rs/log.rs | 43 +++- .../src/bin/php_sidecar_mockgen.rs | 15 +- components-rs/remote_config.rs | 2 +- components-rs/sidecar.h | 9 +- components-rs/sidecar.rs | 76 +++++-- components-rs/telemetry.rs | 22 +- config.m4 | 1 + config.w32 | 1 + ext/ddtrace.c | 21 +- ext/sidecar.c | 6 + ext/sidecar.h | 10 + ext/sidecar_shm.c | 207 ++++++++++++++++++ ext/sidecar_shm.h | 39 ++++ 16 files changed, 622 insertions(+), 122 deletions(-) create mode 100644 ext/sidecar_shm.c create mode 100644 ext/sidecar_shm.h diff --git a/Cargo.lock b/Cargo.lock index b40383d3bee..bcb7435d1b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1088,6 +1088,106 @@ dependencies = [ "syn 2.0.96", ] +[[package]] +name = "data-pipeline" +version = "0.0.1" +dependencies = [ + "anyhow", + "arc-swap", + "bytes", + "clap", + "criterion", + "datadog-trace-protobuf", + "datadog-trace-stats", + "datadog-trace-utils", + "ddcommon 0.0.1", + "ddtelemetry", + "either", + "http", + "http-body-util", + "httpmock", + "hyper", + "hyper-util", + "libdd-ddsketch", + "libdd-dogstatsd-client", + "libdd-log", + "libdd-tinybytes", + "rand 0.8.5", + "regex", + "rmp-serde", + "serde", + "serde_json", + "sha2", + "tempfile", + "tokio", + "tokio-util", + "tracing", + "uuid", +] + +[[package]] +name = "datadog-alloc" +version = "21.0.0" +source = "git+https://github.com/DataDog/libdatadog?rev=0b59f64c4fc08105e5b73c5a0752ced3cf8f653e#0b59f64c4fc08105e5b73c5a0752ced3cf8f653e" +dependencies = [ + "allocator-api2", + "libc 0.2.177", + "windows-sys 0.52.0", +] + +[[package]] +name = "datadog-crashtracker" +version = "0.0.1" +dependencies = [ + "anyhow", + "backtrace", + "blazesym", + "cc", + "chrono", + "criterion", + "ddcommon 0.0.1", + "ddtelemetry", + "goblin", + "http", + "libc 0.2.177", + "nix", + "num-derive", + "num-traits", + "os_info", + "page_size", + "portable-atomic", + "rand 0.8.5", + "schemars", + "serde", + "serde_json", + "symbolic-common", + "symbolic-demangle", + "tempfile", + "thiserror 1.0.69", + "tokio", + "uuid", + "windows 0.59.0", +] + +[[package]] +name = "datadog-crashtracker-ffi" +version = "0.0.1" +dependencies = [ + "anyhow", + "build_common", + "datadog-crashtracker", + "ddcommon 0.0.1", + "ddcommon-ffi", + "function_name", + "libc 0.2.177", + "serde", + "serde_json", + "symbolic-common", + "symbolic-demangle", + "tempfile", + "windows 0.59.0", +] + [[package]] name = "datadog-ipc" version = "0.1.0" @@ -1166,6 +1266,17 @@ dependencies = [ "uuid", ] +[[package]] +name = "datadog-log" +version = "0.0.1" +dependencies = [ + "chrono", + "tempfile", + "tracing", + "tracing-appender", + "tracing-subscriber", +] + [[package]] name = "datadog-php-profiling" version = "0.0.0" diff --git a/components-rs/common.h b/components-rs/common.h index bfe1502bce3..5d815bca4fb 100644 --- a/components-rs/common.h +++ b/components-rs/common.h @@ -306,6 +306,17 @@ typedef enum ddog_Log { DDOG_LOG_HOOK_TRACE = (5 | (4 << 4)), } ddog_Log; +typedef enum ddog_InBodyLocation { + DDOG_IN_BODY_LOCATION_NONE, + DDOG_IN_BODY_LOCATION_START, + DDOG_IN_BODY_LOCATION_END, +} ddog_InBodyLocation; + +typedef enum ddog_EvaluateAt { + DDOG_EVALUATE_AT_ENTRY, + DDOG_EVALUATE_AT_EXIT, +} ddog_EvaluateAt; + typedef enum ddog_MetricKind { DDOG_METRIC_KIND_COUNT, DDOG_METRIC_KIND_GAUGE, @@ -313,6 +324,36 @@ typedef enum ddog_MetricKind { DDOG_METRIC_KIND_DISTRIBUTION, } ddog_MetricKind; +typedef enum ddog_SpanProbeTarget { + DDOG_SPAN_PROBE_TARGET_ACTIVE, + DDOG_SPAN_PROBE_TARGET_ROOT, +} ddog_SpanProbeTarget; + +typedef enum ddog_ProbeStatus { + DDOG_PROBE_STATUS_RECEIVED, + DDOG_PROBE_STATUS_INSTALLED, + DDOG_PROBE_STATUS_EMITTING, + DDOG_PROBE_STATUS_ERROR, + DDOG_PROBE_STATUS_BLOCKED, + DDOG_PROBE_STATUS_WARNING, +} ddog_ProbeStatus; + +typedef enum ddog_ConfigurationOrigin { + DDOG_CONFIGURATION_ORIGIN_ENV_VAR, + DDOG_CONFIGURATION_ORIGIN_CODE, + DDOG_CONFIGURATION_ORIGIN_DD_CONFIG, + DDOG_CONFIGURATION_ORIGIN_REMOTE_CONFIG, + DDOG_CONFIGURATION_ORIGIN_DEFAULT, + DDOG_CONFIGURATION_ORIGIN_LOCAL_STABLE_CONFIG, + DDOG_CONFIGURATION_ORIGIN_FLEET_STABLE_CONFIG, +} ddog_ConfigurationOrigin; + +typedef enum ddog_MetricType { + DDOG_METRIC_TYPE_GAUGE, + DDOG_METRIC_TYPE_COUNT, + DDOG_METRIC_TYPE_DISTRIBUTION, +} ddog_MetricType; + typedef enum ddog_MetricNamespace { DDOG_METRIC_NAMESPACE_TRACERS, DDOG_METRIC_NAMESPACE_PROFILERS, @@ -327,20 +368,16 @@ typedef enum ddog_MetricNamespace { DDOG_METRIC_NAMESPACE_SIDECAR, } ddog_MetricNamespace; -typedef enum ddog_MetricType { - DDOG_METRIC_TYPE_GAUGE, - DDOG_METRIC_TYPE_COUNT, - DDOG_METRIC_TYPE_DISTRIBUTION, -} ddog_MetricType; - -typedef enum ddog_ProbeStatus { - DDOG_PROBE_STATUS_RECEIVED, - DDOG_PROBE_STATUS_INSTALLED, - DDOG_PROBE_STATUS_EMITTING, - DDOG_PROBE_STATUS_ERROR, - DDOG_PROBE_STATUS_BLOCKED, - DDOG_PROBE_STATUS_WARNING, -} ddog_ProbeStatus; +typedef enum ddog_RemoteConfigProduct { + DDOG_REMOTE_CONFIG_PRODUCT_AGENT_CONFIG, + DDOG_REMOTE_CONFIG_PRODUCT_AGENT_TASK, + DDOG_REMOTE_CONFIG_PRODUCT_APM_TRACING, + DDOG_REMOTE_CONFIG_PRODUCT_ASM, + DDOG_REMOTE_CONFIG_PRODUCT_ASM_DATA, + DDOG_REMOTE_CONFIG_PRODUCT_ASM_DD, + DDOG_REMOTE_CONFIG_PRODUCT_ASM_FEATURES, + DDOG_REMOTE_CONFIG_PRODUCT_LIVE_DEBUGGER, +} ddog_RemoteConfigProduct; typedef enum ddog_RemoteConfigCapabilities { DDOG_REMOTE_CONFIG_CAPABILITIES_ASM_ACTIVATION = 1, @@ -388,22 +425,6 @@ typedef enum ddog_RemoteConfigCapabilities { DDOG_REMOTE_CONFIG_CAPABILITIES_ASM_TRACE_TAGGING_RULES = 43, } ddog_RemoteConfigCapabilities; -typedef enum ddog_RemoteConfigProduct { - DDOG_REMOTE_CONFIG_PRODUCT_AGENT_CONFIG, - DDOG_REMOTE_CONFIG_PRODUCT_AGENT_TASK, - DDOG_REMOTE_CONFIG_PRODUCT_APM_TRACING, - DDOG_REMOTE_CONFIG_PRODUCT_ASM, - DDOG_REMOTE_CONFIG_PRODUCT_ASM_DATA, - DDOG_REMOTE_CONFIG_PRODUCT_ASM_DD, - DDOG_REMOTE_CONFIG_PRODUCT_ASM_FEATURES, - DDOG_REMOTE_CONFIG_PRODUCT_LIVE_DEBUGGER, -} ddog_RemoteConfigProduct; - -typedef enum ddog_SpanProbeTarget { - DDOG_SPAN_PROBE_TARGET_ACTIVE, - DDOG_SPAN_PROBE_TARGET_ROOT, -} ddog_SpanProbeTarget; - typedef struct ddog_DebuggerPayload ddog_DebuggerPayload; typedef struct ddog_DslString ddog_DslString; @@ -774,17 +795,17 @@ typedef struct ddog_DebuggerValue ddog_DebuggerValue; #define ddog_EVALUATOR_RESULT_REDACTED (const void*)-2 -typedef enum ddog_DebuggerType { - DDOG_DEBUGGER_TYPE_DIAGNOSTICS, - DDOG_DEBUGGER_TYPE_LOGS, -} ddog_DebuggerType; - typedef enum ddog_FieldType { DDOG_FIELD_TYPE_STATIC, DDOG_FIELD_TYPE_ARG, DDOG_FIELD_TYPE_LOCAL, } ddog_FieldType; +typedef enum ddog_DebuggerType { + DDOG_DEBUGGER_TYPE_DIAGNOSTICS, + DDOG_DEBUGGER_TYPE_LOGS, +} ddog_DebuggerType; + typedef struct ddog_Entry ddog_Entry; typedef struct ddog_HashMap_CowStr__Value ddog_HashMap_CowStr__Value; @@ -913,16 +934,6 @@ typedef struct ddog_OwnedCharSlice { void (*free)(ddog_CharSlice); } ddog_OwnedCharSlice; -typedef enum ddog_LogLevel { - DDOG_LOG_LEVEL_ERROR, - DDOG_LOG_LEVEL_WARN, - DDOG_LOG_LEVEL_DEBUG, -} ddog_LogLevel; - -typedef enum ddog_TelemetryWorkerBuilderBoolProperty { - DDOG_TELEMETRY_WORKER_BUILDER_BOOL_PROPERTY_CONFIG_TELEMETRY_DEBUG_LOGGING_ENABLED, -} ddog_TelemetryWorkerBuilderBoolProperty; - typedef enum ddog_TelemetryWorkerBuilderEndpointProperty { DDOG_TELEMETRY_WORKER_BUILDER_ENDPOINT_PROPERTY_CONFIG_ENDPOINT, } ddog_TelemetryWorkerBuilderEndpointProperty; @@ -941,6 +952,16 @@ typedef enum ddog_TelemetryWorkerBuilderStrProperty { DDOG_TELEMETRY_WORKER_BUILDER_STR_PROPERTY_RUNTIME_ID, } ddog_TelemetryWorkerBuilderStrProperty; +typedef enum ddog_TelemetryWorkerBuilderBoolProperty { + DDOG_TELEMETRY_WORKER_BUILDER_BOOL_PROPERTY_CONFIG_TELEMETRY_DEBUG_LOGGING_ENABLED, +} ddog_TelemetryWorkerBuilderBoolProperty; + +typedef enum ddog_LogLevel { + DDOG_LOG_LEVEL_ERROR, + DDOG_LOG_LEVEL_WARN, + DDOG_LOG_LEVEL_DEBUG, +} ddog_LogLevel; + typedef struct ddog_TelemetryWorkerBuilder ddog_TelemetryWorkerBuilder; /** @@ -1097,6 +1118,29 @@ typedef enum ddog_crasht_FileType { DDOG_CRASHT_FILE_TYPE_PE, } ddog_crasht_FileType; +/** + * Stacktrace collection occurs in the context of a crashing process. + * If the stack is sufficiently corruputed, it is possible (but unlikely), + * for stack trace collection itself to crash. + * We recommend fully enabling stacktrace collection, but having an environment + * variable to allow downgrading the collector. + */ +typedef enum ddog_crasht_StacktraceCollection { + /** + * Stacktrace collection occurs in the + */ + DDOG_CRASHT_STACKTRACE_COLLECTION_DISABLED, + DDOG_CRASHT_STACKTRACE_COLLECTION_WITHOUT_SYMBOLS, + /** + * This option uses `backtrace::resolve_frame_unsynchronized()` to gather symbol information + * and also unwind inlined functions. Enabling this feature will not only provide symbolic + * details, but may also yield additional or less stack frames compared to other + * configurations. + */ + DDOG_CRASHT_STACKTRACE_COLLECTION_ENABLED_WITH_INPROCESS_SYMBOLS, + DDOG_CRASHT_STACKTRACE_COLLECTION_ENABLED_WITH_SYMBOLS_IN_RECEIVER, +} ddog_crasht_StacktraceCollection; + /** * This enum represents operations a the tracked library might be engaged in. * Currently only implemented for profiling. @@ -1120,6 +1164,12 @@ typedef enum ddog_crasht_OpTypes { DDOG_CRASHT_OP_TYPES_SIZE, } ddog_crasht_OpTypes; +typedef enum ddog_crasht_ErrorKind { + DDOG_CRASHT_ERROR_KIND_PANIC, + DDOG_CRASHT_ERROR_KIND_UNHANDLED_EXCEPTION, + DDOG_CRASHT_ERROR_KIND_UNIX_SIGNAL, +} ddog_crasht_ErrorKind; + /** * See https://man7.org/linux/man-pages/man2/sigaction.2.html * MUST REMAIN IN SYNC WITH THE ENUM IN emit_sigcodes.c @@ -1192,28 +1242,23 @@ typedef enum ddog_crasht_SignalNames { DDOG_CRASHT_SIGNAL_NAMES_UNKNOWN, } ddog_crasht_SignalNames; -/** - * Stacktrace collection occurs in the context of a crashing process. - * If the stack is sufficiently corruputed, it is possible (but unlikely), - * for stack trace collection itself to crash. - * We recommend fully enabling stacktrace collection, but having an environment - * variable to allow downgrading the collector. - */ -typedef enum ddog_crasht_StacktraceCollection { - /** - * Stacktrace collection occurs in the - */ - DDOG_CRASHT_STACKTRACE_COLLECTION_DISABLED, - DDOG_CRASHT_STACKTRACE_COLLECTION_WITHOUT_SYMBOLS, - /** - * This option uses `backtrace::resolve_frame_unsynchronized()` to gather symbol information - * and also unwind inlined functions. Enabling this feature will not only provide symbolic - * details, but may also yield additional or less stack frames compared to other - * configurations. - */ - DDOG_CRASHT_STACKTRACE_COLLECTION_ENABLED_WITH_INPROCESS_SYMBOLS, - DDOG_CRASHT_STACKTRACE_COLLECTION_ENABLED_WITH_SYMBOLS_IN_RECEIVER, -} ddog_crasht_StacktraceCollection; +typedef enum ddog_crasht_BuildIdType { + DDOG_CRASHT_BUILD_ID_TYPE_GNU, + DDOG_CRASHT_BUILD_ID_TYPE_GO, + DDOG_CRASHT_BUILD_ID_TYPE_PDB, + DDOG_CRASHT_BUILD_ID_TYPE_SHA1, +} ddog_crasht_BuildIdType; + +typedef enum ddog_crasht_FileType { + DDOG_CRASHT_FILE_TYPE_APK, + DDOG_CRASHT_FILE_TYPE_ELF, + DDOG_CRASHT_FILE_TYPE_PE, +} ddog_crasht_FileType; + +typedef enum ddog_crasht_DemangleOptions { + DDOG_CRASHT_DEMANGLE_OPTIONS_COMPLETE, + DDOG_CRASHT_DEMANGLE_OPTIONS_NAME_ONLY, +} ddog_crasht_DemangleOptions; typedef struct ddog_crasht_CrashInfo ddog_crasht_CrashInfo; diff --git a/components-rs/lib.rs b/components-rs/lib.rs index 5cbfc845312..2884f7d07d9 100644 --- a/components-rs/lib.rs +++ b/components-rs/lib.rs @@ -3,11 +3,11 @@ #![feature(linkage)] #![allow(static_mut_refs)] // remove with move to Rust 2024 edition +pub mod bytes; pub mod log; pub mod remote_config; pub mod sidecar; pub mod telemetry; -pub mod bytes; use libdd_common::entity_id::{get_container_id, set_cgroup_file}; use http::uri::{PathAndQuery, Scheme}; diff --git a/components-rs/log.rs b/components-rs/log.rs index 0954aab626d..56f100a4b71 100644 --- a/components-rs/log.rs +++ b/components-rs/log.rs @@ -1,3 +1,5 @@ +use ddcommon_ffi::slice::AsBytes; +use ddcommon_ffi::CharSlice; use std::cell::RefCell; use std::collections::{BTreeSet, HashMap}; use std::ffi::c_char; @@ -5,13 +7,13 @@ use std::fmt::Debug; use std::str::FromStr; use tracing::Level; use tracing_core::{Event, Field, LevelFilter, Subscriber}; -use tracing_subscriber::EnvFilter; -use tracing_subscriber::fmt::{FmtContext, FormatEvent, FormatFields}; use tracing_subscriber::fmt::format::Writer; +use tracing_subscriber::fmt::{FmtContext, FormatEvent, FormatFields}; use tracing_subscriber::registry::LookupSpan; use tracing_subscriber::util::SubscriberInitExt; use libdd_common_ffi::CharSlice; use libdd_common_ffi::slice::AsBytes; +use tracing_subscriber::EnvFilter; pub const LOG_ONCE: isize = 1 << 3; @@ -66,7 +68,10 @@ pub extern "C" fn ddog_shall_log(category: Log) -> bool { with_target!(category, tracing::event_enabled!()) } -pub fn log(category: Log, msg: S) where S: AsRef + tracing::Value { +pub fn log(category: Log, msg: S) +where + S: AsRef + tracing::Value, +{ let once = (category as isize & LOG_ONCE) != 0; if once { with_target!(category, tracing::event!(once = true, msg)); @@ -99,16 +104,20 @@ impl tracing_core::field::Visit for LogVisitor { } impl FormatEvent for LogFormatter - where - S: Subscriber + for<'a> LookupSpan<'a>, - N: for<'a> FormatFields<'a> + 'static { +where + S: Subscriber + for<'a> LookupSpan<'a>, + N: for<'a> FormatFields<'a> + 'static, +{ fn format_event( &self, _ctx: &FmtContext<'_, S, N>, _writer: Writer<'_>, - event: &Event<'_> + event: &Event<'_>, ) -> core::fmt::Result { - let mut visitor = LogVisitor { msg: None, once: false }; + let mut visitor = LogVisitor { + msg: None, + once: false, + }; event.record(&mut visitor); fn fmt_msg(event: &Event<'_>, msg: &str, suffix: &str) -> String { @@ -149,10 +158,14 @@ impl FormatEvent for LogFormatter COUNTERS.with(|counter| { let mut counter = counter.borrow_mut(); - *counter.entry(event.metadata().level().to_owned()).or_default() += 1; + *counter + .entry(event.metadata().level().to_owned()) + .or_default() += 1; }); - cb(unsafe { CharSlice::from_raw_parts(msg.as_ptr() as *const c_char, msg.len() - 1) }); + cb(unsafe { + CharSlice::from_raw_parts(msg.as_ptr() as *const c_char, msg.len() - 1) + }); } } Ok(()) @@ -175,7 +188,10 @@ pub unsafe extern "C" fn ddog_set_log_level(level: CharSlice, once: bool) { set_log_subscriber(subscriber) } -fn set_log_subscriber(subscriber: S) where S: SubscriberInitExt { +fn set_log_subscriber(subscriber: S) +where + S: SubscriberInitExt, +{ TRACING_GUARDS.replace(None); // drop first to avoid a prior guard to reset the thread local subscriber it upon replace() TRACING_GUARDS.replace(Some(subscriber.set_default())); } @@ -183,7 +199,10 @@ fn set_log_subscriber(subscriber: S) where S: SubscriberInitExt { #[no_mangle] pub unsafe extern "C" fn ddog_log(category: Log, once: bool, msg: CharSlice) { if once { - with_target!(category, tracing::event!(once = true, "{}", msg.to_utf8_lossy())); + with_target!( + category, + tracing::event!(once = true, "{}", msg.to_utf8_lossy()) + ); } else { with_target!(category, tracing::event!("{}", msg.to_utf8_lossy())); } diff --git a/components-rs/php_sidecar_mockgen/src/bin/php_sidecar_mockgen.rs b/components-rs/php_sidecar_mockgen/src/bin/php_sidecar_mockgen.rs index a31c034e07e..b02332de7df 100644 --- a/components-rs/php_sidecar_mockgen/src/bin/php_sidecar_mockgen.rs +++ b/components-rs/php_sidecar_mockgen/src/bin/php_sidecar_mockgen.rs @@ -41,7 +41,8 @@ fn main() { } let source_modified = fs::metadata("mock_php_syms.c").unwrap().modified().unwrap(); - if fs::metadata("mock_php.shared_lib").map_or(true, |m| m.modified().unwrap() < source_modified) { + if fs::metadata("mock_php.shared_lib").map_or(true, |m| m.modified().unwrap() < source_modified) + { env::set_var("OPT_LEVEL", "2"); let mut cc_build = cc::Build::new(); @@ -76,11 +77,17 @@ fn main() { } }; - let comma_separated = bin.iter().map(|byte| format!("{byte:#X}")).collect::>().join(","); - let out = format!(r#" + let comma_separated = bin + .iter() + .map(|byte| format!("{byte:#X}")) + .collect::>() + .join(","); + let out = format!( + r#" const unsigned char DDTRACE_MOCK_PHP[] = {{{comma_separated}}}; const void *DDTRACE_MOCK_PHP_SIZE = (void *) sizeof(DDTRACE_MOCK_PHP); - "#); + "# + ); if let Err(err) = fs::write(output_path, out) { eprintln!("Failed generating {:?}: {}", output_path, err); diff --git a/components-rs/remote_config.rs b/components-rs/remote_config.rs index 4693d2209c6..e3b671c5efa 100644 --- a/components-rs/remote_config.rs +++ b/components-rs/remote_config.rs @@ -6,11 +6,11 @@ use datadog_live_debugger_ffi::evaluator::{ddog_register_expr_evaluator, Evaluat use datadog_live_debugger_ffi::send_data::{ ddog_debugger_diagnostics_create_unboxed, ddog_snapshot_redacted_type, }; +use datadog_remote_config::config::dynamic::{Configs, TracingSamplingRuleProvenance}; use datadog_remote_config::fetch::ConfigInvariants; use datadog_remote_config::{ RemoteConfigCapabilities, RemoteConfigData, RemoteConfigProduct, Target, }; -use datadog_remote_config::config::dynamic::{Configs, TracingSamplingRuleProvenance}; use datadog_sidecar::service::blocking::SidecarTransport; use datadog_sidecar::service::{InstanceId, QueueId}; use datadog_sidecar::shm_remote_config::{RemoteConfigManager, RemoteConfigUpdate}; diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h index 4746e0d2163..a87ee2721bf 100644 --- a/components-rs/sidecar.h +++ b/components-rs/sidecar.h @@ -86,12 +86,13 @@ void ddog_remote_config_reader_drop(struct ddog_RemoteConfigReader*); void ddog_sidecar_transport_drop(struct ddog_SidecarTransport*); -/** - * # Safety - * Caller must ensure the process is safe to fork, at the time when this method is called - */ ddog_MaybeError ddog_sidecar_connect(struct ddog_SidecarTransport **connection); +ddog_MaybeError ddog_sidecar_connect_master(int32_t master_pid); + +ddog_MaybeError ddog_sidecar_connect_worker(int32_t master_pid, + struct ddog_SidecarTransport **connection); + ddog_MaybeError ddog_sidecar_ping(struct ddog_SidecarTransport **transport); ddog_MaybeError ddog_sidecar_flush_traces(struct ddog_SidecarTransport **transport); diff --git a/components-rs/sidecar.rs b/components-rs/sidecar.rs index 0b94deeeab9..060d2c4b32f 100644 --- a/components-rs/sidecar.rs +++ b/components-rs/sidecar.rs @@ -1,9 +1,21 @@ +use datadog_ipc::rate_limiter::{AnyLimiter, ShmLimiterMemory}; +use datadog_sidecar::config::{self, AppSecConfig, LogMethod}; +use datadog_sidecar::service::blocking::{acquire_exception_hash_rate_limiter, SidecarTransport}; +use datadog_sidecar::service::exception_hash_rate_limiter::ExceptionHashRateLimiter; +use datadog_sidecar::tracer::shm_limiter_path; +use ddcommon::rate_limiter::{Limiter, LocalLimiter}; +use ddcommon_ffi::slice::AsBytes; +use ddcommon_ffi::{self as ffi, CharSlice, MaybeError}; +use ddtelemetry_ffi::try_c; +use lazy_static::{lazy_static, LazyStatic}; +#[cfg(windows)] +use spawn_worker::get_trampoline_target_data; +#[cfg(any(windows, php_shared_build))] +use spawn_worker::LibDependency; use std::ffi::{c_char, CStr, OsStr}; use std::ops::DerefMut; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; -use lazy_static::{lazy_static, LazyStatic}; -use tracing::warn; #[cfg(windows)] use std::os::windows::ffi::OsStrExt; use std::sync::Mutex; @@ -23,12 +35,13 @@ use spawn_worker::LibDependency; #[cfg(windows)] use spawn_worker::get_trampoline_target_data; +use tracing::warn; #[cfg(php_shared_build)] extern "C" { - #[linkage="extern_weak"] + #[linkage = "extern_weak"] static DDTRACE_MOCK_PHP: *mut u8; - #[linkage="extern_weak"] + #[linkage = "extern_weak"] static DDTRACE_MOCK_PHP_SIZE: *mut usize; } @@ -36,8 +49,7 @@ extern "C" { fn run_sidecar(mut cfg: config::Config) -> anyhow::Result { if !unsafe { DDTRACE_MOCK_PHP_SIZE }.is_null() { let mock = unsafe { std::slice::from_raw_parts(DDTRACE_MOCK_PHP, *DDTRACE_MOCK_PHP_SIZE) }; - cfg.library_dependencies - .push(LibDependency::Binary(mock)); + cfg.library_dependencies.push(LibDependency::Binary(mock)); } datadog_sidecar::start_or_connect_to_sidecar(cfg) } @@ -54,7 +66,8 @@ pub static mut DDOG_PHP_FUNCTION: *const u8 = std::ptr::null(); #[cfg(windows)] fn run_sidecar(mut cfg: config::Config) -> anyhow::Result { let php_dll = get_trampoline_target_data(unsafe { DDOG_PHP_FUNCTION })?; - cfg.library_dependencies.push(LibDependency::Path(php_dll.into())); + cfg.library_dependencies + .push(LibDependency::Path(php_dll.into())); datadog_sidecar::start_or_connect_to_sidecar(cfg) } @@ -145,12 +158,13 @@ pub extern "C" fn ddog_sidecar_connect_php( } } #[cfg(windows)] - let log_level = log_level.to_utf8_lossy().as_ref().into(); + let log_level = log_level.to_utf8_lossy().as_ref().into(); #[cfg(not(windows))] - let log_level = OsStr::from_bytes(log_level.as_bytes()).into(); - cfg.child_env.insert(OsStr::new("DD_TRACE_LOG_LEVEL").into(), log_level); + let log_level = OsStr::from_bytes(log_level.as_bytes()).into(); + cfg.child_env + .insert(OsStr::new("DD_TRACE_LOG_LEVEL").into(), log_level); } - + let reconnect_fn = on_reconnect.map(|on_reconnect| { let cfg = cfg.clone(); Box::new(move || { @@ -159,7 +173,7 @@ pub extern "C" fn ddog_sidecar_connect_php( Some(transport) }) as Box _> }); - + let mut stream = try_c!(sidecar_connect(cfg)); stream.reconnect_fn = reconnect_fn; *connection = Box::into_raw(stream); @@ -185,17 +199,23 @@ pub extern "C" fn ddtrace_sidecar_reconnect( }); } - lazy_static! { - pub static ref SHM_LIMITER: Option> = ShmLimiterMemory::open(&shm_limiter_path()).map_or_else(|e| { - warn!("Attempt to use the SHM_LIMITER failed: {e:?}"); - None - }, Some); - - pub static ref EXCEPTION_HASH_LIMITER: Option = ExceptionHashRateLimiter::open().map_or_else(|e| { - warn!("Attempt to use the EXCEPTION_HASH_LIMITER failed: {e:?}"); - None - }, Some); + pub static ref SHM_LIMITER: Option> = + ShmLimiterMemory::open(&shm_limiter_path()).map_or_else( + |e| { + warn!("Attempt to use the SHM_LIMITER failed: {e:?}"); + None + }, + Some + ); + pub static ref EXCEPTION_HASH_LIMITER: Option = + ExceptionHashRateLimiter::open().map_or_else( + |e| { + warn!("Attempt to use the EXCEPTION_HASH_LIMITER failed: {e:?}"); + None + }, + Some + ); } pub struct MaybeShmLimiter(Option); @@ -227,12 +247,20 @@ pub extern "C" fn ddog_shm_limiter_inc(limiter: &MaybeShmLimiter, limit: u32) -> } #[no_mangle] -pub extern "C" fn ddog_exception_hash_limiter_inc(connection: &mut SidecarTransport, hash: u64, granularity_seconds: u32) -> bool { +pub extern "C" fn ddog_exception_hash_limiter_inc( + connection: &mut SidecarTransport, + hash: u64, + granularity_seconds: u32, +) -> bool { if let Some(limiter) = &*EXCEPTION_HASH_LIMITER { if let Some(limiter) = limiter.find(hash) { return limiter.inc(); } } - let _ = acquire_exception_hash_rate_limiter(connection, hash, Duration::from_secs(granularity_seconds as u64)); + let _ = acquire_exception_hash_rate_limiter( + connection, + hash, + Duration::from_secs(granularity_seconds as u64), + ); true } diff --git a/components-rs/telemetry.rs b/components-rs/telemetry.rs index c572bceea84..512e92fe614 100644 --- a/components-rs/telemetry.rs +++ b/components-rs/telemetry.rs @@ -303,19 +303,25 @@ unsafe fn ddog_sidecar_telemetry_cache_get_or_update<'a>( let env_str = env.to_utf8_lossy(); // I hate you, borrow checker, you get an unsafe from me! - if let Some(cached_entry) = (&mut *(cache as *mut ShmCacheMap)).get_mut(&(service_str.as_ref(), env_str.as_ref())) { + if let Some(cached_entry) = + (&mut *(cache as *mut ShmCacheMap)).get_mut(&(service_str.as_ref(), env_str.as_ref())) + { refresh_cache(cached_entry); return cached_entry; } let shm_path = path_for_telemetry(&service_str, &env_str); - let reader = OneWayShmReader::::new(open_named_shm(&shm_path).ok(), shm_path); - let cached_entry = cache.entry(ShmCacheKey(service_str.into(), env_str.into())).insert(ShmCache { - reader, - config_sent: false, - integrations: HashSet::new(), - composer_paths: HashSet::new(), - }).into_mut(); + let reader = + OneWayShmReader::::new(open_named_shm(&shm_path).ok(), shm_path); + let cached_entry = cache + .entry(ShmCacheKey(service_str.into(), env_str.into())) + .insert(ShmCache { + reader, + config_sent: false, + integrations: HashSet::new(), + composer_paths: HashSet::new(), + }) + .into_mut(); refresh_cache(cached_entry); cached_entry diff --git a/config.m4 b/config.m4 index a30f435268f..b18cae3cb2c 100644 --- a/config.m4 +++ b/config.m4 @@ -208,6 +208,7 @@ if test "$PHP_DDTRACE" != "no"; then ext/remote_config.c \ ext/serializer.c \ ext/sidecar.c \ + ext/sidecar_shm.c \ ext/signals.c \ ext/span.c \ ext/startup_logging.c \ diff --git a/config.w32 b/config.w32 index bb74057a216..fa28dc7c18e 100644 --- a/config.w32 +++ b/config.w32 @@ -54,6 +54,7 @@ if (PHP_DDTRACE != 'no') { DDTRACE_EXT_SOURCES += " remote_config.c"; DDTRACE_EXT_SOURCES += " serializer.c"; DDTRACE_EXT_SOURCES += " sidecar.c"; + DDTRACE_EXT_SOURCES += " sidecar_shm.c"; DDTRACE_EXT_SOURCES += " span.c"; DDTRACE_EXT_SOURCES += " startup_logging.c"; DDTRACE_EXT_SOURCES += " telemetry.c"; diff --git a/ext/ddtrace.c b/ext/ddtrace.c index ab3077d8432..5009e7987fa 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -2617,6 +2617,9 @@ PHP_FUNCTION(DDTrace_Internal_add_span_flag) { void dd_internal_handle_fork(void) { // CHILD PROCESS #ifndef _WIN32 + ddtrace_pid_t current_pid = getpid(); + bool is_child_process = (ddtrace_master_pid != 0 && current_pid != ddtrace_master_pid); + if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) { ddtrace_coms_curl_shutdown(); ddtrace_coms_clean_background_sender_after_fork(); @@ -2636,7 +2639,23 @@ void dd_internal_handle_fork(void) { } ddtrace_seed_prng(); ddtrace_generate_runtime_id(); - ddtrace_reset_sidecar(); + +#ifndef _WIN32 + // If we're a child process and using sidecar, reuse the master's sidecar connection + // Don't call ddtrace_reset_sidecar() which would create a new sidecar instance + if (is_child_process && get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) { + // Child process reuses master's sidecar - just update instance ID + ddtrace_force_new_instance_id(); + // Reconnect with fork flag + if (ddtrace_sidecar) { + ddtrace_sidecar_submit_root_span_data(); + } + } else +#endif + { + // Not a child process, or not using sidecar - normal reset + ddtrace_reset_sidecar(); + } if (!get_DD_TRACE_FORKED_PROCESS()) { ddtrace_disable_tracing_in_current_request(); } diff --git a/ext/sidecar.c b/ext/sidecar.c index 4ae29295b64..e5f988938cc 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -23,6 +23,7 @@ ZEND_EXTERN_MODULE_GLOBALS(ddtrace); ddog_Endpoint *ddtrace_endpoint; ddog_Endpoint *dogstatsd_endpoint; // always set when ddtrace_endpoint is set struct ddog_InstanceId *ddtrace_sidecar_instance_id; +ddtrace_pid_t ddtrace_master_pid = 0; static uint8_t dd_sidecar_formatted_session_id[36]; static inline void dd_set_endpoint_test_token(ddog_Endpoint *endpoint) { @@ -214,6 +215,11 @@ void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) { ddtrace_set_non_resettable_sidecar_globals(); ddtrace_set_resettable_sidecar_globals(); + // Store master PID for later fork detection +#ifndef _WIN32 + ddtrace_master_pid = getpid(); +#endif + ddog_init_remote_config(get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), appsec_activation, appsec_config); ddtrace_sidecar = dd_sidecar_connection_factory(); diff --git a/ext/sidecar.h b/ext/sidecar.h index b3593ceaf1c..5d8cc47fa7b 100644 --- a/ext/sidecar.h +++ b/ext/sidecar.h @@ -6,10 +6,20 @@ #include "ddtrace_export.h" #include "ddtrace.h" #include "zend_string.h" +#ifndef _WIN32 +#include +// Platform-specific PID type +typedef pid_t ddtrace_pid_t; +#else +#include +// Windows uses int for process IDs +typedef int ddtrace_pid_t; +#endif extern ddog_SidecarTransport *ddtrace_sidecar; extern ddog_Endpoint *ddtrace_endpoint; extern struct ddog_InstanceId *ddtrace_sidecar_instance_id; +extern ddtrace_pid_t ddtrace_master_pid; DDTRACE_PUBLIC const uint8_t *ddtrace_get_formatted_session_id(void); struct telemetry_rc_info { diff --git a/ext/sidecar_shm.c b/ext/sidecar_shm.c new file mode 100644 index 00000000000..9a5d70cb348 --- /dev/null +++ b/ext/sidecar_shm.c @@ -0,0 +1,207 @@ +#include "sidecar_shm.h" +#include "ddtrace.h" +#include "sidecar.h" +#include "threads.h" +#include +#include + +ZEND_EXTERN_MODULE_GLOBALS(ddtrace); + +// Global registry for shared memory handles +static ddtrace_shm_registry_t *shm_registry = NULL; + +// Helper function to generate key from (service, env) +static zend_string *ddtrace_shm_make_key(const char *service, const char *env) { + if (!env || !*env) { + env = "none"; + } + return zend_strpprintf(0, "%s:%s", service, env); +} + +// Initialize the SHM registry (called during MINIT) +void ddtrace_shm_registry_init(void) { + if (shm_registry) { + return; // Already initialized + } + + shm_registry = (ddtrace_shm_registry_t *)malloc(sizeof(ddtrace_shm_registry_t)); + if (!shm_registry) { + LOG(ERROR, "Failed to allocate memory for SHM registry"); + return; + } + + shm_registry->shm_map = (HashTable *)malloc(sizeof(HashTable)); + zend_hash_init(shm_registry->shm_map, 8, NULL, NULL, 1); + shm_registry->mutex = tsrm_mutex_alloc(); + + LOG(DEBUG, "SHM registry initialized"); +} + +// Destroy the SHM registry (called during MSHUTDOWN) +void ddtrace_shm_registry_destroy(void) { + if (!shm_registry) { + return; + } + + if (shm_registry->mutex) { + tsrm_mutex_lock(shm_registry->mutex); + } + + // Free all SHM entries + if (shm_registry->shm_map) { + ddtrace_shm_entry_t *entry; + ZEND_HASH_FOREACH_PTR(shm_registry->shm_map, entry) { + if (entry) { + if (entry->service) { + free(entry->service); + } + if (entry->env) { + free(entry->env); + } + if (entry->handle) { + ddog_drop_anon_shm_handle(entry->handle); + } + free(entry); + } + } ZEND_HASH_FOREACH_END(); + + zend_hash_destroy(shm_registry->shm_map); + free(shm_registry->shm_map); + } + + if (shm_registry->mutex) { + tsrm_mutex_unlock(shm_registry->mutex); + tsrm_mutex_free(shm_registry->mutex); + } + + free(shm_registry); + shm_registry = NULL; + + LOG(DEBUG, "SHM registry destroyed"); +} + +// Get or create anonymous SHM handle for a (service, env) tuple +ddog_ShmHandle *ddtrace_shm_get_or_create(const char *service, const char *env) { + if (!shm_registry || !service) { + return NULL; + } + + zend_string *key = ddtrace_shm_make_key(service, env); + ddog_ShmHandle *handle = NULL; + + tsrm_mutex_lock(shm_registry->mutex); + + // Check if entry already exists + ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); + if (entry) { + entry->ref_count++; + handle = entry->handle; + LOG(DEBUG, "SHM handle found for %s:%s (ref_count=%d)", service, env, entry->ref_count); + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); + return handle; + } + + // Create new entry + entry = (ddtrace_shm_entry_t *)malloc(sizeof(ddtrace_shm_entry_t)); + if (!entry) { + LOG(ERROR, "Failed to allocate memory for SHM entry"); + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); + return NULL; + } + + entry->service = strdup(service); + entry->env = env ? strdup(env) : strdup("none"); + entry->ref_count = 1; + + // Allocate anonymous shared memory + ddog_MaybeError maybe_error = ddog_alloc_anon_shm_handle(128 * 1024, &entry->handle); // 128KB default + if (maybe_error.tag == DDOG_OPTION_ERROR_SOME_ERROR) { + ddog_CharSlice error = ddog_Error_message(&maybe_error.some); + LOG(ERROR, "Failed to allocate anonymous SHM for %s:%s: %.*s", + service, env, (int)error.len, error.ptr); + ddog_MaybeError_drop(maybe_error); + + free(entry->service); + free(entry->env); + free(entry); + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); + return NULL; + } + + handle = entry->handle; + + // Add to hash table + zend_hash_add_new_ptr(shm_registry->shm_map, key, entry); + + LOG(DEBUG, "Created new SHM handle for %s:%s (ref_count=1)", service, env); + + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); + + return handle; +} + +// Increment reference count for a SHM handle +void ddtrace_shm_addref(const char *service, const char *env) { + if (!shm_registry || !service) { + return; + } + + zend_string *key = ddtrace_shm_make_key(service, env); + + tsrm_mutex_lock(shm_registry->mutex); + + ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); + if (entry) { + entry->ref_count++; + LOG(DEBUG, "SHM addref for %s:%s (ref_count=%d)", service, env, entry->ref_count); + } else { + LOG(WARN, "Attempted to addref non-existent SHM entry for %s:%s", service, env); + } + + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); +} + +// Decrement reference count and free if zero +void ddtrace_shm_release(const char *service, const char *env) { + if (!shm_registry || !service) { + return; + } + + zend_string *key = ddtrace_shm_make_key(service, env); + + tsrm_mutex_lock(shm_registry->mutex); + + ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); + if (entry) { + entry->ref_count--; + LOG(DEBUG, "SHM release for %s:%s (ref_count=%d)", service, env, entry->ref_count); + + if (entry->ref_count <= 0) { + // Free the entry + LOG(DEBUG, "Freeing SHM handle for %s:%s", service, env); + + if (entry->handle) { + ddog_drop_anon_shm_handle(entry->handle); + } + if (entry->service) { + free(entry->service); + } + if (entry->env) { + free(entry->env); + } + + zend_hash_del(shm_registry->shm_map, key); + free(entry); + } + } else { + LOG(WARN, "Attempted to release non-existent SHM entry for %s:%s", service, env); + } + + tsrm_mutex_unlock(shm_registry->mutex); + zend_string_release(key); +} diff --git a/ext/sidecar_shm.h b/ext/sidecar_shm.h new file mode 100644 index 00000000000..794258b5d45 --- /dev/null +++ b/ext/sidecar_shm.h @@ -0,0 +1,39 @@ +#ifndef DD_SIDECAR_SHM_H +#define DD_SIDECAR_SHM_H + +#include +#include +#include +#include "threads.h" + +// Entry for tracking shared memory per (service, env) tuple +typedef struct { + char *service; + char *env; + ddog_ShmHandle *handle; + int ref_count; +} ddtrace_shm_entry_t; + +// Global registry for shared memory handles +typedef struct { + HashTable *shm_map; // Maps "service:env" -> ddtrace_shm_entry_t + MUTEX_T mutex; // Protects the hash table +} ddtrace_shm_registry_t; + +// Initialize the SHM registry (called during MINIT) +void ddtrace_shm_registry_init(void); + +// Destroy the SHM registry (called during MSHUTDOWN) +void ddtrace_shm_registry_destroy(void); + +// Get or create anonymous SHM handle for a (service, env) tuple +// Returns NULL on failure +ddog_ShmHandle *ddtrace_shm_get_or_create(const char *service, const char *env); + +// Increment reference count for a SHM handle +void ddtrace_shm_addref(const char *service, const char *env); + +// Decrement reference count and free if zero +void ddtrace_shm_release(const char *service, const char *env); + +#endif // DD_SIDECAR_SHM_H From 31448a7b49236aa7f76037169b772df425f09c28 Mon Sep 17 00:00:00 2001 From: Alexandre Rulleau Date: Wed, 5 Nov 2025 16:56:48 +0100 Subject: [PATCH 2/4] feat: fix threaded connection Signed-off-by: Alexandre Rulleau --- components-rs/ddtrace.h | 57 ++++++++++++ components-rs/sidecar.h | 2 + ext/ddtrace.c | 48 +++++++---- ext/sidecar.c | 186 +++++++++++++++++++++++++++++++++------- ext/sidecar.h | 2 + 5 files changed, 248 insertions(+), 47 deletions(-) diff --git a/components-rs/ddtrace.h b/components-rs/ddtrace.h index 35d06a61820..decf8da1a33 100644 --- a/components-rs/ddtrace.h +++ b/components-rs/ddtrace.h @@ -43,6 +43,63 @@ ddog_Configurator *ddog_library_configurator_new_dummy(bool debug_logs, ddog_Cha int posix_spawn_file_actions_addchdir_np(void *file_actions, const char *path); +void ddog_init_span_func(void (*free_func)(struct _zend_string*), + void (*addref_func)(struct _zend_string*)); + +void ddog_set_span_service_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); + +void ddog_set_span_name_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); + +void ddog_set_span_resource_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); + +void ddog_set_span_type_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); + +void ddog_add_span_meta_zstr(ddog_SpanBytes *ptr, + struct _zend_string *key, + struct _zend_string *val); + +void ddog_add_CharSlice_span_meta_zstr(ddog_SpanBytes *ptr, + ddog_CharSlice key, + struct _zend_string *val); + +void ddog_add_zstr_span_meta_str(ddog_SpanBytes *ptr, struct _zend_string *key, const char *val); + +void ddog_add_str_span_meta_str(ddog_SpanBytes *ptr, const char *key, const char *val); + +void ddog_add_str_span_meta_zstr(ddog_SpanBytes *ptr, const char *key, struct _zend_string *val); + +void ddog_add_str_span_meta_CharSlice(ddog_SpanBytes *ptr, const char *key, ddog_CharSlice val); + +void ddog_del_span_meta_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); + +void ddog_del_span_meta_str(ddog_SpanBytes *ptr, const char *key); + +bool ddog_has_span_meta_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); + +bool ddog_has_span_meta_str(ddog_SpanBytes *ptr, const char *key); + +ddog_CharSlice ddog_get_span_meta_str(ddog_SpanBytes *span, const char *key); + +void ddog_add_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key, double val); + +bool ddog_has_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); + +void ddog_del_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); + +void ddog_add_span_metrics_str(ddog_SpanBytes *ptr, const char *key, double val); + +bool ddog_get_span_metrics_str(ddog_SpanBytes *ptr, const char *key, double *result); + +void ddog_del_span_metrics_str(ddog_SpanBytes *ptr, const char *key); + +void ddog_add_span_meta_struct_zstr(ddog_SpanBytes *ptr, + struct _zend_string *key, + struct _zend_string *val); + +void ddog_add_zstr_span_meta_struct_CharSlice(ddog_SpanBytes *ptr, + struct _zend_string *key, + ddog_CharSlice val); + bool ddog_shall_log(enum ddog_Log category); void ddog_set_error_log_level(bool once); diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h index a87ee2721bf..a7b75235000 100644 --- a/components-rs/sidecar.h +++ b/components-rs/sidecar.h @@ -93,6 +93,8 @@ ddog_MaybeError ddog_sidecar_connect_master(int32_t master_pid); ddog_MaybeError ddog_sidecar_connect_worker(int32_t master_pid, struct ddog_SidecarTransport **connection); +ddog_MaybeError ddog_sidecar_shutdown_master_listener(void); + ddog_MaybeError ddog_sidecar_ping(struct ddog_SidecarTransport **transport); ddog_MaybeError ddog_sidecar_flush_traces(struct ddog_SidecarTransport **transport); diff --git a/ext/ddtrace.c b/ext/ddtrace.c index 5009e7987fa..a93e15dce22 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -433,10 +433,6 @@ static void dd_activate_once(void) { // must run before the first zai_hook_activate as ddtrace_telemetry_setup installs a global hook if (!ddtrace_disable) { - bool appsec_activation = false; - bool appsec_config = false; - -#ifndef _WIN32 // Only disable sidecar sender when explicitly disabled bool bgs_fallback = DD_SIDECAR_TRACE_SENDER_DEFAULT && get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_SIDECAR_TRACE_SENDER].name_index == ZAI_CONFIG_ORIGIN_DEFAULT && !get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(); zend_string *bgs_service = NULL; @@ -452,15 +448,15 @@ static void dd_activate_once(void) { } } - // if we're to enable appsec, we need to enable sidecar + // If we're to enable appsec, we need to enable sidecar + bool appsec_activation = false; + bool appsec_config = false; bool enable_sidecar = ddtrace_sidecar_maybe_enable_appsec(&appsec_activation, &appsec_config); if (!enable_sidecar) { enable_sidecar = get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER(); } - if (enable_sidecar) -#endif - { + if (enable_sidecar) { bool request_startup = PG(during_request_startup); PG(during_request_startup) = false; ddtrace_sidecar_setup(appsec_activation, appsec_config); @@ -1552,6 +1548,10 @@ static PHP_MINIT_FUNCTION(ddtrace) { ddtrace_signals_minit(); #endif + // Store master PID for threaded sidecar connectivity + // The actual sidecar setup happens in first RINIT via pthread_once + ddtrace_sidecar_minit(false, false); + return SUCCESS; } @@ -2641,19 +2641,33 @@ void dd_internal_handle_fork(void) { ddtrace_generate_runtime_id(); #ifndef _WIN32 - // If we're a child process and using sidecar, reuse the master's sidecar connection - // Don't call ddtrace_reset_sidecar() which would create a new sidecar instance - if (is_child_process && get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) { - // Child process reuses master's sidecar - just update instance ID - ddtrace_force_new_instance_id(); - // Reconnect with fork flag - if (ddtrace_sidecar) { - ddtrace_sidecar_submit_root_span_data(); + // Handle sidecar after fork with threaded connectivity + if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) { + if (is_child_process) { + // Child process connects to master's sidecar as a worker + ddtrace_force_new_instance_id(); + + // Connect to master as worker using threaded connectivity + if (ddtrace_sidecar_connect_worker_after_fork()) { + LOG(DEBUG, "Child process connected to master sidecar as worker (child_pid=%d, master_pid=%d)", + current_pid, ddtrace_master_pid); + + // Submit root span data with the new connection + if (ddtrace_sidecar) { + ddtrace_sidecar_submit_root_span_data(); + } + } else { + LOG(WARN, "Failed to connect child process as worker to master sidecar (child_pid=%d, master_pid=%d)", + current_pid, ddtrace_master_pid); + // Fallback: reset sidecar on failure + ddtrace_reset_sidecar(); + } } + // Parent process: keep existing connection, no action needed } else #endif { - // Not a child process, or not using sidecar - normal reset + // Not using sidecar - normal reset for non-sidecar trace sender ddtrace_reset_sidecar(); } if (!get_DD_TRACE_FORKED_PROCESS()) { diff --git a/ext/sidecar.c b/ext/sidecar.c index e5f988938cc..a998a30f579 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -153,6 +153,9 @@ static void dd_sidecar_on_reconnect(ddog_SidecarTransport *transport) { } +// Forward declaration for reconnection handler +static ddog_SidecarTransport *dd_sidecar_connection_factory(void); + static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { // Should not happen, unless the agent url is malformed if (!ddtrace_endpoint) { @@ -173,13 +176,18 @@ static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { } ddog_SidecarTransport *sidecar_transport; - if (!ddtrace_ffi_try("Failed connecting to the sidecar", ddog_sidecar_connect_php(&sidecar_transport, logpath, dd_zend_string_to_CharSlice(get_global_DD_TRACE_LOG_LEVEL()), get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), dd_sidecar_on_reconnect, ddtrace_endpoint))) { + // With threaded connectivity, use connect_worker to connect to master's listener + if (!ddtrace_ffi_try("Failed connecting to sidecar as worker", + ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { dd_free_endpoints(); return NULL; } dd_sidecar_post_connect(&sidecar_transport, is_fork, logpath); + // Set up automatic reconnection handler + ddtrace_sidecar_reconnect(&sidecar_transport, dd_sidecar_connection_factory); + return sidecar_transport; } @@ -211,27 +219,153 @@ bool ddtrace_sidecar_maybe_enable_appsec(bool *appsec_activation, bool *appsec_c #endif } -void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) { - ddtrace_set_non_resettable_sidecar_globals(); - ddtrace_set_resettable_sidecar_globals(); +// Called in MINIT - stores master PID for threaded connectivity +void ddtrace_sidecar_minit(bool appsec_activation, bool appsec_config) { + UNUSED(appsec_activation, appsec_config); + + // Store master PID for threaded connectivity only if not already set + // This ensures that after fork, child processes keep the parent's PID + if (ddtrace_master_pid == 0) { +#ifdef _WIN32 + ddtrace_master_pid = _getpid(); +#else + ddtrace_master_pid = getpid(); +#endif + LOG(DEBUG, "Sidecar MINIT: Stored master PID=%d", ddtrace_master_pid); + } else { + LOG(DEBUG, "Sidecar MINIT: Master PID already set to %d, current PID=%d", + ddtrace_master_pid, +#ifdef _WIN32 + _getpid() +#else + getpid() +#endif + ); + } +} - // Store master PID for later fork detection +// Setup master sidecar connection - called once on first RINIT +static void ddtrace_sidecar_setup_master(bool appsec_activation, bool appsec_config) { + // Check if we're in a forked child process + // pthread_once state is not inherited across fork, so child processes + // might try to call this function. They should NOT start a new master listener. #ifndef _WIN32 - ddtrace_master_pid = getpid(); + ddtrace_pid_t current_pid = getpid(); + + LOG(DEBUG, "ddtrace_sidecar_setup_master called: current_pid=%d, ddtrace_master_pid=%d", + current_pid, ddtrace_master_pid); + + bool is_child_process = (ddtrace_master_pid != 0 && current_pid != ddtrace_master_pid); + + if (is_child_process) { + LOG(DEBUG, "Skipping master sidecar setup in child process (child_pid=%d, master_pid=%d)", + current_pid, ddtrace_master_pid); + // Child processes should only connect as workers, which is handled in dd_internal_handle_fork() + return; + } + + LOG(DEBUG, "Proceeding with master sidecar setup for PID=%d", current_pid); #endif + ddtrace_set_non_resettable_sidecar_globals(); + ddtrace_set_resettable_sidecar_globals(); + ddog_init_remote_config(get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), appsec_activation, appsec_config); - ddtrace_sidecar = dd_sidecar_connection_factory(); - if (!ddtrace_sidecar) { // Something went wrong + // Start master listener for worker processes/threads to connect to + if (!ddtrace_ffi_try("Failed starting sidecar master listener", ddog_sidecar_connect_master((int32_t)ddtrace_master_pid))) { + LOG(WARN, "Failed to start sidecar master listener"); + if (ddtrace_endpoint) { + dd_free_endpoints(); + } + return; + } + + // Master process also needs a transport - connect as first worker + ddog_SidecarTransport *sidecar_transport = NULL; + if (!ddtrace_ffi_try("Failed connecting master to sidecar", ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { + LOG(WARN, "Failed to connect master process to sidecar"); if (ddtrace_endpoint) { dd_free_endpoints(); } + return; + } + + char logpath[MAXPATHLEN]; + int error_fd = atomic_load(&ddtrace_error_log_fd); + if (error_fd == -1 || ddtrace_get_fd_path(error_fd, logpath) < 0) { + *logpath = 0; } + dd_sidecar_post_connect(&sidecar_transport, false, logpath); + ddtrace_sidecar = sidecar_transport; + if (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED()) { ddtrace_telemetry_first_init(); } + + LOG(DEBUG, "Sidecar master setup complete, PID=%d", ddtrace_master_pid); +} + +// Called in RINIT - handles git metadata and root span data +void ddtrace_sidecar_rinit(void) { + if (get_DD_TRACE_GIT_METADATA_ENABLED()) { + zval git_object; + ZVAL_UNDEF(&git_object); + ddtrace_inject_git_metadata(&git_object); + if (Z_TYPE(git_object) == IS_OBJECT) { + ddtrace_git_metadata *git_metadata = (ddtrace_git_metadata *) Z_OBJ(git_object); + if (Z_TYPE(git_metadata->property_commit) == IS_STRING) { + UNUSED(ddog_Vec_Tag_push(&DDTRACE_G(active_global_tags), DDOG_CHARSLICE_C("git.commit.sha"), + dd_zend_string_to_CharSlice(Z_STR(git_metadata->property_commit)))); + } + if (Z_TYPE(git_metadata->property_repository) == IS_STRING) { + UNUSED(ddog_Vec_Tag_push(&DDTRACE_G(active_global_tags), DDOG_CHARSLICE_C("git.repository_url"), + dd_zend_string_to_CharSlice(Z_STR(git_metadata->property_repository)))); + } + OBJ_RELEASE(&git_metadata->std); + } + } + + ddtrace_sidecar_submit_root_span_data_direct_defaults(&ddtrace_sidecar, NULL); +} + +// Setup function called via pthread_once on first RINIT +void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) { + // This is called on first RINIT when config is available + // Set up the master sidecar connection with threaded connectivity + ddtrace_sidecar_setup_master(appsec_activation, appsec_config); +} + +// Connect child process to master sidecar as worker after fork +bool ddtrace_sidecar_connect_worker_after_fork(void) { + if (!ddtrace_endpoint || ddtrace_master_pid == 0) { + return false; + } + + // Connect to master as worker using threaded connectivity + ddog_SidecarTransport *sidecar_transport = NULL; + if (!ddtrace_ffi_try("Failed connecting worker to sidecar after fork", + ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { + return false; + } + + char logpath[MAXPATHLEN]; + int error_fd = atomic_load(&ddtrace_error_log_fd); + if (error_fd == -1 || ddtrace_get_fd_path(error_fd, logpath) < 0) { + *logpath = 0; + } + + // Post-connect with fork flag set to true + dd_sidecar_post_connect(&sidecar_transport, true, logpath); + + // Drop old transport and use the new worker connection + if (ddtrace_sidecar) { + ddog_sidecar_transport_drop(ddtrace_sidecar); + } + ddtrace_sidecar = sidecar_transport; + + return true; } void ddtrace_sidecar_ensure_active(void) { @@ -261,6 +395,20 @@ void ddtrace_sidecar_finalize(bool clear_id) { } void ddtrace_sidecar_shutdown(void) { + // Shutdown master listener thread if this is the master process +#ifndef _WIN32 + ddtrace_pid_t current_pid = getpid(); + if (ddtrace_master_pid != 0 && current_pid == ddtrace_master_pid) { + LOG(DEBUG, "Shutting down master listener thread (PID=%d)", current_pid); + ddtrace_ffi_try("Failed shutting down master listener", ddog_sidecar_shutdown_master_listener()); + } +#else + if (ddtrace_master_pid != 0 && _getpid() == ddtrace_master_pid) { + LOG(DEBUG, "Shutting down master listener thread (PID=%d)", _getpid()); + ddtrace_ffi_try("Failed shutting down master listener", ddog_sidecar_shutdown_master_listener()); + } +#endif + if (ddtrace_sidecar_instance_id) { ddog_sidecar_instanceId_drop(ddtrace_sidecar_instance_id); } @@ -563,28 +711,6 @@ void ddtrace_sidecar_activate(void) { } ZEND_HASH_FOREACH_END(); } -void ddtrace_sidecar_rinit(void) { - if (get_DD_TRACE_GIT_METADATA_ENABLED()) { - zval git_object; - ZVAL_UNDEF(&git_object); - ddtrace_inject_git_metadata(&git_object); - if (Z_TYPE(git_object) == IS_OBJECT) { - ddtrace_git_metadata *git_metadata = (ddtrace_git_metadata *) Z_OBJ(git_object); - if (Z_TYPE(git_metadata->property_commit) == IS_STRING) { - UNUSED(ddog_Vec_Tag_push(&DDTRACE_G(active_global_tags), DDOG_CHARSLICE_C("git.commit.sha"), - dd_zend_string_to_CharSlice(Z_STR(git_metadata->property_commit)))); - } - if (Z_TYPE(git_metadata->property_repository) == IS_STRING) { - UNUSED(ddog_Vec_Tag_push(&DDTRACE_G(active_global_tags), DDOG_CHARSLICE_C("git.repository_url"), - dd_zend_string_to_CharSlice(Z_STR(git_metadata->property_repository)))); - } - OBJ_RELEASE(&git_metadata->std); - } - } - - ddtrace_sidecar_submit_root_span_data_direct_defaults(&ddtrace_sidecar, NULL); -} - void ddtrace_sidecar_rshutdown(void) { ddog_Vec_Tag_drop(DDTRACE_G(active_global_tags)); } diff --git a/ext/sidecar.h b/ext/sidecar.h index 5d8cc47fa7b..4e9a288cece 100644 --- a/ext/sidecar.h +++ b/ext/sidecar.h @@ -30,7 +30,9 @@ struct telemetry_rc_info { }; DDTRACE_PUBLIC struct telemetry_rc_info ddtrace_get_telemetry_rc_info(void); +void ddtrace_sidecar_minit(bool appsec_activation, bool appsec_config); void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config); +bool ddtrace_sidecar_connect_worker_after_fork(void); bool ddtrace_sidecar_maybe_enable_appsec(bool *appsec_activation, bool *appsec_config); void ddtrace_sidecar_ensure_active(void); void ddtrace_sidecar_finalize(bool clear_id); From bc5275732e8ffbc5b839033e605dd3e0d23642d6 Mon Sep 17 00:00:00 2001 From: Alexandre Rulleau Date: Mon, 10 Nov 2025 17:23:00 +0100 Subject: [PATCH 3/4] retrigger pipeline --- Cargo.lock | 111 ------------------- appsec/third_party/libddwaf | 2 +- components-rs/common.h | 82 ++++---------- components-rs/crashtracker.h | 4 +- components-rs/ddtrace.h | 63 +---------- components-rs/log.rs | 6 +- components-rs/sidecar.rs | 27 ++--- config.m4 | 1 - config.w32 | 1 - ext/ddtrace.c | 11 +- ext/sidecar.c | 55 ++-------- ext/sidecar.h | 2 - ext/sidecar_shm.c | 207 ----------------------------------- ext/sidecar_shm.h | 39 ------- libdatadog | 2 +- 15 files changed, 44 insertions(+), 569 deletions(-) delete mode 100644 ext/sidecar_shm.c delete mode 100644 ext/sidecar_shm.h diff --git a/Cargo.lock b/Cargo.lock index bcb7435d1b8..b40383d3bee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1088,106 +1088,6 @@ dependencies = [ "syn 2.0.96", ] -[[package]] -name = "data-pipeline" -version = "0.0.1" -dependencies = [ - "anyhow", - "arc-swap", - "bytes", - "clap", - "criterion", - "datadog-trace-protobuf", - "datadog-trace-stats", - "datadog-trace-utils", - "ddcommon 0.0.1", - "ddtelemetry", - "either", - "http", - "http-body-util", - "httpmock", - "hyper", - "hyper-util", - "libdd-ddsketch", - "libdd-dogstatsd-client", - "libdd-log", - "libdd-tinybytes", - "rand 0.8.5", - "regex", - "rmp-serde", - "serde", - "serde_json", - "sha2", - "tempfile", - "tokio", - "tokio-util", - "tracing", - "uuid", -] - -[[package]] -name = "datadog-alloc" -version = "21.0.0" -source = "git+https://github.com/DataDog/libdatadog?rev=0b59f64c4fc08105e5b73c5a0752ced3cf8f653e#0b59f64c4fc08105e5b73c5a0752ced3cf8f653e" -dependencies = [ - "allocator-api2", - "libc 0.2.177", - "windows-sys 0.52.0", -] - -[[package]] -name = "datadog-crashtracker" -version = "0.0.1" -dependencies = [ - "anyhow", - "backtrace", - "blazesym", - "cc", - "chrono", - "criterion", - "ddcommon 0.0.1", - "ddtelemetry", - "goblin", - "http", - "libc 0.2.177", - "nix", - "num-derive", - "num-traits", - "os_info", - "page_size", - "portable-atomic", - "rand 0.8.5", - "schemars", - "serde", - "serde_json", - "symbolic-common", - "symbolic-demangle", - "tempfile", - "thiserror 1.0.69", - "tokio", - "uuid", - "windows 0.59.0", -] - -[[package]] -name = "datadog-crashtracker-ffi" -version = "0.0.1" -dependencies = [ - "anyhow", - "build_common", - "datadog-crashtracker", - "ddcommon 0.0.1", - "ddcommon-ffi", - "function_name", - "libc 0.2.177", - "serde", - "serde_json", - "symbolic-common", - "symbolic-demangle", - "tempfile", - "windows 0.59.0", -] - [[package]] name = "datadog-ipc" version = "0.1.0" @@ -1266,17 +1166,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "datadog-log" -version = "0.0.1" -dependencies = [ - "chrono", - "tempfile", - "tracing", - "tracing-appender", - "tracing-subscriber", -] - [[package]] name = "datadog-php-profiling" version = "0.0.0" diff --git a/appsec/third_party/libddwaf b/appsec/third_party/libddwaf index 00e895f2c50..bea83f92723 160000 --- a/appsec/third_party/libddwaf +++ b/appsec/third_party/libddwaf @@ -1 +1 @@ -Subproject commit 00e895f2c507a714062aa88ed41466aec10d2e01 +Subproject commit bea83f92723ef578b592148edae9cce9171d8ac0 diff --git a/components-rs/common.h b/components-rs/common.h index 5d815bca4fb..38703355a30 100644 --- a/components-rs/common.h +++ b/components-rs/common.h @@ -264,34 +264,6 @@ typedef struct _zend_string _zend_string; #define ddog_MultiTargetFetcher_DEFAULT_CLIENTS_LIMIT 100 -typedef enum ddog_ConfigurationOrigin { - DDOG_CONFIGURATION_ORIGIN_ENV_VAR, - DDOG_CONFIGURATION_ORIGIN_CODE, - DDOG_CONFIGURATION_ORIGIN_DD_CONFIG, - DDOG_CONFIGURATION_ORIGIN_REMOTE_CONFIG, - DDOG_CONFIGURATION_ORIGIN_DEFAULT, - DDOG_CONFIGURATION_ORIGIN_LOCAL_STABLE_CONFIG, - DDOG_CONFIGURATION_ORIGIN_FLEET_STABLE_CONFIG, -} ddog_ConfigurationOrigin; - -typedef enum ddog_DynamicConfigUpdateMode { - DDOG_DYNAMIC_CONFIG_UPDATE_MODE_READ, - DDOG_DYNAMIC_CONFIG_UPDATE_MODE_READ_WRITE, - DDOG_DYNAMIC_CONFIG_UPDATE_MODE_WRITE, - DDOG_DYNAMIC_CONFIG_UPDATE_MODE_RESTORE, -} ddog_DynamicConfigUpdateMode; - -typedef enum ddog_EvaluateAt { - DDOG_EVALUATE_AT_ENTRY, - DDOG_EVALUATE_AT_EXIT, -} ddog_EvaluateAt; - -typedef enum ddog_InBodyLocation { - DDOG_IN_BODY_LOCATION_NONE, - DDOG_IN_BODY_LOCATION_START, - DDOG_IN_BODY_LOCATION_END, -} ddog_InBodyLocation; - typedef enum ddog_Log { DDOG_LOG_ERROR = 1, DDOG_LOG_WARN = 2, @@ -306,6 +278,13 @@ typedef enum ddog_Log { DDOG_LOG_HOOK_TRACE = (5 | (4 << 4)), } ddog_Log; +typedef enum ddog_DynamicConfigUpdateMode { + DDOG_DYNAMIC_CONFIG_UPDATE_MODE_READ, + DDOG_DYNAMIC_CONFIG_UPDATE_MODE_READ_WRITE, + DDOG_DYNAMIC_CONFIG_UPDATE_MODE_WRITE, + DDOG_DYNAMIC_CONFIG_UPDATE_MODE_RESTORE, +} ddog_DynamicConfigUpdateMode; + typedef enum ddog_InBodyLocation { DDOG_IN_BODY_LOCATION_NONE, DDOG_IN_BODY_LOCATION_START, @@ -457,6 +436,8 @@ typedef struct ddog_SidecarActionsBuffer ddog_SidecarActionsBuffer; */ typedef struct ddog_SidecarTransport ddog_SidecarTransport; +typedef struct _zend_string *ddog_OwnedZendString; + /** * Holds the raw parts of a Rust Vec; it should only be created from Rust, * never from C. @@ -472,8 +453,6 @@ typedef struct ddog_Tag { const struct ddog_DslString *value; } ddog_Tag; -typedef struct _zend_string *ddog_OwnedZendString; - typedef struct _zend_string *(*ddog_DynamicConfigUpdate)(ddog_CharSlice config, ddog_OwnedZendString value, enum ddog_DynamicConfigUpdateMode mode); @@ -803,6 +782,7 @@ typedef enum ddog_FieldType { typedef enum ddog_DebuggerType { DDOG_DEBUGGER_TYPE_DIAGNOSTICS, + DDOG_DEBUGGER_TYPE_SNAPSHOTS, DDOG_DEBUGGER_TYPE_LOGS, } ddog_DebuggerType; @@ -1086,38 +1066,6 @@ typedef struct ddog_SenderParameters { ddog_CharSlice url; } ddog_SenderParameters; -typedef enum ddog_crasht_BuildIdType { - DDOG_CRASHT_BUILD_ID_TYPE_GNU, - DDOG_CRASHT_BUILD_ID_TYPE_GO, - DDOG_CRASHT_BUILD_ID_TYPE_PDB, - DDOG_CRASHT_BUILD_ID_TYPE_SHA1, -} ddog_crasht_BuildIdType; - -/** - * Result type for runtime callback registration - */ -typedef enum ddog_crasht_CallbackResult { - DDOG_CRASHT_CALLBACK_RESULT_OK, - DDOG_CRASHT_CALLBACK_RESULT_ERROR, -} ddog_crasht_CallbackResult; - -typedef enum ddog_crasht_DemangleOptions { - DDOG_CRASHT_DEMANGLE_OPTIONS_COMPLETE, - DDOG_CRASHT_DEMANGLE_OPTIONS_NAME_ONLY, -} ddog_crasht_DemangleOptions; - -typedef enum ddog_crasht_ErrorKind { - DDOG_CRASHT_ERROR_KIND_PANIC, - DDOG_CRASHT_ERROR_KIND_UNHANDLED_EXCEPTION, - DDOG_CRASHT_ERROR_KIND_UNIX_SIGNAL, -} ddog_crasht_ErrorKind; - -typedef enum ddog_crasht_FileType { - DDOG_CRASHT_FILE_TYPE_APK, - DDOG_CRASHT_FILE_TYPE_ELF, - DDOG_CRASHT_FILE_TYPE_PE, -} ddog_crasht_FileType; - /** * Stacktrace collection occurs in the context of a crashing process. * If the stack is sufficiently corruputed, it is possible (but unlikely), @@ -1260,6 +1208,14 @@ typedef enum ddog_crasht_DemangleOptions { DDOG_CRASHT_DEMANGLE_OPTIONS_NAME_ONLY, } ddog_crasht_DemangleOptions; +/** + * Result type for runtime callback registration + */ +typedef enum ddog_crasht_CallbackResult { + DDOG_CRASHT_CALLBACK_RESULT_OK, + DDOG_CRASHT_CALLBACK_RESULT_ERROR, +} ddog_crasht_CallbackResult; + typedef struct ddog_crasht_CrashInfo ddog_crasht_CrashInfo; typedef struct ddog_crasht_CrashInfoBuilder ddog_crasht_CrashInfoBuilder; @@ -1336,7 +1292,7 @@ typedef struct ddog_crasht_Config { /** * Timeout in milliseconds before the signal handler starts tearing things down to return. * If 0, uses the default timeout as specified in - * `datadog_crashtracker::shared::constants::DD_CRASHTRACK_DEFAULT_TIMEOUT`. Otherwise, uses + * `libdd_crashtracker::shared::constants::DD_CRASHTRACK_DEFAULT_TIMEOUT`. Otherwise, uses * the specified timeout value. * This is given as a uint32_t, but the actual timeout needs to fit inside of an i32 (max * 2^31-1). This is a limitation of the various interfaces used to guarantee the timeout. diff --git a/components-rs/crashtracker.h b/components-rs/crashtracker.h index 42209ff18cc..43c14991ba7 100644 --- a/components-rs/crashtracker.h +++ b/components-rs/crashtracker.h @@ -854,7 +854,7 @@ struct ddog_StringWrapperResult ddog_crasht_demangle(ddog_CharSlice name, * signal handler is dangerous, so we fork a sidecar to do the stuff we aren't * allowed to do in the handler. * - * See comments in [datadog-crashtracker/lib.rs] for a full architecture description. + * See comments in [libdd-crashtracker/lib.rs] for a full architecture description. * # Safety * No safety concerns */ @@ -868,7 +868,7 @@ DDOG_CHECK_RETURN struct ddog_VoidResult ddog_crasht_receiver_entry_point_stdin( * signal handler is dangerous, so we fork a sidecar to do the stuff we aren't * allowed to do in the handler. * - * See comments in [datadog-crashtracker/lib.rs] for a full architecture + * See comments in [libdd-crashtracker/lib.rs] for a full architecture * description. * # Safety * No safety concerns diff --git a/components-rs/ddtrace.h b/components-rs/ddtrace.h index decf8da1a33..89039bc88db 100644 --- a/components-rs/ddtrace.h +++ b/components-rs/ddtrace.h @@ -43,8 +43,9 @@ ddog_Configurator *ddog_library_configurator_new_dummy(bool debug_logs, ddog_Cha int posix_spawn_file_actions_addchdir_np(void *file_actions, const char *path); -void ddog_init_span_func(void (*free_func)(struct _zend_string*), - void (*addref_func)(struct _zend_string*)); +void ddog_init_span_func(void (*free_func)(ddog_OwnedZendString), + void (*addref_func)(struct _zend_string*), + ddog_OwnedZendString (*init_func)(ddog_CharSlice)); void ddog_set_span_service_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); @@ -238,62 +239,4 @@ ddog_MaybeError ddog_sidecar_telemetry_filter_flush(struct ddog_SidecarTransport ddog_CharSlice service, ddog_CharSlice env); -void ddog_init_span_func(void (*free_func)(ddog_OwnedZendString), - void (*addref_func)(struct _zend_string*), - ddog_OwnedZendString (*init_func)(ddog_CharSlice)); - -void ddog_set_span_service_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); - -void ddog_set_span_name_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); - -void ddog_set_span_resource_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); - -void ddog_set_span_type_zstr(ddog_SpanBytes *ptr, struct _zend_string *str); - -void ddog_add_span_meta_zstr(ddog_SpanBytes *ptr, - struct _zend_string *key, - struct _zend_string *val); - -void ddog_add_CharSlice_span_meta_zstr(ddog_SpanBytes *ptr, - ddog_CharSlice key, - struct _zend_string *val); - -void ddog_add_zstr_span_meta_str(ddog_SpanBytes *ptr, struct _zend_string *key, const char *val); - -void ddog_add_str_span_meta_str(ddog_SpanBytes *ptr, const char *key, const char *val); - -void ddog_add_str_span_meta_zstr(ddog_SpanBytes *ptr, const char *key, struct _zend_string *val); - -void ddog_add_str_span_meta_CharSlice(ddog_SpanBytes *ptr, const char *key, ddog_CharSlice val); - -void ddog_del_span_meta_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); - -void ddog_del_span_meta_str(ddog_SpanBytes *ptr, const char *key); - -bool ddog_has_span_meta_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); - -bool ddog_has_span_meta_str(ddog_SpanBytes *ptr, const char *key); - -ddog_CharSlice ddog_get_span_meta_str(ddog_SpanBytes *span, const char *key); - -void ddog_add_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key, double val); - -bool ddog_has_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); - -void ddog_del_span_metrics_zstr(ddog_SpanBytes *ptr, struct _zend_string *key); - -void ddog_add_span_metrics_str(ddog_SpanBytes *ptr, const char *key, double val); - -bool ddog_get_span_metrics_str(ddog_SpanBytes *ptr, const char *key, double *result); - -void ddog_del_span_metrics_str(ddog_SpanBytes *ptr, const char *key); - -void ddog_add_span_meta_struct_zstr(ddog_SpanBytes *ptr, - struct _zend_string *key, - struct _zend_string *val); - -void ddog_add_zstr_span_meta_struct_CharSlice(ddog_SpanBytes *ptr, - struct _zend_string *key, - ddog_CharSlice val); - #endif /* DDTRACE_PHP_H */ diff --git a/components-rs/log.rs b/components-rs/log.rs index 56f100a4b71..c9be5722df8 100644 --- a/components-rs/log.rs +++ b/components-rs/log.rs @@ -1,5 +1,5 @@ -use ddcommon_ffi::slice::AsBytes; -use ddcommon_ffi::CharSlice; +use libdd_common_ffi::slice::AsBytes; +use libdd_common_ffi::CharSlice; use std::cell::RefCell; use std::collections::{BTreeSet, HashMap}; use std::ffi::c_char; @@ -11,8 +11,6 @@ use tracing_subscriber::fmt::format::Writer; use tracing_subscriber::fmt::{FmtContext, FormatEvent, FormatFields}; use tracing_subscriber::registry::LookupSpan; use tracing_subscriber::util::SubscriberInitExt; -use libdd_common_ffi::CharSlice; -use libdd_common_ffi::slice::AsBytes; use tracing_subscriber::EnvFilter; pub const LOG_ONCE: isize = 1 << 3; diff --git a/components-rs/sidecar.rs b/components-rs/sidecar.rs index 060d2c4b32f..ba434a87370 100644 --- a/components-rs/sidecar.rs +++ b/components-rs/sidecar.rs @@ -3,15 +3,16 @@ use datadog_sidecar::config::{self, AppSecConfig, LogMethod}; use datadog_sidecar::service::blocking::{acquire_exception_hash_rate_limiter, SidecarTransport}; use datadog_sidecar::service::exception_hash_rate_limiter::ExceptionHashRateLimiter; use datadog_sidecar::tracer::shm_limiter_path; -use ddcommon::rate_limiter::{Limiter, LocalLimiter}; -use ddcommon_ffi::slice::AsBytes; -use ddcommon_ffi::{self as ffi, CharSlice, MaybeError}; -use ddtelemetry_ffi::try_c; use lazy_static::{lazy_static, LazyStatic}; -#[cfg(windows)] -use spawn_worker::get_trampoline_target_data; +use libdd_common::rate_limiter::{Limiter, LocalLimiter}; +use libdd_common::Endpoint; +use libdd_common_ffi::slice::AsBytes; +use libdd_common_ffi::{self as ffi, CharSlice, MaybeError}; +use libdd_telemetry_ffi::try_c; #[cfg(any(windows, php_shared_build))] use spawn_worker::LibDependency; +#[cfg(windows)] +use spawn_worker::get_trampoline_target_data; use std::ffi::{c_char, CStr, OsStr}; use std::ops::DerefMut; #[cfg(unix)] @@ -20,20 +21,6 @@ use std::os::unix::ffi::OsStrExt; use std::os::windows::ffi::OsStrExt; use std::sync::Mutex; use std::time::Duration; -use datadog_sidecar::config::{self, AppSecConfig, LogMethod}; -use datadog_sidecar::service::blocking::{acquire_exception_hash_rate_limiter, SidecarTransport}; -use libdd_common::rate_limiter::{Limiter, LocalLimiter}; -use datadog_ipc::rate_limiter::{AnyLimiter, ShmLimiterMemory}; -use datadog_sidecar::service::exception_hash_rate_limiter::ExceptionHashRateLimiter; -use datadog_sidecar::tracer::shm_limiter_path; -use libdd_common::Endpoint; -use libdd_common_ffi::slice::AsBytes; -use libdd_common_ffi::{CharSlice, self as ffi, MaybeError}; -use libdd_telemetry_ffi::try_c; -#[cfg(any(windows, php_shared_build))] -use spawn_worker::LibDependency; -#[cfg(windows)] -use spawn_worker::get_trampoline_target_data; use tracing::warn; diff --git a/config.m4 b/config.m4 index b18cae3cb2c..a30f435268f 100644 --- a/config.m4 +++ b/config.m4 @@ -208,7 +208,6 @@ if test "$PHP_DDTRACE" != "no"; then ext/remote_config.c \ ext/serializer.c \ ext/sidecar.c \ - ext/sidecar_shm.c \ ext/signals.c \ ext/span.c \ ext/startup_logging.c \ diff --git a/config.w32 b/config.w32 index fa28dc7c18e..bb74057a216 100644 --- a/config.w32 +++ b/config.w32 @@ -54,7 +54,6 @@ if (PHP_DDTRACE != 'no') { DDTRACE_EXT_SOURCES += " remote_config.c"; DDTRACE_EXT_SOURCES += " serializer.c"; DDTRACE_EXT_SOURCES += " sidecar.c"; - DDTRACE_EXT_SOURCES += " sidecar_shm.c"; DDTRACE_EXT_SOURCES += " span.c"; DDTRACE_EXT_SOURCES += " startup_logging.c"; DDTRACE_EXT_SOURCES += " telemetry.c"; diff --git a/ext/ddtrace.c b/ext/ddtrace.c index a93e15dce22..86957292f6e 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -433,6 +433,7 @@ static void dd_activate_once(void) { // must run before the first zai_hook_activate as ddtrace_telemetry_setup installs a global hook if (!ddtrace_disable) { +#ifndef _WIN32 // Only disable sidecar sender when explicitly disabled bool bgs_fallback = DD_SIDECAR_TRACE_SENDER_DEFAULT && get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_SIDECAR_TRACE_SENDER].name_index == ZAI_CONFIG_ORIGIN_DEFAULT && !get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(); zend_string *bgs_service = NULL; @@ -447,6 +448,7 @@ static void dd_activate_once(void) { bgs_service = ddtrace_default_service_name(); } } +#endif // If we're to enable appsec, we need to enable sidecar bool appsec_activation = false; @@ -1548,8 +1550,6 @@ static PHP_MINIT_FUNCTION(ddtrace) { ddtrace_signals_minit(); #endif - // Store master PID for threaded sidecar connectivity - // The actual sidecar setup happens in first RINIT via pthread_once ddtrace_sidecar_minit(false, false); return SUCCESS; @@ -2641,33 +2641,26 @@ void dd_internal_handle_fork(void) { ddtrace_generate_runtime_id(); #ifndef _WIN32 - // Handle sidecar after fork with threaded connectivity if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) { if (is_child_process) { - // Child process connects to master's sidecar as a worker ddtrace_force_new_instance_id(); - // Connect to master as worker using threaded connectivity if (ddtrace_sidecar_connect_worker_after_fork()) { LOG(DEBUG, "Child process connected to master sidecar as worker (child_pid=%d, master_pid=%d)", current_pid, ddtrace_master_pid); - // Submit root span data with the new connection if (ddtrace_sidecar) { ddtrace_sidecar_submit_root_span_data(); } } else { LOG(WARN, "Failed to connect child process as worker to master sidecar (child_pid=%d, master_pid=%d)", current_pid, ddtrace_master_pid); - // Fallback: reset sidecar on failure ddtrace_reset_sidecar(); } } - // Parent process: keep existing connection, no action needed } else #endif { - // Not using sidecar - normal reset for non-sidecar trace sender ddtrace_reset_sidecar(); } if (!get_DD_TRACE_FORKED_PROCESS()) { diff --git a/ext/sidecar.c b/ext/sidecar.c index a998a30f579..6f62525717a 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -153,7 +153,6 @@ static void dd_sidecar_on_reconnect(ddog_SidecarTransport *transport) { } -// Forward declaration for reconnection handler static ddog_SidecarTransport *dd_sidecar_connection_factory(void); static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { @@ -176,7 +175,6 @@ static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { } ddog_SidecarTransport *sidecar_transport; - // With threaded connectivity, use connect_worker to connect to master's listener if (!ddtrace_ffi_try("Failed connecting to sidecar as worker", ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { dd_free_endpoints(); @@ -185,7 +183,6 @@ static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { dd_sidecar_post_connect(&sidecar_transport, is_fork, logpath); - // Set up automatic reconnection handler ddtrace_sidecar_reconnect(&sidecar_transport, dd_sidecar_connection_factory); return sidecar_transport; @@ -219,52 +216,26 @@ bool ddtrace_sidecar_maybe_enable_appsec(bool *appsec_activation, bool *appsec_c #endif } -// Called in MINIT - stores master PID for threaded connectivity void ddtrace_sidecar_minit(bool appsec_activation, bool appsec_config) { UNUSED(appsec_activation, appsec_config); - // Store master PID for threaded connectivity only if not already set - // This ensures that after fork, child processes keep the parent's PID if (ddtrace_master_pid == 0) { #ifdef _WIN32 ddtrace_master_pid = _getpid(); #else ddtrace_master_pid = getpid(); #endif - LOG(DEBUG, "Sidecar MINIT: Stored master PID=%d", ddtrace_master_pid); - } else { - LOG(DEBUG, "Sidecar MINIT: Master PID already set to %d, current PID=%d", - ddtrace_master_pid, -#ifdef _WIN32 - _getpid() -#else - getpid() -#endif - ); } } -// Setup master sidecar connection - called once on first RINIT static void ddtrace_sidecar_setup_master(bool appsec_activation, bool appsec_config) { - // Check if we're in a forked child process - // pthread_once state is not inherited across fork, so child processes - // might try to call this function. They should NOT start a new master listener. #ifndef _WIN32 ddtrace_pid_t current_pid = getpid(); - - LOG(DEBUG, "ddtrace_sidecar_setup_master called: current_pid=%d, ddtrace_master_pid=%d", - current_pid, ddtrace_master_pid); - bool is_child_process = (ddtrace_master_pid != 0 && current_pid != ddtrace_master_pid); if (is_child_process) { - LOG(DEBUG, "Skipping master sidecar setup in child process (child_pid=%d, master_pid=%d)", - current_pid, ddtrace_master_pid); - // Child processes should only connect as workers, which is handled in dd_internal_handle_fork() return; } - - LOG(DEBUG, "Proceeding with master sidecar setup for PID=%d", current_pid); #endif ddtrace_set_non_resettable_sidecar_globals(); @@ -272,7 +243,6 @@ static void ddtrace_sidecar_setup_master(bool appsec_activation, bool appsec_con ddog_init_remote_config(get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), appsec_activation, appsec_config); - // Start master listener for worker processes/threads to connect to if (!ddtrace_ffi_try("Failed starting sidecar master listener", ddog_sidecar_connect_master((int32_t)ddtrace_master_pid))) { LOG(WARN, "Failed to start sidecar master listener"); if (ddtrace_endpoint) { @@ -281,7 +251,6 @@ static void ddtrace_sidecar_setup_master(bool appsec_activation, bool appsec_con return; } - // Master process also needs a transport - connect as first worker ddog_SidecarTransport *sidecar_transport = NULL; if (!ddtrace_ffi_try("Failed connecting master to sidecar", ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { LOG(WARN, "Failed to connect master process to sidecar"); @@ -303,11 +272,8 @@ static void ddtrace_sidecar_setup_master(bool appsec_activation, bool appsec_con if (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED()) { ddtrace_telemetry_first_init(); } - - LOG(DEBUG, "Sidecar master setup complete, PID=%d", ddtrace_master_pid); } -// Called in RINIT - handles git metadata and root span data void ddtrace_sidecar_rinit(void) { if (get_DD_TRACE_GIT_METADATA_ENABLED()) { zval git_object; @@ -330,20 +296,15 @@ void ddtrace_sidecar_rinit(void) { ddtrace_sidecar_submit_root_span_data_direct_defaults(&ddtrace_sidecar, NULL); } -// Setup function called via pthread_once on first RINIT void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) { - // This is called on first RINIT when config is available - // Set up the master sidecar connection with threaded connectivity ddtrace_sidecar_setup_master(appsec_activation, appsec_config); } -// Connect child process to master sidecar as worker after fork bool ddtrace_sidecar_connect_worker_after_fork(void) { if (!ddtrace_endpoint || ddtrace_master_pid == 0) { return false; } - // Connect to master as worker using threaded connectivity ddog_SidecarTransport *sidecar_transport = NULL; if (!ddtrace_ffi_try("Failed connecting worker to sidecar after fork", ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { @@ -356,10 +317,8 @@ bool ddtrace_sidecar_connect_worker_after_fork(void) { *logpath = 0; } - // Post-connect with fork flag set to true dd_sidecar_post_connect(&sidecar_transport, true, logpath); - // Drop old transport and use the new worker connection if (ddtrace_sidecar) { ddog_sidecar_transport_drop(ddtrace_sidecar); } @@ -395,31 +354,31 @@ void ddtrace_sidecar_finalize(bool clear_id) { } void ddtrace_sidecar_shutdown(void) { - // Shutdown master listener thread if this is the master process + // Shutdown master listener thread FIRST if this is the master process + // This must happen before dropping the sidecar transport to ensure clean shutdown #ifndef _WIN32 ddtrace_pid_t current_pid = getpid(); if (ddtrace_master_pid != 0 && current_pid == ddtrace_master_pid) { - LOG(DEBUG, "Shutting down master listener thread (PID=%d)", current_pid); ddtrace_ffi_try("Failed shutting down master listener", ddog_sidecar_shutdown_master_listener()); } #else if (ddtrace_master_pid != 0 && _getpid() == ddtrace_master_pid) { - LOG(DEBUG, "Shutting down master listener thread (PID=%d)", _getpid()); ddtrace_ffi_try("Failed shutting down master listener", ddog_sidecar_shutdown_master_listener()); } #endif - if (ddtrace_sidecar_instance_id) { - ddog_sidecar_instanceId_drop(ddtrace_sidecar_instance_id); + if (ddtrace_sidecar) { + ddog_sidecar_transport_drop(ddtrace_sidecar); } if (ddtrace_endpoint) { dd_free_endpoints(); } - if (ddtrace_sidecar) { - ddog_sidecar_transport_drop(ddtrace_sidecar); + if (ddtrace_sidecar_instance_id) { + ddog_sidecar_instanceId_drop(ddtrace_sidecar_instance_id); } + } void ddtrace_force_new_instance_id(void) { diff --git a/ext/sidecar.h b/ext/sidecar.h index 4e9a288cece..cf3df833c54 100644 --- a/ext/sidecar.h +++ b/ext/sidecar.h @@ -8,11 +8,9 @@ #include "zend_string.h" #ifndef _WIN32 #include -// Platform-specific PID type typedef pid_t ddtrace_pid_t; #else #include -// Windows uses int for process IDs typedef int ddtrace_pid_t; #endif diff --git a/ext/sidecar_shm.c b/ext/sidecar_shm.c deleted file mode 100644 index 9a5d70cb348..00000000000 --- a/ext/sidecar_shm.c +++ /dev/null @@ -1,207 +0,0 @@ -#include "sidecar_shm.h" -#include "ddtrace.h" -#include "sidecar.h" -#include "threads.h" -#include -#include - -ZEND_EXTERN_MODULE_GLOBALS(ddtrace); - -// Global registry for shared memory handles -static ddtrace_shm_registry_t *shm_registry = NULL; - -// Helper function to generate key from (service, env) -static zend_string *ddtrace_shm_make_key(const char *service, const char *env) { - if (!env || !*env) { - env = "none"; - } - return zend_strpprintf(0, "%s:%s", service, env); -} - -// Initialize the SHM registry (called during MINIT) -void ddtrace_shm_registry_init(void) { - if (shm_registry) { - return; // Already initialized - } - - shm_registry = (ddtrace_shm_registry_t *)malloc(sizeof(ddtrace_shm_registry_t)); - if (!shm_registry) { - LOG(ERROR, "Failed to allocate memory for SHM registry"); - return; - } - - shm_registry->shm_map = (HashTable *)malloc(sizeof(HashTable)); - zend_hash_init(shm_registry->shm_map, 8, NULL, NULL, 1); - shm_registry->mutex = tsrm_mutex_alloc(); - - LOG(DEBUG, "SHM registry initialized"); -} - -// Destroy the SHM registry (called during MSHUTDOWN) -void ddtrace_shm_registry_destroy(void) { - if (!shm_registry) { - return; - } - - if (shm_registry->mutex) { - tsrm_mutex_lock(shm_registry->mutex); - } - - // Free all SHM entries - if (shm_registry->shm_map) { - ddtrace_shm_entry_t *entry; - ZEND_HASH_FOREACH_PTR(shm_registry->shm_map, entry) { - if (entry) { - if (entry->service) { - free(entry->service); - } - if (entry->env) { - free(entry->env); - } - if (entry->handle) { - ddog_drop_anon_shm_handle(entry->handle); - } - free(entry); - } - } ZEND_HASH_FOREACH_END(); - - zend_hash_destroy(shm_registry->shm_map); - free(shm_registry->shm_map); - } - - if (shm_registry->mutex) { - tsrm_mutex_unlock(shm_registry->mutex); - tsrm_mutex_free(shm_registry->mutex); - } - - free(shm_registry); - shm_registry = NULL; - - LOG(DEBUG, "SHM registry destroyed"); -} - -// Get or create anonymous SHM handle for a (service, env) tuple -ddog_ShmHandle *ddtrace_shm_get_or_create(const char *service, const char *env) { - if (!shm_registry || !service) { - return NULL; - } - - zend_string *key = ddtrace_shm_make_key(service, env); - ddog_ShmHandle *handle = NULL; - - tsrm_mutex_lock(shm_registry->mutex); - - // Check if entry already exists - ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); - if (entry) { - entry->ref_count++; - handle = entry->handle; - LOG(DEBUG, "SHM handle found for %s:%s (ref_count=%d)", service, env, entry->ref_count); - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); - return handle; - } - - // Create new entry - entry = (ddtrace_shm_entry_t *)malloc(sizeof(ddtrace_shm_entry_t)); - if (!entry) { - LOG(ERROR, "Failed to allocate memory for SHM entry"); - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); - return NULL; - } - - entry->service = strdup(service); - entry->env = env ? strdup(env) : strdup("none"); - entry->ref_count = 1; - - // Allocate anonymous shared memory - ddog_MaybeError maybe_error = ddog_alloc_anon_shm_handle(128 * 1024, &entry->handle); // 128KB default - if (maybe_error.tag == DDOG_OPTION_ERROR_SOME_ERROR) { - ddog_CharSlice error = ddog_Error_message(&maybe_error.some); - LOG(ERROR, "Failed to allocate anonymous SHM for %s:%s: %.*s", - service, env, (int)error.len, error.ptr); - ddog_MaybeError_drop(maybe_error); - - free(entry->service); - free(entry->env); - free(entry); - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); - return NULL; - } - - handle = entry->handle; - - // Add to hash table - zend_hash_add_new_ptr(shm_registry->shm_map, key, entry); - - LOG(DEBUG, "Created new SHM handle for %s:%s (ref_count=1)", service, env); - - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); - - return handle; -} - -// Increment reference count for a SHM handle -void ddtrace_shm_addref(const char *service, const char *env) { - if (!shm_registry || !service) { - return; - } - - zend_string *key = ddtrace_shm_make_key(service, env); - - tsrm_mutex_lock(shm_registry->mutex); - - ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); - if (entry) { - entry->ref_count++; - LOG(DEBUG, "SHM addref for %s:%s (ref_count=%d)", service, env, entry->ref_count); - } else { - LOG(WARN, "Attempted to addref non-existent SHM entry for %s:%s", service, env); - } - - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); -} - -// Decrement reference count and free if zero -void ddtrace_shm_release(const char *service, const char *env) { - if (!shm_registry || !service) { - return; - } - - zend_string *key = ddtrace_shm_make_key(service, env); - - tsrm_mutex_lock(shm_registry->mutex); - - ddtrace_shm_entry_t *entry = zend_hash_find_ptr(shm_registry->shm_map, key); - if (entry) { - entry->ref_count--; - LOG(DEBUG, "SHM release for %s:%s (ref_count=%d)", service, env, entry->ref_count); - - if (entry->ref_count <= 0) { - // Free the entry - LOG(DEBUG, "Freeing SHM handle for %s:%s", service, env); - - if (entry->handle) { - ddog_drop_anon_shm_handle(entry->handle); - } - if (entry->service) { - free(entry->service); - } - if (entry->env) { - free(entry->env); - } - - zend_hash_del(shm_registry->shm_map, key); - free(entry); - } - } else { - LOG(WARN, "Attempted to release non-existent SHM entry for %s:%s", service, env); - } - - tsrm_mutex_unlock(shm_registry->mutex); - zend_string_release(key); -} diff --git a/ext/sidecar_shm.h b/ext/sidecar_shm.h deleted file mode 100644 index 794258b5d45..00000000000 --- a/ext/sidecar_shm.h +++ /dev/null @@ -1,39 +0,0 @@ -#ifndef DD_SIDECAR_SHM_H -#define DD_SIDECAR_SHM_H - -#include -#include -#include -#include "threads.h" - -// Entry for tracking shared memory per (service, env) tuple -typedef struct { - char *service; - char *env; - ddog_ShmHandle *handle; - int ref_count; -} ddtrace_shm_entry_t; - -// Global registry for shared memory handles -typedef struct { - HashTable *shm_map; // Maps "service:env" -> ddtrace_shm_entry_t - MUTEX_T mutex; // Protects the hash table -} ddtrace_shm_registry_t; - -// Initialize the SHM registry (called during MINIT) -void ddtrace_shm_registry_init(void); - -// Destroy the SHM registry (called during MSHUTDOWN) -void ddtrace_shm_registry_destroy(void); - -// Get or create anonymous SHM handle for a (service, env) tuple -// Returns NULL on failure -ddog_ShmHandle *ddtrace_shm_get_or_create(const char *service, const char *env); - -// Increment reference count for a SHM handle -void ddtrace_shm_addref(const char *service, const char *env); - -// Decrement reference count and free if zero -void ddtrace_shm_release(const char *service, const char *env); - -#endif // DD_SIDECAR_SHM_H diff --git a/libdatadog b/libdatadog index 629bce09547..c2690e73166 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit 629bce09547abc77d7bbda623921f97eb5611949 +Subproject commit c2690e731665922fafd7e13575e56b7dd72d7be9 From 7efee66bb1635eb09c48e3898cdc89af27ded4bb Mon Sep 17 00:00:00 2001 From: Alexandre Rulleau Date: Fri, 14 Nov 2025 16:20:47 +0100 Subject: [PATCH 4/4] fix: thread shutdown leaks Signed-off-by: Alexandre Rulleau --- appsec/third_party/libddwaf | 2 +- components-rs/sidecar.h | 2 + dockerfiles/ci/bookworm/.env | 4 +- dockerfiles/ci/bookworm/php-8.3/suppr.txt | 2 + dockerfiles/ci/bookworm/php-8.4/suppr.txt | 2 + dockerfiles/ci/bookworm/php-8.5/suppr.txt | 2 + ext/ddtrace.c | 9 ++- ext/handlers_pcntl.c | 74 +++++++++++++++++++++++ ext/sidecar.c | 26 +++++++- ext/sidecar.h | 2 + libdatadog | 2 +- 11 files changed, 121 insertions(+), 6 deletions(-) diff --git a/appsec/third_party/libddwaf b/appsec/third_party/libddwaf index bea83f92723..00e895f2c50 160000 --- a/appsec/third_party/libddwaf +++ b/appsec/third_party/libddwaf @@ -1 +1 @@ -Subproject commit bea83f92723ef578b592148edae9cce9171d8ac0 +Subproject commit 00e895f2c507a714062aa88ed41466aec10d2e01 diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h index a7b75235000..5aea6f0fafe 100644 --- a/components-rs/sidecar.h +++ b/components-rs/sidecar.h @@ -95,6 +95,8 @@ ddog_MaybeError ddog_sidecar_connect_worker(int32_t master_pid, ddog_MaybeError ddog_sidecar_shutdown_master_listener(void); +ddog_MaybeError ddog_sidecar_clear_inherited_listener(void); + ddog_MaybeError ddog_sidecar_ping(struct ddog_SidecarTransport **transport); ddog_MaybeError ddog_sidecar_flush_traces(struct ddog_SidecarTransport **transport); diff --git a/dockerfiles/ci/bookworm/.env b/dockerfiles/ci/bookworm/.env index 1a92ca601b5..654839246c9 100644 --- a/dockerfiles/ci/bookworm/.env +++ b/dockerfiles/ci/bookworm/.env @@ -1,2 +1,2 @@ -BOOKWORM_CURRENT_VERSION=6 -BOOKWORM_NEXT_VERSION=7 +BOOKWORM_CURRENT_VERSION=5 +BOOKWORM_NEXT_VERSION=6 diff --git a/dockerfiles/ci/bookworm/php-8.3/suppr.txt b/dockerfiles/ci/bookworm/php-8.3/suppr.txt index 741c68ed250..dc215f490a6 100644 --- a/dockerfiles/ci/bookworm/php-8.3/suppr.txt +++ b/dockerfiles/ci/bookworm/php-8.3/suppr.txt @@ -5,3 +5,5 @@ leak:_dl_map_object_deps leak:__res_context_send leak:_dl_make_tlsdesc_dynamic leak:_dl_catch_exception +leak:__cxa_thread_atexit +leak:CURRENT_PARKER diff --git a/dockerfiles/ci/bookworm/php-8.4/suppr.txt b/dockerfiles/ci/bookworm/php-8.4/suppr.txt index 39c643abb53..8f09d3ddad7 100644 --- a/dockerfiles/ci/bookworm/php-8.4/suppr.txt +++ b/dockerfiles/ci/bookworm/php-8.4/suppr.txt @@ -1 +1,3 @@ leak:timer_create +leak:__cxa_thread_atexit +leak:CURRENT_PARKER diff --git a/dockerfiles/ci/bookworm/php-8.5/suppr.txt b/dockerfiles/ci/bookworm/php-8.5/suppr.txt index 39c643abb53..8f09d3ddad7 100644 --- a/dockerfiles/ci/bookworm/php-8.5/suppr.txt +++ b/dockerfiles/ci/bookworm/php-8.5/suppr.txt @@ -1 +1,3 @@ leak:timer_create +leak:__cxa_thread_atexit +leak:CURRENT_PARKER diff --git a/ext/ddtrace.c b/ext/ddtrace.c index 86957292f6e..987384f9065 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -1605,7 +1605,11 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) { ddtrace_user_req_shutdown(); - ddtrace_sidecar_shutdown(); + // Only shutdown sidecar in MSHUTDOWN for non-CLI SAPIs. + // CLI SAPI shuts down in RSHUTDOWN to allow thread joins before ASAN checks. + if (strcmp(sapi_module.name, "cli") != 0) { + ddtrace_sidecar_shutdown(); + } ddtrace_live_debugger_mshutdown(); @@ -2643,6 +2647,9 @@ void dd_internal_handle_fork(void) { #ifndef _WIN32 if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && ddtrace_sidecar) { if (is_child_process) { + // Clear inherited listener state - child doesn't own the master listener thread + ddtrace_ffi_try("Failed clearing inherited listener state", ddog_sidecar_clear_inherited_listener()); + ddtrace_force_new_instance_id(); if (ddtrace_sidecar_connect_worker_after_fork()) { diff --git a/ext/handlers_pcntl.c b/ext/handlers_pcntl.c index acff140cfe4..6353d58d156 100644 --- a/ext/handlers_pcntl.c +++ b/ext/handlers_pcntl.c @@ -27,18 +27,92 @@ static zif_handler dd_pcntl_forkx_handler = NULL; #define JOIN_BGS_BEFORE_FORK 1 #endif +static bool dd_master_listener_was_active = false; + static void dd_prefork() { #if JOIN_BGS_BEFORE_FORK if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) { ddtrace_coms_flush_shutdown_writer_synchronous(); } #endif + +#ifndef _WIN32 + // Check if master listener is active before fork + dd_master_listener_was_active = (ddtrace_master_pid != 0 && getpid() == ddtrace_master_pid); + + if (dd_master_listener_was_active) { + // Shutdown master listener before fork to avoid Tokio runtime corruption. + // Tokio's async runtime and fork() are fundamentally incompatible - forking + // while Tokio threads are active causes state corruption in the parent process. + // See: https://github.com/tokio-rs/tokio/issues/4301 + + // First close parent's worker connection to prevent handler thread deadlock + if (ddtrace_sidecar) { + ddog_sidecar_transport_drop(ddtrace_sidecar); + ddtrace_sidecar = NULL; + } + + // Then shutdown the master listener and its Tokio runtime + ddog_sidecar_shutdown_master_listener(); + } +#endif } +static void dd_postfork_parent() { +#ifndef _WIN32 + // Restart master listener in parent if it was active before fork + if (dd_master_listener_was_active) { + // Reinitialize master listener with fresh Tokio runtime. + // This recreates the async runtime that was shut down before fork. + + // First, restart the master listener thread + if (!ddtrace_ffi_try("Failed restarting sidecar master listener after fork", + ddog_sidecar_connect_master((int32_t)ddtrace_master_pid))) { + LOG(WARN, "Failed to restart sidecar master listener after fork"); + dd_master_listener_was_active = false; + return; + } + + // Then reconnect to it as a worker + ddog_SidecarTransport *sidecar_transport = NULL; + if (!ddtrace_ffi_try("Failed reconnecting master to sidecar after fork", + ddog_sidecar_connect_worker((int32_t)ddtrace_master_pid, &sidecar_transport))) { + LOG(WARN, "Failed to reconnect master process to sidecar after fork"); + dd_master_listener_was_active = false; + return; + } + + ddtrace_sidecar = sidecar_transport; + dd_master_listener_was_active = false; + } +#endif +} + +// Declare LSAN runtime interface functions +#if defined(__SANITIZE_ADDRESS__) && !defined(_WIN32) +void __lsan_disable(void); +void __lsan_enable(void); +#endif + static void dd_handle_fork(zval *return_value) { if (Z_LVAL_P(return_value) == 0) { + // CHILD PROCESS + // Disable ASAN leak detection in child to avoid false positives from inherited + // Tokio TLS and runtime state that can't be properly cleaned up after fork +#if defined(__SANITIZE_ADDRESS__) && !defined(_WIN32) + // The child inherits Tokio runtime thread-local storage from the parent's + // master listener and connection handlers. These TLS destructors reference + // threads that don't exist in the child, causing spurious leak reports. + // We disable leak detection in the child since the inherited memory will + // be reclaimed when the child process exits. + __lsan_disable(); +#endif dd_internal_handle_fork(); } else { + // PARENT PROCESS + // Restart master listener if it was shut down before fork + dd_postfork_parent(); + #if JOIN_BGS_BEFORE_FORK if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) { ddtrace_coms_restart_writer(); diff --git a/ext/sidecar.c b/ext/sidecar.c index 6f62525717a..7ad27863e84 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -9,6 +9,7 @@ #include #include #include +#include
#include "sidecar.h" #include "live_debugger.h" #include "telemetry.h" @@ -153,7 +154,8 @@ static void dd_sidecar_on_reconnect(ddog_SidecarTransport *transport) { } -static ddog_SidecarTransport *dd_sidecar_connection_factory(void); +// Forward declaration (non-static for export to handlers_pcntl.c) +ddog_SidecarTransport *dd_sidecar_connection_factory(void); static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) { // Should not happen, unless the agent url is malformed @@ -672,6 +674,28 @@ void ddtrace_sidecar_activate(void) { void ddtrace_sidecar_rshutdown(void) { ddog_Vec_Tag_drop(DDTRACE_G(active_global_tags)); + + // For CLI SAPI, shut down the master listener thread here in RSHUTDOWN + // since the process will exit after this request. For other SAPIs, + // the master listener persists across requests and shuts down in MSHUTDOWN. + if (strcmp(sapi_module.name, "cli") == 0) { +#ifndef _WIN32 + ddtrace_pid_t current_pid = getpid(); + if (ddtrace_master_pid != 0 && current_pid == ddtrace_master_pid) { + // CRITICAL: Close the worker connection BEFORE shutting down the master listener. + // The master process has its own worker connection to the master listener. + // If we don't close it first, the handler thread waiting on this connection + // will never exit, causing the listener thread join to timeout. + if (ddtrace_sidecar) { + ddog_sidecar_transport_drop(ddtrace_sidecar); + ddtrace_sidecar = NULL; + } + + ddtrace_ffi_try("Failed shutting down master listener in RSHUTDOWN", + ddog_sidecar_shutdown_master_listener()); + } +#endif + } } bool ddtrace_alter_test_session_token(zval *old_value, zval *new_value, zend_string *new_str) { diff --git a/ext/sidecar.h b/ext/sidecar.h index cf3df833c54..6c82f3081a3 100644 --- a/ext/sidecar.h +++ b/ext/sidecar.h @@ -44,6 +44,8 @@ ddog_Endpoint *ddtrace_sidecar_agent_endpoint(void); void ddtrace_sidecar_submit_root_span_data_direct_defaults(ddog_SidecarTransport **transport, ddtrace_root_span_data *root); void ddtrace_sidecar_submit_root_span_data_direct(ddog_SidecarTransport **transport, ddtrace_root_span_data *root, zend_string *cfg_service, zend_string *cfg_env, zend_string *cfg_version); +ddog_SidecarTransport *dd_sidecar_connection_factory(void); + void ddtrace_sidecar_send_debugger_data(ddog_Vec_DebuggerPayload payloads); void ddtrace_sidecar_send_debugger_datum(ddog_DebuggerPayload *payload); diff --git a/libdatadog b/libdatadog index c2690e73166..a0ffd63bbcb 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit c2690e731665922fafd7e13575e56b7dd72d7be9 +Subproject commit a0ffd63bbcbee90b7afaf545ecc16ed895427a55