diff --git a/Cargo.toml b/Cargo.toml index 56d3d4aa..6dc8dbf2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ cc = "1.0" cf-rustracing = "1.2.1" cf-rustracing-jaeger = "1.2.2" clap = "4.4" +crossbeam-utils = { version = "0.8", default-features = false } darling = "0.21" erased-serde = "0.4" futures-util = "0.3" diff --git a/foundations-macros/src/metrics/mod.rs b/foundations-macros/src/metrics/mod.rs index 9e414cee..ee8f9648 100644 --- a/foundations-macros/src/metrics/mod.rs +++ b/foundations-macros/src/metrics/mod.rs @@ -103,8 +103,6 @@ fn expand_from_parsed(args: MacroArgs, extern_: Mod) -> proc_macro2::TokenStream fns, } = extern_; - let reexports = quote! { #foundations::reexports_for_macros }; - // This should be using `Span::def_site` but it is currently unstable. let metrics_struct = Ident::new(&format!("__{mod_name}_Metrics"), Span::call_site()); @@ -152,8 +150,8 @@ fn expand_from_parsed(args: MacroArgs, extern_: Mod) -> proc_macro2::TokenStream #(#label_set_structs)* #[allow(non_upper_case_globals)] - static #metrics_struct: #reexports::once_cell::sync::Lazy<#metrics_struct> = - #reexports::once_cell::sync::Lazy::new(|| { + static #metrics_struct: ::std::sync::LazyLock<#metrics_struct> = + ::std::sync::LazyLock::new(|| { #init_registry #init_opt_registry @@ -424,8 +422,8 @@ mod tests { struct __empty_Metrics {} #[allow(non_upper_case_globals)] - static __empty_Metrics: ::foundations::reexports_for_macros::once_cell::sync::Lazy<__empty_Metrics> = - ::foundations::reexports_for_macros::once_cell::sync::Lazy::new(|| { __empty_Metrics {} }); + static __empty_Metrics: ::std::sync::LazyLock<__empty_Metrics> = + ::std::sync::LazyLock::new(|| { __empty_Metrics {} }); } }; @@ -459,8 +457,8 @@ mod tests { } #[allow(non_upper_case_globals)] - static __oxy_Metrics: tarmac::reexports_for_macros::once_cell::sync::Lazy<__oxy_Metrics> = - tarmac::reexports_for_macros::once_cell::sync::Lazy::new(|| { + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { let registry = &mut *tarmac::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), false, true); __oxy_Metrics { @@ -516,8 +514,8 @@ mod tests { } #[allow(non_upper_case_globals)] - static __oxy_Metrics: ::foundations::reexports_for_macros::once_cell::sync::Lazy<__oxy_Metrics> = - ::foundations::reexports_for_macros::once_cell::sync::Lazy::new(|| { + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { let opt_registry = &mut *::foundations::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), true, true); __oxy_Metrics { @@ -577,8 +575,8 @@ mod tests { } #[allow(non_upper_case_globals)] - static __oxy_Metrics: ::foundations::reexports_for_macros::once_cell::sync::Lazy<__oxy_Metrics> = - ::foundations::reexports_for_macros::once_cell::sync::Lazy::new(|| { + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { let registry = &mut *::foundations::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), false, false); let opt_registry = &mut *::foundations::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), true, false); @@ -682,8 +680,8 @@ mod tests { } #[allow(non_upper_case_globals)] - static __oxy_Metrics: ::foundations::reexports_for_macros::once_cell::sync::Lazy<__oxy_Metrics> = - ::foundations::reexports_for_macros::once_cell::sync::Lazy::new(|| { + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { let registry = &mut *::foundations::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), false, true); __oxy_Metrics { @@ -777,8 +775,8 @@ mod tests { } #[allow(non_upper_case_globals)] - static __oxy_Metrics: ::foundations::reexports_for_macros::once_cell::sync::Lazy<__oxy_Metrics> = - ::foundations::reexports_for_macros::once_cell::sync::Lazy::new(|| { + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { let registry = &mut *::foundations::telemetry::metrics::internal::Registries::get_subsystem(stringify!(oxy), false, true); __oxy_Metrics { diff --git a/foundations/Cargo.toml b/foundations/Cargo.toml index 4f44dbb7..08d0ea5b 100644 --- a/foundations/Cargo.toml +++ b/foundations/Cargo.toml @@ -102,7 +102,6 @@ tokio-runtime-metrics = [ # Enables logging functionality. logging = [ "dep:governor", - "dep:once_cell", "dep:parking_lot", "dep:slog-async", "dep:slog-json", @@ -111,13 +110,13 @@ logging = [ "dep:thread_local", "dep:futures-util", "dep:serde", + "dep:crossbeam-utils", ] # Enables distributed tracing functionality. tracing = [ "dep:foundations-macros", "dep:governor", - "dep:once_cell", "dep:parking_lot", "dep:rand", "dep:cf-rustracing-jaeger", @@ -130,13 +129,13 @@ tracing = [ "dep:slab", "dep:libc", "dep:http", + "dep:crossbeam-utils", ] # Enables metrics functionality. metrics = [ "dep:foundations-macros", "dep:erased-serde", - "dep:once_cell", "dep:parking_lot", "dep:prometheus-client", "dep:prometheus", @@ -188,6 +187,7 @@ foundations-macros = { workspace = true, optional = true, default-features = fal cf-rustracing = { workspace = true, optional = true } cf-rustracing-jaeger = { workspace = true, optional = true } clap = { workspace = true, optional = true } +crossbeam-utils = { workspace = true, optional = true } erased-serde = { workspace = true, optional = true } futures-util = { workspace = true, optional = true } governor = { workspace = true, optional = true } diff --git a/foundations/src/lib.rs b/foundations/src/lib.rs index cafc25fe..391a6340 100644 --- a/foundations/src/lib.rs +++ b/foundations/src/lib.rs @@ -100,7 +100,7 @@ pub mod security; pub mod reexports_for_macros { #[cfg(feature = "tracing")] pub use cf_rustracing; - #[cfg(any(feature = "metrics", feature = "security"))] + #[cfg(feature = "security")] pub use once_cell; #[cfg(feature = "metrics")] pub use parking_lot; diff --git a/foundations/src/telemetry/log/init.rs b/foundations/src/telemetry/log/init.rs index b75f3961..ab0de0f6 100644 --- a/foundations/src/telemetry/log/init.rs +++ b/foundations/src/telemetry/log/init.rs @@ -11,7 +11,7 @@ use crate::telemetry::log::retry_writer::RetryPipeWriter; use crate::telemetry::scope::ScopeStack; use crate::telemetry::settings::{LogFormat, LogOutput, LoggingSettings}; use crate::{BootstrapResult, ServiceInfo}; -use once_cell::sync::{Lazy, OnceCell}; +use crossbeam_utils::CachePadded; use slog::{ Discard, Drain, FnValue, Fuse, LevelFilter, Logger, Never, OwnedKV, SendSyncRefUnwindSafeDrain, SendSyncRefUnwindSafeKV, SendSyncUnwindSafeDrain, @@ -23,15 +23,17 @@ use std::fs::File; use std::io; use std::io::BufWriter; use std::panic::RefUnwindSafe; -use std::sync::Arc; +use std::sync::{Arc, LazyLock, OnceLock}; type FilteredDrain = LevelFilter< FieldFilteringDrain>, >; -static HARNESS: OnceCell = OnceCell::new(); +// These singletons are accessed _very often_, and each access requires an atomic load to +// ensure initialization. Make sure nobody else invalidates our cache lines. +static HARNESS: CachePadded> = CachePadded::new(OnceLock::new()); -static NOOP_HARNESS: Lazy = Lazy::new(|| { +static NOOP_HARNESS: CachePadded> = CachePadded::new(LazyLock::new(|| { let root_drain = Arc::new(Discard); let noop_log = LoggerWithKvNestingTracking::new(Logger::root(Arc::clone(&root_drain), slog::o!())); @@ -42,7 +44,7 @@ static NOOP_HARNESS: Lazy = Lazy::new(|| { settings: Default::default(), log_scope_stack: Default::default(), } -}); +})); pub(crate) struct LogHarness { pub(crate) root_log: SharedLog, @@ -53,7 +55,7 @@ pub(crate) struct LogHarness { impl LogHarness { pub(crate) fn get() -> &'static Self { - HARNESS.get().unwrap_or(&NOOP_HARNESS) + HARNESS.get().unwrap_or_else(|| &**NOOP_HARNESS) } #[cfg(any(test, feature = "testing"))] diff --git a/foundations/src/telemetry/memory_profiler.rs b/foundations/src/telemetry/memory_profiler.rs index 0130a2ad..b085166d 100644 --- a/foundations/src/telemetry/memory_profiler.rs +++ b/foundations/src/telemetry/memory_profiler.rs @@ -7,13 +7,15 @@ use std::fs::File; use std::io::Read; use std::os::raw::c_char; use std::sync::mpsc::{self}; +use std::sync::OnceLock; use tempfile::NamedTempFile; use tokio::sync::{oneshot, Mutex as AsyncMutex}; +// TODO(once_cell_try): replace with `std::sync::OnceLock` static PROFILER: OnceCell> = OnceCell::new(); -static HEAP_PROFILE_REQUEST_SENDER: OnceCell< +static HEAP_PROFILE_REQUEST_SENDER: OnceLock< AsyncMutex>>>, -> = OnceCell::new(); +> = OnceLock::new(); mod control { use super::*; diff --git a/foundations/src/telemetry/metrics/internal.rs b/foundations/src/telemetry/metrics/internal.rs index b0263d3a..43289ad0 100644 --- a/foundations/src/telemetry/metrics/internal.rs +++ b/foundations/src/telemetry/metrics/internal.rs @@ -1,7 +1,6 @@ use super::{info_metric, ExtraProducer, InfoMetric}; use crate::telemetry::settings::{MetricsSettings, ServiceNameFormat}; use crate::{Result, ServiceInfo}; -use once_cell::sync::OnceCell; use prometheus_client::encoding::text::{encode, EncodeMetric}; use prometheus_client::registry::Registry; use prometools::serde::InfoGauge; @@ -9,8 +8,9 @@ use std::any::TypeId; use std::borrow::Cow; use std::collections::HashMap; use std::ops::DerefMut; +use std::sync::OnceLock; -static REGISTRIES: OnceCell = OnceCell::new(); +static REGISTRIES: OnceLock = OnceLock::new(); enum MetricsServiceName { Prefix(String), diff --git a/foundations/src/telemetry/tracing/init.rs b/foundations/src/telemetry/tracing/init.rs index ab77ca28..89155999 100644 --- a/foundations/src/telemetry/tracing/init.rs +++ b/foundations/src/telemetry/tracing/init.rs @@ -3,35 +3,38 @@ use super::output_jaeger_thrift_udp; use crate::telemetry::scope::ScopeStack; use crate::telemetry::settings::{SamplingStrategy, TracesOutput, TracingSettings}; use crate::telemetry::tracing::live::ActiveRoots; +use crate::telemetry::tracing::rate_limit::RateLimitingProbabilisticSampler; use crate::{BootstrapResult, ServiceInfo}; +use cf_rustracing::sampler::{NullSampler, PassiveSampler, Sampler}; use cf_rustracing_jaeger::span::SpanReceiver; +use crossbeam_utils::CachePadded; use futures_util::future::BoxFuture; -use once_cell::sync::{Lazy, OnceCell}; +use std::sync::{LazyLock, OnceLock}; #[cfg(feature = "telemetry-otlp-grpc")] use super::output_otlp_grpc; -use cf_rustracing::sampler::{PassiveSampler, Sampler}; #[cfg(feature = "testing")] use std::borrow::Cow; -use crate::telemetry::tracing::rate_limit::RateLimitingProbabilisticSampler; - -static HARNESS: OnceCell = OnceCell::new(); +// These singletons are accessed _very often_, and each access requires an atomic load to +// ensure initialization. Make sure nobody else invalidates our cache lines. +static HARNESS: CachePadded> = CachePadded::new(OnceLock::new()); -static NOOP_HARNESS: Lazy = Lazy::new(|| { - let (noop_tracer, _) = Tracer::new(RateLimitingProbabilisticSampler::default().boxed()); +static NOOP_HARNESS: CachePadded> = + CachePadded::new(LazyLock::new(|| { + let (noop_tracer, _) = Tracer::new(NullSampler.boxed()); - TracingHarness { - tracer: noop_tracer, - span_scope_stack: Default::default(), + TracingHarness { + tracer: noop_tracer, + span_scope_stack: Default::default(), - #[cfg(feature = "testing")] - test_tracer_scope_stack: Default::default(), + #[cfg(feature = "testing")] + test_tracer_scope_stack: Default::default(), - active_roots: Default::default(), - } -}); + active_roots: Default::default(), + } + })); pub(crate) struct TracingHarness { tracer: Tracer, @@ -46,7 +49,7 @@ pub(crate) struct TracingHarness { impl TracingHarness { pub(crate) fn get() -> &'static Self { - HARNESS.get().unwrap_or(&NOOP_HARNESS) + HARNESS.get().unwrap_or_else(|| &**NOOP_HARNESS) } #[cfg(feature = "testing")] diff --git a/foundations/src/telemetry/tracing/internal.rs b/foundations/src/telemetry/tracing/internal.rs index dd511376..5f971d8e 100644 --- a/foundations/src/telemetry/tracing/internal.rs +++ b/foundations/src/telemetry/tracing/internal.rs @@ -13,8 +13,6 @@ use std::sync::Arc; pub(crate) type Tracer = cf_rustracing::Tracer, SpanContextState>; -static INACTIVE_SPAN: RwLock = RwLock::new(Span::inactive()); - /// Shared span with mutability and additional reference tracking for /// ad-hoc inspection. #[derive(Clone, Debug)] @@ -29,19 +27,13 @@ impl SharedSpanHandle { TracingHarness::get().active_roots.track(span) } - pub(crate) fn read(&self) -> parking_lot::RwLockReadGuard<'_, Span> { - match self { - SharedSpanHandle::Tracked(handle) => handle.read(), - SharedSpanHandle::Untracked(rw_lock) => rw_lock.read(), - SharedSpanHandle::Inactive => INACTIVE_SPAN.read(), - } - } + pub(crate) fn with_read(&self, f: impl FnOnce(&Span) -> R) -> R { + static INACTIVE: Span = Span::inactive(); - pub(crate) fn write(&self) -> parking_lot::RwLockWriteGuard<'_, Span> { match self { - SharedSpanHandle::Tracked(handle) => handle.write(), - SharedSpanHandle::Untracked(rw_lock) => rw_lock.write(), - SharedSpanHandle::Inactive => INACTIVE_SPAN.write(), + SharedSpanHandle::Tracked(handle) => f(&handle.read()), + SharedSpanHandle::Untracked(rw_lock) => f(&rw_lock.read()), + SharedSpanHandle::Inactive => f(&INACTIVE), } } } @@ -51,6 +43,8 @@ impl From for Arc> { match value { SharedSpanHandle::Tracked(handle) => Arc::clone(&handle), SharedSpanHandle::Untracked(rw_lock) => rw_lock, + // This is only used in `rustracing_span()`, which should rarely + // need to be called. Allocating a fresh Arc every time is thus fine. SharedSpanHandle::Inactive => Arc::new(RwLock::new(Span::inactive())), } } @@ -78,16 +72,23 @@ impl From for SharedSpan { } pub fn write_current_span(write_fn: impl FnOnce(&mut Span)) { - if let Some(span) = current_span() { - if span.is_sampled { - write_fn(&mut span.inner.write()); - } - } + let span = match current_span() { + Some(span) if span.is_sampled => span, + _ => return, + }; + + let mut span_guard = match &span.inner { + SharedSpanHandle::Tracked(handle) => handle.write(), + SharedSpanHandle::Untracked(rw_lock) => rw_lock.write(), + SharedSpanHandle::Inactive => unreachable!("inactive spans can't be sampled"), + }; + + write_fn(&mut span_guard); } pub(crate) fn create_span(name: impl Into>) -> SharedSpan { match current_span() { - Some(parent) => parent.inner.read().child(name, |o| o.start()), + Some(parent) => parent.inner.with_read(|s| s.child(name, |o| o.start())), None => start_trace(name, Default::default()), } .into() @@ -150,8 +151,11 @@ fn link_new_trace_with_current( root_span_name: &str, new_trace_root_span: &mut Span, ) { - let current_span_lock = current_span.inner.read(); - let mut new_trace_ref_span = create_fork_ref_span(root_span_name, ¤t_span_lock); + let (mut new_trace_ref_span, current_trace_id) = current_span.inner.with_read(|s| { + let trace_id = span_trace_id(s); + let ref_span = create_fork_ref_span(root_span_name, s); + (ref_span, trace_id) + }); if let Some(trace_id) = span_trace_id(&*new_trace_root_span) { new_trace_ref_span.set_tag(|| { @@ -164,7 +168,7 @@ fn link_new_trace_with_current( new_trace_ref_span.set_tag(|| Tag::new("trace_id", trace_id)); } - if let Some(trace_id) = span_trace_id(¤t_span_lock) { + if let Some(trace_id) = current_trace_id { new_trace_root_span.set_tag(|| Tag::new("trace_id", trace_id)); } @@ -194,13 +198,9 @@ pub(crate) fn fork_trace(fork_name: impl Into>) -> SharedSpan .into() } -fn create_fork_ref_span( - fork_name: &str, - current_span_lock: &parking_lot::RwLockReadGuard, -) -> Span { +fn create_fork_ref_span(fork_name: &str, current_span: &Span) -> Span { let fork_ref_span_name = format!("[{fork_name} ref]"); - - current_span_lock.child(fork_ref_span_name, |o| o.start()) + current_span.child(fork_ref_span_name, |o| o.start()) } fn should_sample(sampling_ratio: f64) -> bool { diff --git a/foundations/src/telemetry/tracing/mod.rs b/foundations/src/telemetry/tracing/mod.rs index 41a6cedd..9cc909e8 100644 --- a/foundations/src/telemetry/tracing/mod.rs +++ b/foundations/src/telemetry/tracing/mod.rs @@ -236,7 +236,7 @@ pub struct StartTraceOptions { /// /// Returns `None` if the span is not sampled and don't have associated trace. pub fn trace_id() -> Option { - span_trace_id(¤t_span()?.inner.read()) + current_span()?.inner.with_read(span_trace_id) } /// Returns tracing state for the current span that can be serialized and passed to other services @@ -288,9 +288,7 @@ pub fn trace_id() -> Option { pub fn state_for_trace_stitching() -> Option { current_span()? .inner - .read() - .context() - .map(|c| c.state().clone()) + .with_read(|s| Some(s.context()?.state().clone())) } /// Returns the value to be used as a W3C traceparent header.