From db3947b82c0a0afefa479c9d0447019f038226f8 Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Wed, 21 Jan 2026 13:36:47 +0000 Subject: [PATCH 1/6] Refactor metrics handles and lazy statics --- .gitignore | 2 +- CHANGELOG.md | 12 +++++++++ metricus/src/counter.rs | 33 +++++++++-------------- metricus/src/histogram.rs | 26 +++++++++++------- metricus/src/lib.rs | 49 +++++++++++++--------------------- metricus_agent/src/exporter.rs | 8 +++--- metricus_macros/src/lib.rs | 6 ++--- 7 files changed, 67 insertions(+), 69 deletions(-) create mode 100644 CHANGELOG.md diff --git a/.gitignore b/.gitignore index 762f710..35e886f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -/target +**/target /.idea Cargo.lock **/*.jsonl diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..335dd69 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,12 @@ +# Changelog + +## Unreleased +- Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus release stores for the global handle. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. +- Switch counters/histograms and proc-macro statics to `LazyCell` with `LazyCell::force`, dropping `UnsafeCell` and `LazyLock` implementations. This narrows the API surface to a single lazy init path and avoids raw mutable access inside statics while preserving lazy initialization semantics. +- Update exporter publish signatures so the top-level API and UDP exporter accept `HashMap` directly, while stream/unix datagram keep the `Counters`/`Histograms` aliases. This aligns the public entry point with the concrete map used by aggregation and keeps the alias-based internal interfaces intact to avoid extra refactors. + In practice, `Counters`/`Histograms` are just aliases for `HashMap` and `HashMap`, so the change only makes the public signatures explicit about the underlying type. This removes an unnecessary layer of indirection at the API boundary without changing behavior. + +This refactor makes `MetricsHandle` immutable because it is just a vtable pointer and backend synchronization lives behind it, so callers do not need mutable access to the handle. With an immutable global handle, we can remove `static mut` and `&mut` aliasing, and then use `LazyCell::force` to get shared references to counters and histograms without `UnsafeCell`, making the API cleaner and safer. + +Breaking: `CounterOps`/`HistogramOps` are no longer implemented for `LazyLock>`. Code using `LazyLock` should switch to `LazyCell` or use a plain `Counter`/`Histogram` value. +Behavioral note: `set_metrics` should run before worker threads start; late initialization may observe the old handle because loads are now relaxed. diff --git a/metricus/src/counter.rs b/metricus/src/counter.rs index b30fe89..0066f65 100644 --- a/metricus/src/counter.rs +++ b/metricus/src/counter.rs @@ -1,9 +1,8 @@ //! A `Counter` proxy struct for managing a metrics counter. -use crate::access::get_metrics_mut; +use crate::access::get_metrics; use crate::{Id, Tags}; -use std::cell::{LazyCell, UnsafeCell}; -use std::sync::LazyLock; +use std::cell::LazyCell; /// Provides methods to create a new counter, increment it, and /// increment it by a specified amount. It automatically deletes the counter @@ -61,7 +60,7 @@ impl Counter { /// let counter = Counter::new("user_count", empty_tags()); /// ``` pub fn new(name: &str, tags: Tags) -> Self { - let counter_id = get_metrics_mut().new_counter(name, tags); + let counter_id = get_metrics().new_counter(name, tags); Self { id: counter_id } } @@ -84,7 +83,7 @@ impl Counter { impl Drop for Counter { fn drop(&mut self) { - get_metrics_mut().delete_counter(self.id); + get_metrics().delete_counter(self.id); } } @@ -116,31 +115,25 @@ pub trait CounterOps { } impl CounterOps for Counter { + #[inline] fn increment(&self) { - get_metrics_mut().increment_counter(self.id); + get_metrics().increment_counter(self.id); } + #[inline] fn increment_by(&self, delta: u64) { - get_metrics_mut().increment_counter_by(self.id, delta); + get_metrics().increment_counter_by(self.id, delta); } } -impl CounterOps for LazyCell> { +impl Counter> CounterOps for LazyCell { + #[inline] fn increment(&self) { - unsafe { &mut *self.get() }.increment() + LazyCell::force(self).increment() } + #[inline] fn increment_by(&self, delta: u64) { - unsafe { &mut *self.get() }.increment_by(delta) - } -} - -impl CounterOps for LazyLock> { - fn increment(&self) { - unsafe { &mut *self.get() }.increment() - } - - fn increment_by(&self, delta: u64) { - unsafe { &mut *self.get() }.increment_by(delta) + LazyCell::force(self).increment_by(delta) } } diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index 6fe4582..43f7916 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -1,10 +1,10 @@ //! A `Histogram` proxy struct for managing a metrics histogram. -use crate::access::get_metrics_mut; +use crate::access::get_metrics; use crate::{Id, Tags}; #[cfg(feature = "rdtsc")] use quanta::Clock; -use std::cell::{LazyCell, UnsafeCell}; +use std::cell::LazyCell; #[cfg(not(feature = "rdtsc"))] use std::time::Instant; @@ -70,7 +70,7 @@ impl Histogram { /// let histogram = Histogram::new("login_duration", empty_tags()); /// ``` pub fn new(name: &str, tags: Tags) -> Self { - let histogram_id = get_metrics_mut().new_histogram(name, tags); + let histogram_id = get_metrics().new_histogram(name, tags); Self { id: histogram_id, #[cfg(feature = "rdtsc")] @@ -124,10 +124,12 @@ pub trait HistogramOps { } impl HistogramOps for Histogram { + #[inline] fn record(&self, value: u64) { - get_metrics_mut().record(self.id, value); + get_metrics().record(self.id, value); } + #[inline] fn span(&self) -> Span<'_> { Span { histogram: self, @@ -138,29 +140,33 @@ impl HistogramOps for Histogram { } } + #[inline] fn with_span R, R>(&self, f: F) -> R { let _span = self.span(); f() } } -impl HistogramOps for LazyCell> { +impl Histogram> HistogramOps for LazyCell { + #[inline] fn record(&self, value: u64) { - unsafe { &mut *self.get() }.record(value) + LazyCell::force(self).record(value) } + #[inline] fn span(&self) -> Span<'_> { - unsafe { &mut *self.get() }.span() + LazyCell::force(self).span() } - fn with_span R, R>(&self, f: F) -> R { - unsafe { &mut *self.get() }.with_span(f) + #[inline] + fn with_span R, R>(&self, f: G) -> R { + LazyCell::force(self).with_span(f) } } impl Drop for Histogram { fn drop(&mut self) { - get_metrics_mut().delete_histogram(self.id); + get_metrics().delete_histogram(self.id); } } diff --git a/metricus/src/lib.rs b/metricus/src/lib.rs index 804b8c2..2090091 100644 --- a/metricus/src/lib.rs +++ b/metricus/src/lib.rs @@ -204,17 +204,15 @@ struct MetricsHolder { } /// Initially set to no-op backend. -static mut METRICS: MetricsHolder = MetricsHolder { +static METRICS: MetricsHolder = MetricsHolder { handle: AtomicRef::new(&NO_OP_METRICS_HANDLE), }; -/// Set a new metrics backend. This should be called as early as possible. Otherwise, -/// all metrics calls will delegate to the `NoOpMetrics`. +/// Set a new metrics backend. This should be called before any worker threads start so +/// hot-path loads can use relaxed ordering. Otherwise, all metrics calls will delegate +/// to the `NoOpMetrics`. pub fn set_metrics(metrics: impl Metrics) { - #[allow(static_mut_refs)] - unsafe { &mut METRICS } - .handle - .set(Box::leak(Box::new(metrics.into_handle())), Ordering::SeqCst); + METRICS.handle.set(Box::leak(Box::new(metrics.into_handle()))); } /// Get name of the active metrics backend. @@ -241,37 +239,37 @@ pub struct MetricsHandle { impl MetricsHandle { #[inline] - fn new_counter(&mut self, name: &str, tags: Tags) -> Id { + fn new_counter(&self, name: &str, tags: Tags) -> Id { (self.vtable.new_counter)(self.ptr, name, tags) } #[inline] - fn delete_counter(&mut self, id: Id) { + fn delete_counter(&self, id: Id) { (self.vtable.delete_counter)(self.ptr, id) } #[inline] - fn increment_counter_by(&mut self, id: Id, delta: u64) { + fn increment_counter_by(&self, id: Id, delta: u64) { (self.vtable.increment_counter_by)(self.ptr, id, delta) } #[inline] - fn increment_counter(&mut self, id: Id) { + fn increment_counter(&self, id: Id) { (self.vtable.increment_counter)(self.ptr, id) } #[inline] - fn new_histogram(&mut self, name: &str, tags: Tags) -> Id { + fn new_histogram(&self, name: &str, tags: Tags) -> Id { (self.vtable.new_histogram)(self.ptr, name, tags) } #[inline] - fn delete_histogram(&mut self, id: Id) { + fn delete_histogram(&self, id: Id) { (self.vtable.delete_histogram)(self.ptr, id) } #[inline] - fn record(&mut self, id: Id, value: u64) { + fn record(&self, id: Id, value: u64) { (self.vtable.record)(self.ptr, id, value) } } @@ -288,32 +286,21 @@ impl AtomicRef { } #[inline] - pub fn get(&self, order: Ordering) -> &T { - unsafe { &*self.ptr.load(order) } - } - - #[inline] - pub fn get_mut(&mut self, order: Ordering) -> &mut T { - unsafe { &mut *self.ptr.load(order) } + pub fn get(&self) -> &T { + unsafe { &*self.ptr.load(Ordering::Relaxed) } } #[inline] - pub fn set(&self, new_ref: &T, order: Ordering) { - self.ptr.store(new_ref as *const T as *mut T, order); + pub fn set(&self, new_ref: &T) { + self.ptr.store(new_ref as *const T as *mut T, Ordering::Release); } } mod access { use crate::{METRICS, MetricsHandle}; - use std::sync::atomic::Ordering; - - #[allow(static_mut_refs)] - pub fn get_metrics_mut() -> &'static mut MetricsHandle { - unsafe { &mut METRICS }.handle.get_mut(Ordering::Acquire) - } - #[allow(static_mut_refs)] + #[inline(always)] pub fn get_metrics() -> &'static MetricsHandle { - unsafe { &METRICS }.handle.get(Ordering::Acquire) + METRICS.handle.get() } } diff --git a/metricus_agent/src/exporter.rs b/metricus_agent/src/exporter.rs index 0db698f..a63ae09 100644 --- a/metricus_agent/src/exporter.rs +++ b/metricus_agent/src/exporter.rs @@ -100,13 +100,13 @@ impl UdpExporter { self.buffer.clear(); Ok(()) } - fn publish_counters(&mut self, counters: &Counters, timestamp: u64) -> std::io::Result<()> { + fn publish_counters(&mut self, counters: &HashMap, timestamp: u64) -> std::io::Result<()> { self.publish_metrics(counters, timestamp, |encoder, item, timestamp, buffer| { encoder.encode_counter(item, timestamp, buffer) }) } - fn publish_histograms(&mut self, histograms: &Histograms, timestamp: u64) -> std::io::Result<()> { + fn publish_histograms(&mut self, histograms: &HashMap, timestamp: u64) -> std::io::Result<()> { self.publish_metrics(histograms, timestamp, |encoder, item, timestamp, buffer| { encoder.encode_histogram(item, timestamp, buffer) }) @@ -209,7 +209,7 @@ impl TryFrom for StreamExporter { } impl StreamExporter { - fn publish_counters(&mut self, counters: &HashMap, timestamp: u64) -> std::io::Result<()> { + fn publish_counters(&mut self, counters: &Counters, timestamp: u64) -> std::io::Result<()> { for counter in counters.values() { self.encoder.encode_counter(counter, timestamp, &mut self.writer)?; } @@ -217,7 +217,7 @@ impl StreamExporter { Ok(()) } - fn publish_histograms(&mut self, histograms: &HashMap, timestamp: u64) -> std::io::Result<()> { + fn publish_histograms(&mut self, histograms: &Histograms, timestamp: u64) -> std::io::Result<()> { for histogram in histograms.values() { self.encoder.encode_histogram(histogram, timestamp, &mut self.writer)?; } diff --git a/metricus_macros/src/lib.rs b/metricus_macros/src/lib.rs index d37284b..1076014 100644 --- a/metricus_macros/src/lib.rs +++ b/metricus_macros/src/lib.rs @@ -129,7 +129,7 @@ pub fn counter(attr: TokenStream, item: TokenStream) -> TokenStream { #(#attrs)* #fn_vis #fn_async #fn_unsafe fn #fn_name #fn_generics (#fn_args) #fn_output #fn_where_clause { - static mut COUNTER: core::cell::LazyCell> = core::cell::LazyCell::new(|| core::cell::UnsafeCell::new(metricus::Counter::new(#measurement, &[ #(#tags),* ]))); + static mut COUNTER: core::cell::LazyCell = core::cell::LazyCell::new(|| metricus::Counter::new(#measurement, &[ #(#tags),* ])); #[allow(static_mut_refs)] unsafe { metricus::CounterOps::increment(&COUNTER); } @@ -235,7 +235,7 @@ pub fn counter_with_id(attr: TokenStream, item: TokenStream) -> TokenStream { #(#attrs)* #fn_vis #fn_async #fn_unsafe fn #fn_name #fn_generics (#fn_args) #fn_output #fn_where_clause { - static mut COUNTER: core::cell::LazyCell> = core::cell::LazyCell::new(|| core::cell::UnsafeCell::new(metricus::Counter::new_with_id(#counter_id))); + static mut COUNTER: core::cell::LazyCell = core::cell::LazyCell::new(|| metricus::Counter::new_with_id(#counter_id)); #[allow(static_mut_refs)] unsafe { metricus::CounterOps::increment(&COUNTER); } @@ -362,7 +362,7 @@ pub fn span(attr: TokenStream, item: TokenStream) -> TokenStream { #(#attrs)* #fn_vis #fn_async #fn_unsafe fn #fn_name #fn_generics (#fn_args) #fn_output #fn_where_clause { - static mut HISTOGRAM: core::cell::LazyCell> = core::cell::LazyCell::new(|| core::cell::UnsafeCell::new(metricus::Histogram::new(#measurement, &[ #(#tags),* ]))); + static mut HISTOGRAM: core::cell::LazyCell = core::cell::LazyCell::new(|| metricus::Histogram::new(#measurement, &[ #(#tags),* ])); #[allow(static_mut_refs)] let _span = unsafe { metricus::HistogramOps::span(&HISTOGRAM) }; From 2eaff5701fd658a40287b9f281918bb4c45f6dfb Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Wed, 21 Jan 2026 14:02:01 +0000 Subject: [PATCH 2/6] Avoid span timing work for no-op metrics --- CHANGELOG.md | 1 + metricus/src/histogram.rs | 50 +++++++++++++++++++++++++++------------ metricus/src/lib.rs | 7 +++++- 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 335dd69..463a45e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- Skip span timing work when the no-op metrics handle is active, returning a no-op span that avoids `Instant::now()`/`rdtsc` and record calls. This reduces overhead when metrics are disabled while keeping behavior unchanged when a real backend is set. - Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus release stores for the global handle. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. - Switch counters/histograms and proc-macro statics to `LazyCell` with `LazyCell::force`, dropping `UnsafeCell` and `LazyLock` implementations. This narrows the API surface to a single lazy init path and avoids raw mutable access inside statics while preserving lazy initialization semantics. - Update exporter publish signatures so the top-level API and UDP exporter accept `HashMap` directly, while stream/unix datagram keep the `Counters`/`Histograms` aliases. This aligns the public entry point with the concrete map used by aggregation and keeps the alias-based internal interfaces intact to avoid extra refactors. diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index 43f7916..126f38d 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -1,6 +1,6 @@ //! A `Histogram` proxy struct for managing a metrics histogram. -use crate::access::get_metrics; +use crate::access::{get_metrics, is_noop}; use crate::{Id, Tags}; #[cfg(feature = "rdtsc")] use quanta::Clock; @@ -131,12 +131,17 @@ impl HistogramOps for Histogram { #[inline] fn span(&self) -> Span<'_> { + if is_noop() { + return Span { state: SpanState::NoOp }; + } Span { - histogram: self, - #[cfg(feature = "rdtsc")] - start_raw: self.clock.raw(), - #[cfg(not(feature = "rdtsc"))] - start_instant: Instant::now(), + state: SpanState::Active { + histogram: self, + #[cfg(feature = "rdtsc")] + start_raw: self.clock.raw(), + #[cfg(not(feature = "rdtsc"))] + start_instant: Instant::now(), + }, } } @@ -172,22 +177,37 @@ impl Drop for Histogram { /// Used for measuring how long given operation takes. The duration is recorded in nanoseconds. pub struct Span<'a> { - histogram: &'a Histogram, - #[cfg(feature = "rdtsc")] - start_raw: u64, - #[cfg(not(feature = "rdtsc"))] - start_instant: Instant, + state: SpanState<'a>, +} + +enum SpanState<'a> { + Active { + histogram: &'a Histogram, + #[cfg(feature = "rdtsc")] + start_raw: u64, + #[cfg(not(feature = "rdtsc"))] + start_instant: Instant, + }, + NoOp, } impl Drop for Span<'_> { fn drop(&mut self) { #[cfg(feature = "rdtsc")] { - let end_raw = self.histogram.clock.raw(); - let elapsed = self.histogram.clock.delta_as_nanos(self.start_raw, end_raw); - self.histogram.record(elapsed); + if let SpanState::Active { histogram, start_raw } = &self.state { + let end_raw = histogram.clock.raw(); + let elapsed = histogram.clock.delta_as_nanos(*start_raw, end_raw); + histogram.record(elapsed); + } } #[cfg(not(feature = "rdtsc"))] - self.histogram.record(self.start_instant.elapsed().as_nanos() as u64); + if let SpanState::Active { + histogram, + start_instant, + } = &self.state + { + histogram.record(start_instant.elapsed().as_nanos() as u64); + } } } diff --git a/metricus/src/lib.rs b/metricus/src/lib.rs index 2090091..b7b3663 100644 --- a/metricus/src/lib.rs +++ b/metricus/src/lib.rs @@ -297,10 +297,15 @@ impl AtomicRef { } mod access { - use crate::{METRICS, MetricsHandle}; + use crate::{METRICS, MetricsHandle, NO_OP_METRICS_HANDLE}; #[inline(always)] pub fn get_metrics() -> &'static MetricsHandle { METRICS.handle.get() } + + #[inline(always)] + pub fn is_noop() -> bool { + std::ptr::eq(get_metrics(), &NO_OP_METRICS_HANDLE) + } } From f6743cfcb1ec2a4f3632d0869db59f9b62687f4c Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Wed, 21 Jan 2026 14:05:09 +0000 Subject: [PATCH 3/6] Reduce non-rdtsc span conversion overhead --- CHANGELOG.md | 1 + metricus/src/histogram.rs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 463a45e..597b0f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - Skip span timing work when the no-op metrics handle is active, returning a no-op span that avoids `Instant::now()`/`rdtsc` and record calls. This reduces overhead when metrics are disabled while keeping behavior unchanged when a real backend is set. +- Avoid u128 nanosecond conversion on non-`rdtsc` span drops by computing nanos from seconds + subsecond nanos with wrapping arithmetic. This keeps the u64 return value behavior while trimming a small amount of conversion overhead. - Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus release stores for the global handle. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. - Switch counters/histograms and proc-macro statics to `LazyCell` with `LazyCell::force`, dropping `UnsafeCell` and `LazyLock` implementations. This narrows the API surface to a single lazy init path and avoids raw mutable access inside statics while preserving lazy initialization semantics. - Update exporter publish signatures so the top-level API and UDP exporter accept `HashMap` directly, while stream/unix datagram keep the `Counters`/`Histograms` aliases. This aligns the public entry point with the concrete map used by aggregation and keeps the alias-based internal interfaces intact to avoid extra refactors. diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index 126f38d..fdea125 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -207,7 +207,12 @@ impl Drop for Span<'_> { start_instant, } = &self.state { - histogram.record(start_instant.elapsed().as_nanos() as u64); + let elapsed = start_instant.elapsed(); + let nanos = elapsed + .as_secs() + .wrapping_mul(1_000_000_000) + .wrapping_add(u64::from(elapsed.subsec_nanos())); + histogram.record(nanos); } } } From c8bbb5f7f1b9733710ff1400a8ee264d6ec10fec Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Wed, 21 Jan 2026 14:14:24 +0000 Subject: [PATCH 4/6] Cache metrics handle in counters and histograms --- CHANGELOG.md | 3 ++- metricus/src/counter.rs | 27 +++++++++++++++++++-------- metricus/src/histogram.rs | 28 +++++++++++++++++++++------- metricus/src/lib.rs | 14 +++++--------- 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 597b0f5..4ab2b82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Skip span timing work when the no-op metrics handle is active, returning a no-op span that avoids `Instant::now()`/`rdtsc` and record calls. This reduces overhead when metrics are disabled while keeping behavior unchanged when a real backend is set. - Avoid u128 nanosecond conversion on non-`rdtsc` span drops by computing nanos from seconds + subsecond nanos with wrapping arithmetic. This keeps the u64 return value behavior while trimming a small amount of conversion overhead. +- Cache the active metrics handle inside `Counter`/`Histogram` so hot-path operations avoid an atomic load. This tightens the initialization contract: `set_metrics` must run before any counters/histograms (including macro statics) are created. - Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus release stores for the global handle. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. - Switch counters/histograms and proc-macro statics to `LazyCell` with `LazyCell::force`, dropping `UnsafeCell` and `LazyLock` implementations. This narrows the API surface to a single lazy init path and avoids raw mutable access inside statics while preserving lazy initialization semantics. - Update exporter publish signatures so the top-level API and UDP exporter accept `HashMap` directly, while stream/unix datagram keep the `Counters`/`Histograms` aliases. This aligns the public entry point with the concrete map used by aggregation and keeps the alias-based internal interfaces intact to avoid extra refactors. @@ -11,4 +12,4 @@ This refactor makes `MetricsHandle` immutable because it is just a vtable pointer and backend synchronization lives behind it, so callers do not need mutable access to the handle. With an immutable global handle, we can remove `static mut` and `&mut` aliasing, and then use `LazyCell::force` to get shared references to counters and histograms without `UnsafeCell`, making the API cleaner and safer. Breaking: `CounterOps`/`HistogramOps` are no longer implemented for `LazyLock>`. Code using `LazyLock` should switch to `LazyCell` or use a plain `Counter`/`Histogram` value. -Behavioral note: `set_metrics` should run before worker threads start; late initialization may observe the old handle because loads are now relaxed. +Behavioral note: `set_metrics` must run before counters/histograms are created (including macro statics) and before worker threads start; metrics cache the handle and relaxed loads may otherwise observe the old backend. diff --git a/metricus/src/counter.rs b/metricus/src/counter.rs index 0066f65..d55f468 100644 --- a/metricus/src/counter.rs +++ b/metricus/src/counter.rs @@ -1,7 +1,7 @@ //! A `Counter` proxy struct for managing a metrics counter. use crate::access::get_metrics; -use crate::{Id, Tags}; +use crate::{Id, MetricsHandle, Tags}; use std::cell::LazyCell; /// Provides methods to create a new counter, increment it, and @@ -35,9 +35,15 @@ use std::cell::LazyCell; /// /// my_function_with_tags(); /// ```` -#[derive(Debug)] pub struct Counter { id: Id, + handle: &'static MetricsHandle, +} + +impl std::fmt::Debug for Counter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Counter").field("id", &self.id).finish() + } } impl Counter { @@ -60,8 +66,12 @@ impl Counter { /// let counter = Counter::new("user_count", empty_tags()); /// ``` pub fn new(name: &str, tags: Tags) -> Self { - let counter_id = get_metrics().new_counter(name, tags); - Self { id: counter_id } + let metrics = get_metrics(); + let counter_id = metrics.new_counter(name, tags); + Self { + id: counter_id, + handle: metrics, + } } /// Create a counter object without registering it. @@ -77,13 +87,14 @@ impl Counter { /// let counter = Counter::new_with_id(1); /// ``` pub fn new_with_id(id: Id) -> Self { - Self { id } + let metrics = get_metrics(); + Self { id, handle: metrics } } } impl Drop for Counter { fn drop(&mut self) { - get_metrics().delete_counter(self.id); + self.handle.delete_counter(self.id); } } @@ -117,12 +128,12 @@ pub trait CounterOps { impl CounterOps for Counter { #[inline] fn increment(&self) { - get_metrics().increment_counter(self.id); + self.handle.increment_counter(self.id); } #[inline] fn increment_by(&self, delta: u64) { - get_metrics().increment_counter_by(self.id, delta); + self.handle.increment_counter_by(self.id, delta); } } diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index fdea125..f2cded3 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -1,7 +1,7 @@ //! A `Histogram` proxy struct for managing a metrics histogram. -use crate::access::{get_metrics, is_noop}; -use crate::{Id, Tags}; +use crate::access::get_metrics; +use crate::{Id, MetricsHandle, Tags}; #[cfg(feature = "rdtsc")] use quanta::Clock; use std::cell::LazyCell; @@ -41,13 +41,25 @@ use std::time::Instant; /// my_function_with_tags(); /// ```` -#[derive(Debug)] pub struct Histogram { id: Id, + handle: &'static MetricsHandle, #[cfg(feature = "rdtsc")] clock: Clock, } +impl std::fmt::Debug for Histogram { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut debug = f.debug_struct("Histogram"); + debug.field("id", &self.id); + #[cfg(feature = "rdtsc")] + { + debug.field("clock", &self.clock); + } + debug.finish() + } +} + impl Histogram { /// Creates a new histogram with the specified name and tags. /// Units of measurement are not defined by the histogram itself but should be implied @@ -70,9 +82,11 @@ impl Histogram { /// let histogram = Histogram::new("login_duration", empty_tags()); /// ``` pub fn new(name: &str, tags: Tags) -> Self { - let histogram_id = get_metrics().new_histogram(name, tags); + let metrics = get_metrics(); + let histogram_id = metrics.new_histogram(name, tags); Self { id: histogram_id, + handle: metrics, #[cfg(feature = "rdtsc")] clock: Clock::new(), } @@ -126,12 +140,12 @@ pub trait HistogramOps { impl HistogramOps for Histogram { #[inline] fn record(&self, value: u64) { - get_metrics().record(self.id, value); + self.handle.record(self.id, value); } #[inline] fn span(&self) -> Span<'_> { - if is_noop() { + if std::ptr::eq(self.handle, &crate::NO_OP_METRICS_HANDLE) { return Span { state: SpanState::NoOp }; } Span { @@ -171,7 +185,7 @@ impl Histogram> HistogramOps for LazyCell { impl Drop for Histogram { fn drop(&mut self) { - get_metrics().delete_histogram(self.id); + self.handle.delete_histogram(self.id); } } diff --git a/metricus/src/lib.rs b/metricus/src/lib.rs index b7b3663..42b118d 100644 --- a/metricus/src/lib.rs +++ b/metricus/src/lib.rs @@ -208,9 +208,10 @@ static METRICS: MetricsHolder = MetricsHolder { handle: AtomicRef::new(&NO_OP_METRICS_HANDLE), }; -/// Set a new metrics backend. This should be called before any worker threads start so -/// hot-path loads can use relaxed ordering. Otherwise, all metrics calls will delegate -/// to the `NoOpMetrics`. +/// Set a new metrics backend. This must be called before any counters or histograms are +/// created (including through macros), because metric objects cache the active handle. +/// It should also be called before any worker threads start so hot-path loads can use +/// relaxed ordering. Otherwise, all metrics calls will delegate to the `NoOpMetrics`. pub fn set_metrics(metrics: impl Metrics) { METRICS.handle.set(Box::leak(Box::new(metrics.into_handle()))); } @@ -297,15 +298,10 @@ impl AtomicRef { } mod access { - use crate::{METRICS, MetricsHandle, NO_OP_METRICS_HANDLE}; + use crate::{METRICS, MetricsHandle}; #[inline(always)] pub fn get_metrics() -> &'static MetricsHandle { METRICS.handle.get() } - - #[inline(always)] - pub fn is_noop() -> bool { - std::ptr::eq(get_metrics(), &NO_OP_METRICS_HANDLE) - } } From 92a6c8c0790aed2d66afe3cd845a0200ac7169e3 Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Sat, 24 Jan 2026 22:54:59 +0000 Subject: [PATCH 5/6] Fix metrics handle Sync and clean warnings --- CHANGELOG.md | 4 ++++ Cargo.toml | 2 +- metricus/src/histogram.rs | 1 - metricus/src/lib.rs | 7 +++++++ metricus_allocator/README.md | 7 ++++++- metricus_allocator/src/lib.rs | 5 +++-- 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ab2b82..5ec900b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog ## Unreleased +- Mark `MetricsHandle` as `Send` and `Sync` with an explicit safety contract: it is only safe if backends are either thread-safe (internally synchronized) or are used in a strictly single-threaded manner. This allows allocator counters to live in a `static LazyLock` and fixes the `*mut u8`-derived `Sync` error without changing runtime behavior. +- Clarify allocator instrumentation behavior: allocation counters cache the active handle on first use, so `set_metrics` must run before allocator instrumentation is enabled if you expect these counters to emit. +- Move the release configuration to `[workspace.metadata.release]` to eliminate Cargo's unused manifest key warning while preserving the same metadata for release tooling. +- Remove an extra blank line after the `Histogram` doc comment to satisfy `clippy::empty_line_after_doc_comments` (no behavior change). - Skip span timing work when the no-op metrics handle is active, returning a no-op span that avoids `Instant::now()`/`rdtsc` and record calls. This reduces overhead when metrics are disabled while keeping behavior unchanged when a real backend is set. - Avoid u128 nanosecond conversion on non-`rdtsc` span drops by computing nanos from seconds + subsecond nanos with wrapping arithmetic. This keeps the u64 return value behavior while trimming a small amount of conversion overhead. - Cache the active metrics handle inside `Counter`/`Histogram` so hot-path operations avoid an atomic load. This tightens the initialization contract: `set_metrics` must run before any counters/histograms (including macro statics) are created. diff --git a/Cargo.toml b/Cargo.toml index fef5433..9d18793 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ members = [ ] resolver = "3" -[release] +[workspace.metadata.release] workspace = true [workspace.dependencies] diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index f2cded3..b81a368 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -40,7 +40,6 @@ use std::time::Instant; /// /// my_function_with_tags(); /// ```` - pub struct Histogram { id: Id, handle: &'static MetricsHandle, diff --git a/metricus/src/lib.rs b/metricus/src/lib.rs index 42b118d..d2cb5e5 100644 --- a/metricus/src/lib.rs +++ b/metricus/src/lib.rs @@ -25,6 +25,8 @@ pub const fn empty_tags() -> Tags<'static> { } /// Common interface for metrics backend. Each new backend must implement this trait. +/// Implementations must be safe for concurrent use or be used in a strictly single-threaded +/// manner, because the global metrics handle can be shared across threads. pub trait Metrics { fn name(&self) -> &'static str; @@ -238,6 +240,11 @@ pub struct MetricsHandle { name: &'static str, } +// Safety: the handle is immutable and backend implementations are responsible for +// any internal synchronization if they are used across threads. +unsafe impl Send for MetricsHandle {} +unsafe impl Sync for MetricsHandle {} + impl MetricsHandle { #[inline] fn new_counter(&self, name: &str, tags: Tags) -> Id { diff --git a/metricus_allocator/README.md b/metricus_allocator/README.md index 3bc2987..cfdbd05 100644 --- a/metricus_allocator/README.md +++ b/metricus_allocator/README.md @@ -1,3 +1,8 @@ # metricus-allocator -Contains allocator. \ No newline at end of file +Contains allocator. + +## Usage notes + +- Call `metricus::set_metrics` before enabling allocator instrumentation if you expect allocation counters to emit. +- Call `enable_allocator_instrumentation` for each thread that should report allocation metrics. diff --git a/metricus_allocator/src/lib.rs b/metricus_allocator/src/lib.rs index 3b4cec4..47fcfc9 100644 --- a/metricus_allocator/src/lib.rs +++ b/metricus_allocator/src/lib.rs @@ -134,8 +134,9 @@ pub fn enable_allocator_instrumentation() { static COUNTERS: LazyLock = LazyLock::new(|| Counters { // `counter_with_id` creates a counter object without registering it. - // This is used for allocation and de-allocation counters, which are special cases that are initialised before the metrics backend is created. - // In that case, the `Counter` is created with the `NoOpBackend`, so we defer the registration of the counters until the actual backend is ready. + // These allocation counters are created lazily on first use and cache the active metrics handle. + // If they are initialized before `set_metrics`, they will remain bound to the no-op backend. + // Ensure the backend is set before enabling allocator instrumentation if you want these to emit. alloc_count: Counter::new_with_id(ALLOC_COUNTER_ID), alloc_bytes: Counter::new_with_id(ALLOC_BYTES_COUNTER_ID), dealloc_count: Counter::new_with_id(DEALLOC_COUNTER_ID), From a9d2668d5a7635de4589a4e4af221827baabd068 Mon Sep 17 00:00:00 2001 From: Dmitriy Kumshayev Date: Mon, 26 Jan 2026 01:14:02 +0000 Subject: [PATCH 6/6] Align exporter aliases, span feature, and metrics ordering --- CHANGELOG.md | 6 +++--- metricus/Cargo.toml | 4 ++-- metricus/src/histogram.rs | 28 +++++++++++++++++++++++----- metricus/src/lib.rs | 15 +++++++++------ metricus_agent/src/exporter.rs | 10 +++++----- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ec900b..01c95aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,10 @@ - Skip span timing work when the no-op metrics handle is active, returning a no-op span that avoids `Instant::now()`/`rdtsc` and record calls. This reduces overhead when metrics are disabled while keeping behavior unchanged when a real backend is set. - Avoid u128 nanosecond conversion on non-`rdtsc` span drops by computing nanos from seconds + subsecond nanos with wrapping arithmetic. This keeps the u64 return value behavior while trimming a small amount of conversion overhead. - Cache the active metrics handle inside `Counter`/`Histogram` so hot-path operations avoid an atomic load. This tightens the initialization contract: `set_metrics` must run before any counters/histograms (including macro statics) are created. -- Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus release stores for the global handle. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. +- Replace mutable global metrics access with an immutable handle, moving metrics vtable calls to `&self` and using relaxed loads plus an explicit SeqCst store for the global handle in `set_metrics`. This removes `static mut` access and `get_mut` usage, reducing unsafe mutable aliasing while keeping hot-path reads fast when initialization happens before worker threads. - Switch counters/histograms and proc-macro statics to `LazyCell` with `LazyCell::force`, dropping `UnsafeCell` and `LazyLock` implementations. This narrows the API surface to a single lazy init path and avoids raw mutable access inside statics while preserving lazy initialization semantics. -- Update exporter publish signatures so the top-level API and UDP exporter accept `HashMap` directly, while stream/unix datagram keep the `Counters`/`Histograms` aliases. This aligns the public entry point with the concrete map used by aggregation and keeps the alias-based internal interfaces intact to avoid extra refactors. - In practice, `Counters`/`Histograms` are just aliases for `HashMap` and `HashMap`, so the change only makes the public signatures explicit about the underlying type. This removes an unnecessary layer of indirection at the API boundary without changing behavior. +- Keep exporter publish signatures using the `Counters`/`Histograms` aliases (including top-level/UDP exporters) for consistency, even though they are just `HashMap` type aliases. +- Add a default-on `span` feature: when disabled, `Span` becomes a no-op type and timing code is compiled out; when enabled, span timing behaves as before. This refactor makes `MetricsHandle` immutable because it is just a vtable pointer and backend synchronization lives behind it, so callers do not need mutable access to the handle. With an immutable global handle, we can remove `static mut` and `&mut` aliasing, and then use `LazyCell::force` to get shared references to counters and histograms without `UnsafeCell`, making the API cleaner and safer. diff --git a/metricus/Cargo.toml b/metricus/Cargo.toml index 8ff5322..19e046b 100644 --- a/metricus/Cargo.toml +++ b/metricus/Cargo.toml @@ -12,7 +12,8 @@ categories.workspace = true rust-version.workspace = true [features] -default = [] +default = ["span"] +span = [] rdtsc = ["dep:quanta"] [dependencies] @@ -33,4 +34,3 @@ harness = false name = "dispatch" path = "benches/dispatch.rs" harness = false - diff --git a/metricus/src/histogram.rs b/metricus/src/histogram.rs index b81a368..8ad7b8c 100644 --- a/metricus/src/histogram.rs +++ b/metricus/src/histogram.rs @@ -2,10 +2,12 @@ use crate::access::get_metrics; use crate::{Id, MetricsHandle, Tags}; -#[cfg(feature = "rdtsc")] +#[cfg(all(feature = "span", feature = "rdtsc"))] use quanta::Clock; use std::cell::LazyCell; -#[cfg(not(feature = "rdtsc"))] +#[cfg(not(feature = "span"))] +use std::marker::PhantomData; +#[cfg(all(feature = "span", not(feature = "rdtsc")))] use std::time::Instant; /// Facilitates the creation of a new histogram, recording of values, and @@ -43,7 +45,7 @@ use std::time::Instant; pub struct Histogram { id: Id, handle: &'static MetricsHandle, - #[cfg(feature = "rdtsc")] + #[cfg(all(feature = "span", feature = "rdtsc"))] clock: Clock, } @@ -51,7 +53,7 @@ impl std::fmt::Debug for Histogram { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut debug = f.debug_struct("Histogram"); debug.field("id", &self.id); - #[cfg(feature = "rdtsc")] + #[cfg(all(feature = "span", feature = "rdtsc"))] { debug.field("clock", &self.clock); } @@ -86,7 +88,7 @@ impl Histogram { Self { id: histogram_id, handle: metrics, - #[cfg(feature = "rdtsc")] + #[cfg(all(feature = "span", feature = "rdtsc"))] clock: Clock::new(), } } @@ -143,6 +145,7 @@ impl HistogramOps for Histogram { } #[inline] + #[cfg(feature = "span")] fn span(&self) -> Span<'_> { if std::ptr::eq(self.handle, &crate::NO_OP_METRICS_HANDLE) { return Span { state: SpanState::NoOp }; @@ -158,6 +161,12 @@ impl HistogramOps for Histogram { } } + #[inline] + #[cfg(not(feature = "span"))] + fn span(&self) -> Span<'_> { + Span { _marker: PhantomData } + } + #[inline] fn with_span R, R>(&self, f: F) -> R { let _span = self.span(); @@ -189,10 +198,18 @@ impl Drop for Histogram { } /// Used for measuring how long given operation takes. The duration is recorded in nanoseconds. +#[cfg(feature = "span")] pub struct Span<'a> { state: SpanState<'a>, } +/// No-op span used when the `span` feature is disabled. +#[cfg(not(feature = "span"))] +pub struct Span<'a> { + _marker: PhantomData<&'a ()>, +} + +#[cfg(feature = "span")] enum SpanState<'a> { Active { histogram: &'a Histogram, @@ -204,6 +221,7 @@ enum SpanState<'a> { NoOp, } +#[cfg(feature = "span")] impl Drop for Span<'_> { fn drop(&mut self) { #[cfg(feature = "rdtsc")] diff --git a/metricus/src/lib.rs b/metricus/src/lib.rs index d2cb5e5..52258db 100644 --- a/metricus/src/lib.rs +++ b/metricus/src/lib.rs @@ -215,7 +215,9 @@ static METRICS: MetricsHolder = MetricsHolder { /// It should also be called before any worker threads start so hot-path loads can use /// relaxed ordering. Otherwise, all metrics calls will delegate to the `NoOpMetrics`. pub fn set_metrics(metrics: impl Metrics) { - METRICS.handle.set(Box::leak(Box::new(metrics.into_handle()))); + METRICS + .handle + .set(Box::leak(Box::new(metrics.into_handle())), Ordering::SeqCst); } /// Get name of the active metrics backend. @@ -294,21 +296,22 @@ impl AtomicRef { } #[inline] - pub fn get(&self) -> &T { - unsafe { &*self.ptr.load(Ordering::Relaxed) } + pub fn get(&self, ordering: Ordering) -> &T { + unsafe { &*self.ptr.load(ordering) } } #[inline] - pub fn set(&self, new_ref: &T) { - self.ptr.store(new_ref as *const T as *mut T, Ordering::Release); + pub fn set(&self, new_ref: &T, ordering: Ordering) { + self.ptr.store(new_ref as *const T as *mut T, ordering); } } mod access { use crate::{METRICS, MetricsHandle}; + use std::sync::atomic::Ordering; #[inline(always)] pub fn get_metrics() -> &'static MetricsHandle { - METRICS.handle.get() + METRICS.handle.get(Ordering::Relaxed) } } diff --git a/metricus_agent/src/exporter.rs b/metricus_agent/src/exporter.rs index a63ae09..20b414c 100644 --- a/metricus_agent/src/exporter.rs +++ b/metricus_agent/src/exporter.rs @@ -1,4 +1,4 @@ -use crate::aggregator::{Counter, Counters, Encoder, Histogram, Histograms}; +use crate::aggregator::{Counters, Encoder, Histograms}; use crate::config::{ExporterSource, FileConfig, UdpConfig, UnixSocketConfig}; use log::warn; use metricus::Id; @@ -35,7 +35,7 @@ impl TryFrom for Exporter { } impl Exporter { - pub fn publish_counters(&mut self, counters: &HashMap, timestamp: u64) -> std::io::Result<()> { + pub fn publish_counters(&mut self, counters: &Counters, timestamp: u64) -> std::io::Result<()> { match self { Exporter::NoOp => Ok(()), Exporter::Udp(exporter) => exporter.publish_counters(counters, timestamp), @@ -45,7 +45,7 @@ impl Exporter { } } - pub fn publish_histograms(&mut self, histograms: &HashMap, timestamp: u64) -> std::io::Result<()> { + pub fn publish_histograms(&mut self, histograms: &Histograms, timestamp: u64) -> std::io::Result<()> { match self { Exporter::NoOp => Ok(()), Exporter::Udp(exporter) => exporter.publish_histograms(histograms, timestamp), @@ -100,13 +100,13 @@ impl UdpExporter { self.buffer.clear(); Ok(()) } - fn publish_counters(&mut self, counters: &HashMap, timestamp: u64) -> std::io::Result<()> { + fn publish_counters(&mut self, counters: &Counters, timestamp: u64) -> std::io::Result<()> { self.publish_metrics(counters, timestamp, |encoder, item, timestamp, buffer| { encoder.encode_counter(item, timestamp, buffer) }) } - fn publish_histograms(&mut self, histograms: &HashMap, timestamp: u64) -> std::io::Result<()> { + fn publish_histograms(&mut self, histograms: &Histograms, timestamp: u64) -> std::io::Result<()> { self.publish_metrics(histograms, timestamp, |encoder, item, timestamp, buffer| { encoder.encode_histogram(item, timestamp, buffer) })