From cbba70923b78de742e9c1b435f5ad80c28c03859 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 15:56:48 +0200 Subject: [PATCH 01/12] Add support for Profiling --- sentry-core/Cargo.toml | 12 +- sentry-core/src/client.rs | 18 +- sentry-core/src/clientoptions.rs | 12 ++ sentry-core/src/performance.rs | 276 +++++++++++++++++++++++++- sentry-types/src/protocol/envelope.rs | 21 +- sentry-types/src/protocol/mod.rs | 1 + sentry-types/src/protocol/profile.rs | 110 ++++++++++ sentry-types/src/protocol/v7.rs | 1 + sentry/Cargo.toml | 1 + sentry/src/transports/ratelimit.rs | 6 + 10 files changed, 439 insertions(+), 19 deletions(-) create mode 100644 sentry-types/src/protocol/profile.rs diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index ca2fb22d..a37c9236 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -25,7 +25,8 @@ client = ["rand"] # I would love to just have a `log` feature, but this is used inside a macro, # and macros actually expand features (and extern crate) where they are used! debug-logs = ["log_"] -test = ["client"] +test = ["client", "profiling"] +profiling = ["pprof", "lazy_static", "mut_static", "build_id", "uuid", "sys-info", "findshlibs"] [dependencies] log_ = { package = "log", version = "0.4.8", optional = true, features = ["std"] } @@ -33,7 +34,14 @@ once_cell = "1" rand = { version = "0.8.1", optional = true } sentry-types = { version = "0.27.0", path = "../sentry-types" } serde = { version = "1.0.104", features = ["derive"] } -serde_json = "1.0.46" +serde_json = { version = "1.0.46" } +lazy_static = { version = "1.4.0", optional = true } +mut_static = { version = "5.0.0", optional = true } +pprof = { version = "0.10.0", optional = true } +uuid = { version = "1.0.0", features = ["v4", "serde"], optional = true } +sys-info = { version = "0.9.1", optional = true } +build_id = { version = "0.2.1", optional = true } +findshlibs = { version = "=0.10.2", optional = true } [dev-dependencies] # Because we re-export all the public API in `sentry`, we actually run all the diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 50bbea8b..53b63c8c 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -215,8 +215,8 @@ impl Client { scope.update_session_from_event(&event); } - if !self.sample_should_send() { - None + if !self.sample_should_send(self.options.sample_rate) { + return None; } else { Some(event) } @@ -338,19 +338,9 @@ impl Client { } } - fn sample_should_send(&self) -> bool { - let rate = self.options.sample_rate; - if rate >= 1.0 { - true - } else { - random::() <= rate - } - } - /// Returns a random boolean with a probability defined - /// by the [`ClientOptions`]'s `traces_sample_rate` - pub fn sample_traces_should_send(&self) -> bool { - let rate = self.options.traces_sample_rate; + /// by rate + pub fn sample_should_send(&self, rate: f32) -> bool { if rate >= 1.0 { true } else { diff --git a/sentry-core/src/clientoptions.rs b/sentry-core/src/clientoptions.rs index 72846ece..c2559e0f 100644 --- a/sentry-core/src/clientoptions.rs +++ b/sentry-core/src/clientoptions.rs @@ -74,6 +74,14 @@ pub struct ClientOptions { pub sample_rate: f32, /// The sample rate for tracing transactions. (0.0 - 1.0, defaults to 0.0) pub traces_sample_rate: f32, + /// Enables profiling + pub enable_profiling: bool, + /// The sample rate for profiling a transactions. (0.0 - 1.0, defaults to 0.0) + /// + /// This is dependent on `traces_sample_rate`. The probability of sending a profile + /// is given by `traces_sample_rate * profiles_sample_rate`. + /// If a given transaction is not sent, then the profile won't be sent neither. + pub profiles_sample_rate: f32, /// Maximum number of breadcrumbs. (defaults to 100) pub max_breadcrumbs: usize, /// Attaches stacktraces to messages. @@ -183,6 +191,8 @@ impl fmt::Debug for ClientOptions { .field("environment", &self.environment) .field("sample_rate", &self.sample_rate) .field("traces_sample_rate", &self.traces_sample_rate) + .field("enable_profiling", &self.enable_profiling) + .field("profiles_sample_rate", &self.profiles_sample_rate) .field("max_breadcrumbs", &self.max_breadcrumbs) .field("attach_stacktrace", &self.attach_stacktrace) .field("send_default_pii", &self.send_default_pii) @@ -215,6 +225,8 @@ impl Default for ClientOptions { environment: None, sample_rate: 1.0, traces_sample_rate: 0.0, + enable_profiling: false, + profiles_sample_rate: 0.0, max_breadcrumbs: 100, attach_stacktrace: false, send_default_pii: false, diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 02deb332..7e4e4569 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -1,6 +1,22 @@ use std::sync::Arc; use std::sync::Mutex; +#[cfg(feature = "profiling")] +use findshlibs::{SharedLibrary, SharedLibraryId, TargetSharedLibrary, TARGET_SUPPORTED}; +#[cfg(feature = "profiling")] +use lazy_static::lazy_static; +#[cfg(feature = "profiling")] +use mut_static::MutStatic; +#[cfg(feature = "profiling")] +use sentry_types::{CodeId, DebugId, Uuid}; + +#[cfg(feature = "profiling")] +use sentry_types::protocol::v7::Profile; +#[cfg(feature = "profiling")] +use sentry_types::protocol::v7::{ + DebugImage, DebugMeta, RustFrame, Sample, SampledProfile, SymbolicDebugImage, TraceId, +}; + use crate::{protocol, Hub}; #[cfg(feature = "client")] @@ -9,6 +25,24 @@ use crate::Client; #[cfg(feature = "client")] const MAX_SPANS: usize = 1_000; +#[cfg(feature = "profiling")] +use build_id; +#[cfg(feature = "profiling")] +use sys_info; +#[cfg(feature = "profiling")] +use uuid; + +#[cfg(feature = "profiling")] +lazy_static! { + static ref PROFILER_RUNNING: Mutex = Mutex::new(false); + static ref PROFILE_INNER: MutStatic = { + MutStatic::from(ProfileInner { + transaction_id: "".to_string(), + profiler_guard: None, + }) + }; +} + // global API: /// Start a new Performance Monitoring Transaction. @@ -273,6 +307,14 @@ pub(crate) struct TransactionInner { pub(crate) transaction: Option>, } +#[derive(Default)] +#[cfg(feature = "profiling")] +struct ProfileInner { + transaction_id: String, + #[cfg(feature = "profiling")] + profiler_guard: Option>, +} + type TransactionArc = Arc>; /// A running Performance Monitoring Transaction. @@ -297,8 +339,9 @@ impl Transaction { let (sampled, mut transaction) = match client.as_ref() { Some(client) => ( - ctx.sampled - .unwrap_or_else(|| client.sample_traces_should_send()), + ctx.sampled.unwrap_or_else(|| { + client.sample_should_send(client.options().traces_sample_rate) + }), Some(protocol::Transaction { name: Some(ctx.name), ..Default::default() @@ -313,6 +356,42 @@ impl Transaction { transaction = None; client = None; } + // if the transaction was sampled then a profile, linked to the transaction, + // might as well be sampled + #[cfg(feature = "profiling")] + if sampled { + if let Some(client) = client.as_ref() { + let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); + // if the profile is sampled and currently there is no other profile + // being collected + if transaction.is_some() + && client.options().enable_profiling + && !*profiler_running + && client.sample_should_send(client.options().profiles_sample_rate) + { + let profile_guard_builder = pprof::ProfilerGuardBuilder::default() + .frequency(100) + .blocklist(&["libc", "libgcc", "pthread", "vdso"]) + .build(); + + match profile_guard_builder { + Ok(guard_builder) => { + *profiler_running = true; + let mut profile_inner = PROFILE_INNER.write().unwrap(); + profile_inner.transaction_id = + transaction.as_ref().unwrap().event_id.clone().to_string(); + profile_inner.profiler_guard = Some(guard_builder); + } + Err(err) => { + sentry_debug!( + "could not start the profiler due to the following error: {:?}", + err + ); + } + } + } + } + } Self { inner: Arc::new(Mutex::new(TransactionInner { @@ -404,9 +483,45 @@ impl Transaction { transaction.environment = opts.environment.clone(); transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); + #[cfg(feature = "profiling")] + let mut profile: Option = None; + + #[cfg(feature = "profiling")] + if client.options().enable_profiling{ + let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); + if *profiler_running { + let mut profile_inner = PROFILE_INNER.write().unwrap(); + // if the transaction that is ending is the same that started the + // profiler, then the profile should be added to the envelope too + if profile_inner.transaction_id == transaction.event_id.to_string() { + match profile_inner.profiler_guard.as_ref().unwrap().report().build_unresolved() { + Ok(report) => { + profile = Some(get_profile_from_report( + &report, + inner.context.trace_id, + transaction.event_id.clone(), + transaction.name.as_ref().unwrap().clone(), + )); + } + Err(err) => { + sentry_debug!("could not build the profile result due to the error: {}", err); + } + } + // in both cases (Ok or Err), reset profile_inner + *profile_inner = ProfileInner::default(); + *profiler_running = false; + } + } + } + let mut envelope = protocol::Envelope::new(); envelope.add_item(transaction); + #[cfg(feature = "profiling")] + if let Some(profile) = profile { + envelope.add_item(profile); + } + client.send_envelope(envelope) } } @@ -594,6 +709,163 @@ fn parse_sentry_trace(header: &str) -> Option { Some(SentryTrace(trace_id, parent_span_id, parent_sampled)) } +/// Converts an ELF object identifier into a `DebugId`. +/// +/// The identifier data is first truncated or extended to match 16 byte size of +/// Uuids. If the data is declared in little endian, the first three Uuid fields +/// are flipped to match the big endian expected by the breakpad processor. +/// +/// The `DebugId::appendix` field is always `0` for ELF. +#[cfg(feature = "profiling")] +fn debug_id_from_build_id(build_id: &[u8]) -> Option { + const UUID_SIZE: usize = 16; + let mut data = [0u8; UUID_SIZE]; + let len = build_id.len().min(UUID_SIZE); + data[0..len].copy_from_slice(&build_id[0..len]); + + #[cfg(target_endian = "little")] + { + // The ELF file targets a little endian architecture. Convert to + // network byte order (big endian) to match the Breakpad processor's + // expectations. For big endian object files, this is not needed. + data[0..4].reverse(); // uuid field 1 + data[4..6].reverse(); // uuid field 2 + data[6..8].reverse(); // uuid field 3 + } + + Uuid::from_slice(&data).map(DebugId::from_uuid).ok() +} + +#[cfg(feature = "profiling")] +/// Returns the list of loaded libraries/images. +pub fn debug_images() -> Vec { + let mut images = vec![]; + if !TARGET_SUPPORTED { + return images; + } + + //crate:: ::{CodeId, DebugId, Uuid}; + TargetSharedLibrary::each(|shlib| { + let maybe_debug_id = shlib.debug_id().and_then(|id| match id { + SharedLibraryId::Uuid(bytes) => Some(DebugId::from_uuid(Uuid::from_bytes(bytes))), + SharedLibraryId::GnuBuildId(ref id) => debug_id_from_build_id(id), + SharedLibraryId::PdbSignature(guid, age) => DebugId::from_guid_age(&guid, age).ok(), + _ => None, + }); + + let debug_id = match maybe_debug_id { + Some(debug_id) => debug_id, + None => return, + }; + + let mut name = shlib.name().to_string_lossy().to_string(); + if name.is_empty() { + name = std::env::current_exe() + .map(|x| x.display().to_string()) + .unwrap_or_else(|_| "
".to_string()); + } + + let code_id = shlib.id().map(|id| CodeId::new(format!("{}", id))); + let debug_name = shlib.debug_name().map(|n| n.to_string_lossy().to_string()); + + // For windows, the `virtual_memory_bias` actually returns the real + // `module_base`, which is the address that sentry uses for symbolication. + // Going via the segments means that the `image_addr` would be offset in + // a way that symbolication yields wrong results. + let (image_addr, image_vmaddr) = if cfg!(windows) { + (shlib.virtual_memory_bias().0.into(), 0.into()) + } else { + ( + shlib.actual_load_addr().0.into(), + shlib.stated_load_addr().0.into(), + ) + }; + + images.push( + SymbolicDebugImage { + id: debug_id, + name, + arch: None, + image_addr, + image_size: shlib.len() as u64, + image_vmaddr, + code_id, + debug_file: debug_name, + } + .into(), + ); + }); + + images +} + +#[cfg(feature = "profiling")] +fn get_profile_from_report( + rep: &pprof::UnresolvedReport, + trace_id: TraceId, + transaction_id: sentry_types::Uuid, + transaction_name: String, +) -> Profile { + let mut samples: Vec = Vec::new(); + + for sample in rep.data.keys() { + let mut frames: Vec = Vec::new(); + for frame in &sample.frames { + frames.push(RustFrame { + instruction_addr: format!("{:p}", frame.ip()), + }) + } + samples.push(Sample { + frames: frames, + thread_name: String::from_utf8_lossy(&sample.thread_name[0..sample.thread_name_length]) + .into_owned(), + thread_id: sample.thread_id, + nanos_relative_to_start: sample + .sample_timestamp + .duration_since(rep.timing.start_time) + .unwrap() + .as_nanos() as u64, + }); + } + let sampled_profile = SampledProfile { + start_time_nanos: rep + .timing + .start_time + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() as u64, + start_time_secs: rep + .timing + .start_time + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(), + duration_nanos: rep.timing.duration.as_nanos() as u64, + samples, + }; + use std::time::SystemTime; + let profile: Profile = Profile { + duration_ns: sampled_profile.duration_nanos, + debug_meta: DebugMeta { + sdk_info: None, + images: debug_images(), + }, + platform: "rust".to_string(), + architecture: std::env::consts::ARCH.to_string(), + trace_id: trace_id, + transaction_name: transaction_name, + transaction_id: transaction_id, + profile_id: uuid::Uuid::new_v4(), + sampled_profile: sampled_profile, + os_name: sys_info::os_type().unwrap(), + os_version: sys_info::os_release().unwrap(), + version_name: env!("CARGO_PKG_VERSION").to_string(), + version_code: build_id::get().to_simple().to_string(), + }; + + return profile; +} + impl std::fmt::Display for SentryTrace { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}-{}", self.0, self.1)?; diff --git a/sentry-types/src/protocol/envelope.rs b/sentry-types/src/protocol/envelope.rs index a2586526..c8358c22 100644 --- a/sentry-types/src/protocol/envelope.rs +++ b/sentry-types/src/protocol/envelope.rs @@ -6,7 +6,7 @@ use uuid::Uuid; use super::{ attachment::AttachmentType, - v7::{Attachment, Event, SessionAggregates, SessionUpdate, Transaction}, + v7::{Attachment, Event, Profile, SessionAggregates, SessionUpdate, Transaction}, }; /// Raised if a envelope cannot be parsed from a given input. @@ -58,6 +58,9 @@ enum EnvelopeItemType { /// An Attachment Item type. #[serde(rename = "attachment")] Attachment, + /// A Profile Item Type + #[serde(rename = "profile")] + Profile, } /// An Envelope Item Header. @@ -104,6 +107,9 @@ pub enum EnvelopeItem { /// See the [Attachment Item documentation](https://develop.sentry.dev/sdk/envelopes/#attachment) /// for more details. Attachment(Attachment), + /// An Profile Item. + /// + Profile(Profile), // TODO: // etc… } @@ -138,6 +144,12 @@ impl From for EnvelopeItem { } } +impl From for EnvelopeItem { + fn from(profile: Profile) -> Self { + EnvelopeItem::Profile(profile) + } +} + /// An Iterator over the items of an Envelope. #[derive(Clone)] pub struct EnvelopeItemIter<'s> { @@ -282,6 +294,11 @@ impl Envelope { writeln!(writer)?; continue; } + EnvelopeItem::Profile(profile) => { + profile.to_writer(&mut writer)?; + writeln!(writer)?; + continue; + } } let item_type = match item { EnvelopeItem::Event(_) => "event", @@ -289,6 +306,7 @@ impl Envelope { EnvelopeItem::SessionAggregates(_) => "sessions", EnvelopeItem::Transaction(_) => "transaction", EnvelopeItem::Attachment(_) => unreachable!(), + EnvelopeItem::Profile(_) => unreachable!(), }; writeln!( writer, @@ -412,6 +430,7 @@ impl Envelope { content_type: header.content_type, ty: header.attachment_type, })), + EnvelopeItemType::Profile => serde_json::from_slice(payload).map(EnvelopeItem::Profile), } .map_err(EnvelopeError::InvalidItemPayload)?; diff --git a/sentry-types/src/protocol/mod.rs b/sentry-types/src/protocol/mod.rs index 72873bd3..3d6ed71e 100644 --- a/sentry-types/src/protocol/mod.rs +++ b/sentry-types/src/protocol/mod.rs @@ -11,4 +11,5 @@ pub use v7 as latest; mod attachment; mod envelope; +mod profile; mod session; diff --git a/sentry-types/src/protocol/profile.rs b/sentry-types/src/protocol/profile.rs new file mode 100644 index 00000000..375cb24a --- /dev/null +++ b/sentry-types/src/protocol/profile.rs @@ -0,0 +1,110 @@ +use super::v7::{DebugMeta, TraceId}; +use serde::{Deserialize, Serialize, Serializer}; +use uuid::Uuid; + +fn serialize_id(uuid: &Uuid, serializer: S) -> Result { + serializer.serialize_some(&uuid.as_simple().to_string()) +} + +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] +/// Represents a Symbol +pub struct RustFrame { + /// Raw instruction address + pub instruction_addr: String, +} + +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] +/// Represents a Sample +pub struct Sample { + /// List of symbols + pub frames: Vec, + /// The thread name + pub thread_name: String, + /// The thread id + pub thread_id: u64, + /// Nanoseconds elapsed between when the profiler started and when this sample was collected + pub nanos_relative_to_start: u64, +} + +#[derive(Debug, Default, Deserialize, Serialize, Clone, PartialEq)] +/// Represents a collected Profile +pub struct SampledProfile { + /// Collection start time in nanoseconds + pub start_time_nanos: u64, + /// Collection start time in seconds + pub start_time_secs: u64, + /// Collection duration in nanoseconds + pub duration_nanos: u64, + /// List of collected samples + pub samples: Vec, +} + +#[derive(Debug, Default, Deserialize, Serialize, Clone, PartialEq)] +/// Represents a Profile Envelope ItemType +pub struct Profile { + /// Duration in nanoseconds of the Profile + pub duration_ns: u64, + /// List of debug images + pub debug_meta: DebugMeta, + /// Platform is `rust` + pub platform: String, + /// A string describing the architecture of the CPU that is currently in use + /// https://doc.rust-lang.org/std/env/consts/constant.ARCH.html + pub architecture: String, + /// The trace ID + pub trace_id: TraceId, + /// The name of the transaction this profile belongs to + pub transaction_name: String, + #[serde(serialize_with = "serialize_id")] + /// The ID of the transaction this profile belongs to + pub transaction_id: Uuid, + /// The ID of the event + #[serde(serialize_with = "serialize_id")] + pub profile_id: Uuid, + /// Represents the profile collected + pub sampled_profile: SampledProfile, + #[serde(rename = "device_os_name")] + /// OS name + pub os_name: String, + #[serde(rename = "device_os_version")] + /// OS version + pub os_version: String, + /// Package version + pub version_name: String, + /// Current binary build ID. See https://docs.rs/build_id/latest/build_id/ + pub version_code: String, +} + +#[derive(Debug, Default, Deserialize, Serialize)] +struct ProfileItemHeader { + content_type: String, + #[serde(skip_serializing_if = "String::is_empty")] + file_name: String, + #[serde(rename = "type")] + typez: String, + length: usize, +} + +impl Profile { + /// Writes the attachment and its headers to the provided `Writer`. + pub fn to_writer(&self, writer: &mut W) -> std::io::Result<()> + where + W: std::io::Write, + { + let serialized_profile = serde_json::to_string(self).unwrap(); + + writeln!( + writer, + "{}", + serde_json::to_string(&ProfileItemHeader { + content_type: "application/json".to_string(), + file_name: format!("{}.trace", self.trace_id).to_string(), + typez: "profile".to_string(), + length: serialized_profile.len(), + })? + )?; + + writer.write_all(&serialized_profile.as_bytes())?; + Ok(()) + } +} diff --git a/sentry-types/src/protocol/v7.rs b/sentry-types/src/protocol/v7.rs index adf13646..7dc94e26 100644 --- a/sentry-types/src/protocol/v7.rs +++ b/sentry-types/src/protocol/v7.rs @@ -26,6 +26,7 @@ use crate::utils::{ts_rfc3339_opt, ts_seconds_float}; pub use super::attachment::*; pub use super::envelope::*; +pub use super::profile::*; pub use super::session::*; /// An arbitrary (JSON) value. diff --git a/sentry/Cargo.toml b/sentry/Cargo.toml index d96d2ae4..803be63a 100644 --- a/sentry/Cargo.toml +++ b/sentry/Cargo.toml @@ -47,6 +47,7 @@ native-tls = ["reqwest_/default-tls"] rustls = ["reqwest_/rustls-tls"] ureq = ["ureq_/tls", "httpdate"] ureq-native-tls = ["ureq_/native-tls", "httpdate"] +profiling = ["sentry-core/profiling"] [dependencies] sentry-core = { version = "0.27.0", path = "../sentry-core", features = ["client"] } diff --git a/sentry/src/transports/ratelimit.rs b/sentry/src/transports/ratelimit.rs index 0aa36bb7..e419f999 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, + profile: Option, } impl RateLimiter { @@ -56,6 +57,7 @@ impl RateLimiter { "session" => self.session = new_time, "transaction" => self.transaction = new_time, "attachment" => self.attachment = new_time, + "profile" => self.profile = new_time, _ => {} } } @@ -89,6 +91,7 @@ impl RateLimiter { RateLimitingCategory::Session => self.session, RateLimitingCategory::Transaction => self.transaction, RateLimitingCategory::Attachment => self.attachment, + RateLimitingCategory::Profile => self.profile, }?; time_left.duration_since(SystemTime::now()).ok() } @@ -112,6 +115,7 @@ impl RateLimiter { } EnvelopeItem::Transaction(_) => RateLimitingCategory::Transaction, EnvelopeItem::Attachment(_) => RateLimitingCategory::Attachment, + EnvelopeItem::Profile(_) => RateLimitingCategory::Profile, _ => RateLimitingCategory::Any, }) }) @@ -131,6 +135,8 @@ pub enum RateLimitingCategory { Transaction, /// Rate Limit pertaining to Attachments. Attachment, + /// Rate Limit pertaining to Profiles. + Profile, } #[cfg(test)] From 98d750ab096a831317720a79a99a717ed1fa3e80 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 16:37:52 +0200 Subject: [PATCH 02/12] Fix cargo doc warnings --- sentry-types/src/protocol/profile.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-types/src/protocol/profile.rs b/sentry-types/src/protocol/profile.rs index 375cb24a..c5b39ff1 100644 --- a/sentry-types/src/protocol/profile.rs +++ b/sentry-types/src/protocol/profile.rs @@ -49,7 +49,7 @@ pub struct Profile { /// Platform is `rust` pub platform: String, /// A string describing the architecture of the CPU that is currently in use - /// https://doc.rust-lang.org/std/env/consts/constant.ARCH.html + /// pub architecture: String, /// The trace ID pub trace_id: TraceId, @@ -71,7 +71,7 @@ pub struct Profile { pub os_version: String, /// Package version pub version_name: String, - /// Current binary build ID. See https://docs.rs/build_id/latest/build_id/ + /// Current binary build ID. See pub version_code: String, } From 552e9363ba58ca9f08c435f5a699fd76e45ab08b Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 16:44:25 +0200 Subject: [PATCH 03/12] fix linting --- sentry-core/src/client.rs | 2 +- sentry-types/src/protocol/profile.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 53b63c8c..ef44840a 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -216,7 +216,7 @@ impl Client { } if !self.sample_should_send(self.options.sample_rate) { - return None; + None } else { Some(event) } diff --git a/sentry-types/src/protocol/profile.rs b/sentry-types/src/protocol/profile.rs index c5b39ff1..f1da305f 100644 --- a/sentry-types/src/protocol/profile.rs +++ b/sentry-types/src/protocol/profile.rs @@ -98,13 +98,13 @@ impl Profile { "{}", serde_json::to_string(&ProfileItemHeader { content_type: "application/json".to_string(), - file_name: format!("{}.trace", self.trace_id).to_string(), + file_name: format!("{}.trace", self.trace_id), typez: "profile".to_string(), length: serialized_profile.len(), })? )?; - writer.write_all(&serialized_profile.as_bytes())?; + writer.write_all(serialized_profile.as_bytes())?; Ok(()) } } From c9e7f43ba1766aef0ba2cca8b3a3230abd2384e1 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 16:57:17 +0200 Subject: [PATCH 04/12] Add target_os conditional compilation This is meant to exclude compilation of nix dependent code on windows --- sentry-core/src/performance.rs | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 7e4e4569..87bda9f9 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -1,18 +1,18 @@ use std::sync::Arc; use std::sync::Mutex; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use findshlibs::{SharedLibrary, SharedLibraryId, TargetSharedLibrary, TARGET_SUPPORTED}; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use lazy_static::lazy_static; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use mut_static::MutStatic; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use sentry_types::{CodeId, DebugId, Uuid}; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use sentry_types::protocol::v7::Profile; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use sentry_types::protocol::v7::{ DebugImage, DebugMeta, RustFrame, Sample, SampledProfile, SymbolicDebugImage, TraceId, }; @@ -25,14 +25,14 @@ use crate::Client; #[cfg(feature = "client")] const MAX_SPANS: usize = 1_000; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use build_id; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use sys_info; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] use uuid; -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] lazy_static! { static ref PROFILER_RUNNING: Mutex = Mutex::new(false); static ref PROFILE_INNER: MutStatic = { @@ -308,10 +308,10 @@ pub(crate) struct TransactionInner { } #[derive(Default)] -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] struct ProfileInner { transaction_id: String, - #[cfg(feature = "profiling")] + #[cfg(all(feature = "profiling", not(target_os = "windows")))] profiler_guard: Option>, } @@ -358,7 +358,7 @@ impl Transaction { } // if the transaction was sampled then a profile, linked to the transaction, // might as well be sampled - #[cfg(feature = "profiling")] + #[cfg(all(feature = "profiling", not(target_os = "windows")))] if sampled { if let Some(client) = client.as_ref() { let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); @@ -483,10 +483,10 @@ impl Transaction { transaction.environment = opts.environment.clone(); transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); - #[cfg(feature = "profiling")] + #[cfg(all(feature = "profiling", not(target_os = "windows")))] let mut profile: Option = None; - #[cfg(feature = "profiling")] + #[cfg(all(feature = "profiling", not(target_os = "windows")))] if client.options().enable_profiling{ let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); if *profiler_running { @@ -517,7 +517,7 @@ impl Transaction { let mut envelope = protocol::Envelope::new(); envelope.add_item(transaction); - #[cfg(feature = "profiling")] + #[cfg(all(feature = "profiling", not(target_os = "windows")))] if let Some(profile) = profile { envelope.add_item(profile); } @@ -716,7 +716,7 @@ fn parse_sentry_trace(header: &str) -> Option { /// are flipped to match the big endian expected by the breakpad processor. /// /// The `DebugId::appendix` field is always `0` for ELF. -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] fn debug_id_from_build_id(build_id: &[u8]) -> Option { const UUID_SIZE: usize = 16; let mut data = [0u8; UUID_SIZE]; @@ -736,7 +736,7 @@ fn debug_id_from_build_id(build_id: &[u8]) -> Option { Uuid::from_slice(&data).map(DebugId::from_uuid).ok() } -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] /// Returns the list of loaded libraries/images. pub fn debug_images() -> Vec { let mut images = vec![]; @@ -799,7 +799,7 @@ pub fn debug_images() -> Vec { images } -#[cfg(feature = "profiling")] +#[cfg(all(feature = "profiling", not(target_os = "windows")))] fn get_profile_from_report( rep: &pprof::UnresolvedReport, trace_id: TraceId, From 0020a8dd98060356952e50ec085eb133cfa456e6 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 17:03:42 +0200 Subject: [PATCH 05/12] Fix error reported by make lint --- sentry-core/src/performance.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 87bda9f9..cad555e3 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -25,13 +25,6 @@ use crate::Client; #[cfg(feature = "client")] const MAX_SPANS: usize = 1_000; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use build_id; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use sys_info; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use uuid; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] lazy_static! { static ref PROFILER_RUNNING: Mutex = Mutex::new(false); @@ -499,7 +492,7 @@ impl Transaction { profile = Some(get_profile_from_report( &report, inner.context.trace_id, - transaction.event_id.clone(), + transaction.event_id, transaction.name.as_ref().unwrap().clone(), )); } @@ -816,7 +809,7 @@ fn get_profile_from_report( }) } samples.push(Sample { - frames: frames, + frames, thread_name: String::from_utf8_lossy(&sample.thread_name[0..sample.thread_name_length]) .into_owned(), thread_id: sample.thread_id, @@ -852,18 +845,18 @@ fn get_profile_from_report( }, platform: "rust".to_string(), architecture: std::env::consts::ARCH.to_string(), - trace_id: trace_id, - transaction_name: transaction_name, - transaction_id: transaction_id, + trace_id, + transaction_name, + transaction_id, profile_id: uuid::Uuid::new_v4(), - sampled_profile: sampled_profile, + sampled_profile, os_name: sys_info::os_type().unwrap(), os_version: sys_info::os_release().unwrap(), version_name: env!("CARGO_PKG_VERSION").to_string(), version_code: build_id::get().to_simple().to_string(), }; - return profile; + profile } impl std::fmt::Display for SentryTrace { From d29e807d617c07742ab7096d71225921e08f7371 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Mon, 27 Jun 2022 17:19:50 +0200 Subject: [PATCH 06/12] Exclude pprof dependency when compiling on windows --- sentry-core/Cargo.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index a37c9236..c40f9563 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -37,12 +37,15 @@ serde = { version = "1.0.104", features = ["derive"] } serde_json = { version = "1.0.46" } lazy_static = { version = "1.4.0", optional = true } mut_static = { version = "5.0.0", optional = true } -pprof = { version = "0.10.0", optional = true } +#pprof = { version = "0.10.0", optional = true } uuid = { version = "1.0.0", features = ["v4", "serde"], optional = true } sys-info = { version = "0.9.1", optional = true } build_id = { version = "0.2.1", optional = true } findshlibs = { version = "=0.10.2", optional = true } +[target.'cfg(not(target_os = "windows"))'.dependencies] +pprof = { version = "0.10.0", optional = true } + [dev-dependencies] # Because we re-export all the public API in `sentry`, we actually run all the # doctests using the `sentry` crate. This also takes care of the doctest From ebe52bf817925b6989e03aead88079a6b2daf580 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 14:11:27 +0200 Subject: [PATCH 07/12] Address PR feedback * refactor into a profiling.rs module * remove lazy_Static and mut_static * replace mutex with AtomicBool * remove profiling feature from test --- sentry-core/Cargo.toml | 4 +- sentry-core/src/lib.rs | 3 + sentry-core/src/performance.rs | 256 ++------------------------ sentry-core/src/profiling.rs | 248 +++++++++++++++++++++++++ sentry-types/src/protocol/envelope.rs | 8 +- sentry-types/src/protocol/profile.rs | 9 +- 6 files changed, 276 insertions(+), 252 deletions(-) create mode 100644 sentry-core/src/profiling.rs diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index c40f9563..19d08bcf 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -25,8 +25,8 @@ client = ["rand"] # I would love to just have a `log` feature, but this is used inside a macro, # and macros actually expand features (and extern crate) where they are used! debug-logs = ["log_"] -test = ["client", "profiling"] -profiling = ["pprof", "lazy_static", "mut_static", "build_id", "uuid", "sys-info", "findshlibs"] +test = ["client"] +profiling = ["pprof", "build_id", "uuid", "sys-info", "findshlibs"] [dependencies] log_ = { package = "log", version = "0.4.8", optional = true, features = ["std"] } diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index 93a7cff8..e92d1c0a 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -92,6 +92,9 @@ pub use crate::client::Client; #[cfg(feature = "test")] pub mod test; +#[cfg(all(feature = "profiling", not(target_os = "windows")))] +mod profiling; + // public api from other crates #[doc(inline)] pub use sentry_types as types; diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index cad555e3..c71be309 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -1,22 +1,13 @@ +#[cfg(all(feature = "profiling", not(target_os = "windows")))] +use std::sync::atomic::AtomicBool; use std::sync::Arc; use std::sync::Mutex; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use findshlibs::{SharedLibrary, SharedLibraryId, TargetSharedLibrary, TARGET_SUPPORTED}; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use lazy_static::lazy_static; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use mut_static::MutStatic; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use sentry_types::{CodeId, DebugId, Uuid}; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] use sentry_types::protocol::v7::Profile; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use sentry_types::protocol::v7::{ - DebugImage, DebugMeta, RustFrame, Sample, SampledProfile, SymbolicDebugImage, TraceId, -}; +#[cfg(all(feature = "profiling", not(target_os = "windows")))] +use crate::profiling; use crate::{protocol, Hub}; #[cfg(feature = "client")] @@ -26,15 +17,7 @@ use crate::Client; const MAX_SPANS: usize = 1_000; #[cfg(all(feature = "profiling", not(target_os = "windows")))] -lazy_static! { - static ref PROFILER_RUNNING: Mutex = Mutex::new(false); - static ref PROFILE_INNER: MutStatic = { - MutStatic::from(ProfileInner { - transaction_id: "".to_string(), - profiler_guard: None, - }) - }; -} +static PROFILER_RUNNING: AtomicBool = AtomicBool::new(false); // global API: @@ -296,16 +279,10 @@ pub(crate) struct TransactionInner { #[cfg(feature = "client")] client: Option>, sampled: bool, - context: protocol::TraceContext, + pub(crate) context: protocol::TraceContext, pub(crate) transaction: Option>, -} - -#[derive(Default)] -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -struct ProfileInner { - transaction_id: String, #[cfg(all(feature = "profiling", not(target_os = "windows")))] - profiler_guard: Option>, + pub(crate) profiler_guard: Option, } type TransactionArc = Arc>; @@ -352,37 +329,11 @@ impl Transaction { // if the transaction was sampled then a profile, linked to the transaction, // might as well be sampled #[cfg(all(feature = "profiling", not(target_os = "windows")))] + let mut profiler_guard: Option = None; + #[cfg(all(feature = "profiling", not(target_os = "windows")))] if sampled { if let Some(client) = client.as_ref() { - let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); - // if the profile is sampled and currently there is no other profile - // being collected - if transaction.is_some() - && client.options().enable_profiling - && !*profiler_running - && client.sample_should_send(client.options().profiles_sample_rate) - { - let profile_guard_builder = pprof::ProfilerGuardBuilder::default() - .frequency(100) - .blocklist(&["libc", "libgcc", "pthread", "vdso"]) - .build(); - - match profile_guard_builder { - Ok(guard_builder) => { - *profiler_running = true; - let mut profile_inner = PROFILE_INNER.write().unwrap(); - profile_inner.transaction_id = - transaction.as_ref().unwrap().event_id.clone().to_string(); - profile_inner.profiler_guard = Some(guard_builder); - } - Err(err) => { - sentry_debug!( - "could not start the profiler due to the following error: {:?}", - err - ); - } - } - } + profiler_guard = profiling::start_profiling(&PROFILER_RUNNING, client); } } @@ -392,6 +343,8 @@ impl Transaction { sampled, context, transaction, + #[cfg(all(feature = "profiling", not(target_os = "windows")))] + profiler_guard, })), } } @@ -411,6 +364,8 @@ impl Transaction { sampled, context, transaction: None, + #[cfg(all(feature = "profiling", not(target_os = "windows")))] + profiler_guard: None, })), } } @@ -478,33 +433,9 @@ impl Transaction { #[cfg(all(feature = "profiling", not(target_os = "windows")))] let mut profile: Option = None; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] if client.options().enable_profiling{ - let mut profiler_running = PROFILER_RUNNING.lock().unwrap(); - if *profiler_running { - let mut profile_inner = PROFILE_INNER.write().unwrap(); - // if the transaction that is ending is the same that started the - // profiler, then the profile should be added to the envelope too - if profile_inner.transaction_id == transaction.event_id.to_string() { - match profile_inner.profiler_guard.as_ref().unwrap().report().build_unresolved() { - Ok(report) => { - profile = Some(get_profile_from_report( - &report, - inner.context.trace_id, - transaction.event_id, - transaction.name.as_ref().unwrap().clone(), - )); - } - Err(err) => { - sentry_debug!("could not build the profile result due to the error: {}", err); - } - } - // in both cases (Ok or Err), reset profile_inner - *profile_inner = ProfileInner::default(); - *profiler_running = false; - } - } + profile = profiling::finish_profiling(&PROFILER_RUNNING, &transaction, &mut inner); } let mut envelope = protocol::Envelope::new(); @@ -702,163 +633,6 @@ fn parse_sentry_trace(header: &str) -> Option { Some(SentryTrace(trace_id, parent_span_id, parent_sampled)) } -/// Converts an ELF object identifier into a `DebugId`. -/// -/// The identifier data is first truncated or extended to match 16 byte size of -/// Uuids. If the data is declared in little endian, the first three Uuid fields -/// are flipped to match the big endian expected by the breakpad processor. -/// -/// The `DebugId::appendix` field is always `0` for ELF. -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -fn debug_id_from_build_id(build_id: &[u8]) -> Option { - const UUID_SIZE: usize = 16; - let mut data = [0u8; UUID_SIZE]; - let len = build_id.len().min(UUID_SIZE); - data[0..len].copy_from_slice(&build_id[0..len]); - - #[cfg(target_endian = "little")] - { - // The ELF file targets a little endian architecture. Convert to - // network byte order (big endian) to match the Breakpad processor's - // expectations. For big endian object files, this is not needed. - data[0..4].reverse(); // uuid field 1 - data[4..6].reverse(); // uuid field 2 - data[6..8].reverse(); // uuid field 3 - } - - Uuid::from_slice(&data).map(DebugId::from_uuid).ok() -} - -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -/// Returns the list of loaded libraries/images. -pub fn debug_images() -> Vec { - let mut images = vec![]; - if !TARGET_SUPPORTED { - return images; - } - - //crate:: ::{CodeId, DebugId, Uuid}; - TargetSharedLibrary::each(|shlib| { - let maybe_debug_id = shlib.debug_id().and_then(|id| match id { - SharedLibraryId::Uuid(bytes) => Some(DebugId::from_uuid(Uuid::from_bytes(bytes))), - SharedLibraryId::GnuBuildId(ref id) => debug_id_from_build_id(id), - SharedLibraryId::PdbSignature(guid, age) => DebugId::from_guid_age(&guid, age).ok(), - _ => None, - }); - - let debug_id = match maybe_debug_id { - Some(debug_id) => debug_id, - None => return, - }; - - let mut name = shlib.name().to_string_lossy().to_string(); - if name.is_empty() { - name = std::env::current_exe() - .map(|x| x.display().to_string()) - .unwrap_or_else(|_| "
".to_string()); - } - - let code_id = shlib.id().map(|id| CodeId::new(format!("{}", id))); - let debug_name = shlib.debug_name().map(|n| n.to_string_lossy().to_string()); - - // For windows, the `virtual_memory_bias` actually returns the real - // `module_base`, which is the address that sentry uses for symbolication. - // Going via the segments means that the `image_addr` would be offset in - // a way that symbolication yields wrong results. - let (image_addr, image_vmaddr) = if cfg!(windows) { - (shlib.virtual_memory_bias().0.into(), 0.into()) - } else { - ( - shlib.actual_load_addr().0.into(), - shlib.stated_load_addr().0.into(), - ) - }; - - images.push( - SymbolicDebugImage { - id: debug_id, - name, - arch: None, - image_addr, - image_size: shlib.len() as u64, - image_vmaddr, - code_id, - debug_file: debug_name, - } - .into(), - ); - }); - - images -} - -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -fn get_profile_from_report( - rep: &pprof::UnresolvedReport, - trace_id: TraceId, - transaction_id: sentry_types::Uuid, - transaction_name: String, -) -> Profile { - let mut samples: Vec = Vec::new(); - - for sample in rep.data.keys() { - let mut frames: Vec = Vec::new(); - for frame in &sample.frames { - frames.push(RustFrame { - instruction_addr: format!("{:p}", frame.ip()), - }) - } - samples.push(Sample { - frames, - thread_name: String::from_utf8_lossy(&sample.thread_name[0..sample.thread_name_length]) - .into_owned(), - thread_id: sample.thread_id, - nanos_relative_to_start: sample - .sample_timestamp - .duration_since(rep.timing.start_time) - .unwrap() - .as_nanos() as u64, - }); - } - let sampled_profile = SampledProfile { - start_time_nanos: rep - .timing - .start_time - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_nanos() as u64, - start_time_secs: rep - .timing - .start_time - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs(), - duration_nanos: rep.timing.duration.as_nanos() as u64, - samples, - }; - use std::time::SystemTime; - let profile: Profile = Profile { - duration_ns: sampled_profile.duration_nanos, - debug_meta: DebugMeta { - sdk_info: None, - images: debug_images(), - }, - platform: "rust".to_string(), - architecture: std::env::consts::ARCH.to_string(), - trace_id, - transaction_name, - transaction_id, - profile_id: uuid::Uuid::new_v4(), - sampled_profile, - os_name: sys_info::os_type().unwrap(), - os_version: sys_info::os_release().unwrap(), - version_name: env!("CARGO_PKG_VERSION").to_string(), - version_code: build_id::get().to_simple().to_string(), - }; - - profile -} - impl std::fmt::Display for SentryTrace { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}-{}", self.0, self.1)?; diff --git a/sentry-core/src/profiling.rs b/sentry-core/src/profiling.rs new file mode 100644 index 00000000..e90efb2e --- /dev/null +++ b/sentry-core/src/profiling.rs @@ -0,0 +1,248 @@ +use std::fmt; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use findshlibs::{SharedLibrary, SharedLibraryId, TargetSharedLibrary, TARGET_SUPPORTED}; + +use crate::TransactionInner; +use sentry_types::protocol::v7::Profile; +use sentry_types::protocol::v7::{ + DebugImage, DebugMeta, RustFrame, Sample, SampledProfile, SymbolicDebugImage, TraceId, + Transaction, +}; +use sentry_types::{CodeId, DebugId, Uuid}; + +#[cfg(feature = "client")] +use crate::Client; + +pub(crate) struct ProfilerGuard(pprof::ProfilerGuard<'static>); + +impl fmt::Debug for ProfilerGuard { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "[ProfilerGuard]") + } +} + +pub(crate) fn start_profiling( + profiler_running: &AtomicBool, + client: &Arc, +) -> Option { + // if profiling is not enabled or the profile was not sampled + // return None immediately + if !client.options().enable_profiling + || !client.sample_should_send(client.options().profiles_sample_rate) + { + return None; + } + + // if no other profile is being collected, then + // start the profiler + if let Ok(false) = + profiler_running.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + { + let profile_guard_builder = pprof::ProfilerGuardBuilder::default() + .frequency(100) + .blocklist(&["libc", "libgcc", "pthread", "vdso"]) + .build(); + + match profile_guard_builder { + Ok(guard_builder) => return Some(ProfilerGuard(guard_builder)), + Err(err) => { + sentry_debug!( + "could not start the profiler due to the following error: {:?}", + err + ); + } + } + } + None +} + +pub(crate) fn finish_profiling( + profiler_running: &AtomicBool, + transaction: &Transaction, + transaction_inner: &mut TransactionInner, +) -> Option { + // if the profiler is running for this transaction, + // then stop it and return the profile + if let Some(profiler_guard) = transaction_inner.profiler_guard.take() { + let mut profile: Option = None; + + match profiler_guard.0.report().build_unresolved() { + Ok(report) => { + profile = Some(get_profile_from_report( + &report, + transaction_inner.context.trace_id, + transaction.event_id, + transaction.name.as_ref().unwrap().clone(), + )); + } + Err(err) => { + sentry_debug!( + "could not build the profile result due to the error: {}", + err + ); + } + } + profiler_running.store(false, Ordering::SeqCst); + return profile; + } + None +} + +/// Converts an ELF object identifier into a `DebugId`. +/// +/// The identifier data is first truncated or extended to match 16 byte size of +/// Uuids. If the data is declared in little endian, the first three Uuid fields +/// are flipped to match the big endian expected by the breakpad processor. +/// +/// The `DebugId::appendix` field is always `0` for ELF. +fn debug_id_from_build_id(build_id: &[u8]) -> Option { + const UUID_SIZE: usize = 16; + let mut data = [0u8; UUID_SIZE]; + let len = build_id.len().min(UUID_SIZE); + data[0..len].copy_from_slice(&build_id[0..len]); + + #[cfg(target_endian = "little")] + { + // The ELF file targets a little endian architecture. Convert to + // network byte order (big endian) to match the Breakpad processor's + // expectations. For big endian object files, this is not needed. + data[0..4].reverse(); // uuid field 1 + data[4..6].reverse(); // uuid field 2 + data[6..8].reverse(); // uuid field 3 + } + + Uuid::from_slice(&data).map(DebugId::from_uuid).ok() +} + +pub fn debug_images() -> Vec { + let mut images = vec![]; + if !TARGET_SUPPORTED { + return images; + } + + //crate:: ::{CodeId, DebugId, Uuid}; + TargetSharedLibrary::each(|shlib| { + let maybe_debug_id = shlib.debug_id().and_then(|id| match id { + SharedLibraryId::Uuid(bytes) => Some(DebugId::from_uuid(Uuid::from_bytes(bytes))), + SharedLibraryId::GnuBuildId(ref id) => debug_id_from_build_id(id), + SharedLibraryId::PdbSignature(guid, age) => DebugId::from_guid_age(&guid, age).ok(), + _ => None, + }); + + let debug_id = match maybe_debug_id { + Some(debug_id) => debug_id, + None => return, + }; + + let mut name = shlib.name().to_string_lossy().to_string(); + if name.is_empty() { + name = std::env::current_exe() + .map(|x| x.display().to_string()) + .unwrap_or_else(|_| "
".to_string()); + } + + let code_id = shlib.id().map(|id| CodeId::new(format!("{}", id))); + let debug_name = shlib.debug_name().map(|n| n.to_string_lossy().to_string()); + + // For windows, the `virtual_memory_bias` actually returns the real + // `module_base`, which is the address that sentry uses for symbolication. + // Going via the segments means that the `image_addr` would be offset in + // a way that symbolication yields wrong results. + let (image_addr, image_vmaddr) = if cfg!(windows) { + (shlib.virtual_memory_bias().0.into(), 0.into()) + } else { + ( + shlib.actual_load_addr().0.into(), + shlib.stated_load_addr().0.into(), + ) + }; + + images.push( + SymbolicDebugImage { + id: debug_id, + name, + arch: None, + image_addr, + image_size: shlib.len() as u64, + image_vmaddr, + code_id, + debug_file: debug_name, + } + .into(), + ); + }); + + images +} + +fn get_profile_from_report( + rep: &pprof::UnresolvedReport, + trace_id: TraceId, + transaction_id: sentry_types::Uuid, + transaction_name: String, +) -> Profile { + use std::time::SystemTime; + + let mut samples: Vec = Vec::new(); + + for sample in rep.data.keys() { + let frames = sample + .frames + .iter() + .map(|frame| RustFrame { + instruction_addr: format!("{:p}", frame.ip()), + }) + .collect(); + + samples.push(Sample { + frames, + thread_name: String::from_utf8_lossy(&sample.thread_name[0..sample.thread_name_length]) + .into_owned(), + thread_id: sample.thread_id, + nanos_relative_to_start: sample + .sample_timestamp + .duration_since(rep.timing.start_time) + .unwrap() + .as_nanos() as u64, + }); + } + let sampled_profile = SampledProfile { + start_time_nanos: rep + .timing + .start_time + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() as u64, + start_time_secs: rep + .timing + .start_time + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(), + duration_nanos: rep.timing.duration.as_nanos() as u64, + samples, + }; + + let profile: Profile = Profile { + duration_ns: sampled_profile.duration_nanos, + debug_meta: DebugMeta { + sdk_info: None, + images: debug_images(), + }, + platform: "rust".to_string(), + architecture: Some(std::env::consts::ARCH.to_string()), + trace_id, + transaction_name, + transaction_id, + profile_id: uuid::Uuid::new_v4(), + sampled_profile, + os_name: sys_info::os_type().unwrap(), + os_version: sys_info::os_release().unwrap(), + version_name: env!("CARGO_PKG_VERSION").to_string(), + version_code: build_id::get().to_simple().to_string(), + }; + + profile +} diff --git a/sentry-types/src/protocol/envelope.rs b/sentry-types/src/protocol/envelope.rs index c8358c22..be94c39a 100644 --- a/sentry-types/src/protocol/envelope.rs +++ b/sentry-types/src/protocol/envelope.rs @@ -294,11 +294,7 @@ impl Envelope { writeln!(writer)?; continue; } - EnvelopeItem::Profile(profile) => { - profile.to_writer(&mut writer)?; - writeln!(writer)?; - continue; - } + EnvelopeItem::Profile(profile) => serde_json::to_writer(&mut item_buf, profile)?, } let item_type = match item { EnvelopeItem::Event(_) => "event", @@ -306,7 +302,7 @@ impl Envelope { EnvelopeItem::SessionAggregates(_) => "sessions", EnvelopeItem::Transaction(_) => "transaction", EnvelopeItem::Attachment(_) => unreachable!(), - EnvelopeItem::Profile(_) => unreachable!(), + EnvelopeItem::Profile(_) => "profile", }; writeln!( writer, diff --git a/sentry-types/src/protocol/profile.rs b/sentry-types/src/protocol/profile.rs index f1da305f..ffcfe9a4 100644 --- a/sentry-types/src/protocol/profile.rs +++ b/sentry-types/src/protocol/profile.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize, Serializer}; use uuid::Uuid; fn serialize_id(uuid: &Uuid, serializer: S) -> Result { - serializer.serialize_some(&uuid.as_simple().to_string()) + serializer.serialize_some(&uuid.as_simple()) } #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] @@ -50,7 +50,8 @@ pub struct Profile { pub platform: String, /// A string describing the architecture of the CPU that is currently in use /// - pub architecture: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub architecture: Option, /// The trace ID pub trace_id: TraceId, /// The name of the transaction this profile belongs to @@ -63,8 +64,8 @@ pub struct Profile { pub profile_id: Uuid, /// Represents the profile collected pub sampled_profile: SampledProfile, - #[serde(rename = "device_os_name")] /// OS name + #[serde(rename = "device_os_name")] pub os_name: String, #[serde(rename = "device_os_version")] /// OS version @@ -85,6 +86,7 @@ struct ProfileItemHeader { length: usize, } +/* impl Profile { /// Writes the attachment and its headers to the provided `Writer`. pub fn to_writer(&self, writer: &mut W) -> std::io::Result<()> @@ -108,3 +110,4 @@ impl Profile { Ok(()) } } +*/ From c52880cb46f7511ec9c995f2ca5784c4db4e7789 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 16:34:08 +0200 Subject: [PATCH 08/12] Refactor start_profiling finish_profiling --- sentry-core/src/performance.rs | 28 +++++------------ sentry-core/src/profiling.rs | 57 ++++++++++++++-------------------- 2 files changed, 32 insertions(+), 53 deletions(-) diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index c71be309..6b770ebd 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -1,11 +1,6 @@ -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use std::sync::atomic::AtomicBool; use std::sync::Arc; use std::sync::Mutex; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -use sentry_types::protocol::v7::Profile; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] use crate::profiling; use crate::{protocol, Hub}; @@ -16,9 +11,6 @@ use crate::Client; #[cfg(feature = "client")] const MAX_SPANS: usize = 1_000; -#[cfg(all(feature = "profiling", not(target_os = "windows")))] -static PROFILER_RUNNING: AtomicBool = AtomicBool::new(false); - // global API: /// Start a new Performance Monitoring Transaction. @@ -329,13 +321,11 @@ impl Transaction { // if the transaction was sampled then a profile, linked to the transaction, // might as well be sampled #[cfg(all(feature = "profiling", not(target_os = "windows")))] - let mut profiler_guard: Option = None; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] - if sampled { - if let Some(client) = client.as_ref() { - profiler_guard = profiling::start_profiling(&PROFILER_RUNNING, client); - } - } + let profiler_guard = if sampled { + client.as_deref().and_then(profiling::start_profiling) + } else { + None + }; Self { inner: Arc::new(Mutex::new(TransactionInner { @@ -432,11 +422,9 @@ impl Transaction { transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); #[cfg(all(feature = "profiling", not(target_os = "windows")))] - let mut profile: Option = None; - #[cfg(all(feature = "profiling", not(target_os = "windows")))] - if client.options().enable_profiling{ - profile = profiling::finish_profiling(&PROFILER_RUNNING, &transaction, &mut inner); - } + let profile = inner.profiler_guard.take().and_then(|profiler_guard| { + profiling::finish_profiling(&transaction, profiler_guard, inner.context.trace_id) + }); let mut envelope = protocol::Envelope::new(); envelope.add_item(transaction); diff --git a/sentry-core/src/profiling.rs b/sentry-core/src/profiling.rs index e90efb2e..9690b05f 100644 --- a/sentry-core/src/profiling.rs +++ b/sentry-core/src/profiling.rs @@ -1,10 +1,8 @@ use std::fmt; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; use findshlibs::{SharedLibrary, SharedLibraryId, TargetSharedLibrary, TARGET_SUPPORTED}; -use crate::TransactionInner; use sentry_types::protocol::v7::Profile; use sentry_types::protocol::v7::{ DebugImage, DebugMeta, RustFrame, Sample, SampledProfile, SymbolicDebugImage, TraceId, @@ -15,6 +13,8 @@ use sentry_types::{CodeId, DebugId, Uuid}; #[cfg(feature = "client")] use crate::Client; +static PROFILER_RUNNING: AtomicBool = AtomicBool::new(false); + pub(crate) struct ProfilerGuard(pprof::ProfilerGuard<'static>); impl fmt::Debug for ProfilerGuard { @@ -23,10 +23,7 @@ impl fmt::Debug for ProfilerGuard { } } -pub(crate) fn start_profiling( - profiler_running: &AtomicBool, - client: &Arc, -) -> Option { +pub(crate) fn start_profiling(client: &Client) -> Option { // if profiling is not enabled or the profile was not sampled // return None immediately if !client.options().enable_profiling @@ -38,7 +35,7 @@ pub(crate) fn start_profiling( // if no other profile is being collected, then // start the profiler if let Ok(false) = - profiler_running.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + PROFILER_RUNNING.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) { let profile_guard_builder = pprof::ProfilerGuardBuilder::default() .frequency(100) @@ -52,6 +49,7 @@ pub(crate) fn start_profiling( "could not start the profiler due to the following error: {:?}", err ); + PROFILER_RUNNING.store(false, Ordering::SeqCst); } } } @@ -59,35 +57,28 @@ pub(crate) fn start_profiling( } pub(crate) fn finish_profiling( - profiler_running: &AtomicBool, transaction: &Transaction, - transaction_inner: &mut TransactionInner, + profiler_guard: ProfilerGuard, + trace_id: TraceId, ) -> Option { - // if the profiler is running for this transaction, - // then stop it and return the profile - if let Some(profiler_guard) = transaction_inner.profiler_guard.take() { - let mut profile: Option = None; - - match profiler_guard.0.report().build_unresolved() { - Ok(report) => { - profile = Some(get_profile_from_report( - &report, - transaction_inner.context.trace_id, - transaction.event_id, - transaction.name.as_ref().unwrap().clone(), - )); - } - Err(err) => { - sentry_debug!( - "could not build the profile result due to the error: {}", - err - ); - } + let profile = match profiler_guard.0.report().build_unresolved() { + Ok(report) => Some(get_profile_from_report( + &report, + trace_id, + transaction.event_id, + transaction.name.as_ref().unwrap().clone(), + )), + Err(err) => { + sentry_debug!( + "could not build the profile result due to the error: {}", + err + ); + None } - profiler_running.store(false, Ordering::SeqCst); - return profile; - } - None + }; + + PROFILER_RUNNING.store(false, Ordering::SeqCst); + profile } /// Converts an ELF object identifier into a `DebugId`. From f71f8d4d6ffbaec183525e4359faa9161a3dc69a Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 16:42:09 +0200 Subject: [PATCH 09/12] Remove unused dependencies --- sentry-core/Cargo.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index 19d08bcf..d110cf8c 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -35,9 +35,6 @@ rand = { version = "0.8.1", optional = true } sentry-types = { version = "0.27.0", path = "../sentry-types" } serde = { version = "1.0.104", features = ["derive"] } serde_json = { version = "1.0.46" } -lazy_static = { version = "1.4.0", optional = true } -mut_static = { version = "5.0.0", optional = true } -#pprof = { version = "0.10.0", optional = true } uuid = { version = "1.0.0", features = ["v4", "serde"], optional = true } sys-info = { version = "0.9.1", optional = true } build_id = { version = "0.2.1", optional = true } From 1b494236918ac3cfa0fe3e2ca5ac6a39ff749b60 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 16:49:49 +0200 Subject: [PATCH 10/12] Remove unused code --- sentry-types/src/protocol/profile.rs | 36 ---------------------------- 1 file changed, 36 deletions(-) diff --git a/sentry-types/src/protocol/profile.rs b/sentry-types/src/protocol/profile.rs index ffcfe9a4..e8396c87 100644 --- a/sentry-types/src/protocol/profile.rs +++ b/sentry-types/src/protocol/profile.rs @@ -75,39 +75,3 @@ pub struct Profile { /// Current binary build ID. See pub version_code: String, } - -#[derive(Debug, Default, Deserialize, Serialize)] -struct ProfileItemHeader { - content_type: String, - #[serde(skip_serializing_if = "String::is_empty")] - file_name: String, - #[serde(rename = "type")] - typez: String, - length: usize, -} - -/* -impl Profile { - /// Writes the attachment and its headers to the provided `Writer`. - pub fn to_writer(&self, writer: &mut W) -> std::io::Result<()> - where - W: std::io::Write, - { - let serialized_profile = serde_json::to_string(self).unwrap(); - - writeln!( - writer, - "{}", - serde_json::to_string(&ProfileItemHeader { - content_type: "application/json".to_string(), - file_name: format!("{}.trace", self.trace_id), - typez: "profile".to_string(), - length: serialized_profile.len(), - })? - )?; - - writer.write_all(serialized_profile.as_bytes())?; - Ok(()) - } -} -*/ From b51befa64882ce92165818a47fafb234c722d47a Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 17:20:52 +0200 Subject: [PATCH 11/12] Update the CHANGELOG --- CHANGELOG.md | 16 ++++++++++++++++ sentry-core/src/performance.rs | 2 ++ 2 files changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e3493b8..821bdc09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## Unreleased + +**Breaking Changes**: + +**Features**: + +- Add support for Profiling feature. ([#479](https://github.com/getsentry/sentry-rust/pull/479)) + +**Internal**: + +**Thank you**: + +Features, fixes and improvements in this release have been contributed by: + +- [@viglia](https://github.com/viglia) + ## 0.27.0 **Breaking Changes**: diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 6b770ebd..40007f85 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -421,6 +421,8 @@ impl Transaction { transaction.environment = opts.environment.clone(); transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); + // if the profiler is running for the given transaction + // then call finish_profiling to return the profile #[cfg(all(feature = "profiling", not(target_os = "windows")))] let profile = inner.profiler_guard.take().and_then(|profiler_guard| { profiling::finish_profiling(&transaction, profiler_guard, inner.context.trace_id) From f5b063c091896d9ac60c20aa0a7bd4c8ea791ab6 Mon Sep 17 00:00:00 2001 From: Francesco Vigliaturo Date: Thu, 30 Jun 2022 17:48:52 +0200 Subject: [PATCH 12/12] Refactor `thank you` section of the CHANGELOG --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 821bdc09..97611b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,6 @@ **Thank you**: -Features, fixes and improvements in this release have been contributed by: - -- [@viglia](https://github.com/viglia) ## 0.27.0