diff --git a/sentry-core/src/performance/headers.rs b/sentry-core/src/performance/headers.rs index a3742135..703b3051 100644 --- a/sentry-core/src/performance/headers.rs +++ b/sentry-core/src/performance/headers.rs @@ -132,6 +132,11 @@ impl TracePropagationContext { }) } + /// Set the `sampled` field, accepting `Option` values. + pub(crate) fn with_maybe_sampled(self, sampled: Option) -> Self { + Self { sampled, ..self } + } + /// Attempts to construct a [`TracePropagationContext`] from the given Sentry trace header. /// /// Returns [`None`] if the header cannot be parsed. diff --git a/sentry-core/src/performance/mod.rs b/sentry-core/src/performance/mod.rs index 390c6081..820ccb3c 100644 --- a/sentry-core/src/performance/mod.rs +++ b/sentry-core/src/performance/mod.rs @@ -10,6 +10,9 @@ use sentry_types::protocol::v7::client_report::Reason as ClientReportReason; use sentry_types::protocol::v7::OrganizationId; use sentry_types::protocol::v7::SpanId; +#[cfg(feature = "client")] +use self::sampling::FinishAction; +use self::sampling::TracingState; #[cfg(feature = "client")] use crate::clientoptions::TracesSamplingStrategy; use crate::{protocol, Hub}; @@ -22,6 +25,7 @@ pub use self::headers::{parse_sentry_trace_header as parse_headers, SentryTrace} pub use self::headers::{HeaderParseError, TracePropagationContext}; mod headers; +mod sampling; #[cfg(feature = "client")] const MAX_SPANS: usize = 1_000; @@ -278,13 +282,13 @@ impl TransactionContext { ( inner.context.trace_id, inner.context.span_id, - Some(inner.sampled), + inner.tracing_state.trace_sampled(), ) } TransactionOrSpan::Span(span) => { - let sampled = span.sampled; + let trace_sampled = span.tracing_state.trace_sampled(); let span = span.span.lock().unwrap(); - (span.trace_id, span.span_id, Some(sampled)) + (span.trace_id, span.span_id, trace_sampled) } }; @@ -676,7 +680,7 @@ impl TransactionOrSpan { pub(crate) struct TransactionInner { #[cfg(feature = "client")] client: Option>, - sampled: bool, + tracing_state: TracingState, pub(crate) context: protocol::TraceContext, pub(crate) transaction: Option>, } @@ -685,16 +689,16 @@ type TransactionArc = Arc>; /// Functional implementation of how a new transaction's sample rate is chosen. /// -/// Split out from `Client.is_transaction_sampled` for testing. +/// Returns `None` when tracing is disabled. #[cfg(feature = "client")] fn transaction_sample_rate( traces_sampling_strategy: &TracesSamplingStrategy, ctx: &TransactionContext, -) -> f32 { +) -> Option { match traces_sampling_strategy { - &TracesSamplingStrategy::FixedRate(rate) => ctx.sampled.map_or(rate, f32::from), - TracesSamplingStrategy::Function(traces_sampler) => traces_sampler(ctx), - TracesSamplingStrategy::Disabled => 0.0, + &TracesSamplingStrategy::FixedRate(rate) => Some(ctx.sampled.map_or(rate, f32::from)), + TracesSamplingStrategy::Function(traces_sampler) => Some(traces_sampler(ctx)), + TracesSamplingStrategy::Disabled => None, } } @@ -714,22 +718,23 @@ fn should_continue_trace( /// Determine whether the new transaction should be sampled. #[cfg(feature = "client")] impl Client { - fn determine_sampling_decision(&self, ctx: &TransactionContext) -> (bool, f32) { + /// Determines the [`TracingState`] based on the provided [`TransactionContext`]. + /// + /// This function performs random sampling according to the appropriate sample rate as needed. + fn determine_tracing_state(&self, ctx: &TransactionContext) -> TracingState { let client_options = self.options(); - let sample_rate = transaction_sample_rate(&client_options.traces_sampling_strategy, ctx); - let sampled = self.sample_should_send(sample_rate); - (sampled, sample_rate) + match transaction_sample_rate(&client_options.traces_sampling_strategy, ctx) { + // A return value of Some(_) indicates tracing is enabled. + Some(sample_rate) => { + let sampled = self.sample_should_send(sample_rate); + TracingState::new_enabled(sampled, sample_rate) + } + // A return value of None indicates tracing is disabled. + None => TracingState::new_disabled(ctx.sampled), + } } } -/// Some metadata associated with a transaction. -#[cfg(feature = "client")] -#[derive(Clone, Debug)] -struct TransactionMetadata { - /// The sample rate used when making the sampling decision for the associated transaction. - sample_rate: f32, -} - /// A running Performance Monitoring Transaction. /// /// The transaction needs to be explicitly finished via [`Transaction::finish`], @@ -738,8 +743,6 @@ struct TransactionMetadata { #[derive(Clone, Debug)] pub struct Transaction { pub(crate) inner: TransactionArc, - #[cfg(feature = "client")] - metadata: TransactionMetadata, } /// Iterable for a transaction's [data attributes](protocol::TraceContext::data). @@ -778,7 +781,7 @@ impl<'a> TransactionData<'a> { impl Transaction { #[cfg(feature = "client")] fn new(client: Option>, mut ctx: TransactionContext) -> Self { - let ((sampled, sample_rate), transaction) = match client.as_ref() { + let (tracing_state, transaction) = match client.as_ref() { Some(client) => { let options = client.options(); let sdk_org_id = options.org_id.or_else(|| options.dsn.as_ref()?.org_id()); @@ -798,20 +801,14 @@ impl Transaction { } ( - client.determine_sampling_decision(&ctx), + client.determine_tracing_state(&ctx), Some(protocol::Transaction { name: Some(ctx.name), ..Default::default() }), ) } - None => ( - ( - ctx.sampled.unwrap_or(false), - ctx.sampled.map_or(0.0, f32::from), - ), - None, - ), + None => (TracingState::new_disabled(ctx.sampled), None), }; let context = protocol::TraceContext { @@ -825,11 +822,10 @@ impl Transaction { Self { inner: Arc::new(Mutex::new(TransactionInner { client, - sampled, + tracing_state, context, transaction, })), - metadata: TransactionMetadata { sample_rate }, } } @@ -841,11 +837,11 @@ impl Transaction { op: Some(ctx.op), ..Default::default() }; - let sampled = ctx.sampled.unwrap_or(false); + let tracing_state = TracingState::new_disabled(ctx.sampled); Self { inner: Arc::new(Mutex::new(TransactionInner { - sampled, + tracing_state, context, transaction: None, })), @@ -943,7 +939,7 @@ impl Transaction { pub fn iter_headers(&self) -> TraceHeadersIter { let inner = self.inner.lock().unwrap(); let trace = TracePropagationContext::new(inner.context.trace_id, inner.context.span_id) - .with_sampled(inner.sampled); + .with_maybe_sampled(inner.tracing_state.trace_sampled()); TraceHeadersIter { sentry_trace: Some(trace.sentry_trace_header()), } @@ -963,7 +959,19 @@ impl Transaction { /// correct results. #[deprecated = "the returned value may not accurately represent the sampling decision"] pub fn is_sampled(&self) -> bool { - self.inner.lock().unwrap().sampled + // Checking that we have a `Send` finish action should at least roughly match the old + // behavior of this function: we only return true for sampled spans when tracing is + // enabled, and false otherwise. + #[cfg(feature = "client")] + { + matches!( + self.inner.lock().unwrap().tracing_state.finish_action(), + FinishAction::Send { .. } + ) + } + + #[cfg(not(feature = "client"))] + false } /// Finishes the Transaction with the provided end timestamp. @@ -974,46 +982,42 @@ impl Transaction { with_client_impl! {{ let mut inner = self.inner.lock().unwrap(); - // Discard `Transaction` unless sampled. - if !inner.sampled { - if let Some(transaction) = inner.transaction.take() { - if let Some(client) = inner.client.as_ref() { + if let (Some(mut transaction), Some(client)) = (inner.transaction.take(), inner.client.take()) { + match inner.tracing_state.finish_action() { + FinishAction::Send { sample_rate } => { + transaction.finish_with_timestamp(_timestamp); + transaction + .contexts + .insert("trace".into(), inner.context.clone().into()); + + Hub::current().with_current_scope(|scope| scope.apply_to_transaction(&mut transaction)); + let opts = client.options(); + transaction.release.clone_from(&opts.release); + transaction.environment.clone_from(&opts.environment); + transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); + transaction.server_name.clone_from(&opts.server_name); + + let mut dsc = protocol::DynamicSamplingContext::new() + .with_trace_id(inner.context.trace_id) + .with_sample_rate(sample_rate) + .with_sampled(true); + if let Some(public_key) = client.dsn().map(|dsn| dsn.public_key()) { + dsc = dsc.with_public_key(public_key.to_owned()); + } + + drop(inner); + + let mut envelope = protocol::Envelope::new().with_headers( + protocol::EnvelopeHeaders::new().with_trace(dsc) + ); + envelope.add_item(transaction); + + client.send_envelope(envelope); + }, + FinishAction::Discard => { client.record_lost_data(&transaction, ClientReportReason::SampleRate); - } - } - return; - } - - if let Some(mut transaction) = inner.transaction.take() { - if let Some(client) = inner.client.take() { - transaction.finish_with_timestamp(_timestamp); - transaction - .contexts - .insert("trace".into(), inner.context.clone().into()); - - Hub::current().with_current_scope(|scope| scope.apply_to_transaction(&mut transaction)); - let opts = client.options(); - transaction.release.clone_from(&opts.release); - transaction.environment.clone_from(&opts.environment); - transaction.sdk = Some(std::borrow::Cow::Owned(client.sdk_info.clone())); - transaction.server_name.clone_from(&opts.server_name); - - let mut dsc = protocol::DynamicSamplingContext::new() - .with_trace_id(inner.context.trace_id) - .with_sample_rate(self.metadata.sample_rate) - .with_sampled(inner.sampled); - if let Some(public_key) = client.dsn().map(|dsn| dsn.public_key()) { - dsc = dsc.with_public_key(public_key.to_owned()); - } - - drop(inner); - - let mut envelope = protocol::Envelope::new().with_headers( - protocol::EnvelopeHeaders::new().with_trace(dsc) - ); - envelope.add_item(transaction); - - client.send_envelope(envelope) + }, + FinishAction::Ignore => (), } } }} @@ -1046,7 +1050,7 @@ impl Transaction { }; Span { transaction: Arc::clone(&self.inner), - sampled: inner.sampled, + tracing_state: inner.tracing_state, span: Arc::new(Mutex::new(span)), } } @@ -1078,7 +1082,7 @@ impl Transaction { }; Span { transaction: Arc::clone(&self.inner), - sampled: inner.sampled, + tracing_state: inner.tracing_state, span: Arc::new(Mutex::new(span)), } } @@ -1126,7 +1130,7 @@ impl DerefMut for Data<'_> { #[derive(Clone, Debug)] pub struct Span { pub(crate) transaction: TransactionArc, - sampled: bool, + tracing_state: TracingState, span: SpanArc, } @@ -1238,8 +1242,9 @@ impl Span { /// trace's distributed tracing headers. pub fn iter_headers(&self) -> TraceHeadersIter { let span = self.span.lock().unwrap(); - let trace = - TracePropagationContext::new(span.trace_id, span.span_id).with_sampled(self.sampled); + let trace = TracePropagationContext::new(span.trace_id, span.span_id) + .with_maybe_sampled(self.tracing_state.trace_sampled()); + TraceHeadersIter { sentry_trace: Some(trace.sentry_trace_header()), } @@ -1258,7 +1263,19 @@ impl Span { /// correct results. #[deprecated = "the returned value may not accurately represent the sampling decision"] pub fn is_sampled(&self) -> bool { - self.sampled + // Checking that we have a `Send` finish action should at least roughly match the old + // behavior of this function: we only return true for sampled spans when tracing is + // enabled, and false otherwise. + #[cfg(feature = "client")] + { + matches!( + self.tracing_state.finish_action(), + FinishAction::Send { .. } + ) + } + + #[cfg(not(feature = "client"))] + false } /// Finishes the Span with the provided end timestamp. @@ -1311,7 +1328,7 @@ impl Span { }; Span { transaction: self.transaction.clone(), - sampled: self.sampled, + tracing_state: self.tracing_state, span: Arc::new(Mutex::new(span)), } } @@ -1343,7 +1360,7 @@ impl Span { }; Span { transaction: self.transaction.clone(), - sampled: self.sampled, + tracing_state: self.tracing_state, span: Arc::new(Mutex::new(span)), } } @@ -1460,40 +1477,40 @@ mod tests { let ctx = TransactionContext::new("noop", "noop"); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), - 0.3 + Some(0.3) ); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.7), &ctx), - 0.7 + Some(0.7) ); let mut ctx = TransactionContext::new("noop", "noop"); ctx.set_sampled(true); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), - 1.0 + Some(1.0) ); ctx.set_sampled(false); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), - 0.0 + Some(0.0) ); let ctx = TransactionContext::new("noop", "noop"); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), - 0.0 + None ); let mut ctx = TransactionContext::new("noop", "noop"); ctx.set_sampled(true); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), - 0.0 + None ); ctx.set_sampled(false); assert_eq!( transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), - 0.0 + None ); // Function and FixedRate are mutually exclusive strategy variants. A function @@ -1501,9 +1518,9 @@ mod tests { let mut ctx = TransactionContext::new("noop", "noop"); let sampler = |_: &TransactionContext| 0.7_f32; let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(0.7)); ctx.set_sampled(false); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(0.7)); let sampler = |ctx: &TransactionContext| match ctx.sampled() { Some(true) => 0.8_f32, @@ -1512,9 +1529,9 @@ mod tests { }; let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); ctx.set_sampled(true); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.8); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(0.8)); ctx.set_sampled(None); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.6); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(0.6)); let sampler = |ctx: &TransactionContext| { if ctx.name() == "must-name" || ctx.operation() == "must-operation" { @@ -1533,11 +1550,11 @@ mod tests { }; let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); let ctx = TransactionContext::new("noop", "must-operation"); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 1.0); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(1.0)); let ctx = TransactionContext::new("must-name", "noop"); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 1.0); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(1.0)); let mut ctx = TransactionContext::new("noop", "noop"); ctx.custom_insert("rate".to_owned(), serde_json::json!(0.7)); - assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); + assert_eq!(transaction_sample_rate(&strategy, &ctx), Some(0.7)); } } diff --git a/sentry-core/src/performance/sampling.rs b/sentry-core/src/performance/sampling.rs new file mode 100644 index 00000000..b16399ef --- /dev/null +++ b/sentry-core/src/performance/sampling.rs @@ -0,0 +1,134 @@ +//! This module contains some types that represent sampling decisions. + +#[cfg(doc)] +use sentry_types::protocol::v7::client_report; + +/// Represents the tracing state of a transaction. +/// +/// The possible representations depend on whether tracing is enabled or disabled in this SDK. +/// +/// ### If tracing is enabled +/// +/// We always have a sampling decision. This decision is propagated from an incoming trace when +/// available, otherwise we make the decision according to the configured sample rate. +/// +/// ### If tracing is disabled +/// +/// For traces started by this SDK, the sampling decision is deferred. No sampling decision is +/// available. +/// +/// If this SDK is continuing an incoming trace, we may have a sampling decision if the incoming +/// trace propagated a sampling decision. As tracing is disabled, the SDK will not sample any +/// spans regardless of the sampling decision, but the incoming tracing decision will again get +/// propagated outwards. +/// +/// When compiled without the `client` crate feature, only disabled traces can be represented, +/// as enabling tracing requires a client. +#[derive(Debug, Clone, Copy)] +pub(super) enum TracingState { + /// Tracing is enabled. In this case, there must be a sampling decision. + #[cfg(feature = "client")] + Enabled(SamplingDecision), + /// Tracing is disabled. In this case, we only have a tracing decision when continuing a trace + /// that has a sampling decision. + Disabled(Option), +} + +impl TracingState { + /// Create a new [`TracingState::Enabled`] with the given sampling decision made at the given + /// sample rate. + #[cfg(feature = "client")] + pub(super) fn new_enabled(sampled: bool, sample_rate: f32) -> Self { + Self::Enabled(SamplingDecision { + sampled, + sample_rate, + }) + } + + /// Create a new [`TracingState::Disabled`] given a sampling decision or `None` if the decision + /// is deferred. + /// + /// The `sample_rate` in the [`SamplingDecision`], if available, is a best-effort estimate of + /// the sample rate because the SDK does not yet read the sample rate propagated in the baggage + /// headers. Therefore, we just assume the sample_rate was `1.0` for sampled traces, and `0.0` + /// for unsampled ones, so that the sample rate is at least consistent with the sampling + /// decision. Once we read the `sample_rate`, this method should be adjusted to use that rate. + pub(super) fn new_disabled(sampled: Option) -> Self { + let decision = sampled.map(|sampled| SamplingDecision { + sampled, + #[cfg(feature = "client")] + sample_rate: sampled.into(), + }); + + Self::Disabled(decision) + } + + /// Return whether this trace is sampled, or `None` if no decision is available. + /// + /// # ⚠️ Caution + /// + /// Never use this method to determine whether the SDK should record spans, as this method + /// may return `Some(true)` when tracing is disabled, namely, when continuing a sampled trace + /// in TwP mode. Use [`Self::finish_action`] for this purpose. + pub(super) fn trace_sampled(&self) -> Option { + match *self { + #[cfg(feature = "client")] + Self::Enabled(SamplingDecision { sampled, .. }) => Some(sampled), + Self::Disabled(decision) => decision.map(|SamplingDecision { sampled, .. }| sampled), + } + } + + /// Determine the correct action to take when spans/transactions in this trace are finished. + /// + /// See [`FinishAction`] for more details. + #[cfg(feature = "client")] + pub(super) fn finish_action(&self) -> FinishAction { + match *self { + Self::Enabled(SamplingDecision { + sampled: true, + sample_rate, + }) => FinishAction::Send { sample_rate }, + + Self::Enabled(SamplingDecision { + sampled: false, + sample_rate: _, + }) => FinishAction::Discard, + + Self::Disabled(_) => FinishAction::Ignore, + } + } +} + +/// The trace's sampling decision. +#[derive(Debug, Clone, Copy)] +pub(super) struct SamplingDecision { + /// The sampling decision. + pub(super) sampled: bool, + /// The sample rate at which the decision was made. + /// + /// Currently, we only use this on the `client` feature, but if needed we can also provide + /// this on non-`client` builds. + #[cfg(feature = "client")] + pub(super) sample_rate: f32, +} + +/// What the SDK should do with spans/transactions when they are finished. +#[cfg(feature = "client")] +#[derive(Debug, Clone, Copy)] +pub(super) enum FinishAction { + /// Send spans/transactions to Sentry. + /// + /// This action should be taken for sampled traces when tracing is enabled. + /// + /// As we may wish to know the sampling rate used to come to the decision to sample when + /// finishing the transaction/span, this variant includes the `sample_rate`. + Send { sample_rate: f32 }, + /// Discard spans/transactions and record a client report with a "sampling rate" reason. + /// + /// This action should be taken for unsampled tracing when tracing is enabled. + Discard, + /// Ignore spans/transactions. Do not send them to Sentry, and do not record a client report. + /// + /// This action should always be taken when tracing is disabled. + Ignore, +}