diff --git a/tests/ext/pcntl/otel_process_context_fork.phpt b/tests/ext/pcntl/otel_process_context_fork.phpt index 2de938987a..1a3266aa8d 100644 --- a/tests/ext/pcntl/otel_process_context_fork.phpt +++ b/tests/ext/pcntl/otel_process_context_fork.phpt @@ -16,6 +16,8 @@ DD_TRACE_GENERATE_ROOT_SPAN=0 --FILE-- meta['runtime-id'] === $parentRuntimeId ); +$parentThreadId = $threadContext->attributes()[4] ?? null; +echo "Parent thread ID matches process ID: "; var_dump( + $parentThreadId === (string) getmypid() +); DDTrace\close_span(); $pid = pcntl_fork(); @@ -106,6 +113,11 @@ if ($pid === 0) { echo "Child mapping was republished: "; var_dump( $childRuntimeId !== $parentRuntimeId ); + $childThreadId = $threadContext->attributes()[4] ?? null; + echo "Child thread ID was refreshed: "; var_dump( + $childThreadId === (string) getmypid() + && $childThreadId !== $parentThreadId + ); DDTrace\close_span(); exit; } @@ -121,8 +133,10 @@ Parent has one mapping: bool(true) Parent mapping is MADV_DONTFORK: bool(true) Parent has runtime ID: bool(true) Parent runtime ID matches tracer: bool(true) +Parent thread ID matches process ID: bool(true) Child has one mapping: bool(true) Child mapping is MADV_DONTFORK: bool(true) Child runtime ID matches tracer: bool(true) Child mapping was republished: bool(true) +Child thread ID was refreshed: bool(true) Child exited successfully: bool(true) diff --git a/tracer/ddtrace.c b/tracer/ddtrace.c index f48dd8af8f..df9b73ef12 100644 --- a/tracer/ddtrace.c +++ b/tracer/ddtrace.c @@ -686,7 +686,7 @@ bool ddtrace_update_remote_config_flags(ddog_RemoteConfigFlags *flags) { #define JOIN_BGS_BEFORE_FORK 1 #endif -void ddtrace_internal_handle_prefork() { +void ddtrace_internal_handle_prefork(void) { #if JOIN_BGS_BEFORE_FORK if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) { ddtrace_coms_flush_shutdown_writer_synchronous(); @@ -694,7 +694,7 @@ void ddtrace_internal_handle_prefork() { #endif } -void ddtrace_internal_handle_postfork() { +void ddtrace_internal_handle_postfork(void) { #if JOIN_BGS_BEFORE_FORK if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) { ddtrace_coms_restart_writer(); @@ -705,6 +705,7 @@ void ddtrace_internal_handle_postfork() { void ddtrace_internal_handle_fork() { #ifdef __linux__ ddtrace_otel_detach(); + ddtrace_otel_tid_fork_handler(); #endif if (DATADOG_G(sidecar)) { // Unconditionally send, even if root span is NULL diff --git a/tracer/otel_context.c b/tracer/otel_context.c index 2276ef60d9..258d252b29 100644 --- a/tracer/otel_context.c +++ b/tracer/otel_context.c @@ -37,6 +37,15 @@ _Static_assert(offsetof(ddtrace_root_span_data, otel_context) % 8 == 0, "unexpec _Static_assert((offsetof(ddtrace_root_span_data, otel_context) + offsetof(datadog_otel_thr_ctx_rec, span_id)) % 8 == 0, "unexpected OTel thread context span_id placement"); +typedef struct { + // The length of the used bytes in .value, excluding null. + uint8_t len; + // On Linux tids are pid_t, which are `int`, so the most negative value is + // the longest when printed (because of sign bit). + char value[sizeof("-2147483648")]; +} ddtrace_otel_cached_tid; + +static __thread ddtrace_otel_cached_tid otel_tid = {0}; __thread void *otel_thread_ctx_v1 __attribute__((visibility("default"), tls_model("global-dynamic"))); static void ddtrace_otel_record_begin_update(datadog_otel_thr_ctx_rec *record); @@ -203,10 +212,20 @@ static void ddtrace_otel_record_set_attrs(datadog_otel_thr_ctx_rec *record, ddtr offset = ddtrace_otel_record_write_attr_zstr(record, offset, DDTRACE_OTEL_ATTR_DEPLOYMENT_ENVIRONMENT_NAME, ddtrace_otel_attr_zstr(env)); offset = ddtrace_otel_record_write_attr_zstr(record, offset, DDTRACE_OTEL_ATTR_SERVICE_VERSION, ddtrace_otel_attr_zstr(version)); - char thread_id[32]; - int thread_id_len = snprintf(thread_id, sizeof(thread_id), "%llu", (unsigned long long)syscall(SYS_gettid)); - if (thread_id_len > 0) { - offset = ddtrace_otel_record_write_attr(record, offset, DDTRACE_OTEL_ATTR_THREAD_ID, thread_id, (size_t)thread_id_len); + if (UNEXPECTED(otel_tid.len == 0)) { // should only happen 1x per thread + long tid = syscall(SYS_gettid); + if (EXPECTED(tid > 0)) { // this syscall is fundamental and shouldn't fail + // tids are just pid_t but there's no safety in casting down to int + // here, if the buffer is big enough, then we're good. + int tid_len = snprintf(otel_tid.value, sizeof otel_tid.value, "%ld", tid); + // Should fit, tids are `int` on Linux. + if (EXPECTED(tid_len > 0 && (size_t)tid_len < sizeof otel_tid.value)) { + otel_tid.len = (uint8_t)tid_len; + } + } + } + if (EXPECTED(otel_tid.len != 0)) { + offset = ddtrace_otel_record_write_attr(record, offset, DDTRACE_OTEL_ATTR_THREAD_ID, otel_tid.value, (size_t)otel_tid.len); } record->attrs_data_size = (uint16_t)offset; @@ -241,3 +260,7 @@ static size_t ddtrace_otel_record_write_attr(datadog_otel_thr_ctx_rec *record, s } static zend_string *ddtrace_otel_attr_zstr(zend_string *value) { return value ? value : ZSTR_EMPTY_ALLOC(); } + +void ddtrace_otel_tid_fork_handler(void) { + otel_tid.len = 0; +} diff --git a/tracer/otel_context.h b/tracer/otel_context.h index 4bf503d36a..c7162b2683 100644 --- a/tracer/otel_context.h +++ b/tracer/otel_context.h @@ -63,6 +63,9 @@ void ddtrace_otel_attach_stack(ddtrace_span_stack *stack); /** Detach the current thread's record. */ void ddtrace_otel_detach(void); +/** Reset the tid after a fork. */ +void ddtrace_otel_tid_fork_handler(void); + END_EXTERN_C() #endif // DDTRACE_OTEL_CONTEXT_H