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

sentry-core: add traces_sampler client option #510

Merged
merged 5 commits into from
Nov 3, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Breaking Changes**:

- The minimum supported Rust version was bumped to **1.60.0** due to requirements from dependencies. ([#498](https://github.com/getsentry/sentry-rust/pull/498))
- Added the `traces_sampler` option to `ClientOptions`. This allows the user to customise sampling rates on a per-transaction basis. ([#510](https://github.com/getsentry/sentry-rust/pull/510))

**Features**:

Expand Down
2 changes: 1 addition & 1 deletion sentry-contexts/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod model_support {
return None;
}

let mut buf = vec![0u8; size as usize];
let mut buf = vec![0u8; size];
let res = libc::sysctlbyname(
c_name.as_ptr() as _,
buf.as_mut_ptr() as *mut c_void,
Expand Down
14 changes: 14 additions & 0 deletions sentry-core/src/clientoptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;
use std::time::Duration;

use crate::constants::USER_AGENT;
use crate::performance::TracesSampler;
use crate::protocol::{Breadcrumb, Event};
use crate::types::Dsn;
use crate::{Integration, IntoDsn, TransportFactory};
Expand Down Expand Up @@ -74,6 +75,11 @@ 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,
/// If given, called with a SamplingContext for each transaction to determine the sampling rate.
///
/// 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)
Expand Down Expand Up @@ -196,6 +202,13 @@ impl fmt::Debug for ClientOptions {
.field("environment", &self.environment)
.field("sample_rate", &self.sample_rate)
.field("traces_sample_rate", &self.traces_sample_rate)
.field(
"traces_sampler",
&self
.traces_sampler
.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)
Expand Down Expand Up @@ -231,6 +244,7 @@ impl Default for ClientOptions {
environment: None,
sample_rate: 1.0,
traces_sample_rate: 0.0,
traces_sampler: None,
enable_profiling: false,
profiles_sample_rate: 0.0,
max_breadcrumbs: 100,
Expand Down
4 changes: 2 additions & 2 deletions sentry-core/src/hub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ pub(crate) struct HubImpl {
impl HubImpl {
pub(crate) fn with<F: FnOnce(&Stack) -> R, R>(&self, f: F) -> R {
let guard = self.stack.read().unwrap_or_else(PoisonError::into_inner);
f(&*guard)
f(&guard)
}

fn with_mut<F: FnOnce(&mut Stack) -> R, R>(&self, f: F) -> R {
let mut guard = self.stack.write().unwrap_or_else(PoisonError::into_inner);
f(&mut *guard)
f(&mut guard)
}

fn is_active_and_usage_safe(&self) -> bool {
Expand Down
88 changes: 78 additions & 10 deletions sentry-core/src/performance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ impl TransactionContext {
}
}

/// A function to be run for each new transaction, to determine the rate at which
/// it should be sampled.
///
/// This function may choose to respect the sampling of the parent transaction (`ctx.sampled`)
/// or ignore it.
pub type TracesSampler = dyn Fn(&TransactionContext) -> f32 + Send + Sync;

// global API types:

/// A wrapper that groups a [`Transaction`] and a [`Span`] together.
Expand Down Expand Up @@ -279,6 +286,37 @@ pub(crate) struct TransactionInner {

type TransactionArc = Arc<Mutex<TransactionInner>>;

/// Functional implementation of how a new transation's sample rate is chosen.
///
/// Split out from `Client.is_transaction_sampled` for testing.
#[cfg(feature = "client")]
fn transaction_sample_rate(
traces_sampler: Option<&TracesSampler>,
ctx: &TransactionContext,
traces_sample_rate: f32,
) -> f32 {
match (traces_sampler, traces_sample_rate) {
(Some(traces_sampler), _) => traces_sampler(ctx),
(None, traces_sample_rate) => ctx
.sampled
.map(|sampled| if sampled { 1.0 } else { 0.0 })
.unwrap_or(traces_sample_rate),
}
}

/// Determine whether the new transaction should be sampled.
#[cfg(feature = "client")]
impl Client {
fn is_transaction_sampled(&self, ctx: &TransactionContext) -> bool {
let client_options = self.options();
self.sample_should_send(transaction_sample_rate(
client_options.traces_sampler.as_deref(),
ctx,
client_options.traces_sample_rate,
))
}
}

/// A running Performance Monitoring Transaction.
///
/// The transaction needs to be explicitly finished via [`Transaction::finish`],
Expand All @@ -292,18 +330,9 @@ pub struct Transaction {
impl Transaction {
#[cfg(feature = "client")]
fn new(mut client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
let context = protocol::TraceContext {
trace_id: ctx.trace_id,
parent_span_id: ctx.parent_span_id,
op: Some(ctx.op),
..Default::default()
};

let (sampled, mut transaction) = match client.as_ref() {
Some(client) => (
ctx.sampled.unwrap_or_else(|| {
client.sample_should_send(client.options().traces_sample_rate)
}),
client.is_transaction_sampled(&ctx),
Some(protocol::Transaction {
name: Some(ctx.name),
#[cfg(all(feature = "profiling", target_family = "unix"))]
Expand All @@ -314,6 +343,13 @@ impl Transaction {
None => (ctx.sampled.unwrap_or(false), None),
};

let context = protocol::TraceContext {
trace_id: ctx.trace_id,
parent_span_id: ctx.parent_span_id,
op: Some(ctx.op),
..Default::default()
};

// throw away the transaction here, which means there is nothing to send
// on `finish`.
if !sampled {
Expand Down Expand Up @@ -679,4 +715,36 @@ mod tests {
assert_eq!(&parsed.0.to_string(), "09e04486820349518ac7b5d2adbf6ba5");
assert_eq!(parsed.2, Some(true));
}

#[cfg(feature = "client")]
#[test]
fn compute_transaction_sample_rate() {
// Global rate used as fallback.
let ctx = TransactionContext::new("noop", "noop");
assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 0.3);
assert_eq!(transaction_sample_rate(None, &ctx, 0.7), 0.7);

// If only global rate, setting sampled overrides it
let mut ctx = TransactionContext::new("noop", "noop");
ctx.set_sampled(true);
assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 1.0);
ctx.set_sampled(false);
assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 0.0);

// If given, sampler function overrides everything else.
let mut ctx = TransactionContext::new("noop", "noop");
assert_eq!(transaction_sample_rate(Some(&|_| { 0.7 }), &ctx, 0.3), 0.7);
ctx.set_sampled(false);
assert_eq!(transaction_sample_rate(Some(&|_| { 0.7 }), &ctx, 0.3), 0.7);
// But the sampler may choose to inspect parent sampling
let sampler = |ctx: &TransactionContext| match ctx.sampled {
Some(true) => 0.8,
Some(false) => 0.4,
None => 0.6,
};
ctx.set_sampled(true);
assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.8);
ctx.set_sampled(None);
assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.6);
}
}
2 changes: 1 addition & 1 deletion sentry-core/src/profiling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ fn get_profile_from_report(
timestamp: rep.timing.start_time,
transactions: vec![TransactionMetadata {
id: transaction.event_id,
name: transaction.name.clone().unwrap_or_else(|| "".to_string()),
name: transaction.name.clone().unwrap_or_default(),
trace_id,
relative_start_ns: 0,
relative_end_ns: transaction
Expand Down
2 changes: 1 addition & 1 deletion sentry-panic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Integration for PanicIntegration {
/// Extract the message of a panic.
pub fn message_from_panic_info<'a>(info: &'a PanicInfo<'_>) -> &'a str {
match info.payload().downcast_ref::<&'static str>() {
Some(s) => *s,
Some(s) => s,
None => match info.payload().downcast_ref::<String>() {
Some(s) => &s[..],
None => "Box<Any>",
Expand Down
2 changes: 1 addition & 1 deletion sentry-types/src/protocol/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ impl Envelope {
let payload_start = std::cmp::min(header_end + 1, slice.len());
let payload_end = match header.length {
Some(len) => {
let payload_end = payload_start + len as usize;
let payload_end = payload_start + len;
if slice.len() < payload_end {
return Err(EnvelopeError::UnexpectedEof);
}
Expand Down
4 changes: 2 additions & 2 deletions sentry-types/src/protocol/v7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ impl Default for SpanId {

impl fmt::Display for SpanId {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{}", hex::encode(&self.0))
write!(fmt, "{}", hex::encode(self.0))
}
}

Expand Down Expand Up @@ -1406,7 +1406,7 @@ impl Default for TraceId {

impl fmt::Display for TraceId {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{}", hex::encode(&self.0))
write!(fmt, "{}", hex::encode(self.0))
}
}

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/transports/curl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl CurlHttpTransport {
let http_proxy = options.http_proxy.as_ref().map(ToString::to_string);
let https_proxy = options.https_proxy.as_ref().map(ToString::to_string);
let dsn = options.dsn.as_ref().unwrap();
let user_agent = options.user_agent.to_owned();
let user_agent = options.user_agent.clone();
let auth = dsn.to_auth(Some(&user_agent)).to_string();
let url = dsn.envelope_api_url().to_string();
let scheme = dsn.scheme();
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/transports/reqwest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl ReqwestHttpTransport {
builder.build().unwrap()
});
let dsn = options.dsn.as_ref().unwrap();
let user_agent = options.user_agent.to_owned();
let user_agent = options.user_agent.clone();
let auth = dsn.to_auth(Some(&user_agent)).to_string();
let url = dsn.envelope_api_url().to_string();

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/transports/surf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl SurfHttpTransport {
}

let dsn = options.dsn.as_ref().unwrap();
let user_agent = options.user_agent.to_owned();
let user_agent = options.user_agent.clone();
let auth = dsn.to_auth(Some(&user_agent)).to_string();
let url = dsn.envelope_api_url().to_string();

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/transports/ureq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl UreqHttpTransport {

builder.build()
});
let user_agent = options.user_agent.to_owned();
let user_agent = options.user_agent.clone();
let auth = dsn.to_auth(Some(&user_agent)).to_string();
let url = dsn.envelope_api_url().to_string();

Expand Down
2 changes: 1 addition & 1 deletion sentry/tests/test_tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn test_tracing() {

#[tracing::instrument(fields(span_field))]
fn function() {
tracing::Span::current().record("span_field", &"some data");
tracing::Span::current().record("span_field", "some data");
}

#[test]
Expand Down