Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(profiling): remove profiling support #595

Merged
merged 1 commit into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 0 additions & 11 deletions sentry-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ client = ["rand"]
# and macros actually expand features (and extern crate) where they are used!
debug-logs = ["dep:log"]
test = ["client"]
profiling = ["pprof", "build_id", "uuid", "sys-info", "findshlibs", "rustc_version_runtime", "libc", "indexmap"]
frame-pointer = ["pprof?/frame-pointer"]

[dependencies]
log = { version = "0.4.8", optional = true, features = ["std"] }
Expand All @@ -37,15 +35,6 @@ sentry-types = { version = "0.31.5", path = "../sentry-types" }
serde = { version = "1.0.104", features = ["derive"] }
serde_json = { version = "1.0.46" }
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 }
rustc_version_runtime = { version = "0.2.1", optional = true }
indexmap = { version = "1.9.1", optional = true }

[target.'cfg(target_family = "unix")'.dependencies]
pprof = { version = "0.11.0", optional = true, default-features = false }
libc = { version = "^0.2.66", optional = true }

[dev-dependencies]
# Because we re-export all the public API in `sentry`, we actually run all the
Expand Down
18 changes: 1 addition & 17 deletions sentry-core/src/clientoptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::sync::Arc;
use std::time::Duration;

use crate::constants::USER_AGENT;
use crate::performance::{ProfilesSampler, TracesSampler};
use crate::performance::TracesSampler;
use crate::protocol::{Breadcrumb, Event};
use crate::types::Dsn;
use crate::{Integration, IntoDsn, TransportFactory};
Expand Down Expand Up @@ -80,17 +80,6 @@ pub struct ClientOptions {
/// Return a sample rate between 0.0 and 1.0 for the transaction in question.
/// Takes priority over the `sample_rate`.
pub traces_sampler: Option<Arc<TracesSampler>>,
/// Enables profiling
pub enable_profiling: bool,
/// The sample rate for profiling a transactions. (0.0 - 1.0, defaults to 0.0)
///
/// This represents the probability that a sampled transaction
/// will send a profile to Sentry
pub profiles_sample_rate: f32,
/// If given, called with a TransactionContext for each profile to determine the sampling rate.
///
/// Return a sample rate between 0.0 and 1.0 for the profile in question.
pub profiles_sampler: Option<Arc<ProfilesSampler>>,
/// Maximum number of breadcrumbs. (defaults to 100)
pub max_breadcrumbs: usize,
/// Attaches stacktraces to messages.
Expand Down Expand Up @@ -213,8 +202,6 @@ impl fmt::Debug for ClientOptions {
.as_ref()
.map(|arc| std::ptr::addr_of!(**arc)),
)
.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)
Expand Down Expand Up @@ -249,9 +236,6 @@ impl Default for ClientOptions {
sample_rate: 1.0,
traces_sample_rate: 0.0,
traces_sampler: None,
enable_profiling: false,
profiles_sample_rate: 0.0,
profiles_sampler: None,
max_breadcrumbs: 100,
attach_stacktrace: false,
send_default_pii: false,
Expand Down
3 changes: 0 additions & 3 deletions sentry-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,6 @@ 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;
Expand Down
70 changes: 1 addition & 69 deletions sentry-core/src/performance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@ use std::collections::BTreeMap;
use std::ops::{Deref, DerefMut};
use std::sync::{Arc, Mutex, MutexGuard};

#[cfg(all(feature = "profiling", target_family = "unix"))]
use crate::profiling;

use crate::{protocol, Hub};

#[cfg(feature = "client")]
Expand Down Expand Up @@ -230,9 +227,6 @@ impl TransactionContext {
/// or ignore it.
pub type TracesSampler = dyn Fn(&TransactionContext) -> f32 + Send + Sync;

/// Same as TracesSampler but for profiles
pub type ProfilesSampler = TracesSampler;

// global API types:

/// A wrapper that groups a [`Transaction`] and a [`Span`] together.
Expand Down Expand Up @@ -360,8 +354,6 @@ pub(crate) struct TransactionInner {
sampled: bool,
pub(crate) context: protocol::TraceContext,
pub(crate) transaction: Option<protocol::Transaction<'static>>,
#[cfg(all(feature = "profiling", target_family = "unix"))]
pub(crate) profiler_guard: Option<profiling::ProfilerGuard>,
}

type TransactionArc = Arc<Mutex<TransactionInner>>;
Expand All @@ -384,18 +376,6 @@ fn transaction_sample_rate(
}
}

#[cfg(all(feature = "profiling", target_family = "unix"))]
fn profile_sample_rate(
profile_sampler: Option<&ProfilesSampler>,
ctx: &TransactionContext,
profiles_sample_rate: f32,
) -> f32 {
match (profile_sampler, profiles_sample_rate) {
(Some(profile_sampler), _) => profile_sampler(ctx),
(None, profiles_sample_rate) => profiles_sample_rate,
}
}

/// Determine whether the new transaction should be sampled.
#[cfg(feature = "client")]
impl Client {
Expand All @@ -407,16 +387,6 @@ impl Client {
client_options.traces_sample_rate,
))
}

#[cfg(all(feature = "profiling", target_family = "unix"))]
pub(crate) fn is_profile_sampled(&self, ctx: &TransactionContext) -> bool {
let client_options = self.options();
self.sample_should_send(profile_sample_rate(
client_options.traces_sampler.as_deref(),
ctx,
client_options.profiles_sample_rate,
))
}
}

/// A running Performance Monitoring Transaction.
Expand All @@ -437,14 +407,6 @@ impl Transaction {
client.is_transaction_sampled(&ctx),
Some(protocol::Transaction {
name: Some(ctx.name.clone()),
#[cfg(all(feature = "profiling", target_family = "unix"))]
active_thread_id: Some(
// NOTE: `pthread_t` is a `usize`, so clippy is wrong complaining about this cast
#[allow(clippy::unnecessary_cast)]
unsafe {
libc::pthread_self() as u64
},
),
..Default::default()
}),
),
Expand All @@ -454,7 +416,7 @@ impl Transaction {
let context = protocol::TraceContext {
trace_id: ctx.trace_id,
parent_span_id: ctx.parent_span_id,
op: Some(ctx.op.clone()),
op: Some(ctx.op),
..Default::default()
};

Expand All @@ -464,25 +426,13 @@ impl Transaction {
transaction = None;
client = None;
}
// if the transaction was sampled then a profile, linked to the transaction,
// might as well be sampled
#[cfg(all(feature = "profiling", target_family = "unix"))]
let profiler_guard = if sampled {
client
.as_deref()
.and_then(|cli| profiling::start_profiling(cli, &ctx))
} else {
None
};

Self {
inner: Arc::new(Mutex::new(TransactionInner {
client,
sampled,
context,
transaction,
#[cfg(all(feature = "profiling", target_family = "unix"))]
profiler_guard,
})),
}
}
Expand All @@ -502,8 +452,6 @@ impl Transaction {
sampled,
context,
transaction: None,
#[cfg(all(feature = "profiling", target_family = "unix"))]
profiler_guard: None,
})),
}
}
Expand Down Expand Up @@ -577,27 +525,11 @@ 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", target_family = "unix"))]
let sample_profile = inner.profiler_guard.take().and_then(|profiler_guard| {
profiling::finish_profiling(&transaction, profiler_guard, inner.context.trace_id)
});
drop(inner);

let mut envelope = protocol::Envelope::new();
envelope.add_item(transaction);

#[cfg(all(feature = "profiling", target_family = "unix"))]
if let Some(sample_profile) = sample_profile {
if !sample_profile.profile.samples.is_empty(){
envelope.add_item(sample_profile);
}
else {
sentry_debug!("the profile is being dropped because it contains no samples");
}
}

client.send_envelope(envelope)
}
}
Expand Down