From da240cc0d05dc22a1f9fc83e3f22be6752fff4a4 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 09:42:17 -0300 Subject: [PATCH 01/13] Create functions to create u64 and f64 exponential histograms --- Cargo.toml | 2 +- src/logfire.rs | 103 +++++++++++++++++++++++++++++++++++++++++-- src/metrics.rs | 53 +++++++++++++++++++--- src/usage/metrics.rs | 14 ++++++ 4 files changed, 162 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2c40e56..3bbd08d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ tonic = { version = "0.13", optional = true } rand = "0.9.0" opentelemetry = { version = "0.30", default-features = false, features = ["trace", "logs"] } -opentelemetry_sdk = { version = "0.30", default-features = false, features = ["trace", "experimental_metrics_custom_reader", "logs"] } +opentelemetry_sdk = { version = "0.30", default-features = false, features = ["trace", "experimental_metrics_custom_reader", "logs", "spec_unstable_metrics_views"] } opentelemetry-otlp = { version = "0.30", default-features = false, features = ["trace", "metrics", "logs"] } futures-util = "0.3" diff --git a/src/logfire.rs b/src/logfire.rs index 37e1260..4a7acee 100644 --- a/src/logfire.rs +++ b/src/logfire.rs @@ -13,7 +13,7 @@ use opentelemetry::{ }; use opentelemetry_sdk::{ logs::{BatchLogProcessor, SdkLoggerProvider}, - metrics::{PeriodicReader, SdkMeterProvider}, + metrics::{Aggregation, Instrument, PeriodicReader, SdkMeterProvider, Stream}, trace::{BatchConfigBuilder, BatchSpanProcessor, SdkTracerProvider}, }; use tracing::{Subscriber, level_filters::LevelFilter, subscriber::DefaultGuard}; @@ -30,6 +30,7 @@ use crate::{ exporters::console::{ConsoleWriter, create_console_processors}, logfire_tracer::{GLOBAL_TRACER, LOCAL_TRACER, LogfireTracer}, }, + metrics, ulid_id_generator::UlidIdGenerator, }; @@ -296,6 +297,21 @@ impl Logfire { meter_provider_builder = meter_provider_builder.with_resource(resource.clone()); } + let view = |i: &Instrument| { + let histograms = metrics::EXPONENTIAL_HISTOGRAMS.read().ok()?; + let scale = histograms.get(i.name())?; + + Stream::builder() + .with_aggregation(Aggregation::Base2ExponentialHistogram { + max_size: 160, + max_scale: *scale, // Upper bound on resolution + record_min_max: true, + }) + .build() + .ok() + }; + meter_provider_builder = meter_provider_builder.with_view(view); + let meter_provider = meter_provider_builder.build(); if let Some(logfire_base_url) = logfire_base_url { @@ -522,9 +538,21 @@ mod tests { time::Duration, }; - use opentelemetry_sdk::trace::{SpanData, SpanProcessor}; + use opentelemetry_sdk::{ + metrics::{ + InMemoryMetricExporter, PeriodicReader, + data::{Metric, MetricData}, + reader::MetricReader, + }, + trace::{SpanData, SpanProcessor}, + }; - use crate::{ConfigureError, Logfire, config::SendToLogfire, configure}; + use crate::{ + ConfigureError, Logfire, + config::{BoxedMetricReader, MetricsOptions, SendToLogfire}, + configure, f64_exponential_histogram, f64_histogram, u64_exponential_histogram, + u64_histogram, + }; #[test] fn test_send_to_logfire() { @@ -699,4 +727,73 @@ mod tests { // Second `shutdown` call should fail logfire.shutdown().unwrap(); } + + #[test] + #[allow(clippy::unwrap_used)] + fn test_exponential_histogram_view() { + let exporter = InMemoryMetricExporter::default(); + let reader = PeriodicReader::builder(exporter.clone()).build(); + let logfire = configure() + .send_to_logfire(false) + .with_metrics(Some(MetricsOptions { + additional_readers: vec![BoxedMetricReader::new(Box::new(reader.clone()))], + })) + .finish() + .expect("failed to configure logfire"); + + let _guard = logfire.shutdown_guard(); + + let f64_hist = f64_histogram("f64_hist").build(); + f64_hist.record(1.0, &[]); + let u64_hist = u64_histogram("u64_hist").build(); + u64_hist.record(20, &[]); + + let f64_exp = f64_exponential_histogram("f64_exp", 20).build(); + f64_exp.record(10.0, &[]); + let u64_exp = u64_exponential_histogram("u64_exp", 20).build(); + u64_exp.record(10, &[]); + + reader.force_flush().unwrap(); + + let metrics = exporter.get_finished_metrics().unwrap(); + + for scope_metics in metrics[0].scope_metrics() { + // Scope name is defined in src/metrics.rs. Using "logfire" as the scope name + // because that's the scope name of the meter used by functions that create histograms. + if scope_metics.scope().name() != "logfire" { + continue; + } + + for (name, expected) in [ + ("f64_hist", false), + ("u64_hist", false), + ("f64_exp", true), + ("u64_exp", true), + ] { + let metric = scope_metics.metrics().find(|m| m.name() == name).unwrap(); + assert_eq!(expected, is_exponential_histogram(metric)); + } + } + } + + fn is_exponential_histogram(metric: &Metric) -> bool { + match metric.data() { + opentelemetry_sdk::metrics::data::AggregatedMetrics::F64(metric_data) => { + is_exponential_histogram_metric_data(metric_data) + } + opentelemetry_sdk::metrics::data::AggregatedMetrics::U64(metric_data) => { + is_exponential_histogram_metric_data(metric_data) + } + opentelemetry_sdk::metrics::data::AggregatedMetrics::I64(metric_data) => { + is_exponential_histogram_metric_data(metric_data) + } + } + } + + fn is_exponential_histogram_metric_data(data: &MetricData) -> bool { + matches!( + data, + opentelemetry_sdk::metrics::data::MetricData::ExponentialHistogram(_) + ) + } } diff --git a/src/metrics.rs b/src/metrics.rs index d724d00..9aed0cc 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; -use std::sync::LazyLock; +use std::collections::HashMap; +use std::sync::{LazyLock, RwLock}; use opentelemetry::metrics::{ AsyncInstrumentBuilder, Counter, Gauge, Histogram, HistogramBuilder, InstrumentBuilder, Meter, @@ -7,14 +8,28 @@ use opentelemetry::metrics::{ }; static METER: LazyLock = LazyLock::new(|| opentelemetry::global::meter("logfire")); +type HistogramName = Cow<'static, str>; +/// A map of histogram name to scale. +/// Histograms that are members of this map will be forced to use `Base2ExponentialHistogram` +/// for aggregation by the meter provider view. +pub static EXPONENTIAL_HISTOGRAMS: LazyLock>> = + LazyLock::new(|| RwLock::new(HashMap::new())); + +#[rustfmt::skip] +macro_rules! metric_doc_header { + (show_header, $method: ident) => { + concat!("Wrapper for [`Meter::", stringify!($method), "`] using Pydantic Logfire's global meter.") + }; + (hide_header, $method: ident) => { "" }; +} /// For methods which are called with an observation. #[rustfmt::skip] macro_rules! make_metric_doc { - ($method: ident, $ty:ty, $var_name:literal, $usage:literal) => { + ($method: ident, $ty:ty, $var_name:literal, $usage:literal, $show_header: ident) => { concat!( -"Wrapper for [`Meter::", stringify!($method), "`] using Pydantic Logfire's global meter. - +metric_doc_header!($show_header, $method), +" # Examples We recommend using this as a static variable, like so: @@ -50,7 +65,7 @@ fn main() -> Result<(), Box> { macro_rules! wrap_method { ($method:ident, $ty:ty, $var_name:literal, $usage:literal) => { - #[doc = make_metric_doc!($method, $ty, $var_name, $usage)] + #[doc = make_metric_doc!($method, $ty, $var_name, $usage, show_header)] pub fn $method(name: impl Into>) -> InstrumentBuilder<'static, $ty> { METER.$method(name) } @@ -59,7 +74,7 @@ macro_rules! wrap_method { macro_rules! wrap_histogram_method { ($method:ident, $ty:ty, $var_name:literal, $usage:literal) => { - #[doc = make_metric_doc!($method, $ty, $var_name, $usage)] + #[doc = make_metric_doc!($method, $ty, $var_name, $usage, show_header)] pub fn $method(name: impl Into>) -> HistogramBuilder<'static, $ty> { METER.$method(name) } @@ -102,6 +117,32 @@ wrap_histogram_method!( "HISTOGRAM.record(1, &[])" ); +#[doc = make_metric_doc!(f64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", hide_header)] +pub fn f64_exponential_histogram( + name: impl Into>, + scale: i8, +) -> HistogramBuilder<'static, Histogram> { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let name = name.into(); + histograms.insert(name.clone(), scale); + f64_histogram(name) +} + +#[doc = make_metric_doc!(u64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", hide_header)] +pub fn u64_exponential_histogram( + name: impl Into>, + scale: i8, +) -> HistogramBuilder<'static, Histogram> { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let name = name.into(); + histograms.insert(name.clone(), scale); + u64_histogram(name) +} + /// For observable methods which take a callback. #[rustfmt::skip] macro_rules! make_observable_metric_doc { diff --git a/src/usage/metrics.rs b/src/usage/metrics.rs index e46b68d..874189d 100644 --- a/src/usage/metrics.rs +++ b/src/usage/metrics.rs @@ -85,6 +85,20 @@ //! .with_unit("By") //! .build() //! }); +//! +//! static QUEUE_LATENCY: LazyLock> = LazyLock::new(|| { +//! logfire::f64_exponential_histogram("queue_latency") +//! .with_description("Latency of items in a queue") +//! .with_unit("ms") +//! .build() +//! }); +//! +//! static MEMORY_USAGE_BYTES: LazyLock> = LazyLock::new(|| { +//! logfire::u64_exponential_histogram("memory_usage_bytes") +//! .with_description("Memory usage in bytes") +//! .with_unit("By") +//! .build() +//! }); //! ``` //! //! ### Up/Down Counters From 0ba378e7869c17427319cbcfd4d3f8f62f8ea7e8 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 09:49:03 -0300 Subject: [PATCH 02/13] fix missing arg in docs --- src/usage/metrics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/usage/metrics.rs b/src/usage/metrics.rs index 874189d..8de977d 100644 --- a/src/usage/metrics.rs +++ b/src/usage/metrics.rs @@ -87,14 +87,14 @@ //! }); //! //! static QUEUE_LATENCY: LazyLock> = LazyLock::new(|| { -//! logfire::f64_exponential_histogram("queue_latency") +//! logfire::f64_exponential_histogram("queue_latency", 20) //! .with_description("Latency of items in a queue") //! .with_unit("ms") //! .build() //! }); //! //! static MEMORY_USAGE_BYTES: LazyLock> = LazyLock::new(|| { -//! logfire::u64_exponential_histogram("memory_usage_bytes") +//! logfire::u64_exponential_histogram("memory_usage_bytes", 20) //! .with_description("Memory usage in bytes") //! .with_unit("By") //! .build() From dccae1c15d0ebac7823a1e6b1dd281efb69cd731 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:24:14 -0300 Subject: [PATCH 03/13] skip instuments that are not of type Histogram --- src/logfire.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/logfire.rs b/src/logfire.rs index 77118d8..942d235 100644 --- a/src/logfire.rs +++ b/src/logfire.rs @@ -16,7 +16,7 @@ use opentelemetry::{ }; use opentelemetry_sdk::{ logs::{BatchLogProcessor, SdkLoggerProvider}, - metrics::{Aggregation, Instrument, PeriodicReader, SdkMeterProvider, Stream}, + metrics::{Aggregation, Instrument, InstrumentKind, PeriodicReader, SdkMeterProvider, Stream}, trace::{BatchConfigBuilder, BatchSpanProcessor, SdkTracerProvider}, }; use tracing::{Subscriber, level_filters::LevelFilter, subscriber::DefaultGuard}; @@ -355,6 +355,9 @@ impl Logfire { } let view = |i: &Instrument| { + if i.kind() != InstrumentKind::Histogram { + return None; + } let histograms = metrics::EXPONENTIAL_HISTOGRAMS.read().ok()?; let scale = histograms.get(i.name())?; @@ -966,9 +969,14 @@ mod tests { let u64_hist = u64_histogram("u64_hist").build(); u64_hist.record(20, &[]); - let f64_exp = f64_exponential_histogram("f64_exp", 20).build(); + let f64_exp = f64_exponential_histogram("f64_exp", 2).build(); + f64_exp.record(1.0, &[]); + f64_exp.record(1.5, &[]); + f64_exp.record(2.0, &[]); + f64_exp.record(3.0, &[]); f64_exp.record(10.0, &[]); - let u64_exp = u64_exponential_histogram("u64_exp", 20).build(); + + let u64_exp = u64_exponential_histogram("u64_exp", 2).build(); u64_exp.record(10, &[]); reader.force_flush().unwrap(); @@ -976,12 +984,6 @@ mod tests { let metrics = exporter.get_finished_metrics().unwrap(); for scope_metics in metrics[0].scope_metrics() { - // Scope name is defined in src/metrics.rs. Using "logfire" as the scope name - // because that's the scope name of the meter used by functions that create histograms. - if scope_metics.scope().name() != "logfire" { - continue; - } - for (name, expected) in [ ("f64_hist", false), ("u64_hist", false), From 86635619cc3fddecf904333f84113c5a1a4bfe7f Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:24:27 -0300 Subject: [PATCH 04/13] fix doc gen / add ExponentialHistogram type --- src/metrics.rs | 248 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 219 insertions(+), 29 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index 9aed0cc..4166bd1 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; use std::collections::HashMap; -use std::sync::{LazyLock, RwLock}; +use std::sync::{Arc, LazyLock, RwLock}; use opentelemetry::metrics::{ AsyncInstrumentBuilder, Counter, Gauge, Histogram, HistogramBuilder, InstrumentBuilder, Meter, @@ -10,25 +10,179 @@ use opentelemetry::metrics::{ static METER: LazyLock = LazyLock::new(|| opentelemetry::global::meter("logfire")); type HistogramName = Cow<'static, str>; /// A map of histogram name to scale. +/// /// Histograms that are members of this map will be forced to use `Base2ExponentialHistogram` /// for aggregation by the meter provider view. +/// +/// Histogram names are removed from the map when the [`ExponentialHistogram`] is dropped. pub static EXPONENTIAL_HISTOGRAMS: LazyLock>> = LazyLock::new(|| RwLock::new(HashMap::new())); +/// An instrument that records a distribution of values. +/// +/// [`ExponentialHistogram`] can be cloned to create multiple handles to the same instrument. If a [`ExponentialHistogram`] needs to be shared, +/// users are recommended to clone the [`ExponentialHistogram`] instead of creating duplicate [`ExponentialHistogram`]s for the same metric. Creating +/// duplicate [`ExponentialHistogram`]s for the same metric could lower SDK performance. +pub struct ExponentialHistogram { + inner: Arc>, +} + +struct Inner { + name: HistogramName, + histogram: Histogram, +} + +impl Drop for Inner { + fn drop(&mut self) { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + + println!( + "\n\naaaaaaa Inner Drop called self.name {:?}\n\n", + &self.name + ); + histograms.remove(&self.name); + } +} + +impl ExponentialHistogram { + /// Adds an additional value to the distribution. + pub fn record(&self, value: T, attributes: &[opentelemetry::KeyValue]) { + self.inner.histogram.record(value, attributes); + } +} + +/// Configuration for building an exponential Histogram. +pub struct ExponentialHistogramBuilder<'a, T> { + name: HistogramName, + scale: i8, + inner: HistogramBuilder<'a, Histogram>, +} + +impl<'a, T> ExponentialHistogramBuilder<'a, T> { + /// Create a new instrument builder + fn new(name: HistogramName, scale: i8, inner: HistogramBuilder<'a, Histogram>) -> Self { + Self { name, scale, inner } + } + + /// Set the description for this instrument + #[must_use] + pub fn with_description>>(mut self, description: S) -> Self { + self.inner = self.inner.with_description(description); + self + } + + /// Set the unit for this instrument. + /// + /// Unit is case sensitive(`kb` is not the same as `kB`). + /// + /// Unit must be: + /// - ASCII string + /// - No longer than 63 characters + #[must_use] + pub fn with_unit>>(mut self, unit: S) -> Self { + self.inner = self.inner.with_unit(unit); + self + } +} + +impl ExponentialHistogramBuilder<'_, u64> { + /// Creates a new instrument. + /// + /// Validates the instrument configuration and creates a new instrument. In + /// case of invalid configuration, a no-op instrument is returned + /// and an error is logged using internal logging. + #[must_use] + pub fn build(self) -> ExponentialHistogram { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + histograms.insert(self.name.clone(), self.scale); + let histogram = self.inner.build(); + + ExponentialHistogram { + inner: Arc::new(Inner { + name: self.name, + histogram, + }), + } + } +} + +impl ExponentialHistogramBuilder<'_, f64> { + /// Creates a new instrument. + /// + /// Validates the instrument configuration and creates a new instrument. In + /// case of invalid configuration, a no-op instrument is returned + /// and an error is logged using internal logging. + #[must_use] + pub fn build(self) -> ExponentialHistogram { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + histograms.insert(self.name.clone(), self.scale); + let histogram = self.inner.build(); + ExponentialHistogram { + inner: Arc::new(Inner { + name: self.name, + histogram, + }), + } + } +} + #[rustfmt::skip] macro_rules! metric_doc_header { - (show_header, $method: ident) => { + (histogram, $method:ident) => { concat!("Wrapper for [`Meter::", stringify!($method), "`] using Pydantic Logfire's global meter.") }; - (hide_header, $method: ident) => { "" }; + (exponential_histogram, $method:ident) => { + concat!("Wrapper for [`Histogram`] using Base2ExponentialHistogram aggregation and Pydantic Logfire's global meter.") + }; +} + +macro_rules! metric_doc_create_metric { + (histogram, $method:ident, $ty:ty, $var_name:literal) => { + concat!( + "static ", + $var_name, + ": LazyLock = LazyLock::new(|| { + logfire::", + stringify!($method), + "(\"my_counter\") + .with_description(\"Just an example\") + .with_unit(\"s\") + .build() +});" + ) + }; + (exponential_histogram, $method:ident, $ty:ty, $var_name:literal) => { + concat!( + "static ", + $var_name, + ": LazyLock = LazyLock::new(|| { + logfire::", + stringify!($method), + "(\"latency\", 20) + .with_description(\"Just an example\") + .with_unit(\"ms\") + .build() +});" + ) + }; } /// For methods which are called with an observation. #[rustfmt::skip] macro_rules! make_metric_doc { - ($method: ident, $ty:ty, $var_name:literal, $usage:literal, $show_header: ident) => { + ($method: ident, $ty:ty, $var_name:literal, $usage:literal, $histogram_type: ident) => { concat!( -metric_doc_header!($show_header, $method), +metric_doc_header!($histogram_type, $method), " # Examples @@ -37,14 +191,9 @@ We recommend using this as a static variable, like so: ```rust use std::sync::LazyLock; use opentelemetry::metrics::AsyncInstrument; - -static ", $var_name, ": LazyLock = LazyLock::new(|| { - logfire::", stringify!($method), "(\"my_counter\") - .with_description(\"Just an example\") - .with_unit(\"s\") - .build() -}); - +", +metric_doc_create_metric!($histogram_type, $method, $ty, $var_name), +" fn main() -> Result<(), Box> { // ensure Pydantic Logfire is configured before accessing the metric for the first time let shutdown_handler = logfire::configure() @@ -65,7 +214,7 @@ fn main() -> Result<(), Box> { macro_rules! wrap_method { ($method:ident, $ty:ty, $var_name:literal, $usage:literal) => { - #[doc = make_metric_doc!($method, $ty, $var_name, $usage, show_header)] + #[doc = make_metric_doc!($method, $ty, $var_name, $usage, histogram)] pub fn $method(name: impl Into>) -> InstrumentBuilder<'static, $ty> { METER.$method(name) } @@ -74,7 +223,7 @@ macro_rules! wrap_method { macro_rules! wrap_histogram_method { ($method:ident, $ty:ty, $var_name:literal, $usage:literal) => { - #[doc = make_metric_doc!($method, $ty, $var_name, $usage, show_header)] + #[doc = make_metric_doc!($method, $ty, $var_name, $usage, histogram)] pub fn $method(name: impl Into>) -> HistogramBuilder<'static, $ty> { METER.$method(name) } @@ -117,30 +266,22 @@ wrap_histogram_method!( "HISTOGRAM.record(1, &[])" ); -#[doc = make_metric_doc!(f64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", hide_header)] +#[doc = make_metric_doc!(f64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] pub fn f64_exponential_histogram( name: impl Into>, scale: i8, -) -> HistogramBuilder<'static, Histogram> { - let mut histograms = EXPONENTIAL_HISTOGRAMS - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); +) -> ExponentialHistogramBuilder<'static, f64> { let name = name.into(); - histograms.insert(name.clone(), scale); - f64_histogram(name) + ExponentialHistogramBuilder::new(name.clone(), scale, f64_histogram(name)) } -#[doc = make_metric_doc!(u64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", hide_header)] +#[doc = make_metric_doc!(u64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] pub fn u64_exponential_histogram( name: impl Into>, scale: i8, -) -> HistogramBuilder<'static, Histogram> { - let mut histograms = EXPONENTIAL_HISTOGRAMS - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); +) -> ExponentialHistogramBuilder<'static, u64> { let name = name.into(); - histograms.insert(name.clone(), scale); - u64_histogram(name) + ExponentialHistogramBuilder::new(name.clone(), scale, u64_histogram(name)) } /// For observable methods which take a callback. @@ -246,3 +387,52 @@ wrap_observable_method!( "GAUGE", "|gauge| gauge.observe(1, &[])" ); + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn exponential_histograms_are_registered_and_removed_on_drop() { + let a = f64_exponential_histogram("f64_exp", 10).build(); + let b = u64_exponential_histogram("u64_exp", 20).build(); + + // Exponential histograms have their names automatically added to EXPONENTIAL_HISTOGRAMS. + { + let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); + assert!(histograms.contains_key("f64_exp")); + assert!(histograms.contains_key("u64_exp")); + } + + drop(a); + drop(b); + + // And have their names removed from the map on drop. + { + let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); + assert!(!histograms.contains_key("f64_exp")); + assert!(!histograms.contains_key("u64_exp")); + } + } + + #[test] + fn exponential_histograms_are_registered_on_build() { + let b1 = f64_exponential_histogram("f64_exp", 10); + let b2 = u64_exponential_histogram("u64_exp", 20); + + { + let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); + assert!(histograms.is_empty()); + } + + let _a = b1.build(); + let _bb = b2.build(); + + { + let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); + assert!(histograms.contains_key("f64_exp")); + assert!(histograms.contains_key("u64_exp")); + } + } +} From 972efe202e59474503997c201437f1c69a519ff2 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:42:08 -0300 Subject: [PATCH 05/13] add missing fields to impl Default for LogfireConfigBuilder { --- src/config.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/config.rs b/src/config.rs index c029407..e04c945 100644 --- a/src/config.rs +++ b/src/config.rs @@ -54,6 +54,9 @@ pub struct LogfireConfigBuilder { impl Default for LogfireConfigBuilder { fn default() -> Self { Self { + environment: None, + service_name: None, + service_version: None, local: false, send_to_logfire: None, token: None, From 2395dfadbe4f7db4210f8cc5e147d4110fc37e5e Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:43:46 -0300 Subject: [PATCH 06/13] release locks as soon as possible --- src/logfire.rs | 9 ++++++--- src/metrics.rs | 47 +++++++++++++++-------------------------------- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/logfire.rs b/src/logfire.rs index c819dea..e9b827a 100644 --- a/src/logfire.rs +++ b/src/logfire.rs @@ -401,13 +401,16 @@ impl Logfire { if i.kind() != InstrumentKind::Histogram { return None; } - let histograms = metrics::EXPONENTIAL_HISTOGRAMS.read().ok()?; - let scale = histograms.get(i.name())?; + + let scale = { + let histograms = metrics::EXPONENTIAL_HISTOGRAMS.read().ok()?; + *histograms.get(i.name())? + }; Stream::builder() .with_aggregation(Aggregation::Base2ExponentialHistogram { max_size: 160, - max_scale: *scale, // Upper bound on resolution + max_scale: scale, // Upper bound on resolution record_min_max: true, }) .build() diff --git a/src/metrics.rs b/src/metrics.rs index 4166bd1..17121bb 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -38,10 +38,6 @@ impl Drop for Inner { .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - println!( - "\n\naaaaaaa Inner Drop called self.name {:?}\n\n", - &self.name - ); histograms.remove(&self.name); } } @@ -95,10 +91,13 @@ impl ExponentialHistogramBuilder<'_, u64> { /// and an error is logged using internal logging. #[must_use] pub fn build(self) -> ExponentialHistogram { - let mut histograms = EXPONENTIAL_HISTOGRAMS - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); - histograms.insert(self.name.clone(), self.scale); + { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + histograms.insert(self.name.clone(), self.scale); + } + let histogram = self.inner.build(); ExponentialHistogram { @@ -118,11 +117,15 @@ impl ExponentialHistogramBuilder<'_, f64> { /// and an error is logged using internal logging. #[must_use] pub fn build(self) -> ExponentialHistogram { - let mut histograms = EXPONENTIAL_HISTOGRAMS - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); - histograms.insert(self.name.clone(), self.scale); + { + let mut histograms = EXPONENTIAL_HISTOGRAMS + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + histograms.insert(self.name.clone(), self.scale); + } + let histogram = self.inner.build(); + ExponentialHistogram { inner: Arc::new(Inner { name: self.name, @@ -415,24 +418,4 @@ mod tests { assert!(!histograms.contains_key("u64_exp")); } } - - #[test] - fn exponential_histograms_are_registered_on_build() { - let b1 = f64_exponential_histogram("f64_exp", 10); - let b2 = u64_exponential_histogram("u64_exp", 20); - - { - let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); - assert!(histograms.is_empty()); - } - - let _a = b1.build(); - let _bb = b2.build(); - - { - let histograms = EXPONENTIAL_HISTOGRAMS.read().unwrap(); - assert!(histograms.contains_key("f64_exp")); - assert!(histograms.contains_key("u64_exp")); - } - } } From 59503a09d68cf140709e2a1e5f788ddbf43c9099 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:45:37 -0300 Subject: [PATCH 07/13] lint --- src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index 17121bb..dc06262 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -140,7 +140,7 @@ macro_rules! metric_doc_header { (histogram, $method:ident) => { concat!("Wrapper for [`Meter::", stringify!($method), "`] using Pydantic Logfire's global meter.") }; - (exponential_histogram, $method:ident) => { + (exponential_histogram, $method:ident) => { concat!("Wrapper for [`Histogram`] using Base2ExponentialHistogram aggregation and Pydantic Logfire's global meter.") }; } From dfe5f594e84e3221f53823b67d2bb0ec98e559d6 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:46:22 -0300 Subject: [PATCH 08/13] fix doc gen --- src/metrics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index dc06262..f7fe9df 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -269,7 +269,7 @@ wrap_histogram_method!( "HISTOGRAM.record(1, &[])" ); -#[doc = make_metric_doc!(f64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] +#[doc = make_metric_doc!(f64_exponential_histogram, ExponentialHistogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] pub fn f64_exponential_histogram( name: impl Into>, scale: i8, @@ -278,7 +278,7 @@ pub fn f64_exponential_histogram( ExponentialHistogramBuilder::new(name.clone(), scale, f64_histogram(name)) } -#[doc = make_metric_doc!(u64_exponential_histogram, Histogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] +#[doc = make_metric_doc!(u64_exponential_histogram, ExponentialHistogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] pub fn u64_exponential_histogram( name: impl Into>, scale: i8, From 78994af649e9d9e222cd8ab6912299027bb37c27 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:50:47 -0300 Subject: [PATCH 09/13] fix doc --- src/usage/metrics.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/usage/metrics.rs b/src/usage/metrics.rs index 8de977d..3bd2797 100644 --- a/src/usage/metrics.rs +++ b/src/usage/metrics.rs @@ -71,6 +71,7 @@ //! ```rust //! use std::sync::LazyLock; //! use opentelemetry::metrics::Histogram; +//! use crate::metrics::ExponentialHistogram; //! //! static REQUEST_DURATION: LazyLock> = LazyLock::new(|| { //! logfire::f64_histogram("http_request_duration") @@ -86,14 +87,14 @@ //! .build() //! }); //! -//! static QUEUE_LATENCY: LazyLock> = LazyLock::new(|| { +//! static QUEUE_LATENCY: LazyLock> = LazyLock::new(|| { //! logfire::f64_exponential_histogram("queue_latency", 20) //! .with_description("Latency of items in a queue") //! .with_unit("ms") //! .build() //! }); //! -//! static MEMORY_USAGE_BYTES: LazyLock> = LazyLock::new(|| { +//! static MEMORY_USAGE_BYTES: LazyLock> = LazyLock::new(|| { //! logfire::u64_exponential_histogram("memory_usage_bytes", 20) //! .with_description("Memory usage in bytes") //! .with_unit("By") From b0288d746732c8f71647af147da58dbc2efdc64c Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:55:02 -0300 Subject: [PATCH 10/13] fix doc gen --- src/metrics.rs | 2 +- src/usage/metrics.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index f7fe9df..ad0bb53 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -166,7 +166,7 @@ macro_rules! metric_doc_create_metric { concat!( "static ", $var_name, - ": LazyLock = LazyLock::new(|| { logfire::", diff --git a/src/usage/metrics.rs b/src/usage/metrics.rs index 3bd2797..6588c2d 100644 --- a/src/usage/metrics.rs +++ b/src/usage/metrics.rs @@ -71,7 +71,7 @@ //! ```rust //! use std::sync::LazyLock; //! use opentelemetry::metrics::Histogram; -//! use crate::metrics::ExponentialHistogram; +//! use logfire::ExponentialHistogram; //! //! static REQUEST_DURATION: LazyLock> = LazyLock::new(|| { //! logfire::f64_histogram("http_request_duration") From c7f308d627783743309640c0d874f290394337e9 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:58:11 -0300 Subject: [PATCH 11/13] fix doc gen --- src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index ad0bb53..a62432b 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -166,7 +166,7 @@ macro_rules! metric_doc_create_metric { concat!( "static ", $var_name, - ": LazyLock<", + ": LazyLock = LazyLock::new(|| { logfire::", From 98cb986c08859a60a6bc8531e237ed8a3fce1a58 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 20:58:33 -0300 Subject: [PATCH 12/13] fix doc gen --- src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index a62432b..30cc85b 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -166,7 +166,7 @@ macro_rules! metric_doc_create_metric { concat!( "static ", $var_name, - ": LazyLock = LazyLock::new(|| { logfire::", From 47e422e261ebf0991967c4051ddf45b2224d96d1 Mon Sep 17 00:00:00 2001 From: PoorlyDefinedBehaviour Date: Wed, 20 Aug 2025 21:01:26 -0300 Subject: [PATCH 13/13] fix doc gen --- src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index 30cc85b..c398369 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -269,7 +269,7 @@ wrap_histogram_method!( "HISTOGRAM.record(1, &[])" ); -#[doc = make_metric_doc!(f64_exponential_histogram, ExponentialHistogram, "HISTOGRAM", "HISTOGRAM.record(1, &[])", exponential_histogram)] +#[doc = make_metric_doc!(f64_exponential_histogram, ExponentialHistogram, "HISTOGRAM", "HISTOGRAM.record(1.0, &[])", exponential_histogram)] pub fn f64_exponential_histogram( name: impl Into>, scale: i8,