Skip to content

Commit

Permalink
Delay sampling of span to finish
Browse files Browse the repository at this point in the history
  • Loading branch information
thomaseizinger committed Nov 29, 2024
1 parent 3f4f126 commit 230d6d8
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 14 deletions.
17 changes: 8 additions & 9 deletions sentry-core/src/performance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,8 @@ impl<'a> TransactionData<'a> {

impl Transaction {
#[cfg(feature = "client")]
fn new(mut client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
let (sampled, mut transaction) = match client.as_ref() {
fn new(client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
let (sampled, transaction) = match client.as_ref() {
Some(client) => (
client.is_transaction_sampled(&ctx),
Some(protocol::Transaction {
Expand All @@ -581,13 +581,6 @@ impl Transaction {
..Default::default()
};

// throw away the transaction here, which means there is nothing to send
// on `finish`.
if !sampled {
transaction = None;
client = None;
}

Self {
inner: Arc::new(Mutex::new(TransactionInner {
client,
Expand Down Expand Up @@ -699,6 +692,12 @@ impl Transaction {
pub fn finish(self) {
with_client_impl! {{
let mut inner = self.inner.lock().unwrap();

// Discard `Transaction` unless sampled.
if !inner.sampled {
return;
}

if let Some(mut transaction) = inner.transaction.take() {
if let Some(client) = inner.client.take() {
transaction.finish();
Expand Down
4 changes: 2 additions & 2 deletions sentry-tracing/tests/breadcrumbs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ mod shared;

#[test]
fn breadcrumbs_should_capture_span_fields() {
let transport = shared::init_sentry();
let transport = shared::init_sentry(0.0); // This test should work even if we are not sampling transactions.

foo();

let data = transport.fetch_and_clear_envelopes();
assert_eq!(data.len(), 2);
assert_eq!(data.len(), 1);

let event = data.first().expect("should have 1 event");
let event = match event.items().next().unwrap() {
Expand Down
4 changes: 2 additions & 2 deletions sentry-tracing/tests/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ use sentry_core::test::TestTransport;

use std::sync::Arc;

pub fn init_sentry() -> Arc<TestTransport> {
pub fn init_sentry(traces_sample_rate: f32) -> Arc<TestTransport> {
use tracing_subscriber::prelude::*;

let transport = TestTransport::new();
let options = ClientOptions {
dsn: Some("https://test@sentry-tracing.com/test".parse().unwrap()),
transport: Some(Arc::new(transport.clone())),
sample_rate: 1.0,
traces_sample_rate: 1.0,
traces_sample_rate,
..ClientOptions::default()
};
Hub::current().bind_client(Some(Arc::new(options.into())));
Expand Down
2 changes: 1 addition & 1 deletion sentry-tracing/tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn function_with_tags(value: i32) {

#[test]
fn should_instrument_function_with_event() {
let transport = shared::init_sentry();
let transport = shared::init_sentry(1.0); // Sample all spans.

function_with_tags(1);

Expand Down

0 comments on commit 230d6d8

Please sign in to comment.