From 9f330907baf8099931e0e2d76fca0338ae866c17 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 17 Oct 2023 17:31:39 +0200 Subject: [PATCH 01/33] Add experimental metrics implementation --- Cargo.lock | 259 +++++++++++++++++++++++--- sentry-core/Cargo.toml | 3 + sentry-core/src/client.rs | 27 +++ sentry-core/src/lib.rs | 2 + sentry-core/src/metrics.rs | 42 +++++ sentry-types/src/protocol/envelope.rs | 20 +- sentry/Cargo.toml | 1 + 7 files changed, 327 insertions(+), 27 deletions(-) create mode 100644 sentry-core/src/metrics.rs diff --git a/Cargo.lock b/Cargo.lock index 6bfe9e43..d646be36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -650,12 +650,34 @@ dependencies = [ "alloc-stdlib", ] +[[package]] +name = "bstr" +version = "1.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c79ad7fb2dd38f3dabd76b09c6a5a20c038fc0213ef1e9afd30eb777f120f019" +dependencies = [ + "memchr", + "serde", +] + [[package]] name = "bumpalo" version = "3.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" +[[package]] +name = "bytecount" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad152d03a2c813c80bb94fedbf3a3f02b28f793e39e7c214c8a0bcc196343de7" + +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "bytes" version = "0.5.6" @@ -677,6 +699,15 @@ dependencies = [ "bytes 1.5.0", ] +[[package]] +name = "cadence" +version = "0.29.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f39286bc075b023101dccdb79456a1334221c768b8faede0c2aff7ed29a9482d" +dependencies = [ + "crossbeam-channel", +] + [[package]] name = "cast" version = "0.3.0" @@ -699,6 +730,16 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f2c685bad3eb3d45a01354cedb7d5faa66194d1d58ba6e267a8de788f79db38" +dependencies = [ + "num-traits", + "serde", +] + [[package]] name = "ciborium" version = "0.2.1" @@ -1380,6 +1421,19 @@ version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6fb8d784f27acf97159b40fc4db5ecd8aa23b9ad5ef69cdd136d3bc80665f0c0" +[[package]] +name = "globset" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "759c97c1e17c55525b57192c06a267cda0ac5210b222d6b82189a2338fa1c13d" +dependencies = [ + "aho-corasick", + "bstr", + "fnv", + "log", + "regex", +] + [[package]] name = "gloo-timers" version = "0.2.6" @@ -1417,12 +1471,30 @@ version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" +[[package]] +name = "hash32" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47d60b12902ba28e2730cd37e95b8c9223af2808df9e902d4df49588d1470606" +dependencies = [ + "byteorder", +] + [[package]] name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +[[package]] +name = "hashbrown" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e" +dependencies = [ + "ahash", +] + [[package]] name = "hashbrown" version = "0.14.0" @@ -1849,6 +1921,15 @@ dependencies = [ "value-bag", ] +[[package]] +name = "lru" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71e7d46de488603ffdd5f30afbc64fbba2378214a2c3a2fb83abf3d33126df17" +dependencies = [ + "hashbrown 0.13.2", +] + [[package]] name = "match_cfg" version = "0.1.0" @@ -2399,6 +2480,99 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" +[[package]] +name = "relay-base-schema" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "relay-common", + "relay-protocol", + "serde", +] + +[[package]] +name = "relay-common" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "chrono", + "globset", + "lru", + "once_cell", + "parking_lot", + "regex", + "sentry-types 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde", +] + +[[package]] +name = "relay-log" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "sentry-core 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", + "tracing", +] + +[[package]] +name = "relay-metrics" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "bytecount", + "fnv", + "hash32", + "itertools", + "relay-base-schema", + "relay-common", + "relay-log", + "relay-statsd", + "relay-system", + "serde", + "serde_json", + "smallvec", + "thiserror", + "tokio", +] + +[[package]] +name = "relay-protocol" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "num-traits", + "relay-common", + "serde", + "serde_json", + "smallvec", + "unicase", + "uuid", +] + +[[package]] +name = "relay-statsd" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "cadence", + "crossbeam-channel", + "parking_lot", + "rand 0.8.5", + "relay-log", +] + +[[package]] +name = "relay-system" +version = "23.10.0" +source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" +dependencies = [ + "futures", + "once_cell", + "relay-log", + "relay-statsd", + "tokio", +] + [[package]] name = "reqwest" version = "0.11.20" @@ -2640,7 +2814,7 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "sentry" -version = "0.31.6" +version = "0.31.7" dependencies = [ "actix-web", "anyhow", @@ -2656,7 +2830,7 @@ dependencies = [ "sentry-anyhow", "sentry-backtrace", "sentry-contexts", - "sentry-core", + "sentry-core 0.31.7", "sentry-debug-images", "sentry-log", "sentry-panic", @@ -2676,62 +2850,64 @@ dependencies = [ [[package]] name = "sentry-actix" -version = "0.31.6" +version = "0.31.7" dependencies = [ "actix-web", "futures", "futures-util", "sentry", - "sentry-core", + "sentry-core 0.31.7", "tokio", ] [[package]] name = "sentry-anyhow" -version = "0.31.6" +version = "0.31.7" dependencies = [ "anyhow", "sentry", "sentry-backtrace", - "sentry-core", + "sentry-core 0.31.7", ] [[package]] name = "sentry-backtrace" -version = "0.31.6" +version = "0.31.7" dependencies = [ "backtrace", "once_cell", "regex", - "sentry-core", + "sentry-core 0.31.7", ] [[package]] name = "sentry-contexts" -version = "0.31.6" +version = "0.31.7" dependencies = [ "hostname", "libc", "os_info", "rustc_version 0.4.0", "sentry", - "sentry-core", + "sentry-core 0.31.7", "uname", ] [[package]] name = "sentry-core" -version = "0.31.6" +version = "0.31.7" dependencies = [ "anyhow", + "cadence", "criterion", "futures", "log", "once_cell", "rand 0.8.5", "rayon", + "relay-metrics", "sentry", - "sentry-types", + "sentry-types 0.31.7", "serde", "serde_json", "thiserror", @@ -2739,41 +2915,53 @@ dependencies = [ "uuid", ] +[[package]] +name = "sentry-core" +version = "0.31.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f51264e4013ed9b16558cce43917b983fa38170de2ca480349ceb57d71d6053" +dependencies = [ + "once_cell", + "sentry-types 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde", + "serde_json", +] + [[package]] name = "sentry-debug-images" -version = "0.31.6" +version = "0.31.7" dependencies = [ "findshlibs", "once_cell", - "sentry-core", + "sentry-core 0.31.7", ] [[package]] name = "sentry-log" -version = "0.31.6" +version = "0.31.7" dependencies = [ "log", "pretty_env_logger", "sentry", - "sentry-core", + "sentry-core 0.31.7", ] [[package]] name = "sentry-panic" -version = "0.31.6" +version = "0.31.7" dependencies = [ "sentry", "sentry-backtrace", - "sentry-core", + "sentry-core 0.31.7", ] [[package]] name = "sentry-slog" -version = "0.31.6" +version = "0.31.7" dependencies = [ "erased-serde", "sentry", - "sentry-core", + "sentry-core 0.31.7", "serde", "serde_json", "slog", @@ -2781,7 +2969,7 @@ dependencies = [ [[package]] name = "sentry-tower" -version = "0.31.6" +version = "0.31.7" dependencies = [ "anyhow", "axum", @@ -2790,7 +2978,7 @@ dependencies = [ "prost", "sentry", "sentry-anyhow", - "sentry-core", + "sentry-core 0.31.7", "tokio", "tonic", "tower", @@ -2801,12 +2989,12 @@ dependencies = [ [[package]] name = "sentry-tracing" -version = "0.31.6" +version = "0.31.7" dependencies = [ "log", "sentry", "sentry-backtrace", - "sentry-core", + "sentry-core 0.31.7", "tokio", "tracing", "tracing-core", @@ -2815,7 +3003,24 @@ dependencies = [ [[package]] name = "sentry-types" -version = "0.31.6" +version = "0.31.7" +dependencies = [ + "debugid", + "hex", + "rand 0.8.5", + "serde", + "serde_json", + "thiserror", + "time 0.3.28", + "url", + "uuid", +] + +[[package]] +name = "sentry-types" +version = "0.31.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e663b3eb62ddfc023c9cf5432daf5f1a4f6acb1df4d78dd80b740b32dd1a740" dependencies = [ "debugid", "hex", @@ -2992,6 +3197,9 @@ name = "smallvec" version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" +dependencies = [ + "serde", +] [[package]] name = "socket2" @@ -3313,6 +3521,7 @@ dependencies = [ "signal-hook-registry", "socket2 0.5.4", "tokio-macros", + "tracing", "windows-sys", ] diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index 8f6c3869..e9bbf3c9 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -26,11 +26,14 @@ client = ["rand"] # and macros actually expand features (and extern crate) where they are used! debug-logs = ["dep:log"] test = ["client"] +UNSTABLE_metrics = ["dep:relay-metrics", "dep:cadence"] [dependencies] +cadence = { version = "0.29.0", optional = true } log = { version = "0.4.8", optional = true, features = ["std"] } once_cell = "1" rand = { version = "0.8.1", optional = true } +relay-metrics = { git = "https://github.com/getsentry/relay", optional = true } sentry-types = { version = "0.31.7", path = "../sentry-types" } serde = { version = "1.0.104", features = ["derive"] } serde_json = { version = "1.0.46" } diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 0dd7c9d1..d94275c6 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -11,6 +11,8 @@ use sentry_types::protocol::v7::SessionUpdate; use sentry_types::random_uuid; use crate::constants::SDK_INFO; +#[cfg(feature = "UNSTABLE_metrics")] +use crate::metrics::MetricFlusher; use crate::protocol::{ClientSdkInfo, Event}; use crate::session::SessionFlusher; use crate::types::{Dsn, Uuid}; @@ -45,6 +47,8 @@ pub struct Client { options: ClientOptions, transport: TransportArc, session_flusher: RwLock>, + #[cfg(feature = "UNSTABLE_metrics")] + metrics_flusher: RwLock>, integrations: Vec<(TypeId, Arc)>, pub(crate) sdk_info: ClientSdkInfo, } @@ -65,10 +69,14 @@ impl Clone for Client { transport.clone(), self.options.session_mode, ))); + #[cfg(feature = "UNSTABLE_metrics")] + let metrics_flusher = RwLock::new(Some(MetricFlusher::new(transport.clone()))); Client { options: self.options.clone(), transport, session_flusher, + #[cfg(feature = "UNSTABLE_metrics")] + metrics_flusher, integrations: self.integrations.clone(), sdk_info: self.sdk_info.clone(), } @@ -136,10 +144,16 @@ impl Client { transport.clone(), options.session_mode, ))); + + #[cfg(feature = "UNSTABLE_metrics")] + let metrics_flusher = RwLock::new(Some(MetricFlusher::new(transport.clone()))); + Client { options, transport, session_flusher, + #[cfg(feature = "UNSTABLE_metrics")] + metrics_flusher, integrations, sdk_info, } @@ -308,11 +322,22 @@ impl Client { } } + #[cfg(feature = "UNSTABLE_metrics")] + pub(crate) fn send_metric(&self, metric: &str) { + if let Some(ref flusher) = *self.metrics_flusher.read().unwrap() { + flusher.send_metric(metric) + } + } + /// Drains all pending events without shutting down. pub fn flush(&self, timeout: Option) -> bool { if let Some(ref flusher) = *self.session_flusher.read().unwrap() { flusher.flush(); } + #[cfg(feature = "UNSTABLE_metrics")] + if let Some(ref flusher) = *self.metrics_flusher.read().unwrap() { + flusher.flush(); + } if let Some(ref transport) = *self.transport.read().unwrap() { transport.flush(timeout.unwrap_or(self.options.shutdown_timeout)) } else { @@ -329,6 +354,8 @@ impl Client { /// `shutdown_timeout` in the client options. pub fn close(&self, timeout: Option) -> bool { drop(self.session_flusher.write().unwrap().take()); + #[cfg(feature = "UNSTABLE_metrics")] + drop(self.metrics_flusher.write().unwrap().take()); let transport_opt = self.transport.write().unwrap().take(); if let Some(transport) = transport_opt { sentry_debug!("client close; request transport to shut down"); diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index aa513aa7..a9f0b3fa 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -141,6 +141,8 @@ pub use crate::transport::{Transport, TransportFactory}; mod client; #[cfg(feature = "client")] mod hub_impl; +#[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] +mod metrics; #[cfg(feature = "client")] mod session; #[cfg(feature = "client")] diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs new file mode 100644 index 00000000..edb283fb --- /dev/null +++ b/sentry-core/src/metrics.rs @@ -0,0 +1,42 @@ +use relay_metrics::{Bucket, UnixTimestamp}; + +use crate::client::TransportArc; +use crate::Client; + +pub struct SentryMetricSink { + client: Client, +} + +impl cadence::MetricSink for SentryMetricSink { + fn emit(&self, metric: &str) -> std::io::Result { + self.client.send_metric(metric); + Ok(metric.len()) + } + + fn flush(&self) -> std::io::Result<()> { + if self.client.flush(None) { + Ok(()) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::Other, + "Flushing Client failed", + )) + } + } +} + +pub struct MetricFlusher { + transport: TransportArc, +} + +impl MetricFlusher { + pub fn new(transport: TransportArc) -> Self { + Self { transport } + } + + pub fn send_metric(&self, metric: &str) { + let parsed_metric = Bucket::parse(metric.as_bytes(), UnixTimestamp::now()); + } + + pub fn flush(&self) {} +} diff --git a/sentry-types/src/protocol/envelope.rs b/sentry-types/src/protocol/envelope.rs index ba95c7a6..10db5136 100644 --- a/sentry-types/src/protocol/envelope.rs +++ b/sentry-types/src/protocol/envelope.rs @@ -4,7 +4,9 @@ use serde::Deserialize; use thiserror::Error; use uuid::Uuid; -use super::v7::{ +use super::v7 as protocol; + +use protocol::{ Attachment, AttachmentType, Event, MonitorCheckIn, SessionAggregates, SessionUpdate, Transaction, }; @@ -42,6 +44,7 @@ struct EnvelopeHeader { /// An Envelope Item Type. #[derive(Clone, Debug, Eq, PartialEq, Deserialize)] +#[non_exhaustive] enum EnvelopeItemType { /// An Event Item type. #[serde(rename = "event")] @@ -61,6 +64,10 @@ enum EnvelopeItemType { /// A Monitor Check In Item Type #[serde(rename = "check_in")] MonitorCheckIn, + /// A Metrics Item type. + #[cfg(feature = "UNSTABLE_metrics")] + #[serde(rename = "statsd")] + Metrics, } /// An Envelope Item Header. @@ -109,6 +116,9 @@ pub enum EnvelopeItem { Attachment(Attachment), /// A MonitorCheckIn item. MonitorCheckIn(MonitorCheckIn), + /// A Metrics Item. + #[cfg(feature = "UNSTABLE_metrics")] + Metrics(Vec), /// This is a sentinel item used to `filter` raw envelopes. Raw, // TODO: @@ -349,6 +359,8 @@ impl Envelope { EnvelopeItem::MonitorCheckIn(check_in) => { serde_json::to_writer(&mut item_buf, check_in)? } + #[cfg(feature = "UNSTABLE_metrics")] + EnvelopeItem::Metrics(metrics) => item_buf.extend_from_slice(metrics.as_bytes()), EnvelopeItem::Raw => { continue; } @@ -358,8 +370,10 @@ impl Envelope { EnvelopeItem::SessionUpdate(_) => "session", EnvelopeItem::SessionAggregates(_) => "sessions", EnvelopeItem::Transaction(_) => "transaction", - EnvelopeItem::Attachment(_) | EnvelopeItem::Raw => unreachable!(), EnvelopeItem::MonitorCheckIn(_) => "check_in", + #[cfg(feature = "UNSTABLE_metrics")] + EnvelopeItem::Metrics(_) => "statsd", + EnvelopeItem::Attachment(_) | EnvelopeItem::Raw => unreachable!(), }; writeln!( writer, @@ -503,6 +517,8 @@ impl Envelope { EnvelopeItemType::MonitorCheckIn => { serde_json::from_slice(payload).map(EnvelopeItem::MonitorCheckIn) } + #[cfg(feature = "UNSTABLE_metrics")] + EnvelopeItemType::Metrics => EnvelopeItem::Metrics(payload.into()), } .map_err(EnvelopeError::InvalidItemPayload)?; diff --git a/sentry/Cargo.toml b/sentry/Cargo.toml index 7aeb2d68..30a2fc8b 100644 --- a/sentry/Cargo.toml +++ b/sentry/Cargo.toml @@ -22,6 +22,7 @@ rustdoc-args = ["--cfg", "doc_cfg"] [features] default = ["backtrace", "contexts", "debug-images", "panic", "transport"] +UNSTABLE_metrics = ["sentry-core/UNSTABLE_metrics"] # default integrations backtrace = ["sentry-backtrace", "sentry-tracing?/backtrace"] From c83b9aacd1860c4bf3acf4d2a724390f728ba148 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 18 Oct 2023 14:13:42 +0200 Subject: [PATCH 02/33] finish up impl --- Cargo.lock | 223 +-------------- sentry-core/Cargo.toml | 3 +- sentry-core/src/lib.rs | 2 + sentry-core/src/metrics.rs | 375 +++++++++++++++++++++++++- sentry-types/Cargo.toml | 1 + sentry-types/src/protocol/envelope.rs | 4 +- 6 files changed, 382 insertions(+), 226 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d646be36..8c4c2bb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -650,34 +650,12 @@ dependencies = [ "alloc-stdlib", ] -[[package]] -name = "bstr" -version = "1.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c79ad7fb2dd38f3dabd76b09c6a5a20c038fc0213ef1e9afd30eb777f120f019" -dependencies = [ - "memchr", - "serde", -] - [[package]] name = "bumpalo" version = "3.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" -[[package]] -name = "bytecount" -version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad152d03a2c813c80bb94fedbf3a3f02b28f793e39e7c214c8a0bcc196343de7" - -[[package]] -name = "byteorder" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" - [[package]] name = "bytes" version = "0.5.6" @@ -730,16 +708,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "chrono" -version = "0.4.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f2c685bad3eb3d45a01354cedb7d5faa66194d1d58ba6e267a8de788f79db38" -dependencies = [ - "num-traits", - "serde", -] - [[package]] name = "ciborium" version = "0.2.1" @@ -1421,19 +1389,6 @@ version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6fb8d784f27acf97159b40fc4db5ecd8aa23b9ad5ef69cdd136d3bc80665f0c0" -[[package]] -name = "globset" -version = "0.4.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "759c97c1e17c55525b57192c06a267cda0ac5210b222d6b82189a2338fa1c13d" -dependencies = [ - "aho-corasick", - "bstr", - "fnv", - "log", - "regex", -] - [[package]] name = "gloo-timers" version = "0.2.6" @@ -1471,30 +1426,12 @@ version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" -[[package]] -name = "hash32" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47d60b12902ba28e2730cd37e95b8c9223af2808df9e902d4df49588d1470606" -dependencies = [ - "byteorder", -] - [[package]] name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" -[[package]] -name = "hashbrown" -version = "0.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e" -dependencies = [ - "ahash", -] - [[package]] name = "hashbrown" version = "0.14.0" @@ -1921,15 +1858,6 @@ dependencies = [ "value-bag", ] -[[package]] -name = "lru" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71e7d46de488603ffdd5f30afbc64fbba2378214a2c3a2fb83abf3d33126df17" -dependencies = [ - "hashbrown 0.13.2", -] - [[package]] name = "match_cfg" version = "0.1.0" @@ -2480,99 +2408,6 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" -[[package]] -name = "relay-base-schema" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "relay-common", - "relay-protocol", - "serde", -] - -[[package]] -name = "relay-common" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "chrono", - "globset", - "lru", - "once_cell", - "parking_lot", - "regex", - "sentry-types 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde", -] - -[[package]] -name = "relay-log" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "sentry-core 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", - "tracing", -] - -[[package]] -name = "relay-metrics" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "bytecount", - "fnv", - "hash32", - "itertools", - "relay-base-schema", - "relay-common", - "relay-log", - "relay-statsd", - "relay-system", - "serde", - "serde_json", - "smallvec", - "thiserror", - "tokio", -] - -[[package]] -name = "relay-protocol" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "num-traits", - "relay-common", - "serde", - "serde_json", - "smallvec", - "unicase", - "uuid", -] - -[[package]] -name = "relay-statsd" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "cadence", - "crossbeam-channel", - "parking_lot", - "rand 0.8.5", - "relay-log", -] - -[[package]] -name = "relay-system" -version = "23.10.0" -source = "git+https://github.com/getsentry/relay#e188c586f81a641a00596a7b39ff87802e835e77" -dependencies = [ - "futures", - "once_cell", - "relay-log", - "relay-statsd", - "tokio", -] - [[package]] name = "reqwest" version = "0.11.20" @@ -2830,7 +2665,7 @@ dependencies = [ "sentry-anyhow", "sentry-backtrace", "sentry-contexts", - "sentry-core 0.31.7", + "sentry-core", "sentry-debug-images", "sentry-log", "sentry-panic", @@ -2856,7 +2691,7 @@ dependencies = [ "futures", "futures-util", "sentry", - "sentry-core 0.31.7", + "sentry-core", "tokio", ] @@ -2867,7 +2702,7 @@ dependencies = [ "anyhow", "sentry", "sentry-backtrace", - "sentry-core 0.31.7", + "sentry-core", ] [[package]] @@ -2877,7 +2712,7 @@ dependencies = [ "backtrace", "once_cell", "regex", - "sentry-core 0.31.7", + "sentry-core", ] [[package]] @@ -2889,7 +2724,7 @@ dependencies = [ "os_info", "rustc_version 0.4.0", "sentry", - "sentry-core 0.31.7", + "sentry-core", "uname", ] @@ -2905,9 +2740,8 @@ dependencies = [ "once_cell", "rand 0.8.5", "rayon", - "relay-metrics", "sentry", - "sentry-types 0.31.7", + "sentry-types", "serde", "serde_json", "thiserror", @@ -2915,25 +2749,13 @@ dependencies = [ "uuid", ] -[[package]] -name = "sentry-core" -version = "0.31.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f51264e4013ed9b16558cce43917b983fa38170de2ca480349ceb57d71d6053" -dependencies = [ - "once_cell", - "sentry-types 0.31.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde", - "serde_json", -] - [[package]] name = "sentry-debug-images" version = "0.31.7" dependencies = [ "findshlibs", "once_cell", - "sentry-core 0.31.7", + "sentry-core", ] [[package]] @@ -2943,7 +2765,7 @@ dependencies = [ "log", "pretty_env_logger", "sentry", - "sentry-core 0.31.7", + "sentry-core", ] [[package]] @@ -2952,7 +2774,7 @@ version = "0.31.7" dependencies = [ "sentry", "sentry-backtrace", - "sentry-core 0.31.7", + "sentry-core", ] [[package]] @@ -2961,7 +2783,7 @@ version = "0.31.7" dependencies = [ "erased-serde", "sentry", - "sentry-core 0.31.7", + "sentry-core", "serde", "serde_json", "slog", @@ -2978,7 +2800,7 @@ dependencies = [ "prost", "sentry", "sentry-anyhow", - "sentry-core 0.31.7", + "sentry-core", "tokio", "tonic", "tower", @@ -2994,7 +2816,7 @@ dependencies = [ "log", "sentry", "sentry-backtrace", - "sentry-core 0.31.7", + "sentry-core", "tokio", "tracing", "tracing-core", @@ -3016,23 +2838,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "sentry-types" -version = "0.31.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e663b3eb62ddfc023c9cf5432daf5f1a4f6acb1df4d78dd80b740b32dd1a740" -dependencies = [ - "debugid", - "hex", - "rand 0.8.5", - "serde", - "serde_json", - "thiserror", - "time 0.3.28", - "url", - "uuid", -] - [[package]] name = "serde" version = "1.0.188" @@ -3197,9 +3002,6 @@ name = "smallvec" version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" -dependencies = [ - "serde", -] [[package]] name = "socket2" @@ -3521,7 +3323,6 @@ dependencies = [ "signal-hook-registry", "socket2 0.5.4", "tokio-macros", - "tracing", "windows-sys", ] diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index e9bbf3c9..07d8fd8e 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -26,14 +26,13 @@ client = ["rand"] # and macros actually expand features (and extern crate) where they are used! debug-logs = ["dep:log"] test = ["client"] -UNSTABLE_metrics = ["dep:relay-metrics", "dep:cadence"] +UNSTABLE_metrics = ["dep:cadence", "sentry-types/UNSTABLE_metrics"] [dependencies] cadence = { version = "0.29.0", optional = true } log = { version = "0.4.8", optional = true, features = ["std"] } once_cell = "1" rand = { version = "0.8.1", optional = true } -relay-metrics = { git = "https://github.com/getsentry/relay", optional = true } sentry-types = { version = "0.31.7", path = "../sentry-types" } serde = { version = "1.0.104", features = ["derive"] } serde_json = { version = "1.0.46" } diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index a9f0b3fa..c9026cff 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -147,6 +147,8 @@ mod metrics; mod session; #[cfg(feature = "client")] pub use crate::client::Client; +#[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] +pub use crate::metrics::SentryMetricSink; // test utilities #[cfg(feature = "test")] diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index edb283fb..a6039218 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,21 +1,47 @@ -use relay_metrics::{Bucket, UnixTimestamp}; +use std::collections::btree_map::Entry; +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::{sync_channel, Receiver, RecvTimeoutError, SyncSender}; +use std::sync::Arc; +use std::thread::{self, JoinHandle}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +use cadence::MetricSink; +use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; -use crate::Client; +use crate::{Client, Hub}; + +#[derive(Debug)] +pub struct SentryMetricSink { + client: Arc, + sink: S, +} -pub struct SentryMetricSink { - client: Client, +impl SentryMetricSink { + pub fn try_new(sink: S) -> Result { + let hub = Hub::current(); + let Some(client) = hub.client() else { + return Err(sink); + }; + + Ok(Self { client, sink }) + } } -impl cadence::MetricSink for SentryMetricSink { +impl MetricSink for SentryMetricSink +where + S: MetricSink, +{ fn emit(&self, metric: &str) -> std::io::Result { self.client.send_metric(metric); - Ok(metric.len()) + self.sink.emit(metric) } fn flush(&self) -> std::io::Result<()> { if self.client.flush(None) { - Ok(()) + self.sink.flush() } else { Err(std::io::Error::new( std::io::ErrorKind::Other, @@ -25,18 +51,345 @@ impl cadence::MetricSink for SentryMetricSink { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +enum MetricType { + Counter, + Distribution, + Set, + Gauge, +} + +impl MetricType { + /// Return the shortcode for this metric type. + pub fn as_str(&self) -> &'static str { + match self { + MetricType::Counter => "c", + MetricType::Distribution => "d", + MetricType::Set => "s", + MetricType::Gauge => "g", + } + } +} + +impl fmt::Display for MetricType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl std::str::FromStr for MetricType { + type Err = (); + + fn from_str(s: &str) -> Result { + Ok(match s { + "c" | "m" => Self::Counter, + "h" | "d" | "ms" => Self::Distribution, + "s" => Self::Set, + "g" => Self::Gauge, + _ => return Err(()), + }) + } +} + +struct GaugeValue { + last: f64, + min: f64, + max: f64, + sum: f64, + count: u64, +} +enum BucketValue { + Counter(f64), + Distribution(Vec), + Set(BTreeSet), + Gauge(GaugeValue), +} +impl BucketValue { + fn distribution(val: f64) -> BucketValue { + Self::Distribution(vec![val]) + } + + fn gauge(val: f64) -> BucketValue { + Self::Gauge(GaugeValue { + last: val, + min: val, + max: val, + sum: val, + count: 1, + }) + } + + fn set_from_str(value: &str) -> BucketValue { + todo!() + } + + fn merge(&mut self, other: BucketValue) -> Result<(), ()> { + match (self, other) { + (BucketValue::Counter(c1), BucketValue::Counter(c2)) => { + *c1 += c2; + } + (BucketValue::Distribution(d1), BucketValue::Distribution(d2)) => { + d1.extend(d2); + } + (BucketValue::Set(s1), BucketValue::Set(s2)) => s1.extend(s2), + (BucketValue::Gauge(g1), BucketValue::Gauge(g2)) => { + g1.last = g2.last; + g1.min = g1.min.min(g2.min); + g1.max = g1.max.max(g2.max); + g1.sum += g2.sum; + g1.count += g2.count; + } + _ => return Err(()), + } + Ok(()) + } +} + +#[derive(PartialEq, Eq, PartialOrd, Ord)] +struct BucketKey { + timestamp: u64, + ty: MetricType, + name: String, + tags: String, +} + +type AggregateMetrics = BTreeMap; + +enum Task { + SendMetrics((BucketKey, BucketValue)), + Flush, + Shutdown, +} + pub struct MetricFlusher { - transport: TransportArc, + sender: SyncSender, + shutdown: Arc, + handle: Option>, } +const FLUSH_INTERVAL: Duration = Duration::from_secs(10); + impl MetricFlusher { pub fn new(transport: TransportArc) -> Self { - Self { transport } + let (sender, receiver) = sync_channel(30); + let shutdown = Arc::new(AtomicBool::new(false)); + let shutdown_worker = shutdown.clone(); + let handle = thread::Builder::new() + .name("sentry-metrics".into()) + .spawn(move || Self::worker_thread(receiver, shutdown_worker, transport)) + .ok(); + + Self { + sender, + shutdown, + handle, + } } pub fn send_metric(&self, metric: &str) { - let parsed_metric = Bucket::parse(metric.as_bytes(), UnixTimestamp::now()); + fn mk_value(ty: MetricType, value: &str) -> Option { + Some(match ty { + MetricType::Counter => BucketValue::Counter(value.parse().ok()?), + MetricType::Distribution => BucketValue::distribution(value.parse().ok()?), + MetricType::Set => BucketValue::set_from_str(value), + MetricType::Gauge => BucketValue::gauge(value.parse().ok()?), + }) + } + + fn parse(metric: &str) -> Option<(BucketKey, BucketValue)> { + let mut components = metric.split('|'); + let mut values = components.next()?.split(':'); + let name = values.next()?; + + let ty: MetricType = components.next().and_then(|s| s.parse().ok())?; + let mut value = mk_value(ty, values.next()?)?; + + for value_s in values { + value.merge(mk_value(ty, value_s)?).ok()?; + } + + let mut tags = ""; + let mut timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + for component in components { + if let Some(component_tags) = component.strip_prefix('#') { + tags = component_tags; + } else if let Some(component_timestamp) = component.strip_prefix('T') { + timestamp = component_timestamp.parse().ok()?; + } + } + + Some(( + BucketKey { + timestamp, + ty, + name: name.into(), + tags: tags.into(), + }, + value, + )) + } + + if let Some(parsed_metric) = parse(metric) { + let _ = self.sender.send(Task::SendMetrics(parsed_metric)); + } + } + + pub fn flush(&self) { + let _ = self.sender.send(Task::Flush); + } + + fn worker_thread(receiver: Receiver, shutdown: Arc, transport: TransportArc) { + let mut buckets = AggregateMetrics::new(); + let mut last_flush = Instant::now(); + + loop { + if shutdown.load(Ordering::SeqCst) { + Self::flush_buckets(buckets, &transport); + return; + } + + let timeout = FLUSH_INTERVAL + .checked_sub(last_flush.elapsed()) + .unwrap_or_else(|| Duration::from_secs(0)); + + match receiver.recv_timeout(timeout) { + Err(RecvTimeoutError::Timeout) | Ok(Task::Flush) => { + // flush + Self::flush_buckets(std::mem::take(&mut buckets), &transport); + last_flush = Instant::now(); + } + Ok(Task::SendMetrics((mut key, value))) => { + // aggregate + let rounding_interval = FLUSH_INTERVAL.as_secs(); + let rounded_timestamp = (key.timestamp / rounding_interval) * rounding_interval; + + key.timestamp = rounded_timestamp; + + match buckets.entry(key) { + Entry::Occupied(mut entry) => { + let _ = entry.get_mut().merge(value); + } + Entry::Vacant(entry) => { + entry.insert(value); + } + } + } + _ => { + // shutdown + Self::flush_buckets(buckets, &transport); + return; + } + } + } + } + + fn flush_buckets(buckets: AggregateMetrics, transport: &TransportArc) { + fn format_payload(buckets: AggregateMetrics) -> std::io::Result> { + use std::io::Write; + let mut out = vec![]; + for (key, value) in buckets { + write!(&mut out, "{}", key.name)?; + + match value { + BucketValue::Counter(c) => { + write!(&mut out, ":{}", c)?; + } + BucketValue::Distribution(d) => { + for v in d { + write!(&mut out, ":{}", v)?; + } + } + BucketValue::Set(s) => { + for v in s { + write!(&mut out, ":{}", v)?; + } + } + BucketValue::Gauge(g) => { + write!( + &mut out, + ":{}:{}:{}:{}:{}", + g.last, g.min, g.max, g.sum, g.count + )?; + } + } + + write!(&mut out, "|{}", key.ty.as_str())?; + if !key.tags.is_empty() { + write!(&mut out, "|#{}", key.tags)?; + } + writeln!(&mut out, "|T{}", key.timestamp)?; + } + + Ok(out) + } + + let Ok(output) = format_payload(buckets) else { + return; + }; + + let mut envelope = Envelope::new(); + envelope.add_item(EnvelopeItem::Metrics(output)); + + if let Some(ref transport) = *transport.read().unwrap() { + transport.send_envelope(envelope); + } } +} + +impl Drop for MetricFlusher { + fn drop(&mut self) { + self.shutdown.store(true, Ordering::SeqCst); + let _ = self.sender.send(Task::Shutdown); + if let Some(handle) = self.handle.take() { + handle.join().unwrap(); + } + } +} + +#[cfg(test)] +mod tests { + use std::str::from_utf8; - pub fn flush(&self) {} + use cadence::{Counted, Distributed}; + + use crate::test::with_captured_envelopes; + + use super::*; + + #[test] + fn test_basic_metrics() { + let envelopes = with_captured_envelopes(|| { + let sink = SentryMetricSink::try_new(cadence::NopMetricSink).unwrap(); + + let client = cadence::StatsdClient::from_sink("sentry.test", sink); + client.count("some.count", 1).unwrap(); + client.count("some.count", 10).unwrap(); + client + .count_with_tags("count.with.tags", 1) + .with_tag("foo", "bar") + .send(); + client.distribution("some.distr", 1).unwrap(); + client.distribution("some.distr", 2).unwrap(); + client.distribution("some.distr", 3).unwrap(); + }); + assert_eq!(envelopes.len(), 1); + + let mut items = envelopes[0].items(); + if let Some(EnvelopeItem::Metrics(metrics)) = items.next() { + let metrics = from_utf8(metrics).unwrap(); + + println!("{metrics}"); + + assert!(metrics.contains("sentry.test.count.with.tags:1|c|#foo:bar|T")); + assert!(metrics.contains("sentry.test.some.count:11|c|T")); + assert!(metrics.contains("sentry.test.some.distr:1:2:3|d|T")); + } else { + panic!("expected metrics"); + } + assert_eq!(items.next(), None); + } } diff --git a/sentry-types/Cargo.toml b/sentry-types/Cargo.toml index 9638f352..06618f31 100644 --- a/sentry-types/Cargo.toml +++ b/sentry-types/Cargo.toml @@ -19,6 +19,7 @@ all-features = true [features] default = ["protocol"] protocol = [] +UNSTABLE_metrics = [] [dependencies] debugid = { version = "0.8.0", features = ["serde"] } diff --git a/sentry-types/src/protocol/envelope.rs b/sentry-types/src/protocol/envelope.rs index 10db5136..cb90e14c 100644 --- a/sentry-types/src/protocol/envelope.rs +++ b/sentry-types/src/protocol/envelope.rs @@ -360,7 +360,7 @@ impl Envelope { serde_json::to_writer(&mut item_buf, check_in)? } #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItem::Metrics(metrics) => item_buf.extend_from_slice(metrics.as_bytes()), + EnvelopeItem::Metrics(metrics) => item_buf.extend_from_slice(metrics), EnvelopeItem::Raw => { continue; } @@ -518,7 +518,7 @@ impl Envelope { serde_json::from_slice(payload).map(EnvelopeItem::MonitorCheckIn) } #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItemType::Metrics => EnvelopeItem::Metrics(payload.into()), + EnvelopeItemType::Metrics => Ok(EnvelopeItem::Metrics(payload.into())), } .map_err(EnvelopeError::InvalidItemPayload)?; From dd22afb10b547077925cd59e2a957b43084ba571 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 18 Oct 2023 14:20:25 +0200 Subject: [PATCH 03/33] implement sets the naive way --- sentry-core/src/metrics.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index a6039218..297db745 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -13,6 +13,10 @@ use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; use crate::{Client, Hub}; +/// A [`cadence`] compatible [`MetricSink`]. +/// +/// This will ingest all the emitted metrics to Sentry as well as forward them +/// to the inner [`MetricSink`]. #[derive(Debug)] pub struct SentryMetricSink { client: Arc, @@ -20,6 +24,7 @@ pub struct SentryMetricSink { } impl SentryMetricSink { + /// Creates a new [`SentryMetricSink`], wrapping the given [`MetricSink`]. pub fn try_new(sink: S) -> Result { let hub = Hub::current(); let Some(client) = hub.client() else { @@ -101,7 +106,7 @@ struct GaugeValue { enum BucketValue { Counter(f64), Distribution(Vec), - Set(BTreeSet), + Set(BTreeSet), Gauge(GaugeValue), } impl BucketValue { @@ -120,7 +125,7 @@ impl BucketValue { } fn set_from_str(value: &str) -> BucketValue { - todo!() + BucketValue::Set([value.into()].into()) } fn merge(&mut self, other: BucketValue) -> Result<(), ()> { From 2fc55aefaa283e3c998e180451e6b85255d91b83 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 18 Oct 2023 14:48:52 +0200 Subject: [PATCH 04/33] do not emit an empty envelope item on shutdown --- sentry-core/src/metrics.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 297db745..44c77cf0 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -293,6 +293,10 @@ impl MetricFlusher { } fn flush_buckets(buckets: AggregateMetrics, transport: &TransportArc) { + if buckets.is_empty() { + return; + } + fn format_payload(buckets: AggregateMetrics) -> std::io::Result> { use std::io::Write; let mut out = vec![]; From 97c9a0cac63a5096406f3c7257207f4ea006e18d Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 20 Oct 2023 10:04:22 +0200 Subject: [PATCH 05/33] review --- sentry-core/src/metrics.rs | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 44c77cf0..facf1c6a 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -161,7 +161,7 @@ struct BucketKey { type AggregateMetrics = BTreeMap; enum Task { - SendMetrics((BucketKey, BucketValue)), + RecordMetric((BucketKey, BucketValue)), Flush, Shutdown, } @@ -239,7 +239,7 @@ impl MetricFlusher { } if let Some(parsed_metric) = parse(metric) { - let _ = self.sender.send(Task::SendMetrics(parsed_metric)); + let _ = self.sender.send(Task::RecordMetric(parsed_metric)); } } @@ -257,9 +257,7 @@ impl MetricFlusher { return; } - let timeout = FLUSH_INTERVAL - .checked_sub(last_flush.elapsed()) - .unwrap_or_else(|| Duration::from_secs(0)); + let timeout = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); match receiver.recv_timeout(timeout) { Err(RecvTimeoutError::Timeout) | Ok(Task::Flush) => { @@ -267,7 +265,7 @@ impl MetricFlusher { Self::flush_buckets(std::mem::take(&mut buckets), &transport); last_flush = Instant::now(); } - Ok(Task::SendMetrics((mut key, value))) => { + Ok(Task::RecordMetric((mut key, value))) => { // aggregate let rounding_interval = FLUSH_INTERVAL.as_secs(); let rounded_timestamp = (key.timestamp / rounding_interval) * rounding_interval; @@ -388,17 +386,16 @@ mod tests { assert_eq!(envelopes.len(), 1); let mut items = envelopes[0].items(); - if let Some(EnvelopeItem::Metrics(metrics)) = items.next() { - let metrics = from_utf8(metrics).unwrap(); + let Some(EnvelopeItem::Metrics(metrics)) = items.next() else { + panic!("expected metrics"); + }; + let metrics = from_utf8(metrics).unwrap(); - println!("{metrics}"); + println!("{metrics}"); - assert!(metrics.contains("sentry.test.count.with.tags:1|c|#foo:bar|T")); - assert!(metrics.contains("sentry.test.some.count:11|c|T")); - assert!(metrics.contains("sentry.test.some.distr:1:2:3|d|T")); - } else { - panic!("expected metrics"); - } + assert!(metrics.contains("sentry.test.count.with.tags:1|c|#foo:bar|T")); + assert!(metrics.contains("sentry.test.some.count:11|c|T")); + assert!(metrics.contains("sentry.test.some.distr:1:2:3|d|T")); assert_eq!(items.next(), None); } } From b6bdece4717704d3eb4575de28d737a153729c3a Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 10:04:58 +0200 Subject: [PATCH 06/33] add rate limit category --- sentry/src/transports/ratelimit.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sentry/src/transports/ratelimit.rs b/sentry/src/transports/ratelimit.rs index 0aa36bb7..d2bea824 100644 --- a/sentry/src/transports/ratelimit.rs +++ b/sentry/src/transports/ratelimit.rs @@ -12,6 +12,7 @@ pub struct RateLimiter { session: Option, transaction: Option, attachment: Option, + statsd: Option, } impl RateLimiter { @@ -56,6 +57,7 @@ impl RateLimiter { "session" => self.session = new_time, "transaction" => self.transaction = new_time, "attachment" => self.attachment = new_time, + "statsd" => self.statsd = new_time, _ => {} } } @@ -89,6 +91,7 @@ impl RateLimiter { RateLimitingCategory::Session => self.session, RateLimitingCategory::Transaction => self.transaction, RateLimitingCategory::Attachment => self.attachment, + RateLimitingCategory::Statsd => self.statsd, }?; time_left.duration_since(SystemTime::now()).ok() } @@ -112,6 +115,7 @@ impl RateLimiter { } EnvelopeItem::Transaction(_) => RateLimitingCategory::Transaction, EnvelopeItem::Attachment(_) => RateLimitingCategory::Attachment, + EnvelopeItem::Statsd(_) => RateLimitingCategory::Statsd, _ => RateLimitingCategory::Any, }) }) @@ -131,6 +135,8 @@ pub enum RateLimitingCategory { Transaction, /// Rate Limit pertaining to Attachments. Attachment, + /// Rate Limit pertaining to metrics. + Statsd, } #[cfg(test)] From 331a4573009d28328f0b9969df5e29541c288e2e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 10:07:44 +0200 Subject: [PATCH 07/33] fix typo --- sentry/src/transports/ratelimit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/transports/ratelimit.rs b/sentry/src/transports/ratelimit.rs index d2bea824..6614e293 100644 --- a/sentry/src/transports/ratelimit.rs +++ b/sentry/src/transports/ratelimit.rs @@ -115,7 +115,7 @@ impl RateLimiter { } EnvelopeItem::Transaction(_) => RateLimitingCategory::Transaction, EnvelopeItem::Attachment(_) => RateLimitingCategory::Attachment, - EnvelopeItem::Statsd(_) => RateLimitingCategory::Statsd, + EnvelopeItem::Metrics(_) => RateLimitingCategory::Statsd, _ => RateLimitingCategory::Any, }) }) From a61de488153f642b551b7782b8bf3a41dd58678e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 11:27:43 +0200 Subject: [PATCH 08/33] fix build --- sentry/src/transports/ratelimit.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry/src/transports/ratelimit.rs b/sentry/src/transports/ratelimit.rs index 6614e293..74632d46 100644 --- a/sentry/src/transports/ratelimit.rs +++ b/sentry/src/transports/ratelimit.rs @@ -115,6 +115,7 @@ impl RateLimiter { } EnvelopeItem::Transaction(_) => RateLimitingCategory::Transaction, EnvelopeItem::Attachment(_) => RateLimitingCategory::Attachment, + #[cfg(feature = "UNSTABLE_metrics")] EnvelopeItem::Metrics(_) => RateLimitingCategory::Statsd, _ => RateLimitingCategory::Any, }) @@ -136,6 +137,7 @@ pub enum RateLimitingCategory { /// Rate Limit pertaining to Attachments. Attachment, /// Rate Limit pertaining to metrics. + #[allow(unused)] Statsd, } From 06c483abe462d999179d6c8ade0b202e00fa847a Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 12:47:17 +0200 Subject: [PATCH 09/33] try to fix flush race on windows --- sentry-core/src/metrics.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index facf1c6a..3333dfa8 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -252,11 +252,6 @@ impl MetricFlusher { let mut last_flush = Instant::now(); loop { - if shutdown.load(Ordering::SeqCst) { - Self::flush_buckets(buckets, &transport); - return; - } - let timeout = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); match receiver.recv_timeout(timeout) { @@ -287,6 +282,11 @@ impl MetricFlusher { return; } } + + if shutdown.load(Ordering::SeqCst) { + Self::flush_buckets(buckets, &transport); + return; + } } } From c6920480ba6c368a9acd0c6e9d6e68f5e18ef24e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 12:47:59 +0200 Subject: [PATCH 10/33] fix doc builds --- sentry-core/src/hub.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-core/src/hub.rs b/sentry-core/src/hub.rs index 775f9e05..ad4b8b14 100644 --- a/sentry-core/src/hub.rs +++ b/sentry-core/src/hub.rs @@ -11,7 +11,7 @@ use crate::{Integration, IntoBreadcrumbs, Scope, ScopeGuard}; /// The central object that can manages scopes and clients. /// /// This can be used to capture events and manage the scope. This object is -/// [`Send`][std::marker::Send] and [`Sync`][std::marker::Sync] so it can be used from +/// [`Send`] and [`Sync`] so it can be used from /// multiple threads if needed. /// /// Each thread has its own thread-local ( see [`Hub::current`]) hub, which is From b568ebc7bcf47cc13e28689495afc528ce7610ba Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 23 Oct 2023 12:58:57 +0200 Subject: [PATCH 11/33] remove forced shutdown from metrics aggregator --- sentry-core/src/metrics.rs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 3333dfa8..9c37c781 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,7 +1,6 @@ use std::collections::btree_map::Entry; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{sync_channel, Receiver, RecvTimeoutError, SyncSender}; use std::sync::Arc; use std::thread::{self, JoinHandle}; @@ -168,7 +167,6 @@ enum Task { pub struct MetricFlusher { sender: SyncSender, - shutdown: Arc, handle: Option>, } @@ -177,18 +175,12 @@ const FLUSH_INTERVAL: Duration = Duration::from_secs(10); impl MetricFlusher { pub fn new(transport: TransportArc) -> Self { let (sender, receiver) = sync_channel(30); - let shutdown = Arc::new(AtomicBool::new(false)); - let shutdown_worker = shutdown.clone(); let handle = thread::Builder::new() .name("sentry-metrics".into()) - .spawn(move || Self::worker_thread(receiver, shutdown_worker, transport)) + .spawn(move || Self::worker_thread(receiver, transport)) .ok(); - Self { - sender, - shutdown, - handle, - } + Self { sender, handle } } pub fn send_metric(&self, metric: &str) { @@ -247,7 +239,7 @@ impl MetricFlusher { let _ = self.sender.send(Task::Flush); } - fn worker_thread(receiver: Receiver, shutdown: Arc, transport: TransportArc) { + fn worker_thread(receiver: Receiver, transport: TransportArc) { let mut buckets = AggregateMetrics::new(); let mut last_flush = Instant::now(); @@ -282,11 +274,6 @@ impl MetricFlusher { return; } } - - if shutdown.load(Ordering::SeqCst) { - Self::flush_buckets(buckets, &transport); - return; - } } } @@ -349,7 +336,6 @@ impl MetricFlusher { impl Drop for MetricFlusher { fn drop(&mut self) { - self.shutdown.store(true, Ordering::SeqCst); let _ = self.sender.send(Task::Shutdown); if let Some(handle) = self.handle.take() { handle.join().unwrap(); From dc5ea74e3a6b28b97e78bffe5991063dd9d9ffd5 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 13:16:16 +0100 Subject: [PATCH 12/33] ref(metrics): Minor review comments --- sentry-core/src/client.rs | 22 +++++++++++----------- sentry-core/src/metrics.rs | 13 +++++++------ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index d94275c6..8c674b01 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -12,7 +12,7 @@ use sentry_types::random_uuid; use crate::constants::SDK_INFO; #[cfg(feature = "UNSTABLE_metrics")] -use crate::metrics::MetricFlusher; +use crate::metrics::MetricAggregator; use crate::protocol::{ClientSdkInfo, Event}; use crate::session::SessionFlusher; use crate::types::{Dsn, Uuid}; @@ -48,7 +48,7 @@ pub struct Client { transport: TransportArc, session_flusher: RwLock>, #[cfg(feature = "UNSTABLE_metrics")] - metrics_flusher: RwLock>, + metric_aggregator: RwLock>, integrations: Vec<(TypeId, Arc)>, pub(crate) sdk_info: ClientSdkInfo, } @@ -70,13 +70,13 @@ impl Clone for Client { self.options.session_mode, ))); #[cfg(feature = "UNSTABLE_metrics")] - let metrics_flusher = RwLock::new(Some(MetricFlusher::new(transport.clone()))); + let metric_aggregator = RwLock::new(Some(MetricAggregator::new(transport.clone()))); Client { options: self.options.clone(), transport, session_flusher, #[cfg(feature = "UNSTABLE_metrics")] - metrics_flusher, + metric_aggregator, integrations: self.integrations.clone(), sdk_info: self.sdk_info.clone(), } @@ -146,14 +146,14 @@ impl Client { ))); #[cfg(feature = "UNSTABLE_metrics")] - let metrics_flusher = RwLock::new(Some(MetricFlusher::new(transport.clone()))); + let metric_aggregator = RwLock::new(Some(MetricAggregator::new(transport.clone()))); Client { options, transport, session_flusher, #[cfg(feature = "UNSTABLE_metrics")] - metrics_flusher, + metric_aggregator, integrations, sdk_info, } @@ -323,9 +323,9 @@ impl Client { } #[cfg(feature = "UNSTABLE_metrics")] - pub(crate) fn send_metric(&self, metric: &str) { - if let Some(ref flusher) = *self.metrics_flusher.read().unwrap() { - flusher.send_metric(metric) + pub(crate) fn add_metric(&self, metric: &str) { + if let Some(ref aggregator) = *self.metric_aggregator.read().unwrap() { + aggregator.add(metric) } } @@ -335,7 +335,7 @@ impl Client { flusher.flush(); } #[cfg(feature = "UNSTABLE_metrics")] - if let Some(ref flusher) = *self.metrics_flusher.read().unwrap() { + if let Some(ref flusher) = *self.metric_aggregator.read().unwrap() { flusher.flush(); } if let Some(ref transport) = *self.transport.read().unwrap() { @@ -355,7 +355,7 @@ impl Client { pub fn close(&self, timeout: Option) -> bool { drop(self.session_flusher.write().unwrap().take()); #[cfg(feature = "UNSTABLE_metrics")] - drop(self.metrics_flusher.write().unwrap().take()); + drop(self.metric_aggregator.write().unwrap().take()); let transport_opt = self.transport.write().unwrap().take(); if let Some(transport) = transport_opt { sentry_debug!("client close; request transport to shut down"); diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 9c37c781..78b0ce03 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -39,7 +39,7 @@ where S: MetricSink, { fn emit(&self, metric: &str) -> std::io::Result { - self.client.send_metric(metric); + self.client.add_metric(metric); self.sink.emit(metric) } @@ -165,14 +165,15 @@ enum Task { Shutdown, } -pub struct MetricFlusher { +pub struct MetricAggregator { sender: SyncSender, handle: Option>, } +const BUCKET_INTERVAL: Duration = Duration::from_secs(10); const FLUSH_INTERVAL: Duration = Duration::from_secs(10); -impl MetricFlusher { +impl MetricAggregator { pub fn new(transport: TransportArc) -> Self { let (sender, receiver) = sync_channel(30); let handle = thread::Builder::new() @@ -183,7 +184,7 @@ impl MetricFlusher { Self { sender, handle } } - pub fn send_metric(&self, metric: &str) { + pub fn add(&self, metric: &str) { fn mk_value(ty: MetricType, value: &str) -> Option { Some(match ty { MetricType::Counter => BucketValue::Counter(value.parse().ok()?), @@ -254,7 +255,7 @@ impl MetricFlusher { } Ok(Task::RecordMetric((mut key, value))) => { // aggregate - let rounding_interval = FLUSH_INTERVAL.as_secs(); + let rounding_interval = BUCKET_INTERVAL.as_secs(); let rounded_timestamp = (key.timestamp / rounding_interval) * rounding_interval; key.timestamp = rounded_timestamp; @@ -334,7 +335,7 @@ impl MetricFlusher { } } -impl Drop for MetricFlusher { +impl Drop for MetricAggregator { fn drop(&mut self) { let _ = self.sender.send(Task::Shutdown); if let Some(handle) = self.handle.take() { From f2c641089bdb16a4d889e744eb160759b108ebbf Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 13:22:43 +0100 Subject: [PATCH 13/33] ref: Separate cadence from metrics feature --- sentry-core/Cargo.toml | 3 +- sentry-core/src/cadence.rs | 90 ++++++++++++++++++++++++++++++++++ sentry-core/src/lib.rs | 6 ++- sentry-core/src/metrics.rs | 99 ++------------------------------------ 4 files changed, 101 insertions(+), 97 deletions(-) create mode 100644 sentry-core/src/cadence.rs diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index b31b72ff..8095cc46 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -26,7 +26,8 @@ client = ["rand"] # and macros actually expand features (and extern crate) where they are used! debug-logs = ["dep:log"] test = ["client"] -UNSTABLE_metrics = ["dep:cadence", "sentry-types/UNSTABLE_metrics"] +UNSTABLE_metrics = ["sentry-types/UNSTABLE_metrics"] +UNSTABLE_cadence = ["dep:cadence", "UNSTABLE_metrics"] [dependencies] cadence = { version = "0.29.0", optional = true } diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs new file mode 100644 index 00000000..10304ac3 --- /dev/null +++ b/sentry-core/src/cadence.rs @@ -0,0 +1,90 @@ +use std::sync::Arc; + +use cadence::MetricSink; + +use crate::{Client, Hub}; + +/// A [`cadence`] compatible [`MetricSink`]. +/// +/// This will ingest all the emitted metrics to Sentry as well as forward them +/// to the inner [`MetricSink`]. +#[derive(Debug)] +pub struct SentryMetricSink { + client: Arc, + sink: S, +} + +impl SentryMetricSink { + /// Creates a new [`SentryMetricSink`], wrapping the given [`MetricSink`]. + pub fn try_new(sink: S) -> Result { + let hub = Hub::current(); + let Some(client) = hub.client() else { + return Err(sink); + }; + + Ok(Self { client, sink }) + } +} + +impl MetricSink for SentryMetricSink +where + S: MetricSink, +{ + fn emit(&self, metric: &str) -> std::io::Result { + self.client.add_metric(metric); + self.sink.emit(metric) + } + + fn flush(&self) -> std::io::Result<()> { + if self.client.flush(None) { + self.sink.flush() + } else { + Err(std::io::Error::new( + std::io::ErrorKind::Other, + "Flushing Client failed", + )) + } + } +} + +#[cfg(test)] +mod tests { + use cadence::{Counted, Distributed}; + use sentry_types::protocol::latest::EnvelopeItem; + + use crate::test::with_captured_envelopes; + + use super::*; + + #[test] + fn test_basic_metrics() { + let envelopes = with_captured_envelopes(|| { + let sink = SentryMetricSink::try_new(cadence::NopMetricSink).unwrap(); + + let client = cadence::StatsdClient::from_sink("sentry.test", sink); + client.count("some.count", 1).unwrap(); + client.count("some.count", 10).unwrap(); + client + .count_with_tags("count.with.tags", 1) + .with_tag("foo", "bar") + .send(); + client.distribution("some.distr", 1).unwrap(); + client.distribution("some.distr", 2).unwrap(); + client.distribution("some.distr", 3).unwrap(); + }); + assert_eq!(envelopes.len(), 1); + + let mut items = envelopes[0].items(); + let Some(EnvelopeItem::Metrics(metrics)) = items.next() else { + panic!("expected metrics"); + }; + let metrics = std::str::from_utf8(metrics).unwrap(); + + println!("{metrics}"); + + assert!(metrics.contains("sentry.test.count.with.tags:1|c|#foo:bar|T")); + assert!(metrics.contains("sentry.test.some.count:11|c|T")); + assert!(metrics.contains("sentry.test.some.distr:1:2:3|d|T")); + assert_eq!(items.next(), None); + } +} diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index c9026cff..a1a1afdb 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -136,6 +136,8 @@ pub use crate::performance::*; pub use crate::scope::{Scope, ScopeGuard}; pub use crate::transport::{Transport, TransportFactory}; +#[cfg(all(feature = "client", feature = "UNSTABLE_cadence"))] +mod cadence; // client feature #[cfg(feature = "client")] mod client; @@ -145,10 +147,10 @@ mod hub_impl; mod metrics; #[cfg(feature = "client")] mod session; +#[cfg(all(feature = "client", feature = "UNSTABLE_cadence"))] +pub use crate::cadence::SentryMetricSink; #[cfg(feature = "client")] pub use crate::client::Client; -#[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] -pub use crate::metrics::SentryMetricSink; // test utilities #[cfg(feature = "test")] diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 78b0ce03..682ab695 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,59 +1,13 @@ use std::collections::btree_map::Entry; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; -use std::sync::mpsc::{sync_channel, Receiver, RecvTimeoutError, SyncSender}; -use std::sync::Arc; +use std::sync::mpsc; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; -use cadence::MetricSink; use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; -use crate::{Client, Hub}; - -/// A [`cadence`] compatible [`MetricSink`]. -/// -/// This will ingest all the emitted metrics to Sentry as well as forward them -/// to the inner [`MetricSink`]. -#[derive(Debug)] -pub struct SentryMetricSink { - client: Arc, - sink: S, -} - -impl SentryMetricSink { - /// Creates a new [`SentryMetricSink`], wrapping the given [`MetricSink`]. - pub fn try_new(sink: S) -> Result { - let hub = Hub::current(); - let Some(client) = hub.client() else { - return Err(sink); - }; - - Ok(Self { client, sink }) - } -} - -impl MetricSink for SentryMetricSink -where - S: MetricSink, -{ - fn emit(&self, metric: &str) -> std::io::Result { - self.client.add_metric(metric); - self.sink.emit(metric) - } - - fn flush(&self) -> std::io::Result<()> { - if self.client.flush(None) { - self.sink.flush() - } else { - Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Flushing Client failed", - )) - } - } -} #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] enum MetricType { @@ -166,7 +120,7 @@ enum Task { } pub struct MetricAggregator { - sender: SyncSender, + sender: mpsc::SyncSender, handle: Option>, } @@ -175,7 +129,7 @@ const FLUSH_INTERVAL: Duration = Duration::from_secs(10); impl MetricAggregator { pub fn new(transport: TransportArc) -> Self { - let (sender, receiver) = sync_channel(30); + let (sender, receiver) = mpsc::sync_channel(30); let handle = thread::Builder::new() .name("sentry-metrics".into()) .spawn(move || Self::worker_thread(receiver, transport)) @@ -240,7 +194,7 @@ impl MetricAggregator { let _ = self.sender.send(Task::Flush); } - fn worker_thread(receiver: Receiver, transport: TransportArc) { + fn worker_thread(receiver: mpsc::Receiver, transport: TransportArc) { let mut buckets = AggregateMetrics::new(); let mut last_flush = Instant::now(); @@ -248,7 +202,7 @@ impl MetricAggregator { let timeout = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); match receiver.recv_timeout(timeout) { - Err(RecvTimeoutError::Timeout) | Ok(Task::Flush) => { + Err(_) | Ok(Task::Flush) => { // flush Self::flush_buckets(std::mem::take(&mut buckets), &transport); last_flush = Instant::now(); @@ -343,46 +297,3 @@ impl Drop for MetricAggregator { } } } - -#[cfg(test)] -mod tests { - use std::str::from_utf8; - - use cadence::{Counted, Distributed}; - - use crate::test::with_captured_envelopes; - - use super::*; - - #[test] - fn test_basic_metrics() { - let envelopes = with_captured_envelopes(|| { - let sink = SentryMetricSink::try_new(cadence::NopMetricSink).unwrap(); - - let client = cadence::StatsdClient::from_sink("sentry.test", sink); - client.count("some.count", 1).unwrap(); - client.count("some.count", 10).unwrap(); - client - .count_with_tags("count.with.tags", 1) - .with_tag("foo", "bar") - .send(); - client.distribution("some.distr", 1).unwrap(); - client.distribution("some.distr", 2).unwrap(); - client.distribution("some.distr", 3).unwrap(); - }); - assert_eq!(envelopes.len(), 1); - - let mut items = envelopes[0].items(); - let Some(EnvelopeItem::Metrics(metrics)) = items.next() else { - panic!("expected metrics"); - }; - let metrics = from_utf8(metrics).unwrap(); - - println!("{metrics}"); - - assert!(metrics.contains("sentry.test.count.with.tags:1|c|#foo:bar|T")); - assert!(metrics.contains("sentry.test.some.count:11|c|T")); - assert!(metrics.contains("sentry.test.some.distr:1:2:3|d|T")); - assert_eq!(items.next(), None); - } -} From 45a4ac16167e8a360d33ec24bf5b291e57a33120 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 13:26:01 +0100 Subject: [PATCH 14/33] ref(metrics): Minor code-level changes --- sentry-core/src/cadence.rs | 10 ++++------ sentry-core/src/metrics.rs | 13 +++++++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 10304ac3..691026fd 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -17,12 +17,10 @@ pub struct SentryMetricSink { impl SentryMetricSink { /// Creates a new [`SentryMetricSink`], wrapping the given [`MetricSink`]. pub fn try_new(sink: S) -> Result { - let hub = Hub::current(); - let Some(client) = hub.client() else { - return Err(sink); - }; - - Ok(Self { client, sink }) + match Hub::current().client() { + Some(client) => Ok(Self { client, sink }), + None => Err(sink), + } } } diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 682ab695..cad640ba 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -9,6 +9,9 @@ use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; +const BUCKET_INTERVAL: Duration = Duration::from_secs(10); +const FLUSH_INTERVAL: Duration = Duration::from_secs(10); + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] enum MetricType { Counter, @@ -56,12 +59,14 @@ struct GaugeValue { sum: f64, count: u64, } + enum BucketValue { Counter(f64), Distribution(Vec), Set(BTreeSet), Gauge(GaugeValue), } + impl BucketValue { fn distribution(val: f64) -> BucketValue { Self::Distribution(vec![val]) @@ -89,7 +94,9 @@ impl BucketValue { (BucketValue::Distribution(d1), BucketValue::Distribution(d2)) => { d1.extend(d2); } - (BucketValue::Set(s1), BucketValue::Set(s2)) => s1.extend(s2), + (BucketValue::Set(s1), BucketValue::Set(s2)) => { + s1.extend(s2); + } (BucketValue::Gauge(g1), BucketValue::Gauge(g2)) => { g1.last = g2.last; g1.min = g1.min.min(g2.min); @@ -99,6 +106,7 @@ impl BucketValue { } _ => return Err(()), } + Ok(()) } } @@ -124,9 +132,6 @@ pub struct MetricAggregator { handle: Option>, } -const BUCKET_INTERVAL: Duration = Duration::from_secs(10); -const FLUSH_INTERVAL: Duration = Duration::from_secs(10); - impl MetricAggregator { pub fn new(transport: TransportArc) -> Self { let (sender, receiver) = mpsc::sync_channel(30); From d24c9b2610dd45afdbb435cfa4cb0bae56efe13c Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 17:47:04 +0100 Subject: [PATCH 15/33] ref(metrics): Refactor to match Python SDK --- sentry-core/src/cadence.rs | 46 ++- sentry-core/src/client.rs | 4 +- sentry-core/src/lib.rs | 2 + sentry-core/src/metrics.rs | 592 +++++++++++++++++++++++++------------ sentry-core/src/units.rs | 293 ++++++++++++++++++ 5 files changed, 749 insertions(+), 188 deletions(-) create mode 100644 sentry-core/src/units.rs diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 691026fd..754469db 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -2,6 +2,8 @@ use std::sync::Arc; use cadence::MetricSink; +use crate::metrics::{Metric, MetricType, MetricValue}; +use crate::units::MetricUnit; use crate::{Client, Hub}; /// A [`cadence`] compatible [`MetricSink`]. @@ -28,9 +30,12 @@ impl MetricSink for SentryMetricSink where S: MetricSink, { - fn emit(&self, metric: &str) -> std::io::Result { - self.client.add_metric(metric); - self.sink.emit(metric) + fn emit(&self, string: &str) -> std::io::Result { + if let Some(metric) = parse_metric(string) { + self.client.add_metric(metric); + } + + self.sink.emit(string) } fn flush(&self) -> std::io::Result<()> { @@ -45,6 +50,41 @@ where } } +fn parse_metric(string: &str) -> Option { + let mut components = string.split('|'); + + let (mri_str, value_str) = components.next()?.split_once(':')?; + let (name, unit) = match mri_str.split_once('@') { + Some((name, unit_str)) => (name, unit_str.parse().ok()?), + None => (mri_str, MetricUnit::None), + }; + + let ty = components.next().and_then(|s| s.parse().ok())?; + let value = match ty { + MetricType::Counter => MetricValue::Counter(value_str.parse().ok()?), + MetricType::Distribution => MetricValue::Distribution(value_str.parse().ok()?), + MetricType::Set => MetricValue::Set(value_str.parse().ok()?), + MetricType::Gauge => MetricValue::Gauge(value_str.parse().ok()?), + }; + + let mut builder = Metric::build(name.to_owned(), value).with_unit(unit); + + for component in components { + if let Some('#') = component.chars().next() { + for pair in string.get(1..)?.split(',') { + let mut key_value = pair.splitn(2, ':'); + + let key = key_value.next()?.to_owned(); + let value = key_value.next().unwrap_or_default().to_owned(); + + builder = builder.with_tag(key, value); + } + } + } + + Some(builder.finish()) +} + #[cfg(test)] mod tests { use cadence::{Counted, Distributed}; diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 8c674b01..a67295f4 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -12,7 +12,7 @@ use sentry_types::random_uuid; use crate::constants::SDK_INFO; #[cfg(feature = "UNSTABLE_metrics")] -use crate::metrics::MetricAggregator; +use crate::metrics::{self, MetricAggregator}; use crate::protocol::{ClientSdkInfo, Event}; use crate::session::SessionFlusher; use crate::types::{Dsn, Uuid}; @@ -323,7 +323,7 @@ impl Client { } #[cfg(feature = "UNSTABLE_metrics")] - pub(crate) fn add_metric(&self, metric: &str) { + pub fn add_metric(&self, metric: metrics::Metric) { if let Some(ref aggregator) = *self.metric_aggregator.read().unwrap() { aggregator.add(metric) } diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index a1a1afdb..5b1f5cd4 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -147,6 +147,8 @@ mod hub_impl; mod metrics; #[cfg(feature = "client")] mod session; +#[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] +mod units; #[cfg(all(feature = "client", feature = "UNSTABLE_cadence"))] pub use crate::cadence::SentryMetricSink; #[cfg(feature = "client")] diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index cad640ba..822a2fc7 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,7 +1,8 @@ -use std::collections::btree_map::Entry; -use std::collections::{BTreeMap, BTreeSet}; +use std::borrow::Cow; +use std::collections::hash_map::{DefaultHasher, Entry}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::fmt; -use std::sync::mpsc; +use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -9,11 +10,15 @@ use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; +use crate::units::DurationUnit; +pub use crate::units::MetricUnit; + const BUCKET_INTERVAL: Duration = Duration::from_secs(10); -const FLUSH_INTERVAL: Duration = Duration::from_secs(10); +const FLUSH_INTERVAL: Duration = Duration::from_secs(5); +const MAX_WEIGHT: usize = 100_000; -#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] -enum MetricType { +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub(crate) enum MetricType { Counter, Distribution, Set, @@ -52,253 +57,474 @@ impl std::str::FromStr for MetricType { } } -struct GaugeValue { - last: f64, - min: f64, - max: f64, - sum: f64, - count: u64, -} - -enum BucketValue { - Counter(f64), - Distribution(Vec), - Set(BTreeSet), - Gauge(GaugeValue), +/// Type used for Counter metric +pub type CounterType = f64; + +/// Type of distribution entries +pub type DistributionType = f64; + +/// Type used for set elements in Set metric +pub type SetType = u32; + +/// Type used for Gauge entries +pub type GaugeType = f64; + +/// A snapshot of values. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct GaugeValue { + /// The last value reported in the bucket. + /// + /// This aggregation is not commutative. + pub last: GaugeType, + /// The minimum value reported in the bucket. + pub min: GaugeType, + /// The maximum value reported in the bucket. + pub max: GaugeType, + /// The sum of all values reported in the bucket. + pub sum: GaugeType, + /// The number of times this bucket was updated with a new value. + pub count: u64, } -impl BucketValue { - fn distribution(val: f64) -> BucketValue { - Self::Distribution(vec![val]) - } - - fn gauge(val: f64) -> BucketValue { - Self::Gauge(GaugeValue { - last: val, - min: val, - max: val, - sum: val, +impl GaugeValue { + /// Creates a gauge snapshot from a single value. + pub fn single(value: GaugeType) -> Self { + Self { + last: value, + min: value, + max: value, + sum: value, count: 1, - }) + } } - fn set_from_str(value: &str) -> BucketValue { - BucketValue::Set([value.into()].into()) + /// Inserts a new value into the gauge. + pub fn insert(&mut self, value: GaugeType) { + self.last = value; + self.min = self.min.min(value); + self.max = self.max.max(value); + self.sum += value; + self.count += 1; } +} - fn merge(&mut self, other: BucketValue) -> Result<(), ()> { - match (self, other) { - (BucketValue::Counter(c1), BucketValue::Counter(c2)) => { +enum BucketValue { + Counter(CounterType), + Distribution(Vec), + Set(BTreeSet), + Gauge(GaugeValue), +} + +impl BucketValue { + pub fn insert(&mut self, value: MetricValue) -> usize { + match (self, value) { + (Self::Counter(c1), MetricValue::Counter(c2)) => { *c1 += c2; + 0 } - (BucketValue::Distribution(d1), BucketValue::Distribution(d2)) => { - d1.extend(d2); + (Self::Distribution(d1), MetricValue::Distribution(d2)) => { + d1.push(d2); + 1 } - (BucketValue::Set(s1), BucketValue::Set(s2)) => { - s1.extend(s2); + (Self::Set(s1), MetricValue::Set(s2)) => { + if s1.insert(s2) { + 1 + } else { + 0 + } } - (BucketValue::Gauge(g1), BucketValue::Gauge(g2)) => { - g1.last = g2.last; - g1.min = g1.min.min(g2.min); - g1.max = g1.max.max(g2.max); - g1.sum += g2.sum; - g1.count += g2.count; + (Self::Gauge(g1), MetricValue::Gauge(g2)) => { + g1.insert(g2); + 0 } - _ => return Err(()), + _ => panic!("invalid metric type"), } + } - Ok(()) + pub fn weight(&self) -> usize { + match self { + BucketValue::Counter(_) => 1, + BucketValue::Distribution(v) => v.len(), + BucketValue::Set(v) => v.len(), + BucketValue::Gauge(_) => 5, + } } } -#[derive(PartialEq, Eq, PartialOrd, Ord)] -struct BucketKey { - timestamp: u64, - ty: MetricType, - name: String, - tags: String, +impl From for BucketValue { + fn from(value: MetricValue) -> Self { + match value { + MetricValue::Counter(v) => Self::Counter(v), + MetricValue::Distribution(v) => Self::Distribution(vec![v]), + MetricValue::Gauge(v) => Self::Gauge(GaugeValue::single(v)), + MetricValue::Set(v) => Self::Set(std::iter::once(v).collect()), + } + } } -type AggregateMetrics = BTreeMap; +pub type MetricStr = Cow<'static, str>; + +type Timestamp = u64; -enum Task { - RecordMetric((BucketKey, BucketValue)), - Flush, - Shutdown, +#[derive(PartialEq, Eq, Hash)] +struct BucketKey { + timestamp: Timestamp, + ty: MetricType, + name: MetricStr, + unit: MetricUnit, + tags: BTreeMap, } -pub struct MetricAggregator { - sender: mpsc::SyncSender, - handle: Option>, +#[derive(Debug)] +pub enum MetricValue { + Counter(CounterType), + Distribution(DistributionType), + Gauge(GaugeType), + Set(SetType), } -impl MetricAggregator { - pub fn new(transport: TransportArc) -> Self { - let (sender, receiver) = mpsc::sync_channel(30); - let handle = thread::Builder::new() - .name("sentry-metrics".into()) - .spawn(move || Self::worker_thread(receiver, transport)) - .ok(); +impl MetricValue { + /// Returns a bucket value representing a set with a single given string value. + pub fn set_from_str(string: &str) -> Self { + Self::Set(hash_set_value(string)) + } - Self { sender, handle } + /// Returns a bucket value representing a set with a single given value. + pub fn set_from_display(display: impl fmt::Display) -> Self { + Self::Set(hash_set_value(&display.to_string())) } - pub fn add(&self, metric: &str) { - fn mk_value(ty: MetricType, value: &str) -> Option { - Some(match ty { - MetricType::Counter => BucketValue::Counter(value.parse().ok()?), - MetricType::Distribution => BucketValue::distribution(value.parse().ok()?), - MetricType::Set => BucketValue::set_from_str(value), - MetricType::Gauge => BucketValue::gauge(value.parse().ok()?), - }) + fn ty(&self) -> MetricType { + match self { + Self::Counter(_) => MetricType::Counter, + Self::Distribution(_) => MetricType::Distribution, + Self::Gauge(_) => MetricType::Gauge, + Self::Set(_) => MetricType::Set, } + } +} - fn parse(metric: &str) -> Option<(BucketKey, BucketValue)> { - let mut components = metric.split('|'); - let mut values = components.next()?.split(':'); - let name = values.next()?; +/// Hashes the given set value. +/// +/// Sets only guarantee 32-bit accuracy, but arbitrary strings are allowed on the protocol. Upon +/// parsing, they are hashed and only used as hashes subsequently. +fn hash_set_value(string: &str) -> u32 { + use std::hash::Hasher; + let mut hasher = DefaultHasher::default(); + hasher.write(string.as_bytes()); + hasher.finish() as u32 +} - let ty: MetricType = components.next().and_then(|s| s.parse().ok())?; - let mut value = mk_value(ty, values.next()?)?; +type BucketMap = BTreeMap>; - for value_s in values { - value.merge(mk_value(ty, value_s)?).ok()?; - } +struct AggregatorInner { + buckets: BucketMap, + weight: usize, + running: bool, + force_flush: bool, +} + +impl AggregatorInner { + pub fn new() -> Self { + Self { + buckets: BTreeMap::new(), + weight: 0, + running: true, + force_flush: false, + } + } - let mut tags = ""; - let mut timestamp = SystemTime::now() + pub fn add(&mut self, mut key: BucketKey, value: MetricValue) { + // Floor timestamp to bucket interval + key.timestamp /= BUCKET_INTERVAL.as_secs(); + key.timestamp *= BUCKET_INTERVAL.as_secs(); + + match self.buckets.entry(key.timestamp).or_default().entry(key) { + Entry::Occupied(mut e) => self.weight += e.get_mut().insert(value), + Entry::Vacant(e) => self.weight += e.insert(value.into()).weight(), + } + } + + pub fn take_buckets(&mut self) -> BucketMap { + if self.force_flush || !self.running { + self.weight = 0; + self.force_flush = false; + std::mem::take(&mut self.buckets) + } else { + let timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() + .saturating_sub(FLUSH_INTERVAL) .as_secs(); - for component in components { - if let Some(component_tags) = component.strip_prefix('#') { - tags = component_tags; - } else if let Some(component_timestamp) = component.strip_prefix('T') { - timestamp = component_timestamp.parse().ok()?; - } - } + // Split all buckets after the cutoff time. `split` contains newer buckets, which should + // remain, so swap them. After the swap, `split` contains all older buckets. + let mut split = self.buckets.split_off(×tamp); + std::mem::swap(&mut split, &mut self.buckets); + + self.weight -= split + .values() + .flat_map(|map| map.values()) + .map(|bucket| bucket.weight()) + .sum::(); - Some(( - BucketKey { - timestamp, - ty, - name: name.into(), - tags: tags.into(), - }, - value, - )) + split } + } + + pub fn weight(&self) -> usize { + self.weight + } +} + +pub struct Metric { + name: MetricStr, + unit: MetricUnit, + value: MetricValue, + tags: BTreeMap, + time: Option, +} + +impl Metric { + pub fn build(name: impl Into, value: MetricValue) -> MetricBuilder { + let metric = Metric { + name: name.into(), + unit: MetricUnit::None, + value, + tags: BTreeMap::new(), + time: None, + }; + + MetricBuilder { metric } + } + + pub fn incr(name: impl Into) -> MetricBuilder { + Self::build(name, MetricValue::Counter(1.0)) + } + + pub fn timing(name: impl Into, timing: Duration) -> MetricBuilder { + Self::build(name, MetricValue::Distribution(timing.as_secs_f64())) + .with_unit(MetricUnit::Duration(DurationUnit::Second)) + } + + pub fn distribution(name: impl Into, value: f64) -> MetricBuilder { + Self::build(name, MetricValue::Distribution(value)) + } - if let Some(parsed_metric) = parse(metric) { - let _ = self.sender.send(Task::RecordMetric(parsed_metric)); + pub fn set(name: impl Into, string: &str) -> MetricBuilder { + Self::build(name, MetricValue::set_from_str(string)) + } + + pub fn gauge(name: impl Into, value: f64) -> MetricBuilder { + Self::build(name, MetricValue::Gauge(value)) + } +} + +#[must_use] +pub struct MetricBuilder { + metric: Metric, +} + +impl MetricBuilder { + pub fn with_unit(mut self, unit: MetricUnit) -> Self { + self.metric.unit = unit; + self + } + + pub fn with_tag(mut self, name: impl Into, value: impl Into) -> Self { + self.metric.tags.insert(name.into(), value.into()); + self + } + + pub fn with_time(mut self, time: SystemTime) -> Self { + self.metric.time = Some(time); + self + } + + pub fn finish(self) -> Metric { + self.metric + } +} + +pub struct MetricAggregator { + inner: Arc>, + handle: Option>, +} + +impl MetricAggregator { + pub fn new(transport: TransportArc) -> Self { + let inner = Arc::new(Mutex::new(AggregatorInner::new())); + let inner_clone = Arc::clone(&inner); + + let handle = thread::Builder::new() + .name("sentry-metrics".into()) + .spawn(move || Self::worker_thread(inner_clone, transport)) + .expect("failed to spawn thread"); + + Self { + inner, + handle: Some(handle), } } - pub fn flush(&self) { - let _ = self.sender.send(Task::Flush); + pub fn add(&self, metric: Metric) { + let Metric { + name, + unit, + value, + tags, + time, + } = metric; + + let timestamp = time + .unwrap_or_else(SystemTime::now) + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let key = BucketKey { + timestamp, + ty: value.ty(), + name, + unit, + tags, + }; + + let mut guard = self.inner.lock().unwrap(); + guard.add(key, value); + + if guard.weight() > MAX_WEIGHT { + if let Some(ref handle) = self.handle { + handle.thread().unpark(); + } + } } - fn worker_thread(receiver: mpsc::Receiver, transport: TransportArc) { - let mut buckets = AggregateMetrics::new(); - let mut last_flush = Instant::now(); + pub fn flush(&self) { + self.inner.lock().unwrap().force_flush = true; + if let Some(ref handle) = self.handle { + handle.thread().unpark(); + } + } + fn worker_thread(inner: Arc>, transport: TransportArc) { loop { - let timeout = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); + let mut guard = inner.lock().unwrap(); + let should_stop = !guard.running; - match receiver.recv_timeout(timeout) { - Err(_) | Ok(Task::Flush) => { - // flush - Self::flush_buckets(std::mem::take(&mut buckets), &transport); - last_flush = Instant::now(); - } - Ok(Task::RecordMetric((mut key, value))) => { - // aggregate - let rounding_interval = BUCKET_INTERVAL.as_secs(); - let rounded_timestamp = (key.timestamp / rounding_interval) * rounding_interval; - - key.timestamp = rounded_timestamp; - - match buckets.entry(key) { - Entry::Occupied(mut entry) => { - let _ = entry.get_mut().merge(value); - } - Entry::Vacant(entry) => { - entry.insert(value); - } - } - } - _ => { - // shutdown - Self::flush_buckets(buckets, &transport); - return; - } + let last_flush = Instant::now(); + let buckets = guard.take_buckets(); + drop(guard); + + if !buckets.is_empty() { + Self::flush_buckets(buckets, &transport); } + + if should_stop { + break; + } + + // Park instead of sleep so we can wake the thread up + let park_time = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); + thread::park_timeout(park_time); } } - fn flush_buckets(buckets: AggregateMetrics, transport: &TransportArc) { - if buckets.is_empty() { - return; + fn flush_buckets(buckets: BucketMap, transport: &TransportArc) { + // The transport is usually available when flush is called. Prefer a short lock and worst + // case throw away the result rather than blocking the transport for too long. + if let Ok(output) = Self::format_payload(buckets) { + let mut envelope = Envelope::new(); + envelope.add_item(EnvelopeItem::Metrics(output)); + + if let Some(ref transport) = *transport.read().unwrap() { + transport.send_envelope(envelope); + } } + } - fn format_payload(buckets: AggregateMetrics) -> std::io::Result> { - use std::io::Write; - let mut out = vec![]; - for (key, value) in buckets { - write!(&mut out, "{}", key.name)?; + fn format_payload(buckets: BucketMap) -> std::io::Result> { + use std::io::Write; + let mut out = vec![]; - match value { - BucketValue::Counter(c) => { - write!(&mut out, ":{}", c)?; - } - BucketValue::Distribution(d) => { - for v in d { - write!(&mut out, ":{}", v)?; - } - } - BucketValue::Set(s) => { - for v in s { - write!(&mut out, ":{}", v)?; - } + for (key, value) in buckets.into_iter().flat_map(|(_, v)| v) { + write!(&mut out, "{}@{}", SafeKey(key.name.as_ref()), key.unit)?; + + match value { + BucketValue::Counter(c) => { + write!(&mut out, ":{}", c)?; + } + BucketValue::Distribution(d) => { + for v in d { + write!(&mut out, ":{}", v)?; } - BucketValue::Gauge(g) => { - write!( - &mut out, - ":{}:{}:{}:{}:{}", - g.last, g.min, g.max, g.sum, g.count - )?; + } + BucketValue::Set(s) => { + for v in s { + write!(&mut out, ":{}", v)?; } } - - write!(&mut out, "|{}", key.ty.as_str())?; - if !key.tags.is_empty() { - write!(&mut out, "|#{}", key.tags)?; + BucketValue::Gauge(g) => { + write!( + &mut out, + ":{}:{}:{}:{}:{}", + g.last, g.min, g.max, g.sum, g.count + )?; } - writeln!(&mut out, "|T{}", key.timestamp)?; } - Ok(out) - } + write!(&mut out, "|{}", key.ty.as_str())?; - let Ok(output) = format_payload(buckets) else { - return; - }; - - let mut envelope = Envelope::new(); - envelope.add_item(EnvelopeItem::Metrics(output)); + if !key.tags.is_empty() { + write!(&mut out, "|#")?; + for (i, (k, v)) in key.tags.into_iter().enumerate() { + if i > 0 { + write!(&mut out, ",")?; + } + write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SaveVal(v.as_ref()))?; + } + } - if let Some(ref transport) = *transport.read().unwrap() { - transport.send_envelope(envelope); + writeln!(&mut out, "|T{}", key.timestamp)?; } + + Ok(out) } } impl Drop for MetricAggregator { fn drop(&mut self) { - let _ = self.sender.send(Task::Shutdown); + self.inner.lock().unwrap().running = false; if let Some(handle) = self.handle.take() { handle.join().unwrap(); } } } + +struct SafeKey<'s>(&'s str); + +impl<'s> fmt::Display for SafeKey<'s> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for c in self.0.chars() { + if c.is_ascii_alphanumeric() || ['_', '-', '.', '/'].contains(&c) { + write!(f, "{}", c)?; + } + } + Ok(()) + } +} + +struct SaveVal<'s>(&'s str); + +impl<'s> fmt::Display for SaveVal<'s> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for c in self.0.chars() { + if c.is_alphanumeric() + || ['_', ':', '/', '@', '.', '{', '}', '[', ']', '$', '-'].contains(&c) + { + write!(f, "{}", c)?; + } + } + Ok(()) + } +} diff --git a/sentry-core/src/units.rs b/sentry-core/src/units.rs new file mode 100644 index 00000000..93236e1e --- /dev/null +++ b/sentry-core/src/units.rs @@ -0,0 +1,293 @@ +//! Type definitions for Sentry metrics. + +use std::fmt; + +/// The unit of measurement of a metric value. +/// +/// Units augment metric values by giving them a magnitude and semantics. There are certain types of +/// units that are subdivided in their precision, such as the [`DurationUnit`] for time +/// measurements. +/// +/// Units and their precisions are uniquely represented by a string identifier. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Default)] +pub enum MetricUnit { + /// A time duration, defaulting to `"millisecond"`. + Duration(DurationUnit), + /// Size of information derived from bytes, defaulting to `"byte"`. + Information(InformationUnit), + /// Fractions such as percentages, defaulting to `"ratio"`. + Fraction(FractionUnit), + /// user-defined units without builtin conversion or default. + Custom(CustomUnit), + /// Untyped value without a unit (`""`). + #[default] + None, +} + +impl MetricUnit { + /// Returns `true` if the metric_unit is [`None`]. + pub fn is_none(&self) -> bool { + matches!(self, Self::None) + } +} + +impl fmt::Display for MetricUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MetricUnit::Duration(u) => u.fmt(f), + MetricUnit::Information(u) => u.fmt(f), + MetricUnit::Fraction(u) => u.fmt(f), + MetricUnit::Custom(u) => u.fmt(f), + MetricUnit::None => f.write_str("none"), + } + } +} + +impl std::str::FromStr for MetricUnit { + type Err = ParseMetricUnitError; + + fn from_str(s: &str) -> Result { + Ok(match s { + "nanosecond" | "ns" => Self::Duration(DurationUnit::NanoSecond), + "microsecond" => Self::Duration(DurationUnit::MicroSecond), + "millisecond" | "ms" => Self::Duration(DurationUnit::MilliSecond), + "second" | "s" => Self::Duration(DurationUnit::Second), + "minute" => Self::Duration(DurationUnit::Minute), + "hour" => Self::Duration(DurationUnit::Hour), + "day" => Self::Duration(DurationUnit::Day), + "week" => Self::Duration(DurationUnit::Week), + + "bit" => Self::Information(InformationUnit::Bit), + "byte" => Self::Information(InformationUnit::Byte), + "kilobyte" => Self::Information(InformationUnit::KiloByte), + "kibibyte" => Self::Information(InformationUnit::KibiByte), + "megabyte" => Self::Information(InformationUnit::MegaByte), + "mebibyte" => Self::Information(InformationUnit::MebiByte), + "gigabyte" => Self::Information(InformationUnit::GigaByte), + "gibibyte" => Self::Information(InformationUnit::GibiByte), + "terabyte" => Self::Information(InformationUnit::TeraByte), + "tebibyte" => Self::Information(InformationUnit::TebiByte), + "petabyte" => Self::Information(InformationUnit::PetaByte), + "pebibyte" => Self::Information(InformationUnit::PebiByte), + "exabyte" => Self::Information(InformationUnit::ExaByte), + "exbibyte" => Self::Information(InformationUnit::ExbiByte), + + "ratio" => Self::Fraction(FractionUnit::Ratio), + "percent" => Self::Fraction(FractionUnit::Percent), + + "" | "none" => Self::None, + _ => Self::Custom(CustomUnit::parse(s)?), + }) + } +} + +/// Time duration units used in [`MetricUnit::Duration`]. +/// +/// Defaults to `millisecond`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum DurationUnit { + /// Nanosecond (`"nanosecond"`), 10^-9 seconds. + NanoSecond, + /// Microsecond (`"microsecond"`), 10^-6 seconds. + MicroSecond, + /// Millisecond (`"millisecond"`), 10^-3 seconds. + MilliSecond, + /// Full second (`"second"`). + Second, + /// Minute (`"minute"`), 60 seconds. + Minute, + /// Hour (`"hour"`), 3600 seconds. + Hour, + /// Day (`"day"`), 86,400 seconds. + Day, + /// Week (`"week"`), 604,800 seconds. + Week, +} + +impl Default for DurationUnit { + fn default() -> Self { + Self::MilliSecond + } +} + +impl fmt::Display for DurationUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NanoSecond => f.write_str("nanosecond"), + Self::MicroSecond => f.write_str("microsecond"), + Self::MilliSecond => f.write_str("millisecond"), + Self::Second => f.write_str("second"), + Self::Minute => f.write_str("minute"), + Self::Hour => f.write_str("hour"), + Self::Day => f.write_str("day"), + Self::Week => f.write_str("week"), + } + } +} + +/// An error parsing a [`MetricUnit`] or one of its variants. +#[derive(Clone, Copy, Debug)] +pub struct ParseMetricUnitError(()); + +/// Size of information derived from bytes, used in [`MetricUnit::Information`]. +/// +/// Defaults to `byte`. See also [Units of +/// information](https://en.wikipedia.org/wiki/Units_of_information). +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum InformationUnit { + /// Bit (`"bit"`), corresponding to 1/8 of a byte. + /// + /// Note that there are computer systems with a different number of bits per byte. + Bit, + /// Byte (`"byte"`). + Byte, + /// Kilobyte (`"kilobyte"`), 10^3 bytes. + KiloByte, + /// Kibibyte (`"kibibyte"`), 2^10 bytes. + KibiByte, + /// Megabyte (`"megabyte"`), 10^6 bytes. + MegaByte, + /// Mebibyte (`"mebibyte"`), 2^20 bytes. + MebiByte, + /// Gigabyte (`"gigabyte"`), 10^9 bytes. + GigaByte, + /// Gibibyte (`"gibibyte"`), 2^30 bytes. + GibiByte, + /// Terabyte (`"terabyte"`), 10^12 bytes. + TeraByte, + /// Tebibyte (`"tebibyte"`), 2^40 bytes. + TebiByte, + /// Petabyte (`"petabyte"`), 10^15 bytes. + PetaByte, + /// Pebibyte (`"pebibyte"`), 2^50 bytes. + PebiByte, + /// Exabyte (`"exabyte"`), 10^18 bytes. + ExaByte, + /// Exbibyte (`"exbibyte"`), 2^60 bytes. + ExbiByte, +} + +impl Default for InformationUnit { + fn default() -> Self { + Self::Byte + } +} + +impl fmt::Display for InformationUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Bit => f.write_str("bit"), + Self::Byte => f.write_str("byte"), + Self::KiloByte => f.write_str("kilobyte"), + Self::KibiByte => f.write_str("kibibyte"), + Self::MegaByte => f.write_str("megabyte"), + Self::MebiByte => f.write_str("mebibyte"), + Self::GigaByte => f.write_str("gigabyte"), + Self::GibiByte => f.write_str("gibibyte"), + Self::TeraByte => f.write_str("terabyte"), + Self::TebiByte => f.write_str("tebibyte"), + Self::PetaByte => f.write_str("petabyte"), + Self::PebiByte => f.write_str("pebibyte"), + Self::ExaByte => f.write_str("exabyte"), + Self::ExbiByte => f.write_str("exbibyte"), + } + } +} + +/// Units of fraction used in [`MetricUnit::Fraction`]. +/// +/// Defaults to `ratio`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum FractionUnit { + /// Floating point fraction of `1`. + Ratio, + /// Ratio expressed as a fraction of `100`. `100%` equals a ratio of `1.0`. + Percent, +} + +impl Default for FractionUnit { + fn default() -> Self { + Self::Ratio + } +} + +impl fmt::Display for FractionUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Ratio => f.write_str("ratio"), + Self::Percent => f.write_str("percent"), + } + } +} + +const CUSTOM_UNIT_MAX_SIZE: usize = 15; + +/// Custom user-defined units without builtin conversion. +#[derive(Clone, Copy, Eq, PartialEq, Hash)] +pub struct CustomUnit([u8; CUSTOM_UNIT_MAX_SIZE]); + +impl CustomUnit { + /// Parses a `CustomUnit` from a string. + pub fn parse(s: &str) -> Result { + if !s.is_ascii() { + return Err(ParseMetricUnitError(())); + } + + let mut unit = Self([0; CUSTOM_UNIT_MAX_SIZE]); + let slice = unit.0.get_mut(..s.len()).ok_or(ParseMetricUnitError(()))?; + slice.copy_from_slice(s.as_bytes()); + unit.0.make_ascii_lowercase(); + Ok(unit) + } + + /// Returns the string representation of this unit. + #[inline] + pub fn as_str(&self) -> &str { + // Safety: The string is already validated to be valid ASCII when + // parsing `CustomUnit`. + unsafe { std::str::from_utf8_unchecked(&self.0).trim_end_matches('\0') } + } +} + +impl fmt::Debug for CustomUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +impl fmt::Display for CustomUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +impl std::str::FromStr for CustomUnit { + type Err = ParseMetricUnitError; + + fn from_str(s: &str) -> Result { + Self::parse(s) + } +} + +impl std::ops::Deref for CustomUnit { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.as_str() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_custom_unit_parse() { + assert_eq!("foo", CustomUnit::parse("Foo").unwrap().as_str()); + assert_eq!( + "0123456789abcde", + CustomUnit::parse("0123456789abcde").unwrap().as_str() + ); + assert!(CustomUnit::parse("this_is_a_unit_that_is_too_long").is_err()); + } +} From b515db3841acf2fa0f9533e28950b2757d9d1bfa Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 18:21:45 +0100 Subject: [PATCH 16/33] fix: First bugfixes --- sentry-core/src/cadence.rs | 2 +- sentry-core/src/client.rs | 1 + sentry-core/src/metrics.rs | 5 ++++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 754469db..07742d96 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -71,7 +71,7 @@ fn parse_metric(string: &str) -> Option { for component in components { if let Some('#') = component.chars().next() { - for pair in string.get(1..)?.split(',') { + for pair in component.get(1..)?.split(',') { let mut key_value = pair.splitn(2, ':'); let key = key_value.next()?.to_owned(); diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index a67295f4..7e4f1b67 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -322,6 +322,7 @@ impl Client { } } + /// Captures a metric and sends it to Sentry after a short delay. #[cfg(feature = "UNSTABLE_metrics")] pub fn add_metric(&self, metric: metrics::Metric) { if let Some(ref aggregator) = *self.metric_aggregator.read().unwrap() { diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 822a2fc7..7240121e 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -448,7 +448,10 @@ impl MetricAggregator { let mut out = vec![]; for (key, value) in buckets.into_iter().flat_map(|(_, v)| v) { - write!(&mut out, "{}@{}", SafeKey(key.name.as_ref()), key.unit)?; + write!(&mut out, "{}", SafeKey(key.name.as_ref()))?; + if key.unit != MetricUnit::None { + write!(&mut out, "@{}", key.unit)?; + } match value { BucketValue::Counter(c) => { From 1267668af09fdf55b2e456f5876840a2ed623b0c Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 19:09:22 +0100 Subject: [PATCH 17/33] fix: Flakey submission --- sentry-core/src/metrics.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 7240121e..3c8fd5d7 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -408,25 +408,22 @@ impl MetricAggregator { } fn worker_thread(inner: Arc>, transport: TransportArc) { - loop { - let mut guard = inner.lock().unwrap(); - let should_stop = !guard.running; + let mut running = true; - let last_flush = Instant::now(); - let buckets = guard.take_buckets(); - drop(guard); + while running { + // Park instead of sleep so we can wake the thread up. Do not account for delays during + // flushing, since we benefit from some drift to spread out metric submissions. + thread::park_timeout(FLUSH_INTERVAL); + + let buckets = { + let mut guard = inner.lock().unwrap(); + running = guard.running; + guard.take_buckets() + }; if !buckets.is_empty() { Self::flush_buckets(buckets, &transport); } - - if should_stop { - break; - } - - // Park instead of sleep so we can wake the thread up - let park_time = FLUSH_INTERVAL.saturating_sub(last_flush.elapsed()); - thread::park_timeout(park_time); } } @@ -499,6 +496,7 @@ impl Drop for MetricAggregator { fn drop(&mut self) { self.inner.lock().unwrap().running = false; if let Some(handle) = self.handle.take() { + handle.thread().unpark(); handle.join().unwrap(); } } From c1649839717b6740dee85581b9a78c4aaf50fd1e Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 19:16:00 +0100 Subject: [PATCH 18/33] feat(metrics): Add a convenience API to track metrics directly --- sentry-core/src/metrics.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 3c8fd5d7..3949c13d 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -9,6 +9,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; +use crate::Hub; use crate::units::DurationUnit; pub use crate::units::MetricUnit; @@ -344,6 +345,12 @@ impl MetricBuilder { pub fn finish(self) -> Metric { self.metric } + + pub fn send(self) { + if let Some(client) = Hub::current().client() { + client.add_metric(self.finish()); + } + } } pub struct MetricAggregator { From ba4afe0dd0c70e36cb50a14a72620e8e7f117980 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 19:30:04 +0100 Subject: [PATCH 19/33] fix: Move statsd parsing to metrics module --- sentry-core/src/cadence.rs | 40 ++--------------------------- sentry-core/src/metrics.rs | 52 +++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 39 deletions(-) diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 07742d96..96287d5e 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -2,8 +2,7 @@ use std::sync::Arc; use cadence::MetricSink; -use crate::metrics::{Metric, MetricType, MetricValue}; -use crate::units::MetricUnit; +use crate::metrics::Metric; use crate::{Client, Hub}; /// A [`cadence`] compatible [`MetricSink`]. @@ -31,7 +30,7 @@ where S: MetricSink, { fn emit(&self, string: &str) -> std::io::Result { - if let Some(metric) = parse_metric(string) { + if let Ok(metric) = Metric::parse_statsd(string) { self.client.add_metric(metric); } @@ -50,41 +49,6 @@ where } } -fn parse_metric(string: &str) -> Option { - let mut components = string.split('|'); - - let (mri_str, value_str) = components.next()?.split_once(':')?; - let (name, unit) = match mri_str.split_once('@') { - Some((name, unit_str)) => (name, unit_str.parse().ok()?), - None => (mri_str, MetricUnit::None), - }; - - let ty = components.next().and_then(|s| s.parse().ok())?; - let value = match ty { - MetricType::Counter => MetricValue::Counter(value_str.parse().ok()?), - MetricType::Distribution => MetricValue::Distribution(value_str.parse().ok()?), - MetricType::Set => MetricValue::Set(value_str.parse().ok()?), - MetricType::Gauge => MetricValue::Gauge(value_str.parse().ok()?), - }; - - let mut builder = Metric::build(name.to_owned(), value).with_unit(unit); - - for component in components { - if let Some('#') = component.chars().next() { - for pair in component.get(1..)?.split(',') { - let mut key_value = pair.splitn(2, ':'); - - let key = key_value.next()?.to_owned(); - let value = key_value.next().unwrap_or_default().to_owned(); - - builder = builder.with_tag(key, value); - } - } - } - - Some(builder.finish()) -} - #[cfg(test)] mod tests { use cadence::{Counted, Distributed}; diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 3949c13d..e5a98c81 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::fmt; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; @@ -299,6 +299,10 @@ impl Metric { MetricBuilder { metric } } + pub fn parse_statsd(string: &str) -> Result { + parse_metric_opt(string).ok_or(ParseMetricError(())) + } + pub fn incr(name: impl Into) -> MetricBuilder { Self::build(name, MetricValue::Counter(1.0)) } @@ -353,6 +357,52 @@ impl MetricBuilder { } } +#[derive(Debug)] +pub struct ParseMetricError(()); + +impl std::error::Error for ParseMetricError {} + +impl fmt::Display for ParseMetricError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("invalid metric string") + } +} + +fn parse_metric_opt(string: &str) -> Option { + let mut components = string.split('|'); + + let (mri_str, value_str) = components.next()?.split_once(':')?; + let (name, unit) = match mri_str.split_once('@') { + Some((name, unit_str)) => (name, unit_str.parse().ok()?), + None => (mri_str, MetricUnit::None), + }; + + let ty = components.next().and_then(|s| s.parse().ok())?; + let value = match ty { + MetricType::Counter => MetricValue::Counter(value_str.parse().ok()?), + MetricType::Distribution => MetricValue::Distribution(value_str.parse().ok()?), + MetricType::Set => MetricValue::Set(value_str.parse().ok()?), + MetricType::Gauge => MetricValue::Gauge(value_str.parse().ok()?), + }; + + let mut builder = Metric::build(name.to_owned(), value).with_unit(unit); + + for component in components { + if let Some('#') = component.chars().next() { + for pair in component.get(1..)?.split(',') { + let mut key_value = pair.splitn(2, ':'); + + let key = key_value.next()?.to_owned(); + let value = key_value.next().unwrap_or_default().to_owned(); + + builder = builder.with_tag(key, value); + } + } + } + + Some(builder.finish()) +} + pub struct MetricAggregator { inner: Arc>, handle: Option>, From bcc665b4156f7698adbbc016c6718a918f0424f0 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 20:25:20 +0100 Subject: [PATCH 20/33] ref(metrics): Reorganize and add docs --- sentry-core/src/lib.rs | 2 +- sentry-core/src/metrics.rs | 375 +++++++++++++++++++++++++++++++------ 2 files changed, 316 insertions(+), 61 deletions(-) diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index 5b1f5cd4..fa9288bd 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -144,7 +144,7 @@ mod client; #[cfg(feature = "client")] mod hub_impl; #[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] -mod metrics; +pub mod metrics; #[cfg(feature = "client")] mod session; #[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index e5a98c81..47847f14 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,3 +1,9 @@ +//! Utilities to track metrics in Sentry. +//! +//! Metrics allow you to track the custom values related to the behavior and performance of your +//! application and send them to Sentry. See [`Metric`] for more information on how to build and +//! capture metrics. + use std::borrow::Cow; use std::collections::hash_map::{DefaultHasher, Entry}; use std::collections::{BTreeMap, BTreeSet, HashMap}; @@ -11,15 +17,130 @@ use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; use crate::Hub; -use crate::units::DurationUnit; -pub use crate::units::MetricUnit; +pub use crate::units::*; const BUCKET_INTERVAL: Duration = Duration::from_secs(10); const FLUSH_INTERVAL: Duration = Duration::from_secs(5); const MAX_WEIGHT: usize = 100_000; +/// Type alias for strings used in [`Metric`] for names and tags. +pub type MetricStr = Cow<'static, str>; + +/// Type used for [`MetricValue::Counter`]. +pub type CounterType = f64; + +/// Type used for [`MetricValue::Distribution`]. +pub type DistributionType = f64; + +/// Type used for [`MetricValue::Set`]. +pub type SetType = u32; + +/// Type used for [`MetricValue::Gauge`]. +pub type GaugeType = f64; + +/// The value of a [`Metric`], indicating its type. +#[derive(Debug)] +pub enum MetricValue { + /// Counts instances of an event. + /// + /// Counters can be incremented and decremented. The default operation is to increment a counter + /// by `1`, although increments by larger values and even floating point values are possible. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric, MetricValue}; + /// + /// Metric::build("my.counter", MetricValue::Counter(1.0)).send(); + /// ``` + Counter(CounterType), + + /// Builds a statistical distribution over values reported. + /// + /// Based on individual reported values, distributions allow to query the maximum, minimum, or + /// average of the reported values, as well as statistical quantiles. With an increasing number + /// of values in the distribution, its accuracy becomes approximate. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric, MetricValue}; + /// + /// Metric::build("my.distribution", MetricValue::Distribution(42.0)).send(); + /// ``` + Distribution(DistributionType), + + /// Counts the number of unique reported values. + /// + /// Sets allow sending arbitrary discrete values, including strings, and store the deduplicated + /// count. With an increasing number of unique values in the set, its accuracy becomes + /// approximate. It is not possible to query individual values from a set. + /// + /// # Example + /// + /// To create a set value, use [`MetricValue::set_from_str`] or + /// [`MetricValue::set_from_display`]. These functions convert the provided argument into a + /// unique hash value, which is then used as the set value. + /// + /// ``` + /// use sentry::metrics::{Metric, MetricValue}; + /// + /// Metric::build("my.set", MetricValue::set_from_str("foo")).send(); + /// ``` + Set(SetType), + + /// Stores absolute snapshots of values. + /// + /// In addition to plain [counters](Self::Counter), gauges store a snapshot of the maximum, + /// minimum and sum of all values, as well as the last reported value. Note that the "last" + /// component of this aggregation is not commutative. Which value is preserved as last value is + /// implementation-defined. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric, MetricValue}; + /// + /// Metric::build("my.gauge", MetricValue::Gauge(42.0)).send(); + /// ``` + Gauge(GaugeType), +} + +impl MetricValue { + /// Returns a set value representing the given string. + pub fn set_from_str(string: &str) -> Self { + Self::Set(hash_set_value(string)) + } + + /// Returns a set value representing the given argument. + pub fn set_from_display(display: impl fmt::Display) -> Self { + Self::Set(hash_set_value(&display.to_string())) + } + + /// Returns the type of the metric value. + fn ty(&self) -> MetricType { + match self { + Self::Counter(_) => MetricType::Counter, + Self::Distribution(_) => MetricType::Distribution, + Self::Gauge(_) => MetricType::Gauge, + Self::Set(_) => MetricType::Set, + } + } +} + +/// Hashes the given set value. +/// +/// Sets only guarantee 32-bit accuracy, but arbitrary strings are allowed on the protocol. Upon +/// parsing, they are hashed and only used as hashes subsequently. +fn hash_set_value(string: &str) -> u32 { + use std::hash::Hasher; + let mut hasher = DefaultHasher::default(); + hasher.write(string.as_bytes()); + hasher.finish() as u32 +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub(crate) enum MetricType { +enum MetricType { Counter, Distribution, Set, @@ -58,21 +179,9 @@ impl std::str::FromStr for MetricType { } } -/// Type used for Counter metric -pub type CounterType = f64; - -/// Type of distribution entries -pub type DistributionType = f64; - -/// Type used for set elements in Set metric -pub type SetType = u32; - -/// Type used for Gauge entries -pub type GaugeType = f64; - /// A snapshot of values. #[derive(Clone, Copy, Debug, PartialEq)] -pub struct GaugeValue { +struct GaugeValue { /// The last value reported in the bucket. /// /// This aggregation is not commutative. @@ -109,6 +218,7 @@ impl GaugeValue { } } +/// The aggregated value of a [`Metric`] bucket. enum BucketValue { Counter(CounterType), Distribution(Vec), @@ -117,6 +227,7 @@ enum BucketValue { } impl BucketValue { + /// Inserts a new value into the bucket and returns the added weight. pub fn insert(&mut self, value: MetricValue) -> usize { match (self, value) { (Self::Counter(c1), MetricValue::Counter(c2)) => { @@ -142,6 +253,7 @@ impl BucketValue { } } + /// Returns the number of values stored in this bucket. pub fn weight(&self) -> usize { match self { BucketValue::Counter(_) => 1, @@ -163,10 +275,10 @@ impl From for BucketValue { } } -pub type MetricStr = Cow<'static, str>; - +/// UNIX timestamp used for buckets. type Timestamp = u64; +/// Composite bucket key for [`BucketMap`]. #[derive(PartialEq, Eq, Hash)] struct BucketKey { timestamp: Timestamp, @@ -176,46 +288,15 @@ struct BucketKey { tags: BTreeMap, } -#[derive(Debug)] -pub enum MetricValue { - Counter(CounterType), - Distribution(DistributionType), - Gauge(GaugeType), - Set(SetType), -} - -impl MetricValue { - /// Returns a bucket value representing a set with a single given string value. - pub fn set_from_str(string: &str) -> Self { - Self::Set(hash_set_value(string)) - } - - /// Returns a bucket value representing a set with a single given value. - pub fn set_from_display(display: impl fmt::Display) -> Self { - Self::Set(hash_set_value(&display.to_string())) - } - - fn ty(&self) -> MetricType { - match self { - Self::Counter(_) => MetricType::Counter, - Self::Distribution(_) => MetricType::Distribution, - Self::Gauge(_) => MetricType::Gauge, - Self::Set(_) => MetricType::Set, - } - } -} - -/// Hashes the given set value. +/// A nested map storing metric buckets. /// -/// Sets only guarantee 32-bit accuracy, but arbitrary strings are allowed on the protocol. Upon -/// parsing, they are hashed and only used as hashes subsequently. -fn hash_set_value(string: &str) -> u32 { - use std::hash::Hasher; - let mut hasher = DefaultHasher::default(); - hasher.write(string.as_bytes()); - hasher.finish() as u32 -} - +/// This map consists of two levels: +/// 1. The rounded UNIX timestamp of buckets. +/// 2. The metric buckets themselves with a corresponding timestamp. +/// +/// This structure allows for efficient dequeueing of buckets that are older than a certain +/// threshold. The buckets are dequeued in order of their timestamp, so the oldest buckets are +/// dequeued first. type BucketMap = BTreeMap>; struct AggregatorInner { @@ -235,6 +316,10 @@ impl AggregatorInner { } } + /// Adds a new bucket to the aggregator. + /// + /// The bucket timestamp is rounded to the nearest bucket interval. Note that this does NOT + /// automatically flush the aggregator if the weight exceeds the weight threshold. pub fn add(&mut self, mut key: BucketKey, value: MetricValue) { // Floor timestamp to bucket interval key.timestamp /= BUCKET_INTERVAL.as_secs(); @@ -246,6 +331,10 @@ impl AggregatorInner { } } + /// Removes and returns all buckets that are ready to flush. + /// + /// Buckets are ready to flush as soon as their time window has closed. For example, a bucket + /// from timestamps `[4600, 4610)` is ready to flush immediately at `4610`. pub fn take_buckets(&mut self) -> BucketMap { if self.force_flush || !self.running { self.weight = 0; @@ -255,7 +344,7 @@ impl AggregatorInner { let timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() - .saturating_sub(FLUSH_INTERVAL) + .saturating_sub(BUCKET_INTERVAL) .as_secs(); // Split all buckets after the cutoff time. `split` contains newer buckets, which should @@ -278,7 +367,58 @@ impl AggregatorInner { } } +/// A metric value that contains a numeric value and metadata to be sent to Sentry. +/// +/// # Units +/// +/// To make the most out of metrics in Sentry, consider assigning a unit during construction. This +/// can be achieved using the [`with_unit`](MetricBuilder::with_unit) builder method. +/// +/// ``` +/// use sentry::metrics::{Metric, MetricUnit, InformationUnit}; +/// +/// Metric::distribution("request.size", 47.2) +/// .with_unit(MetricUnit::Information(InformationUnit::Byte)) +/// .send(); +/// ``` +/// +/// # Sending Metrics +/// +/// Metrics can be sent to Sentry directly using the [`send`](MetricBuilder::send) method on the +/// constructor. This will send the metric to the [`Client`](crate::Client) on the current [`Hub`]. +/// If there is no client on the current hub, the metric is dropped. +/// +/// ``` +/// use sentry::metrics::Metric; +/// +/// Metric::count("requests") +/// .with_tag("method", "GET") +/// .send(); +/// ``` +/// +/// # Sending to a Custom Client +/// +/// Metrics can also be sent to a custom client. This is useful if you want to send metrics to a +/// different Sentry project or with different configuration. To do so, finish building the metric +/// and then add it to the client: +/// +/// ``` +/// use sentry::Hub; +/// use sentry::metrics::Metric; +/// +/// let metric = Metric::count("requests") +/// .with_tag("method", "GET") +/// .finish(); +/// +/// // Obtain a client from somewhere +/// if let Some(client) = Hub::current().client() { +/// client.add_metric(metric); +/// } +/// ``` pub struct Metric { + /// The name of the metric, identifying it in Sentry. + /// + /// The name should consist of name: MetricStr, unit: MetricUnit, value: MetricValue, @@ -287,6 +427,15 @@ pub struct Metric { } impl Metric { + /// Creates a new metric with the stated name and value. + /// + /// The provided name identifies the metric in Sentry. It should consist of alphanumeric + /// characters and `_`, `-`, and `.`. While a single forward slash (`/`) is also allowed in + /// metric names, it has a special meaning and should not be used in regular metric names. All + /// characters that do not match this criteria are sanitized. + /// + /// The value of the metric determines its type. See the [struct-level](self) docs and + /// constructor methods for examples on how to build metrics. pub fn build(name: impl Into, value: MetricValue) -> MetricBuilder { let metric = Metric { name: name.into(), @@ -299,57 +448,162 @@ impl Metric { MetricBuilder { metric } } + /// Parses a metric from a StatsD string. + /// + /// This supports regular StatsD payloads with an extension for tags. In the below example, tags + /// are optional: + /// + /// ```plain + /// :||#:,: + /// ``` + /// + /// Units are encoded into the metric name, separated by an `@`: + /// + /// ```plain + /// @:||#:,: + /// ``` pub fn parse_statsd(string: &str) -> Result { parse_metric_opt(string).ok_or(ParseMetricError(())) } - pub fn incr(name: impl Into) -> MetricBuilder { + /// Builds a metric that increments a [counter](MetricValue::Counter) by the given value. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric}; + /// + /// Metric::incr("operation.total_values", 7.0).send(); + /// ``` + pub fn incr(name: impl Into, value: f64) -> MetricBuilder { + Self::build(name, MetricValue::Counter(value)) + } + + /// Builds a metric that [counts](MetricValue::Counter) the single occurrence of an event. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric}; + /// + /// Metric::count("requests").send(); + /// ``` + pub fn count(name: impl Into) -> MetricBuilder { Self::build(name, MetricValue::Counter(1.0)) } + /// Builds a metric that tracks the duration of an operation. + /// + /// This is a [distribution](MetricValue::Distribution) metric that is tracked in seconds. + /// + /// # Example + /// + /// ``` + /// use std::time::Duration; + /// use sentry::metrics::{Metric}; + /// + /// Metric::timing("operation", Duration::from_secs(1)).send(); + /// ``` pub fn timing(name: impl Into, timing: Duration) -> MetricBuilder { Self::build(name, MetricValue::Distribution(timing.as_secs_f64())) .with_unit(MetricUnit::Duration(DurationUnit::Second)) } + /// Builds a metric that tracks the [distribution](MetricValue::Distribution) of values. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric}; + /// + /// Metric::distribution("operation.batch_size", 42.0).send(); + /// ``` pub fn distribution(name: impl Into, value: f64) -> MetricBuilder { Self::build(name, MetricValue::Distribution(value)) } + /// Builds a metric that tracks the [unique number](MetricValue::Set) of values provided. + /// + /// See [`MetricValue`] for more ways to construct sets. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric}; + /// + /// Metric::set("users", "user1").send(); + /// ``` pub fn set(name: impl Into, string: &str) -> MetricBuilder { Self::build(name, MetricValue::set_from_str(string)) } + /// Builds a metric that tracks the [snapshot](MetricValue::Gauge) of provided values. + /// + /// # Example + /// + /// ``` + /// use sentry::metrics::{Metric}; + /// + /// Metric::gauge("cache.size", 42.0).send(); + /// ``` pub fn gauge(name: impl Into, value: f64) -> MetricBuilder { Self::build(name, MetricValue::Gauge(value)) } } +/// A builder for metrics. +/// +/// Use one of the [`Metric`] constructors to create a new builder. See the struct-level docs for +/// examples of how to build metrics. #[must_use] pub struct MetricBuilder { metric: Metric, } impl MetricBuilder { + /// Sets the unit for the metric. + /// + /// The unit augments the metric value by giving it a magnitude and semantics. Some units have + /// special support when rendering metrics or their values in Sentry, such as for timings. See + /// [`MetricUnit`] for more information on the supported units. The unit can be set to + /// [`MetricUnit::None`] to indicate that the metric has no unit, or to [`MetricUnit::Custom`] + /// to indicate a user-defined unit. + /// + /// By default, the unit is set to [`MetricUnit::None`]. pub fn with_unit(mut self, unit: MetricUnit) -> Self { self.metric.unit = unit; self } + /// Adds a tag to the metric. + /// + /// Tags allow you to add dimensions to metrics. They are key-value pairs that can be filtered + /// or grouped by in Sentry. + /// + /// When sent to Sentry via [`MetricBuilder::send`] or when added to a + /// [`Client`](crate::Client), the client may add default tags to the metrics, such as the + /// `release` or the `environment` from the Scope. pub fn with_tag(mut self, name: impl Into, value: impl Into) -> Self { self.metric.tags.insert(name.into(), value.into()); self } + /// Sets the timestamp for the metric. + /// + /// By default, the timestamp is set to the current time when the metric is built or sent. pub fn with_time(mut self, time: SystemTime) -> Self { self.metric.time = Some(time); self } + /// Builds the metric. pub fn finish(self) -> Metric { self.metric } + /// Sends the metric to the current client. + /// + /// If there is no client on the current [`Hub`], the metric is dropped. pub fn send(self) { if let Some(client) = Hub::current().client() { client.add_metric(self.finish()); @@ -357,6 +611,7 @@ impl MetricBuilder { } } +/// Error emitted from [`Metric::parse_statsd`] for invalid metric strings. #[derive(Debug)] pub struct ParseMetricError(()); @@ -403,7 +658,7 @@ fn parse_metric_opt(string: &str) -> Option { Some(builder.finish()) } -pub struct MetricAggregator { +pub(crate) struct MetricAggregator { inner: Arc>, handle: Option>, } From 32e6baaa9995590f30ba90aec36a94cac98d0c83 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 20:34:03 +0100 Subject: [PATCH 21/33] test: Add a first unit test --- sentry-core/src/metrics.rs | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 47847f14..d2d42ba2 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -841,3 +841,49 @@ impl<'s> fmt::Display for SaveVal<'s> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test::with_captured_envelopes; + + /// Returns the current system time and rounded bucket timestamp. + fn current_time() -> (SystemTime, u64) { + let now = SystemTime::now(); + let timestamp = now.duration_since(UNIX_EPOCH).unwrap_or_default().as_secs(); + let timestamp = timestamp / 10 * 10; + + (now, timestamp) + } + + fn get_single_metrics(envelopes: &[Envelope]) -> &str { + assert_eq!(envelopes.len(), 1, "expected exactly one envelope"); + + let mut items = envelopes[0].items(); + let Some(EnvelopeItem::Metrics(payload)) = items.next() else { + panic!("expected metrics item"); + }; + + std::str::from_utf8(payload).unwrap().trim() + } + + #[test] + fn test_counter() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("requests") + .with_tag("foo", "bar") + .with_time(time) + .send(); + + Metric::incr("requests", 2.0) + .with_tag("foo", "bar") + .with_time(time) + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("requests:3|c|#foo:bar|T{ts}")); + } +} From 0ef19a3c99daedfa0273630eba700218695dc307 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 11 Dec 2023 21:07:57 +0100 Subject: [PATCH 22/33] feat(metrics): Inject default tags --- sentry-core/src/client.rs | 8 +++-- sentry-core/src/metrics.rs | 74 +++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 7e4f1b67..b659d0c3 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -70,7 +70,10 @@ impl Clone for Client { self.options.session_mode, ))); #[cfg(feature = "UNSTABLE_metrics")] - let metric_aggregator = RwLock::new(Some(MetricAggregator::new(transport.clone()))); + let metric_aggregator = RwLock::new(Some(MetricAggregator::new( + transport.clone(), + &self.options, + ))); Client { options: self.options.clone(), transport, @@ -146,7 +149,8 @@ impl Client { ))); #[cfg(feature = "UNSTABLE_metrics")] - let metric_aggregator = RwLock::new(Some(MetricAggregator::new(transport.clone()))); + let metric_aggregator = + RwLock::new(Some(MetricAggregator::new(transport.clone(), &options))); Client { options, diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index d2d42ba2..d495151b 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -15,7 +15,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use sentry_types::protocol::latest::{Envelope, EnvelopeItem}; use crate::client::TransportArc; -use crate::Hub; +use crate::{ClientOptions, Hub}; pub use crate::units::*; @@ -658,19 +658,34 @@ fn parse_metric_opt(string: &str) -> Option { Some(builder.finish()) } +pub(crate) type TagMap = BTreeMap; + +fn get_default_tags(options: &ClientOptions) -> TagMap { + let mut tags = TagMap::new(); + if let Some(ref release) = options.release { + tags.insert("release".into(), release.clone()); + } + if let Some(ref environment) = options.environment { + tags.insert("environment".into(), environment.clone()); + } + tags +} + pub(crate) struct MetricAggregator { inner: Arc>, handle: Option>, } impl MetricAggregator { - pub fn new(transport: TransportArc) -> Self { + pub fn new(transport: TransportArc, options: &ClientOptions) -> Self { + let default_tags = get_default_tags(options); + let inner = Arc::new(Mutex::new(AggregatorInner::new())); let inner_clone = Arc::clone(&inner); let handle = thread::Builder::new() .name("sentry-metrics".into()) - .spawn(move || Self::worker_thread(inner_clone, transport)) + .spawn(move || Self::worker_thread(inner_clone, transport, default_tags)) .expect("failed to spawn thread"); Self { @@ -719,7 +734,7 @@ impl MetricAggregator { } } - fn worker_thread(inner: Arc>, transport: TransportArc) { + fn worker_thread(inner: Arc>, transport: TransportArc, tags: TagMap) { let mut running = true; while running { @@ -734,15 +749,15 @@ impl MetricAggregator { }; if !buckets.is_empty() { - Self::flush_buckets(buckets, &transport); + Self::flush_buckets(buckets, &transport, &tags); } } } - fn flush_buckets(buckets: BucketMap, transport: &TransportArc) { + fn flush_buckets(buckets: BucketMap, transport: &TransportArc, tags: &TagMap) { // The transport is usually available when flush is called. Prefer a short lock and worst // case throw away the result rather than blocking the transport for too long. - if let Ok(output) = Self::format_payload(buckets) { + if let Ok(output) = Self::format_payload(buckets, tags) { let mut envelope = Envelope::new(); envelope.add_item(EnvelopeItem::Metrics(output)); @@ -752,7 +767,7 @@ impl MetricAggregator { } } - fn format_payload(buckets: BucketMap) -> std::io::Result> { + fn format_payload(buckets: BucketMap, tags: &TagMap) -> std::io::Result> { use std::io::Write; let mut out = vec![]; @@ -787,14 +802,13 @@ impl MetricAggregator { write!(&mut out, "|{}", key.ty.as_str())?; - if !key.tags.is_empty() { - write!(&mut out, "|#")?; - for (i, (k, v)) in key.tags.into_iter().enumerate() { - if i > 0 { - write!(&mut out, ",")?; - } - write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SaveVal(v.as_ref()))?; + for (i, (k, v)) in key.tags.iter().chain(tags).enumerate() { + match i { + 0 => write!(&mut out, "|#")?, + _ => write!(&mut out, ",")?, } + + write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SaveVal(v.as_ref()))?; } writeln!(&mut out, "|T{}", key.timestamp)?; @@ -845,7 +859,8 @@ impl<'s> fmt::Display for SaveVal<'s> { #[cfg(test)] mod tests { use super::*; - use crate::test::with_captured_envelopes; + use crate::test::{with_captured_envelopes, with_captured_envelopes_options}; + use crate::ClientOptions; /// Returns the current system time and rounded bucket timestamp. fn current_time() -> (SystemTime, u64) { @@ -886,4 +901,31 @@ mod tests { let metrics = get_single_metrics(&envelopes); assert_eq!(metrics, format!("requests:3|c|#foo:bar|T{ts}")); } + + #[test] + fn test_default_tags() { + let (time, ts) = current_time(); + + let options = ClientOptions { + release: Some("myapp@1.0.0".into()), + environment: Some("production".into()), + ..Default::default() + }; + + let envelopes = with_captured_envelopes_options( + || { + Metric::count("requests") + .with_tag("foo", "bar") + .with_time(time) + .send(); + }, + options, + ); + + let metrics = get_single_metrics(&envelopes); + assert_eq!( + metrics, + format!("requests:1|c|#foo:bar,environment:production,release:myapp@1.0.0|T{ts}") + ); + } } From 12bf258f487cc249929f9027116ad83a36f99cfa Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 09:10:28 +0100 Subject: [PATCH 23/33] ref(metrics): Simplify unit handling --- sentry-core/src/metrics.rs | 55 +++++++++++++++--- sentry-core/src/units.rs | 115 +++++++++++++------------------------ 2 files changed, 87 insertions(+), 83 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index d495151b..626b8e32 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -375,10 +375,10 @@ impl AggregatorInner { /// can be achieved using the [`with_unit`](MetricBuilder::with_unit) builder method. /// /// ``` -/// use sentry::metrics::{Metric, MetricUnit, InformationUnit}; +/// use sentry::metrics::{Metric, InformationUnit}; /// /// Metric::distribution("request.size", 47.2) -/// .with_unit(MetricUnit::Information(InformationUnit::Byte)) +/// .with_unit(InformationUnit::Byte) /// .send(); /// ``` /// @@ -506,7 +506,7 @@ impl Metric { /// ``` pub fn timing(name: impl Into, timing: Duration) -> MetricBuilder { Self::build(name, MetricValue::Distribution(timing.as_secs_f64())) - .with_unit(MetricUnit::Duration(DurationUnit::Second)) + .with_unit(DurationUnit::Second) } /// Builds a metric that tracks the [distribution](MetricValue::Distribution) of values. @@ -570,8 +570,8 @@ impl MetricBuilder { /// to indicate a user-defined unit. /// /// By default, the unit is set to [`MetricUnit::None`]. - pub fn with_unit(mut self, unit: MetricUnit) -> Self { - self.metric.unit = unit; + pub fn with_unit(mut self, unit: impl Into) -> Self { + self.metric.unit = unit.into(); self } @@ -887,19 +887,58 @@ mod tests { let (time, ts) = current_time(); let envelopes = with_captured_envelopes(|| { - Metric::count("requests") + Metric::count("my.metric") .with_tag("foo", "bar") .with_time(time) .send(); - Metric::incr("requests", 2.0) + Metric::incr("my.metric", 2.0) .with_tag("foo", "bar") .with_time(time) .send(); }); let metrics = get_single_metrics(&envelopes); - assert_eq!(metrics, format!("requests:3|c|#foo:bar|T{ts}")); + assert_eq!(metrics, format!("my.metric:3|c|#foo:bar|T{ts}")); + } + + #[test] + fn test_timing() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::timing("my.metric", Duration::from_millis(200)) + .with_tag("foo", "bar") + .with_time(time) + .send(); + + Metric::timing("my.metric", Duration::from_millis(100)) + .with_tag("foo", "bar") + .with_time(time) + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!( + metrics, + format!("my.metric@second:0.2:0.1|d|#foo:bar|T{ts}") + ); + } + + #[test] + fn test_unit() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("my.metric") + .with_tag("foo", "bar") + .with_time(time) + .with_unit("custom") + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric@custom:1|c|#foo:bar|T{ts}")); } #[test] diff --git a/sentry-core/src/units.rs b/sentry-core/src/units.rs index 93236e1e..a873a52a 100644 --- a/sentry-core/src/units.rs +++ b/sentry-core/src/units.rs @@ -1,5 +1,6 @@ //! Type definitions for Sentry metrics. +use std::borrow::Cow; use std::fmt; /// The unit of measurement of a metric value. @@ -9,7 +10,7 @@ use std::fmt; /// measurements. /// /// Units and their precisions are uniquely represented by a string identifier. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Default)] +#[derive(Clone, Debug, Eq, PartialEq, Hash, Default)] pub enum MetricUnit { /// A time duration, defaulting to `"millisecond"`. Duration(DurationUnit), @@ -18,7 +19,7 @@ pub enum MetricUnit { /// Fractions such as percentages, defaulting to `"ratio"`. Fraction(FractionUnit), /// user-defined units without builtin conversion or default. - Custom(CustomUnit), + Custom(Cow<'static, str>), /// Untyped value without a unit (`""`). #[default] None, @@ -76,11 +77,47 @@ impl std::str::FromStr for MetricUnit { "percent" => Self::Fraction(FractionUnit::Percent), "" | "none" => Self::None, - _ => Self::Custom(CustomUnit::parse(s)?), + _ => Self::Custom(s.to_owned().into()), }) } } +impl From for MetricUnit { + fn from(unit: DurationUnit) -> Self { + Self::Duration(unit) + } +} + +impl From for MetricUnit { + fn from(unit: InformationUnit) -> Self { + Self::Information(unit) + } +} + +impl From for MetricUnit { + fn from(unit: FractionUnit) -> Self { + Self::Fraction(unit) + } +} + +impl From<&'static str> for MetricUnit { + fn from(unit: &'static str) -> Self { + Self::Custom(unit.into()) + } +} + +impl From for MetricUnit { + fn from(unit: String) -> Self { + Self::Custom(unit.into()) + } +} + +impl From> for MetricUnit { + fn from(unit: Cow<'static, str>) -> Self { + Self::Custom(unit) + } +} + /// Time duration units used in [`MetricUnit::Duration`]. /// /// Defaults to `millisecond`. @@ -219,75 +256,3 @@ impl fmt::Display for FractionUnit { } } } - -const CUSTOM_UNIT_MAX_SIZE: usize = 15; - -/// Custom user-defined units without builtin conversion. -#[derive(Clone, Copy, Eq, PartialEq, Hash)] -pub struct CustomUnit([u8; CUSTOM_UNIT_MAX_SIZE]); - -impl CustomUnit { - /// Parses a `CustomUnit` from a string. - pub fn parse(s: &str) -> Result { - if !s.is_ascii() { - return Err(ParseMetricUnitError(())); - } - - let mut unit = Self([0; CUSTOM_UNIT_MAX_SIZE]); - let slice = unit.0.get_mut(..s.len()).ok_or(ParseMetricUnitError(()))?; - slice.copy_from_slice(s.as_bytes()); - unit.0.make_ascii_lowercase(); - Ok(unit) - } - - /// Returns the string representation of this unit. - #[inline] - pub fn as_str(&self) -> &str { - // Safety: The string is already validated to be valid ASCII when - // parsing `CustomUnit`. - unsafe { std::str::from_utf8_unchecked(&self.0).trim_end_matches('\0') } - } -} - -impl fmt::Debug for CustomUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} - -impl fmt::Display for CustomUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} - -impl std::str::FromStr for CustomUnit { - type Err = ParseMetricUnitError; - - fn from_str(s: &str) -> Result { - Self::parse(s) - } -} - -impl std::ops::Deref for CustomUnit { - type Target = str; - - fn deref(&self) -> &Self::Target { - self.as_str() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_custom_unit_parse() { - assert_eq!("foo", CustomUnit::parse("Foo").unwrap().as_str()); - assert_eq!( - "0123456789abcde", - CustomUnit::parse("0123456789abcde").unwrap().as_str() - ); - assert!(CustomUnit::parse("this_is_a_unit_that_is_too_long").is_err()); - } -} From d90d14c84d7115cdc3fd0dd3d59a1c4f480b87d3 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 10:30:03 +0100 Subject: [PATCH 24/33] feat(metrics): Further improvements to docs and cadence --- sentry-core/src/cadence.rs | 114 +++++++++++++++++++++----- sentry-core/src/lib.rs | 4 +- sentry-core/src/metrics.rs | 163 ++++++++++++++++++++++++++++--------- sentry-core/src/units.rs | 8 +- 4 files changed, 224 insertions(+), 65 deletions(-) diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 96287d5e..46928d7a 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -1,50 +1,122 @@ +//! [`cadence`] integration for Sentry. +//! +//! [`cadence`] is a popular Statsd client for Rust. The [`SentryMetricSink`] provides a drop-in +//! integration to send metrics captured via `cadence` to Sentry. For direct usage of Sentry +//! metrics, see the [`metrics`](crate::metrics) module. +//! +//! # Usage +//! +//! To use the `cadence` integration, enable the `UNSTABLE_cadence` feature in your `Cargo.toml`. +//! Then, create a [`SentryMetricSink`] and pass it to your `cadence` client: +//! +//! ``` +//! use cadence::StatsdClient; +//! use sentry::cadence::SentryMetricSink; +//! +//! let client = StatsdClient::from_sink("sentry.test", SentryMetricSink::new()); +//! ``` +//! +//! # Side-by-side Usage +//! +//! If you want to send metrics to Sentry and another backend at the same time, you can use +//! [`SentryMetricSink::wrap`] to wrap another [`MetricSink`]: +//! +//! ``` +//! use cadence::{StatsdClient, NopMetricSink}; +//! use sentry::cadence::SentryMetricSink; +//! +//! let sink = SentryMetricSink::wrap(NopMetricSink); +//! let client = StatsdClient::from_sink("sentry.test", sink); +//! ``` + use std::sync::Arc; -use cadence::MetricSink; +use cadence::{MetricSink, NopMetricSink}; use crate::metrics::Metric; use crate::{Client, Hub}; -/// A [`cadence`] compatible [`MetricSink`]. +/// A [`MetricSink`] that sends metrics to Sentry. +/// +/// This metric sends all metrics to Sentry. The Sentry client is internally buffered, so submission +/// will be delayed. /// -/// This will ingest all the emitted metrics to Sentry as well as forward them -/// to the inner [`MetricSink`]. +/// Optionally, this sink can also forward metrics to another [`MetricSink`]. This is useful if you +/// want to send metrics to Sentry and another backend at the same time. Use +/// [`SentryMetricSink::wrap`] to construct such a sink. #[derive(Debug)] -pub struct SentryMetricSink { - client: Arc, +pub struct SentryMetricSink { + client: Option>, sink: S, } -impl SentryMetricSink { +impl SentryMetricSink +where + S: MetricSink, +{ /// Creates a new [`SentryMetricSink`], wrapping the given [`MetricSink`]. - pub fn try_new(sink: S) -> Result { - match Hub::current().client() { - Some(client) => Ok(Self { client, sink }), - None => Err(sink), + pub fn wrap(sink: S) -> Self { + Self { client: None, sink } + } + + /// Creates a new [`SentryMetricSink`] sending data to the given [`Client`]. + pub fn with_client(mut self, client: Arc) -> Self { + self.client = Some(client); + self + } +} + +impl SentryMetricSink { + /// Creates a new [`SentryMetricSink`]. + /// + /// It is not required that a client is available when this sink is created. The sink sends + /// metrics to the client of the Sentry hub that is registered when the metrics are emitted. + pub fn new() -> Self { + Self { + client: None, + sink: NopMetricSink, } } } -impl MetricSink for SentryMetricSink -where - S: MetricSink, -{ +impl Default for SentryMetricSink { + fn default() -> Self { + Self::new() + } +} + +impl MetricSink for SentryMetricSink { fn emit(&self, string: &str) -> std::io::Result { if let Ok(metric) = Metric::parse_statsd(string) { - self.client.add_metric(metric); + if let Some(ref client) = self.client { + client.add_metric(metric); + } else if let Some(client) = Hub::current().client() { + client.add_metric(metric); + } } + // NopMetricSink returns `0`, which is correct as Sentry is buffering the metrics. self.sink.emit(string) } fn flush(&self) -> std::io::Result<()> { - if self.client.flush(None) { - self.sink.flush() + let flushed = if let Some(ref client) = self.client { + client.flush(None) + } else if let Some(client) = Hub::current().client() { + client.flush(None) } else { + true + }; + + let sink_result = self.sink.flush(); + + if !flushed { Err(std::io::Error::new( std::io::ErrorKind::Other, - "Flushing Client failed", + "failed to flush metrics to Sentry", )) + } else { + sink_result } } } @@ -61,9 +133,7 @@ mod tests { #[test] fn test_basic_metrics() { let envelopes = with_captured_envelopes(|| { - let sink = SentryMetricSink::try_new(cadence::NopMetricSink).unwrap(); - - let client = cadence::StatsdClient::from_sink("sentry.test", sink); + let client = cadence::StatsdClient::from_sink("sentry.test", SentryMetricSink::new()); client.count("some.count", 1).unwrap(); client.count("some.count", 10).unwrap(); client diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index fa9288bd..f29454e6 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -137,7 +137,7 @@ pub use crate::scope::{Scope, ScopeGuard}; pub use crate::transport::{Transport, TransportFactory}; #[cfg(all(feature = "client", feature = "UNSTABLE_cadence"))] -mod cadence; +pub mod cadence; // client feature #[cfg(feature = "client")] mod client; @@ -149,8 +149,6 @@ pub mod metrics; mod session; #[cfg(all(feature = "client", feature = "UNSTABLE_metrics"))] mod units; -#[cfg(all(feature = "client", feature = "UNSTABLE_cadence"))] -pub use crate::cadence::SentryMetricSink; #[cfg(feature = "client")] pub use crate::client::Client; diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 626b8e32..205a3c6d 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -1,8 +1,48 @@ //! Utilities to track metrics in Sentry. //! -//! Metrics allow you to track the custom values related to the behavior and performance of your -//! application and send them to Sentry. See [`Metric`] for more information on how to build and -//! capture metrics. +//! Metrics are numerical values that can track anything about your environment over time, from +//! latency to error rates to user signups. +//! +//! Metrics at Sentry come in different flavors, in order to help you track your data in the most +//! efficient and cost-effective way. The types of metrics we currently support are: +//! +//! - **Counters** track a value that can only be incremented. +//! - **Distributions** track a list of values over time in on which you can perform aggregations +//! like max, min, avg. +//! - **Gauges** track a value that can go up and down. +//! - **Sets** track a set of values on which you can perform aggregations such as count_unique. +//! +//! For more information on metrics in Sentry, see [our docs]. +//! +//! # Usage +//! +//! To collect a metric, use the [`Metric`] struct to capture all relevant properties of your +//! metric. Then, use [`send`](Metric::send) to send the metric to Sentry: +//! +//! ``` +//! use std::time::Duration; +//! use sentry::metrics::Metric; +//! +//! Metric::count("requests") +//! .with_tag("method", "GET") +//! .send(); +//! +//! Metric::timing("request.duration", Duration::from_millis(17)) +//! .with_tag("status_code", "200") +//! // unit is added automatically by timing +//! .send(); +//! +//! Metric::set("site.visitors", "user1") +//! .with_unit("user") +//! .send(); +//! ``` +//! +//! # Usage with Cadence +//! +//! [`cadence`] is a popular Statsd client for Rust and can be used to send metrics to Sentry. To +//! use Sentry directly with `cadence`, see the [`sentry-cadence`](crate::cadence) documentation. +//! +//! [our docs]: https://develop.sentry.dev/delightful-developer-metrics/ use std::borrow::Cow; use std::collections::hash_map::{DefaultHasher, Entry}; @@ -372,7 +412,8 @@ impl AggregatorInner { /// # Units /// /// To make the most out of metrics in Sentry, consider assigning a unit during construction. This -/// can be achieved using the [`with_unit`](MetricBuilder::with_unit) builder method. +/// can be achieved using the [`with_unit`](MetricBuilder::with_unit) builder method. See the +/// documentation for more examples on units. /// /// ``` /// use sentry::metrics::{Metric, InformationUnit}; @@ -400,7 +441,7 @@ impl AggregatorInner { /// /// Metrics can also be sent to a custom client. This is useful if you want to send metrics to a /// different Sentry project or with different configuration. To do so, finish building the metric -/// and then add it to the client: +/// and then call [`add_metric`](crate::Client::add_metric) to the client: /// /// ``` /// use sentry::Hub; @@ -722,6 +763,7 @@ impl MetricAggregator { if guard.weight() > MAX_WEIGHT { if let Some(ref handle) = self.handle { + guard.force_flush = true; handle.thread().unpark(); } } @@ -883,7 +925,7 @@ mod tests { } #[test] - fn test_counter() { + fn test_tags() { let (time, ts) = current_time(); let envelopes = with_captured_envelopes(|| { @@ -891,38 +933,10 @@ mod tests { .with_tag("foo", "bar") .with_time(time) .send(); - - Metric::incr("my.metric", 2.0) - .with_tag("foo", "bar") - .with_time(time) - .send(); }); let metrics = get_single_metrics(&envelopes); - assert_eq!(metrics, format!("my.metric:3|c|#foo:bar|T{ts}")); - } - - #[test] - fn test_timing() { - let (time, ts) = current_time(); - - let envelopes = with_captured_envelopes(|| { - Metric::timing("my.metric", Duration::from_millis(200)) - .with_tag("foo", "bar") - .with_time(time) - .send(); - - Metric::timing("my.metric", Duration::from_millis(100)) - .with_tag("foo", "bar") - .with_time(time) - .send(); - }); - - let metrics = get_single_metrics(&envelopes); - assert_eq!( - metrics, - format!("my.metric@second:0.2:0.1|d|#foo:bar|T{ts}") - ); + assert_eq!(metrics, format!("my.metric:1|c|#foo:bar|T{ts}")); } #[test] @@ -931,14 +945,13 @@ mod tests { let envelopes = with_captured_envelopes(|| { Metric::count("my.metric") - .with_tag("foo", "bar") .with_time(time) .with_unit("custom") .send(); }); let metrics = get_single_metrics(&envelopes); - assert_eq!(metrics, format!("my.metric@custom:1|c|#foo:bar|T{ts}")); + assert_eq!(metrics, format!("my.metric@custom:1|c|T{ts}")); } #[test] @@ -967,4 +980,80 @@ mod tests { format!("requests:1|c|#foo:bar,environment:production,release:myapp@1.0.0|T{ts}") ); } + + #[test] + fn test_counter() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("my.metric").with_time(time).send(); + Metric::incr("my.metric", 2.0).with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric:3|c|T{ts}")); + } + + #[test] + fn test_timing() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::timing("my.metric", Duration::from_millis(200)) + .with_time(time) + .send(); + Metric::timing("my.metric", Duration::from_millis(100)) + .with_time(time) + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric@second:0.2:0.1|d|T{ts}")); + } + + #[test] + fn test_distribution() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::distribution("my.metric", 2.0) + .with_time(time) + .send(); + Metric::distribution("my.metric", 1.0) + .with_time(time) + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric:2:1|d|T{ts}")); + } + + #[test] + fn test_set() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::set("my.metric", "hello").with_time(time).send(); + // Duplicate that should not be reflected twice + Metric::set("my.metric", "hello").with_time(time).send(); + Metric::set("my.metric", "world").with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric:3410894750:3817476724|s|T{ts}")); + } + + #[test] + fn test_gauge() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::gauge("my.metric", 2.0).with_time(time).send(); + Metric::gauge("my.metric", 1.0).with_time(time).send(); + Metric::gauge("my.metric", 1.5).with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my.metric:1.5:1:2:4.5:3|g|T{ts}")); + } } diff --git a/sentry-core/src/units.rs b/sentry-core/src/units.rs index a873a52a..bc47af65 100644 --- a/sentry-core/src/units.rs +++ b/sentry-core/src/units.rs @@ -6,10 +6,12 @@ use std::fmt; /// The unit of measurement of a metric value. /// /// Units augment metric values by giving them a magnitude and semantics. There are certain types of -/// units that are subdivided in their precision, such as the [`DurationUnit`] for time -/// measurements. +/// units that are subdivided in their precision: +/// - [`DurationUnit`]: time durations +/// - [`InformationUnit`]: sizes of information +/// - [`FractionUnit`]: fractions /// -/// Units and their precisions are uniquely represented by a string identifier. +/// You are not restricted to these units, but can use any `&'static str` or `String` as a unit. #[derive(Clone, Debug, Eq, PartialEq, Hash, Default)] pub enum MetricUnit { /// A time duration, defaulting to `"millisecond"`. From 524a5a912ecb48ec64f933a3987f370444fb6d36 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 10:53:27 +0100 Subject: [PATCH 25/33] fix(metrics): Sanitation behavior --- sentry-core/src/metrics.rs | 117 ++++++++++++++++++++++++++++++------- 1 file changed, 97 insertions(+), 20 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 205a3c6d..8274b68a 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -47,7 +47,7 @@ use std::borrow::Cow; use std::collections::hash_map::{DefaultHasher, Entry}; use std::collections::{BTreeMap, BTreeSet, HashMap}; -use std::fmt; +use std::fmt::{self, Write}; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -850,7 +850,7 @@ impl MetricAggregator { _ => write!(&mut out, ",")?, } - write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SaveVal(v.as_ref()))?; + write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SafeVal(v.as_ref()))?; } writeln!(&mut out, "|T{}", key.timestamp)?; @@ -870,40 +870,58 @@ impl Drop for MetricAggregator { } } +fn safe_fmt(f: &mut fmt::Formatter<'_>, string: &str, mut check: F) -> fmt::Result +where + F: FnMut(char) -> bool, +{ + let mut valid = true; + + for c in string.chars() { + if check(c) { + valid = true; + f.write_char(c)?; + } else if valid { + valid = false; + f.write_char('_')?; + } + } + + Ok(()) +} + +// Helper that serializes a string into a safe format for metric names or tag keys. struct SafeKey<'s>(&'s str); impl<'s> fmt::Display for SafeKey<'s> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for c in self.0.chars() { - if c.is_ascii_alphanumeric() || ['_', '-', '.', '/'].contains(&c) { - write!(f, "{}", c)?; - } - } - Ok(()) + safe_fmt(f, self.0, |c| { + c.is_ascii_alphanumeric() || matches!(c, '_' | '-' | '.' | '/') + }) } } -struct SaveVal<'s>(&'s str); +// Helper that serializes a string into a safe format for tag values. +struct SafeVal<'s>(&'s str); -impl<'s> fmt::Display for SaveVal<'s> { +impl<'s> fmt::Display for SafeVal<'s> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for c in self.0.chars() { - if c.is_alphanumeric() - || ['_', ':', '/', '@', '.', '{', '}', '[', ']', '$', '-'].contains(&c) - { - write!(f, "{}", c)?; - } - } - Ok(()) + safe_fmt(f, self.0, |c| { + c.is_alphanumeric() + || matches!( + c, + '_' | ':' | '/' | '@' | '.' | '{' | '}' | '[' | ']' | '$' | '-' + ) + }) } } #[cfg(test)] mod tests { - use super::*; use crate::test::{with_captured_envelopes, with_captured_envelopes_options}; use crate::ClientOptions; + use super::*; + /// Returns the current system time and rounded bucket timestamp. fn current_time() -> (SystemTime, u64) { let now = SystemTime::now(); @@ -931,12 +949,13 @@ mod tests { let envelopes = with_captured_envelopes(|| { Metric::count("my.metric") .with_tag("foo", "bar") + .with_tag("and", "more") .with_time(time) .send(); }); let metrics = get_single_metrics(&envelopes); - assert_eq!(metrics, format!("my.metric:1|c|#foo:bar|T{ts}")); + assert_eq!(metrics, format!("my.metric:1|c|#and:more,foo:bar|T{ts}")); } #[test] @@ -954,6 +973,48 @@ mod tests { assert_eq!(metrics, format!("my.metric@custom:1|c|T{ts}")); } + #[test] + fn test_metric_sanitation() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("my$$$metric").with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("my_metric:1|c|T{ts}")); + } + + #[test] + fn test_tag_sanitation() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("my.metric") + .with_tag("foo-bar$$$blub", "%$föö{}") + .with_time(time) + .send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!( + metrics, + format!("my.metric:1|c|#foo-bar_blub:_$föö{{}}|T{ts}") + ); + } + + #[test] + fn test_own_namespace() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("ns/my.metric").with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + assert_eq!(metrics, format!("ns/my.metric:1|c|T{ts}")); + } + #[test] fn test_default_tags() { let (time, ts) = current_time(); @@ -1056,4 +1117,20 @@ mod tests { let metrics = get_single_metrics(&envelopes); assert_eq!(metrics, format!("my.metric:1.5:1:2:4.5:3|g|T{ts}")); } + + #[test] + fn test_multiple() { + let (time, ts) = current_time(); + + let envelopes = with_captured_envelopes(|| { + Metric::count("my.metric").with_time(time).send(); + Metric::distribution("my.dist", 2.0).with_time(time).send(); + }); + + let metrics = get_single_metrics(&envelopes); + println!("{metrics}"); + + assert!(metrics.contains(&format!("my.metric:1|c|T{ts}"))); + assert!(metrics.contains(&format!("my.dist:2|d|T{ts}"))); + } } From c5062511aab2a919cb5846051019c5ca632bccb7 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 10:59:55 +0100 Subject: [PATCH 26/33] fix: Docs --- sentry-core/src/metrics.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 8274b68a..1e667544 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -590,6 +590,16 @@ impl Metric { pub fn gauge(name: impl Into, value: f64) -> MetricBuilder { Self::build(name, MetricValue::Gauge(value)) } + + /// Sends the metric to the current client. + /// + /// When building a metric, you can use [`MetricBuilder::send`] to send the metric directly. If + /// there is no client on the current [`Hub`], the metric is dropped. + pub fn send(self) { + if let Some(client) = Hub::current().client() { + client.add_metric(self); + } + } } /// A builder for metrics. @@ -644,11 +654,10 @@ impl MetricBuilder { /// Sends the metric to the current client. /// - /// If there is no client on the current [`Hub`], the metric is dropped. + /// This is a shorthand for `.finish().send()`. If there is no client on the current [`Hub`], + /// the metric is dropped. pub fn send(self) { - if let Some(client) = Hub::current().client() { - client.add_metric(self.finish()); - } + self.finish().send() } } From c3f72476f30e2c88a676387cfa06e9f37ec22770 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 11:28:10 +0100 Subject: [PATCH 27/33] ref(metrics): Refactor worker into separate struct --- sentry-core/src/metrics.rs | 172 +++++++++++++++++++++---------------- 1 file changed, 98 insertions(+), 74 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 1e667544..605a36c5 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -259,6 +259,7 @@ impl GaugeValue { } /// The aggregated value of a [`Metric`] bucket. +#[derive(Debug)] enum BucketValue { Counter(CounterType), Distribution(Vec), @@ -319,7 +320,7 @@ impl From for BucketValue { type Timestamp = u64; /// Composite bucket key for [`BucketMap`]. -#[derive(PartialEq, Eq, Hash)] +#[derive(Debug, PartialEq, Eq, Hash)] struct BucketKey { timestamp: Timestamp, ty: MetricType, @@ -339,14 +340,15 @@ struct BucketKey { /// dequeued first. type BucketMap = BTreeMap>; -struct AggregatorInner { +#[derive(Debug)] +struct SharedAggregatorState { buckets: BucketMap, weight: usize, running: bool, force_flush: bool, } -impl AggregatorInner { +impl SharedAggregatorState { pub fn new() -> Self { Self { buckets: BTreeMap::new(), @@ -721,71 +723,15 @@ fn get_default_tags(options: &ClientOptions) -> TagMap { tags } -pub(crate) struct MetricAggregator { - inner: Arc>, - handle: Option>, +#[derive(Clone)] +struct Worker { + shared: Arc>, + default_tags: TagMap, + transport: TransportArc, } -impl MetricAggregator { - pub fn new(transport: TransportArc, options: &ClientOptions) -> Self { - let default_tags = get_default_tags(options); - - let inner = Arc::new(Mutex::new(AggregatorInner::new())); - let inner_clone = Arc::clone(&inner); - - let handle = thread::Builder::new() - .name("sentry-metrics".into()) - .spawn(move || Self::worker_thread(inner_clone, transport, default_tags)) - .expect("failed to spawn thread"); - - Self { - inner, - handle: Some(handle), - } - } - - pub fn add(&self, metric: Metric) { - let Metric { - name, - unit, - value, - tags, - time, - } = metric; - - let timestamp = time - .unwrap_or_else(SystemTime::now) - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_secs(); - - let key = BucketKey { - timestamp, - ty: value.ty(), - name, - unit, - tags, - }; - - let mut guard = self.inner.lock().unwrap(); - guard.add(key, value); - - if guard.weight() > MAX_WEIGHT { - if let Some(ref handle) = self.handle { - guard.force_flush = true; - handle.thread().unpark(); - } - } - } - - pub fn flush(&self) { - self.inner.lock().unwrap().force_flush = true; - if let Some(ref handle) = self.handle { - handle.thread().unpark(); - } - } - - fn worker_thread(inner: Arc>, transport: TransportArc, tags: TagMap) { +impl Worker { + pub fn run(self) { let mut running = true; while running { @@ -794,31 +740,31 @@ impl MetricAggregator { thread::park_timeout(FLUSH_INTERVAL); let buckets = { - let mut guard = inner.lock().unwrap(); + let mut guard = self.shared.lock().unwrap(); running = guard.running; guard.take_buckets() }; if !buckets.is_empty() { - Self::flush_buckets(buckets, &transport, &tags); + self.flush_buckets(buckets); } } } - fn flush_buckets(buckets: BucketMap, transport: &TransportArc, tags: &TagMap) { + pub fn flush_buckets(&self, buckets: BucketMap) { // The transport is usually available when flush is called. Prefer a short lock and worst // case throw away the result rather than blocking the transport for too long. - if let Ok(output) = Self::format_payload(buckets, tags) { + if let Ok(output) = self.format_payload(buckets) { let mut envelope = Envelope::new(); envelope.add_item(EnvelopeItem::Metrics(output)); - if let Some(ref transport) = *transport.read().unwrap() { + if let Some(ref transport) = *self.transport.read().unwrap() { transport.send_envelope(envelope); } } } - fn format_payload(buckets: BucketMap, tags: &TagMap) -> std::io::Result> { + fn format_payload(&self, buckets: BucketMap) -> std::io::Result> { use std::io::Write; let mut out = vec![]; @@ -853,7 +799,7 @@ impl MetricAggregator { write!(&mut out, "|{}", key.ty.as_str())?; - for (i, (k, v)) in key.tags.iter().chain(tags).enumerate() { + for (i, (k, v)) in key.tags.iter().chain(&self.default_tags).enumerate() { match i { 0 => write!(&mut out, "|#")?, _ => write!(&mut out, ",")?, @@ -869,9 +815,87 @@ impl MetricAggregator { } } +impl fmt::Debug for Worker { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Worker") + .field("transport", &format_args!("ArcTransport")) + .field("default_tags", &self.default_tags) + .finish() + } +} + +#[derive(Debug)] +pub(crate) struct MetricAggregator { + shared: Arc>, + handle: Option>, +} + +impl MetricAggregator { + pub fn new(transport: TransportArc, options: &ClientOptions) -> Self { + let shared = Arc::new(Mutex::new(SharedAggregatorState::new())); + + let worker = Worker { + shared: Arc::clone(&shared), + default_tags: get_default_tags(options), + transport, + }; + + let handle = thread::Builder::new() + .name("sentry-metrics".into()) + .spawn(move || worker.run()) + .expect("failed to spawn thread"); + + Self { + shared, + handle: Some(handle), + } + } + + pub fn add(&self, metric: Metric) { + let Metric { + name, + unit, + value, + tags, + time, + } = metric; + + let timestamp = time + .unwrap_or_else(SystemTime::now) + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let key = BucketKey { + timestamp, + ty: value.ty(), + name, + unit, + tags, + }; + + let mut guard = self.shared.lock().unwrap(); + guard.add(key, value); + + if guard.weight() > MAX_WEIGHT { + if let Some(ref handle) = self.handle { + guard.force_flush = true; + handle.thread().unpark(); + } + } + } + + pub fn flush(&self) { + self.shared.lock().unwrap().force_flush = true; + if let Some(ref handle) = self.handle { + handle.thread().unpark(); + } + } +} + impl Drop for MetricAggregator { fn drop(&mut self) { - self.inner.lock().unwrap().running = false; + self.shared.lock().unwrap().running = false; if let Some(handle) = self.handle.take() { handle.thread().unpark(); handle.join().unwrap(); From edbbb9571b1399f1a7e209e244c60a921b4030df Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 11:43:23 +0100 Subject: [PATCH 28/33] ref(metrics): Add more Debug impls --- sentry-core/src/metrics.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 605a36c5..b9395242 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -458,6 +458,7 @@ impl SharedAggregatorState { /// client.add_metric(metric); /// } /// ``` +#[derive(Debug)] pub struct Metric { /// The name of the metric, identifying it in Sentry. /// @@ -609,6 +610,7 @@ impl Metric { /// Use one of the [`Metric`] constructors to create a new builder. See the struct-level docs for /// examples of how to build metrics. #[must_use] +#[derive(Debug)] pub struct MetricBuilder { metric: Metric, } From 55b7f28b7c6ce4aa49d481fee67ae2ddca7d5810 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 11:55:55 +0100 Subject: [PATCH 29/33] ref: Rename metric item to "statsd" --- sentry-core/src/cadence.rs | 2 +- sentry-core/src/metrics.rs | 4 ++-- sentry-types/src/protocol/envelope.rs | 8 ++++---- sentry/src/transports/ratelimit.rs | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sentry-core/src/cadence.rs b/sentry-core/src/cadence.rs index 46928d7a..8cd7c3d0 100644 --- a/sentry-core/src/cadence.rs +++ b/sentry-core/src/cadence.rs @@ -147,7 +147,7 @@ mod tests { assert_eq!(envelopes.len(), 1); let mut items = envelopes[0].items(); - let Some(EnvelopeItem::Metrics(metrics)) = items.next() else { + let Some(EnvelopeItem::Statsd(metrics)) = items.next() else { panic!("expected metrics"); }; let metrics = std::str::from_utf8(metrics).unwrap(); diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index b9395242..893c3d8f 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -758,7 +758,7 @@ impl Worker { // case throw away the result rather than blocking the transport for too long. if let Ok(output) = self.format_payload(buckets) { let mut envelope = Envelope::new(); - envelope.add_item(EnvelopeItem::Metrics(output)); + envelope.add_item(EnvelopeItem::Statsd(output)); if let Some(ref transport) = *self.transport.read().unwrap() { transport.send_envelope(envelope); @@ -970,7 +970,7 @@ mod tests { assert_eq!(envelopes.len(), 1, "expected exactly one envelope"); let mut items = envelopes[0].items(); - let Some(EnvelopeItem::Metrics(payload)) = items.next() else { + let Some(EnvelopeItem::Statsd(payload)) = items.next() else { panic!("expected metrics item"); }; diff --git a/sentry-types/src/protocol/envelope.rs b/sentry-types/src/protocol/envelope.rs index cb90e14c..02f77803 100644 --- a/sentry-types/src/protocol/envelope.rs +++ b/sentry-types/src/protocol/envelope.rs @@ -118,7 +118,7 @@ pub enum EnvelopeItem { MonitorCheckIn(MonitorCheckIn), /// A Metrics Item. #[cfg(feature = "UNSTABLE_metrics")] - Metrics(Vec), + Statsd(Vec), /// This is a sentinel item used to `filter` raw envelopes. Raw, // TODO: @@ -360,7 +360,7 @@ impl Envelope { serde_json::to_writer(&mut item_buf, check_in)? } #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItem::Metrics(metrics) => item_buf.extend_from_slice(metrics), + EnvelopeItem::Statsd(statsd) => item_buf.extend_from_slice(statsd), EnvelopeItem::Raw => { continue; } @@ -372,7 +372,7 @@ impl Envelope { EnvelopeItem::Transaction(_) => "transaction", EnvelopeItem::MonitorCheckIn(_) => "check_in", #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItem::Metrics(_) => "statsd", + EnvelopeItem::Statsd(_) => "statsd", EnvelopeItem::Attachment(_) | EnvelopeItem::Raw => unreachable!(), }; writeln!( @@ -518,7 +518,7 @@ impl Envelope { serde_json::from_slice(payload).map(EnvelopeItem::MonitorCheckIn) } #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItemType::Metrics => Ok(EnvelopeItem::Metrics(payload.into())), + EnvelopeItemType::Metrics => Ok(EnvelopeItem::Statsd(payload.into())), } .map_err(EnvelopeError::InvalidItemPayload)?; diff --git a/sentry/src/transports/ratelimit.rs b/sentry/src/transports/ratelimit.rs index 74632d46..a64ce3a9 100644 --- a/sentry/src/transports/ratelimit.rs +++ b/sentry/src/transports/ratelimit.rs @@ -116,7 +116,7 @@ impl RateLimiter { EnvelopeItem::Transaction(_) => RateLimitingCategory::Transaction, EnvelopeItem::Attachment(_) => RateLimitingCategory::Attachment, #[cfg(feature = "UNSTABLE_metrics")] - EnvelopeItem::Metrics(_) => RateLimitingCategory::Statsd, + EnvelopeItem::Statsd(_) => RateLimitingCategory::Statsd, _ => RateLimitingCategory::Any, }) }) @@ -157,8 +157,8 @@ mod tests { rl.update_from_sentry_header( r#" - 30::bar, - 120:invalid:invalid, + 30::bar, + 120:invalid:invalid, 4711:foo;bar;baz;security:project "#, ); From ab14afa17fa3afc601cdb3430df142bce4fcf0b0 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 12:42:58 +0100 Subject: [PATCH 30/33] ref(metrics): Flush synchronously --- sentry-core/src/client.rs | 4 ++-- sentry-core/src/metrics.rs | 48 ++++++++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index b659d0c3..4d0f09e5 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -340,8 +340,8 @@ impl Client { flusher.flush(); } #[cfg(feature = "UNSTABLE_metrics")] - if let Some(ref flusher) = *self.metric_aggregator.read().unwrap() { - flusher.flush(); + if let Some(ref aggregator) = *self.metric_aggregator.read().unwrap() { + aggregator.flush(); } if let Some(ref transport) = *self.transport.read().unwrap() { transport.flush(timeout.unwrap_or(self.options.shutdown_timeout)) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 893c3d8f..62c94aa4 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -734,26 +734,28 @@ struct Worker { impl Worker { pub fn run(self) { - let mut running = true; - - while running { + loop { // Park instead of sleep so we can wake the thread up. Do not account for delays during // flushing, since we benefit from some drift to spread out metric submissions. thread::park_timeout(FLUSH_INTERVAL); let buckets = { let mut guard = self.shared.lock().unwrap(); - running = guard.running; + if !guard.running { + break; + } guard.take_buckets() }; - if !buckets.is_empty() { - self.flush_buckets(buckets); - } + self.flush_buckets(buckets); } } pub fn flush_buckets(&self, buckets: BucketMap) { + if buckets.is_empty() { + return; + } + // The transport is usually available when flush is called. Prefer a short lock and worst // case throw away the result rather than blocking the transport for too long. if let Ok(output) = self.format_payload(buckets) { @@ -828,27 +830,27 @@ impl fmt::Debug for Worker { #[derive(Debug)] pub(crate) struct MetricAggregator { - shared: Arc>, + local_worker: Worker, handle: Option>, } impl MetricAggregator { pub fn new(transport: TransportArc, options: &ClientOptions) -> Self { - let shared = Arc::new(Mutex::new(SharedAggregatorState::new())); - let worker = Worker { - shared: Arc::clone(&shared), + shared: Arc::new(Mutex::new(SharedAggregatorState::new())), default_tags: get_default_tags(options), transport, }; + let local_worker = worker.clone(); + let handle = thread::Builder::new() .name("sentry-metrics".into()) .spawn(move || worker.run()) .expect("failed to spawn thread"); Self { - shared, + local_worker, handle: Some(handle), } } @@ -876,7 +878,7 @@ impl MetricAggregator { tags, }; - let mut guard = self.shared.lock().unwrap(); + let mut guard = self.local_worker.shared.lock().unwrap(); guard.add(key, value); if guard.weight() > MAX_WEIGHT { @@ -888,16 +890,26 @@ impl MetricAggregator { } pub fn flush(&self) { - self.shared.lock().unwrap().force_flush = true; - if let Some(ref handle) = self.handle { - handle.thread().unpark(); - } + let buckets = { + let mut guard = self.local_worker.shared.lock().unwrap(); + guard.force_flush = true; + guard.take_buckets() + }; + + self.local_worker.flush_buckets(buckets); } } impl Drop for MetricAggregator { fn drop(&mut self) { - self.shared.lock().unwrap().running = false; + let buckets = { + let mut guard = self.local_worker.shared.lock().unwrap(); + guard.running = false; + guard.take_buckets() + }; + + self.local_worker.flush_buckets(buckets); + if let Some(handle) = self.handle.take() { handle.thread().unpark(); handle.join().unwrap(); From 0c4064ca3c7de0b96bef364e0ef46c2e537b3c9b Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 14:45:18 +0100 Subject: [PATCH 31/33] ref: Address review feedback --- sentry-core/src/client.rs | 2 +- sentry-core/src/metrics.rs | 232 ++++++++++++++++++------------------- 2 files changed, 117 insertions(+), 117 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 4d0f09e5..a2681140 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -326,7 +326,7 @@ impl Client { } } - /// Captures a metric and sends it to Sentry after a short delay. + /// Captures a metric and sends it to Sentry on the next flush. #[cfg(feature = "UNSTABLE_metrics")] pub fn add_metric(&self, metric: metrics::Metric) { if let Some(ref aggregator) = *self.metric_aggregator.read().unwrap() { diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 62c94aa4..23ae3d25 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -67,16 +67,16 @@ const MAX_WEIGHT: usize = 100_000; pub type MetricStr = Cow<'static, str>; /// Type used for [`MetricValue::Counter`]. -pub type CounterType = f64; +pub type CounterValue = f64; /// Type used for [`MetricValue::Distribution`]. -pub type DistributionType = f64; +pub type DistributionValue = f64; /// Type used for [`MetricValue::Set`]. -pub type SetType = u32; +pub type SetValue = u32; /// Type used for [`MetricValue::Gauge`]. -pub type GaugeType = f64; +pub type GaugeValue = f64; /// The value of a [`Metric`], indicating its type. #[derive(Debug)] @@ -93,7 +93,7 @@ pub enum MetricValue { /// /// Metric::build("my.counter", MetricValue::Counter(1.0)).send(); /// ``` - Counter(CounterType), + Counter(CounterValue), /// Builds a statistical distribution over values reported. /// @@ -108,7 +108,7 @@ pub enum MetricValue { /// /// Metric::build("my.distribution", MetricValue::Distribution(42.0)).send(); /// ``` - Distribution(DistributionType), + Distribution(DistributionValue), /// Counts the number of unique reported values. /// @@ -127,7 +127,7 @@ pub enum MetricValue { /// /// Metric::build("my.set", MetricValue::set_from_str("foo")).send(); /// ``` - Set(SetType), + Set(SetValue), /// Stores absolute snapshots of values. /// @@ -143,7 +143,7 @@ pub enum MetricValue { /// /// Metric::build("my.gauge", MetricValue::Gauge(42.0)).send(); /// ``` - Gauge(GaugeType), + Gauge(GaugeValue), } impl MetricValue { @@ -221,24 +221,24 @@ impl std::str::FromStr for MetricType { /// A snapshot of values. #[derive(Clone, Copy, Debug, PartialEq)] -struct GaugeValue { +struct GaugeSummary { /// The last value reported in the bucket. /// /// This aggregation is not commutative. - pub last: GaugeType, + pub last: GaugeValue, /// The minimum value reported in the bucket. - pub min: GaugeType, + pub min: GaugeValue, /// The maximum value reported in the bucket. - pub max: GaugeType, + pub max: GaugeValue, /// The sum of all values reported in the bucket. - pub sum: GaugeType, + pub sum: GaugeValue, /// The number of times this bucket was updated with a new value. pub count: u64, } -impl GaugeValue { +impl GaugeSummary { /// Creates a gauge snapshot from a single value. - pub fn single(value: GaugeType) -> Self { + pub fn single(value: GaugeValue) -> Self { Self { last: value, min: value, @@ -249,7 +249,7 @@ impl GaugeValue { } /// Inserts a new value into the gauge. - pub fn insert(&mut self, value: GaugeType) { + pub fn insert(&mut self, value: GaugeValue) { self.last = value; self.min = self.min.min(value); self.max = self.max.max(value); @@ -261,10 +261,10 @@ impl GaugeValue { /// The aggregated value of a [`Metric`] bucket. #[derive(Debug)] enum BucketValue { - Counter(CounterType), - Distribution(Vec), - Set(BTreeSet), - Gauge(GaugeValue), + Counter(CounterValue), + Distribution(Vec), + Set(BTreeSet), + Gauge(GaugeSummary), } impl BucketValue { @@ -310,103 +310,10 @@ impl From for BucketValue { match value { MetricValue::Counter(v) => Self::Counter(v), MetricValue::Distribution(v) => Self::Distribution(vec![v]), - MetricValue::Gauge(v) => Self::Gauge(GaugeValue::single(v)), - MetricValue::Set(v) => Self::Set(std::iter::once(v).collect()), - } - } -} - -/// UNIX timestamp used for buckets. -type Timestamp = u64; - -/// Composite bucket key for [`BucketMap`]. -#[derive(Debug, PartialEq, Eq, Hash)] -struct BucketKey { - timestamp: Timestamp, - ty: MetricType, - name: MetricStr, - unit: MetricUnit, - tags: BTreeMap, -} - -/// A nested map storing metric buckets. -/// -/// This map consists of two levels: -/// 1. The rounded UNIX timestamp of buckets. -/// 2. The metric buckets themselves with a corresponding timestamp. -/// -/// This structure allows for efficient dequeueing of buckets that are older than a certain -/// threshold. The buckets are dequeued in order of their timestamp, so the oldest buckets are -/// dequeued first. -type BucketMap = BTreeMap>; - -#[derive(Debug)] -struct SharedAggregatorState { - buckets: BucketMap, - weight: usize, - running: bool, - force_flush: bool, -} - -impl SharedAggregatorState { - pub fn new() -> Self { - Self { - buckets: BTreeMap::new(), - weight: 0, - running: true, - force_flush: false, - } - } - - /// Adds a new bucket to the aggregator. - /// - /// The bucket timestamp is rounded to the nearest bucket interval. Note that this does NOT - /// automatically flush the aggregator if the weight exceeds the weight threshold. - pub fn add(&mut self, mut key: BucketKey, value: MetricValue) { - // Floor timestamp to bucket interval - key.timestamp /= BUCKET_INTERVAL.as_secs(); - key.timestamp *= BUCKET_INTERVAL.as_secs(); - - match self.buckets.entry(key.timestamp).or_default().entry(key) { - Entry::Occupied(mut e) => self.weight += e.get_mut().insert(value), - Entry::Vacant(e) => self.weight += e.insert(value.into()).weight(), + MetricValue::Gauge(v) => Self::Gauge(GaugeSummary::single(v)), + MetricValue::Set(v) => Self::Set(BTreeSet::from([v])), } } - - /// Removes and returns all buckets that are ready to flush. - /// - /// Buckets are ready to flush as soon as their time window has closed. For example, a bucket - /// from timestamps `[4600, 4610)` is ready to flush immediately at `4610`. - pub fn take_buckets(&mut self) -> BucketMap { - if self.force_flush || !self.running { - self.weight = 0; - self.force_flush = false; - std::mem::take(&mut self.buckets) - } else { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .saturating_sub(BUCKET_INTERVAL) - .as_secs(); - - // Split all buckets after the cutoff time. `split` contains newer buckets, which should - // remain, so swap them. After the swap, `split` contains all older buckets. - let mut split = self.buckets.split_off(×tamp); - std::mem::swap(&mut split, &mut self.buckets); - - self.weight -= split - .values() - .flat_map(|map| map.values()) - .map(|bucket| bucket.weight()) - .sum::(); - - split - } - } - - pub fn weight(&self) -> usize { - self.weight - } } /// A metric value that contains a numeric value and metadata to be sent to Sentry. @@ -712,7 +619,100 @@ fn parse_metric_opt(string: &str) -> Option { Some(builder.finish()) } -pub(crate) type TagMap = BTreeMap; +/// UNIX timestamp used for buckets. +type Timestamp = u64; + +/// Composite bucket key for [`BucketMap`]. +#[derive(Debug, PartialEq, Eq, Hash)] +struct BucketKey { + timestamp: Timestamp, + ty: MetricType, + name: MetricStr, + unit: MetricUnit, + tags: BTreeMap, +} + +/// A nested map storing metric buckets. +/// +/// This map consists of two levels: +/// 1. The rounded UNIX timestamp of buckets. +/// 2. The metric buckets themselves with a corresponding timestamp. +/// +/// This structure allows for efficient dequeueing of buckets that are older than a certain +/// threshold. The buckets are dequeued in order of their timestamp, so the oldest buckets are +/// dequeued first. +type BucketMap = BTreeMap>; + +#[derive(Debug)] +struct SharedAggregatorState { + buckets: BucketMap, + weight: usize, + running: bool, + force_flush: bool, +} + +impl SharedAggregatorState { + pub fn new() -> Self { + Self { + buckets: BTreeMap::new(), + weight: 0, + running: true, + force_flush: false, + } + } + + /// Adds a new bucket to the aggregator. + /// + /// The bucket timestamp is rounded to the nearest bucket interval. Note that this does NOT + /// automatically flush the aggregator if the weight exceeds the weight threshold. + pub fn add(&mut self, mut key: BucketKey, value: MetricValue) { + // Floor timestamp to bucket interval + key.timestamp /= BUCKET_INTERVAL.as_secs(); + key.timestamp *= BUCKET_INTERVAL.as_secs(); + + match self.buckets.entry(key.timestamp).or_default().entry(key) { + Entry::Occupied(mut e) => self.weight += e.get_mut().insert(value), + Entry::Vacant(e) => self.weight += e.insert(value.into()).weight(), + } + } + + /// Removes and returns all buckets that are ready to flush. + /// + /// Buckets are ready to flush as soon as their time window has closed. For example, a bucket + /// from timestamps `[4600, 4610)` is ready to flush immediately at `4610`. + pub fn take_buckets(&mut self) -> BucketMap { + if self.force_flush || !self.running { + self.weight = 0; + self.force_flush = false; + std::mem::take(&mut self.buckets) + } else { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .saturating_sub(BUCKET_INTERVAL) + .as_secs(); + + // Split all buckets after the cutoff time. `split` contains newer buckets, which should + // remain, so swap them. After the swap, `split` contains all older buckets. + let mut split = self.buckets.split_off(×tamp); + std::mem::swap(&mut split, &mut self.buckets); + + self.weight -= split + .values() + .flat_map(|map| map.values()) + .map(|bucket| bucket.weight()) + .sum::(); + + split + } + } + + pub fn weight(&self) -> usize { + self.weight + } +} + +type TagMap = BTreeMap; fn get_default_tags(options: &ClientOptions) -> TagMap { let mut tags = TagMap::new(); From 139cdf3922e31b53f5256fd14a590e61ee6dc240 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 14:48:46 +0100 Subject: [PATCH 32/33] ref(metrics): Reduce duplication of bucket key --- sentry-core/src/metrics.rs | 84 +++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/sentry-core/src/metrics.rs b/sentry-core/src/metrics.rs index 23ae3d25..ffb94a07 100644 --- a/sentry-core/src/metrics.rs +++ b/sentry-core/src/metrics.rs @@ -619,19 +619,18 @@ fn parse_metric_opt(string: &str) -> Option { Some(builder.finish()) } -/// UNIX timestamp used for buckets. -type Timestamp = u64; - /// Composite bucket key for [`BucketMap`]. #[derive(Debug, PartialEq, Eq, Hash)] struct BucketKey { - timestamp: Timestamp, ty: MetricType, name: MetricStr, unit: MetricUnit, tags: BTreeMap, } +/// UNIX timestamp used for buckets. +type Timestamp = u64; + /// A nested map storing metric buckets. /// /// This map consists of two levels: @@ -665,12 +664,12 @@ impl SharedAggregatorState { /// /// The bucket timestamp is rounded to the nearest bucket interval. Note that this does NOT /// automatically flush the aggregator if the weight exceeds the weight threshold. - pub fn add(&mut self, mut key: BucketKey, value: MetricValue) { + pub fn add(&mut self, mut timestamp: Timestamp, key: BucketKey, value: MetricValue) { // Floor timestamp to bucket interval - key.timestamp /= BUCKET_INTERVAL.as_secs(); - key.timestamp *= BUCKET_INTERVAL.as_secs(); + timestamp /= BUCKET_INTERVAL.as_secs(); + timestamp *= BUCKET_INTERVAL.as_secs(); - match self.buckets.entry(key.timestamp).or_default().entry(key) { + match self.buckets.entry(timestamp).or_default().entry(key) { Entry::Occupied(mut e) => self.weight += e.get_mut().insert(value), Entry::Vacant(e) => self.weight += e.insert(value.into()).weight(), } @@ -772,47 +771,49 @@ impl Worker { use std::io::Write; let mut out = vec![]; - for (key, value) in buckets.into_iter().flat_map(|(_, v)| v) { - write!(&mut out, "{}", SafeKey(key.name.as_ref()))?; - if key.unit != MetricUnit::None { - write!(&mut out, "@{}", key.unit)?; - } - - match value { - BucketValue::Counter(c) => { - write!(&mut out, ":{}", c)?; + for (timestamp, buckets) in buckets { + for (key, value) in buckets { + write!(&mut out, "{}", SafeKey(key.name.as_ref()))?; + if key.unit != MetricUnit::None { + write!(&mut out, "@{}", key.unit)?; } - BucketValue::Distribution(d) => { - for v in d { - write!(&mut out, ":{}", v)?; + + match value { + BucketValue::Counter(c) => { + write!(&mut out, ":{}", c)?; } - } - BucketValue::Set(s) => { - for v in s { - write!(&mut out, ":{}", v)?; + BucketValue::Distribution(d) => { + for v in d { + write!(&mut out, ":{}", v)?; + } + } + BucketValue::Set(s) => { + for v in s { + write!(&mut out, ":{}", v)?; + } + } + BucketValue::Gauge(g) => { + write!( + &mut out, + ":{}:{}:{}:{}:{}", + g.last, g.min, g.max, g.sum, g.count + )?; } } - BucketValue::Gauge(g) => { - write!( - &mut out, - ":{}:{}:{}:{}:{}", - g.last, g.min, g.max, g.sum, g.count - )?; - } - } - write!(&mut out, "|{}", key.ty.as_str())?; + write!(&mut out, "|{}", key.ty.as_str())?; + + for (i, (k, v)) in key.tags.iter().chain(&self.default_tags).enumerate() { + match i { + 0 => write!(&mut out, "|#")?, + _ => write!(&mut out, ",")?, + } - for (i, (k, v)) in key.tags.iter().chain(&self.default_tags).enumerate() { - match i { - 0 => write!(&mut out, "|#")?, - _ => write!(&mut out, ",")?, + write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SafeVal(v.as_ref()))?; } - write!(&mut out, "{}:{}", SafeKey(k.as_ref()), SafeVal(v.as_ref()))?; + writeln!(&mut out, "|T{}", timestamp)?; } - - writeln!(&mut out, "|T{}", key.timestamp)?; } Ok(out) @@ -871,7 +872,6 @@ impl MetricAggregator { .as_secs(); let key = BucketKey { - timestamp, ty: value.ty(), name, unit, @@ -879,7 +879,7 @@ impl MetricAggregator { }; let mut guard = self.local_worker.shared.lock().unwrap(); - guard.add(key, value); + guard.add(timestamp, key, value); if guard.weight() > MAX_WEIGHT { if let Some(ref handle) = self.handle { From 69080abe666a7bb6495051d7088fede42e96da63 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 12 Dec 2023 15:28:11 +0100 Subject: [PATCH 33/33] meta: Changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ede7372f..919668d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Unreleased + +**Features**: + +- Add experimental implementations for Sentry metrics and a cadence sink. These + require to use the `UNSTABLE_metrics` and `UNSTABLE_cadence` feature flags. + Note that these APIs are still under development and subject to change. + ## 0.32.0 **Features**: